[Top] [All Lists]

Re: REVIEW: Fix CI lookup in leaf-form directories

To: Barry Naujok <bnaujok@xxxxxxx>
Subject: Re: REVIEW: Fix CI lookup in leaf-form directories
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 23 Jun 2008 05:37:18 -0400
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
In-reply-to: <op.uc6088qm3jf8g2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <op.uc6088qm3jf8g2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
On Mon, Jun 23, 2008 at 06:44:58PM +1000, Barry Naujok wrote:
> Along the same lines as the node-form directory patch last week,
> the leaf-form is not handling directory buffers properly and
> locks up.
> Instead of comparing buffer pointers, compare buffer block numbers
> and don't keep buffers hanging around.

Which might be a small performance penalty, but I think that's fine
for the CI lookup case.

The patch looks good to me.

But one things in the original xfs_dir2_leaf_lookup_int that barely
touched by your patch really irks me:

> -     cbp = NULL;
> -     for (lep = &leaf->ents[index], dbp = NULL, curdb = -1;
> +     for (lep = &leaf->ents[index], dbp = NULL, curdb = -1, cidb = -1;
>                               index < be16_to_cpu(leaf->hdr.count) &&
>                               be32_to_cpu(lep->hashval) == args->hashval;
>                               lep++, index++) {

I'd really prefer to have not too much rather unrelated bits in the
for loop.  In fact the use of the for construct here is more than odd
because there is no such things as a loop variable at all.  I think
we'd be much better of in terms of readability with a simple while
loop with where all the initialization is moved out of the loop.

Probably not for this patch, though..

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