xfs
[Top] [All Lists]

Re: [PATCH 3/4] xfs: convert AIL cursors to use struct list_head

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH 3/4] xfs: convert AIL cursors to use struct list_head
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 20 Jul 2011 09:43:24 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1311116636.1964.40.camel@doink>
References: <1310960419-9875-1-git-send-email-david@xxxxxxxxxxxxx> <1310960419-9875-4-git-send-email-david@xxxxxxxxxxxxx> <1311116636.1964.40.camel@doink>
User-agent: Mutt/1.5.20 (2009-06-14)
On Tue, Jul 19, 2011 at 06:03:56PM -0500, Alex Elder wrote:
> On Mon, 2011-07-18 at 13:40 +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > 
> > The list of active AIL cursors uses a roll-your-own linked list with
> > special casing for the AIL push cursor. Simplify this code by
> > replacing the list with standard struct list_head lists, and use a
> > separate list_head to track the active cursors. This allows us to
> > treat the AIL push cursor as a generic cursor rather than as a
> > special case, further simplifying the code.
> > 
> > Further, fix the duplicate push cursor initialisation that the
> > special case handling was hiding, and clean up all the comments
> > around the active cursor list handling.
> > 
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> 
> You didn't fix one thing I mentioned before in a comment.
> I can fix that for you if you like.  Other than that, this
> looks good.
> 
> Reviewed-by: Alex Elder <aelder@xxxxxxx>
> 
> > ---
> >  fs/xfs/xfs_trans_ail.c  |   77 
> > ++++++++++++++++-------------------------------
> >  fs/xfs/xfs_trans_priv.h |    4 +-
> >  2 files changed, 28 insertions(+), 53 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> > index 7908c79..8cd329b 100644
> > --- a/fs/xfs/xfs_trans_ail.c
> > +++ b/fs/xfs/xfs_trans_ail.c
> 
> . . .
> 
> > @@ -181,17 +177,12 @@ xfs_trans_ail_cursor_init(
> 
> . . .
> 
> > - * Get the next item in the traversal and advance the cursor.
> > - * If the cursor was invalidated (inidicated by a lip of 1),
> > - * restart the traversal.
> > + * Get the next item in the traversal and advance the cursor.  If the 
> > cursor
> > + * was invalidated (indicated by a lip of 1), restart the traversal.
>  * was invalidated (indicated by its item pointer's low bit being set),
>  * restart the traversal.

I thought I fixed that. Hmmmm, I did fix a comment that said the
same thing - just it wasn't the one you noticed. :/ I'll fix it up
when I redo the splice code again.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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