Re: [PATCH 2/5] xfs: Introduce writeback context for writepages

Dave Chinner
2016-02-09T21:51:39Z

On Tue, Feb 09, 2016 at 09:22:26AM -0500, Brian Foster wrote:
> On Mon, Feb 08, 2016 at 04:44:15PM +1100, Dave Chinner wrote:
> > + /*
> > + * We have to fail the iohead here because we buffers locked in the
> > + * ioend chain. If we don't do this, we'll deadlock invalidating the
> > + * page as that tries to lock the buffers on the page. Also, because we
> > + * have set pages under writeback, we have to run IO completion to mark
> > + * the error state of the IO appropriately, so we can't cancel the ioend
> > + * directly here. That means we have to mark this page as under
> > + * writeback if we included any buffers from it in the ioend chain.
> > + */
> > + if (count)
> > + xfs_start_page_writeback(page, 0, count);
> > + xfs_writepage_submit(wpc, wbc, err);
>
> We make the xfs_writepage_submit() error case call here because...
...
> > STATIC int
> > +xfs_vm_writepage(
> > + struct page *page,
> > + struct writeback_control *wbc)
> > +{
> > + struct xfs_writepage_ctx wpc = {
> > + .io_type = XFS_IO_OVERWRITE,
> > + };
> > + int ret;
> > +
> > + ret = xfs_do_writepage(page, wbc, &wpc);
> > + if (ret)
> > + return ret;
> > + return xfs_writepage_submit(&wpc, wbc, ret);
>
>
> ... the callers only call it when ret == 0. Can we eliminate the error
> call down in xfs_do_writepage() and just invoke this consistently from
> the writepage(s) callers?

Probably - I think this is left over from an early concoction that
exploded badly when it was stirred too vigorously...

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com