xfs
[Top] [All Lists]

Re: [PATCH 6/8] xfs: don't chain ioends during writepage submission

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 6/8] xfs: don't chain ioends during writepage submission
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 11 Feb 2016 11:26:47 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20160210113609.GC15221@xxxxxxxxxxxxx>
References: <1455094043-9694-1-git-send-email-david@xxxxxxxxxxxxx> <1455094043-9694-7-git-send-email-david@xxxxxxxxxxxxx> <20160210113609.GC15221@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Feb 10, 2016 at 03:36:09AM -0800, Christoph Hellwig wrote:
> > -STATIC void
> > +STATIC int
> >  xfs_submit_ioend(
> >     struct writeback_control *wbc,
> >     xfs_ioend_t             *ioend,
> >     int                     fail)
> 
> No that almost all of the function is rewritten can you rename
> fail to error or status?  fail always suggests a boolean to me and
> is rather confusing.

*nod*

> > + * Return the ioend we finished off so that the caller can submit it
> > + * once it has finished processing the dirty page.
> >   */
> > -STATIC void
> > +STATIC struct xfs_ioend *
> >  xfs_add_to_ioend(
> >     struct inode            *inode,
> >     struct buffer_head      *bh,
> >     xfs_off_t               offset,
> >     struct xfs_writepage_ctx *wpc)
> >  {
> > +   struct xfs_ioend        *prev = NULL;
> > +
> >     if (!wpc->ioend || wpc->io_type != wpc->ioend->io_type ||
> >         bh->b_blocknr != wpc->last_block + 1) {
> >             struct xfs_ioend        *new;
> >  
> > +           prev = wpc->ioend;
> 
> Looking at the new list_head based code it might be either to just
> pass in a pointer to the submit_list and just add the previous ioend
> here directly.

OK.

> >     ret = xfs_do_writepage(page, wbc, &wpc);
> > -   return xfs_writepage_submit(&wpc, wbc, ret);
> > +   if (wpc.ioend)
> > +           xfs_submit_ioend(wbc, wpc.ioend, ret);
> > +   return ret;
> >  }
> >  
> >  STATIC int
> > @@ -1019,7 +1022,9 @@ xfs_vm_writepages(
> >  
> >     xfs_iflags_clear(XFS_I(mapping->host), XFS_ITRUNCATED);
> >     ret = write_cache_pages(mapping, wbc, xfs_do_writepage, &wpc);
> > -   return xfs_writepage_submit(&wpc, wbc, ret);
> > +   if (wpc.ioend)
> > +           xfs_submit_ioend(wbc, wpc.ioend, ret);
> > +   return ret;
> 
> And this is where ignoreing the xfs_setfilesize_trans_alloc errors
> reappears after a previous patch mostly fixed it up.

will fix.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>