Re: [PATCH 6/8] xfs: factor common AIL item deletion code

Dave Chinner
2020-03-27T00:50:18Z

On Wed, Mar 25, 2020 at 10:10:01PM -0700, Darrick J. Wong wrote:
> On Wed, Mar 25, 2020 at 12:42:03PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Factor the common AIL deletion code that does all the wakeups into a
> > helper so we only have one copy of this somewhat tricky code to
> > interface with all the wakeups necessary when the LSN of the log
> > tail changes.
> >
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> This call site didn't have a wake_up_all and now it does; is that going
> to make a difference?

That logic only changedf for xfs_trans_ail_update_bulk()...

> I /think/ the answer is that this function
> usually puts things on the AIL so we won't trigger the ail_empty wakeup;

which only inserts into the AIL, so will never trigger the "wake up
if AIL empty" code that is now there because the AIL will never be
empty...

> and if the AIL was previously empty and we didn't match any log items
> (such that it's still empty) then it's fine to wake up anyone who was
> waiting for the ail to clear out?

If we are calling xfs_trans_ail_update_bulk() with zero log items on
an empty AIL, we are probably doing something else wrong. But doing
a wakeup on anything waiting on an empty log will not hurt anything
in this case, because the log was already empty and there should be
nothing waiting for the log to empty...

So, yes, it is safe and won't cause strange issues....

> If so,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks!

-Dave.
--
Dave Chinner
david@fromorbit.com