xfs
[Top] [All Lists]

Re: [PATCH 10/16] xfs: add a lru to the XFS buffer cache

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 10/16] xfs: add a lru to the XFS buffer cache
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 9 Nov 2010 10:45:12 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20101108231928.GA20647@xxxxxxxxxxxxx>
References: <1289206519-18377-1-git-send-email-david@xxxxxxxxxxxxx> <1289206519-18377-11-git-send-email-david@xxxxxxxxxxxxx> <20101108231928.GA20647@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Mon, Nov 08, 2010 at 06:19:28PM -0500, Christoph Hellwig wrote:
> > @@ -471,6 +546,8 @@ _xfs_buf_find(
> >             /* the buffer keeps the perag reference until it is freed */
> >             new_bp->b_pag = pag;
> >             spin_unlock(&pag->pag_buf_lock);
> > +
> > +           xfs_buf_lru_add(new_bp);
> 
> Why do we add the buffer to the lru when we find it?  Normally we
> would remove it here (unless we want a lazy lru scheme),

Oh, I forgot to remove that from the patch when rewriting it to use
lazy updateѕ. Good catch! (*)

>
> and potentially increment b_lru_ref - although that seems to be
> done by the callers in the next patch.

b_lru_ref is initialised to 1 when the buffer is first initialised,
so it doesn't need to be done here. And yes, the next patch allows
the users of the buffers to set the reclaim reference count
themselves when the buffer is read for prioritisation. ideally I
don't want to have to touch the reclaim state of the buffer during
xfs_buf_find....

Cheers,

Dave.

(*) Catching this sort of bug is exactly why I posted the series
early. Little details are easy to miss in a forest of changes this
large....
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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