Re: [PATCH] xfs: clear ail delwri queued bufs on unmount of shutdown fs

[Brian Foster <bfoster@redhat.com>]

On Mon, Oct 15, 2018 at 12:57:27PM +1100, Dave Chinner wrote:
> On Sat, Oct 13, 2018 at 01:29:14PM +1100, Dave Chinner wrote:
> > On Fri, Oct 12, 2018 at 01:46:54PM -0400, Brian Foster wrote:
> > > In the typical unmount case, the AIL is forced out by the unmount
> > > sequence before the xfsaild task is stopped. Since AIL items are
> > > removed on writeback completion, this means that the AIL
> > > ->ail_buf_list delwri queue has been drained. This is not always
> > > true in the shutdown case, however.
> > > 
> > > It's possible for buffers to sit on a delwri queue for a period of
> > > time across submission attempts if said items are locked or have
> > > been relogged and pinned since first added to the queue.
> > 
> > Can you add this as a comment to xfs_buf_delwri_submit_nowait() to
> > document that callers either need to check that everything was
> > submitted and/or cancel the delwri list before they tear it down?
> 
> I added this comment to the patch when I pulled it in:
> 
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -2055,6 +2055,13 @@ xfs_buf_delwri_submit_buffers(
>   * is only safely useable for callers that can track I/O completion by higher
>   * level means, e.g. AIL pushing as the @buffer_list is consumed in this
>   * function.
> + *
> + * Note: this function will skip buffers it would block on, and in doing so
> + * leaves them on @buffer_list so they can be retried on a later pass. As such,
> + * it is up to the caller to ensure that the buffer list is fully submitted or
> + * cancelled appropriately when they are finished with the list. Failure to
> + * cancel or resubmit the list until it is empty will result in leaked buffers
> + * at unmount time.
>   */
>  int
>  xfs_buf_delwri_submit_nowait(
> 
> Does that look reasonable?
> 

Looks fine to me, thanks!

Brian

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com