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
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
