Re: [PATCH 5/9] xfs: convert the xfsaild threads to a workqueue

Dave Chinner
2011-04-07T00:05:33Z

On Wed, Apr 06, 2011 at 02:12:56PM -0400, Christoph Hellwig wrote:
> On Wed, Apr 06, 2011 at 04:19:14PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Similar to the xfssyncd, the per-filesystem xfsaild threads can be
> > converted to a global workqueue and run periodically by delayed
> > works. This makes sense for the AIL pushing because it uses
> > variable timeouts depending on the work that needs to be done.
> >
> > By removing the xfsaild, we simplify the AIL pushing code and
> > remove the need to spread the code to implement the threading
> > and pushing across multiple files.
> >
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> > fs/xfs/linux-2.6/xfs_super.c | 115 ++++++++++++------------------------
> > fs/xfs/xfs_trans_ail.c | 133 ++++++++++++++++++++++++------------------
> > fs/xfs/xfs_trans_priv.h | 15 +++--
> > 3 files changed, 121 insertions(+), 142 deletions(-)
>
>
>
>
> > + tout = 0;
> > if (!count) {
> > /* We're past our target or empty, so idle */
> > - last_pushed_lsn = 0;
> > + ailp->xa_last_pushed_lsn = 0;
> > +
> > + /*
> > + * Check for an updated push target before clearing the
> > + * XFS_AIL_PUSHING_BIT. If the target changed, we've got more
> > + * work to do.
> > + */
> > + smp_rmb();
> > + if (ailp->xa_target == target)
> > + clear_bit(XFS_AIL_PUSHING_BIT, &ailp->xa_flags);
> > + else
> > + tout = 50;
>
> I thjink this could be simplified a bit by just returning here if we
> have matched the target, and then making the queueing up of work later
> on unconditional.

Ok, that's reasonable.

> > +/*
> > + * xfs_trans_ail_push
> > + *
>
> Wha'ts the point of mentioning the function name like this in comments?

None really, was just being consistent with other functions. Will
fix.

> > + /*
> > + * En??ure that the new target is noticed in push code before it clears
>
> There's some weird character here which my mailer can't display.

Oops.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com