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

[Dave Chinner <david@fromorbit.com>]

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@fromorbit.com