xfs
[Top] [All Lists]

Re: [PATCH] Always reset btree cursor after an insert

To: Lachlan McIlroy <lachlan@xxxxxxx>
Subject: Re: [PATCH] Always reset btree cursor after an insert
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 16 Jun 2008 15:01:50 +1000
Cc: xfs-dev <xfs-dev@xxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <4855CE2D.70505@xxxxxxx>
Mail-followup-to: Lachlan McIlroy <lachlan@xxxxxxx>, xfs-dev <xfs-dev@xxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
References: <4855CE2D.70505@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.17+20080114 (2008-01-14)
On Mon, Jun 16, 2008 at 12:21:33PM +1000, Lachlan McIlroy wrote:
> After a btree insert operation a cursor can be invalid due to block
> splits and a maybe a new root block.  We reset the cursor in
> xfs_bmbt_insert() in the cases where we think we need to but it
> isn't enough as we still see assertions.  Just do what we do elsewhere
> and reset the cursor unconditionally.

Ok, so you should also kill the new code in the btree insert that
revalidates the btree cursor. IIRC, this was the only place it was
needed for....

> --- fs/xfs/xfs_bmap.c_1.392   2008-06-03 12:20:14.000000000 +1000
> +++ fs/xfs/xfs_bmap.c 2008-06-16 12:11:47.000000000 +1000
> @@ -1745,11 +1745,17 @@ xfs_bmap_add_extent_unwritten_real(
>                       if ((error = xfs_bmbt_insert(cur, &i)))
>                               goto done;
>                       ASSERT(i == 1);
> -                     if ((error = xfs_bmbt_increment(cur, 0, &i)))
> +                     /*
> +                      * Reset the cursor, don't trust it after any insert
> +                      * operation.
> +                      */

/*
 * reset the cursor to the position of the new extent we are about
 * to insert as we can't trust it after the previous insert
 */

> +                     if ((error = xfs_bmbt_lookup_eq(cur, new->br_startoff,
> +                                     new->br_startblock, new->br_blockcount,
> +                                     &i)))
>                               goto done;


                        error = xfs_bmbt_lookup_eq(cur, new->br_startoff,
                                        br_startblock, new->br_blockcount, &i);
                        if (error)
                                goto done;

> -                     ASSERT(i == 1);
> +                     ASSERT(i == 0);

ASSERT? How about a WANT_CORRUPTED_GOTO()?

>                       /* new middle extent - newext */
> -                     cur->bc_rec.b = *new;
> +                     cur->bc_rec.b.br_state = new->br_state;
>                       if ((error = xfs_bmbt_insert(cur, &i)))
>                               goto done;
>                       ASSERT(i == 1);

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx


<Prev in Thread] Current Thread [Next in Thread>