xfs
[Top] [All Lists]

Re: [PATCH 4/6] xfs: reduce the number of log forces from tail pushing

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH 4/6] xfs: reduce the number of log forces from tail pushing
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 26 Aug 2011 09:47:31 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1314305826.3136.102.camel@doink>
References: <1314256626-11136-1-git-send-email-david@xxxxxxxxxxxxx> <1314256626-11136-5-git-send-email-david@xxxxxxxxxxxxx> <1314305826.3136.102.camel@doink>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Aug 25, 2011 at 03:57:06PM -0500, Alex Elder wrote:
> On Thu, 2011-08-25 at 17:17 +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > 
> > The AIL push code will issue a log force on ever single push loop
> > that it exits and has encountered pinned items. It doesn't rescan
> > these pinned items until it revisits the AIL from the start. Hence
> > we only need to force the log once per walk from the start of the
> > AIL to the target LSN.
> > 
> > This results in numbers like this:
> > 
> >     xs_push_ail_flush.....         1456
> >     xs_log_force.........          1485
> > 
> > For an 8-way 50M inode create workload - almost all the log forces
> > are coming from the AIL pushing code.
> > 
> > Reduce the number of log forces by only forcing the log if the
> > previous walk found pinned buffers. This reduces the numbers to:
> > 
> >     xs_push_ail_flush.....          665
> >     xs_log_force.........           682
> > 
> > For the same test.
> > 
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> This looks good to me and if you don't update it
> I can take it as-is.  A couple trivial things below
> if you decide to update.
> 
> Reviewed-by: Alex Elder <aelder@xxxxxxx>
> 
> > ---
> >  fs/xfs/xfs_trans_ail.c  |   28 ++++++++++++++++------------
> >  fs/xfs/xfs_trans_priv.h |    1 +
> >  2 files changed, 17 insertions(+), 12 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> > index c15aa29..dd966e0 100644
> > --- a/fs/xfs/xfs_trans_ail.c
> > +++ b/fs/xfs/xfs_trans_ail.c
> > @@ -372,12 +372,24 @@ xfs_ail_worker(
> >     xfs_lsn_t               lsn;
> >     xfs_lsn_t               target;
> >     long                    tout = 10;
> > -   int                     flush_log = 0;
> >     int                     stuck = 0;
> >     int                     count = 0;
> >     int                     push_xfsbufd = 0;
> >  
> > +   /*
> > +    * If last time we ran we encountered pinned items, force the log first,
> > +    * wait for it and then push again.
>           * and wait for it before we push it again.
>           */
> 

OK.

> > +    */
> >     spin_lock(&ailp->xa_lock);
> > +   if (ailp->xa_last_pushed_lsn == 0 && ailp->xa_log_flush &&
> > +       !list_empty(&ailp->xa_ail)) {
> > +           ailp->xa_log_flush = 0;
> > +           spin_unlock(&ailp->xa_lock);
> > +           XFS_STATS_INC(xs_push_ail_flush);
> > +           xfs_log_force(mp, XFS_LOG_SYNC);
> > +           spin_lock(&ailp->xa_lock);
> > +   }
> 
> This is a definite improvement over the previous version.
> 
> > +
> >     target = ailp->xa_target;
> >     lip = xfs_trans_ail_cursor_first(ailp, &cur, ailp->xa_last_pushed_lsn);
> >     if (!lip || XFS_FORCED_SHUTDOWN(mp)) {
> > @@ -391,6 +403,7 @@ xfs_ail_worker(
> >  
> >     XFS_STATS_INC(xs_push_ail);
> >  
> > +
> >     /*
> >      * While the item we are looking at is below the given threshold
> >      * try to flush it out. We'd like not to stop until we've at least
> 
> Kill this hunk.

Yep, will do.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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