From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH v2 2/2] xfs: use sync buffer I/O for sync delwri queue submission
Date: Mon, 18 Jun 2018 07:47:45 -0400 [thread overview]
Message-ID: <20180618114744.GA28320@bfoster> (raw)
In-Reply-To: <20180618112130.GA24243@infradead.org>
On Mon, Jun 18, 2018 at 04:21:30AM -0700, Christoph Hellwig wrote:
> On Fri, Jun 15, 2018 at 08:39:56AM -0400, Brian Foster wrote:
> > Ok, codewise I don't have much of a preference, but I don't think it's
> > worth redoing the regression testing and lowmem testing and whatnot just
> > to change how the guts are refactored here. What's the endgame?
>
> Avoid the nasty duplication as much as possible. I think your patch
> already goes a long way towards that, but I'd rather go all the way.
>
That's what I figured, thanks.
> > I came
> > up with the following on top of patch 2. Compile tested only, and I can
> > refold the _common() helper back into the caller and invert the
> > nowait logic or whatnot..
>
> Mostly looks fine, except that it seems pointless to have
> __xfs_buf_submit_common around instead of merging it into the only
> caller.
>
Yes..
> > + if (!(bp->b_flags & XBF_ASYNC) && !sync_nowait)
>
> Also I'd rather pass in a
>
> 'bool wait' parameter.
>
> xfs_buf_submit() would set it based on XBF_ASYNC, and the delwri
> code would just set it to false explicitly.
A patch with the above changes is essentially what I put through some
testing over the weekend. It doesn't look like anything exploded, so
I'll try to get it cleaned up and posted later today.
Brian
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
prev parent reply other threads:[~2018-06-18 11:47 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-13 11:05 [PATCH v2 0/2] xfs: fix buffer delwri queue state race Brian Foster
2018-06-13 11:05 ` [PATCH v2 1/2] xfs: refactor buffer submission into a common helper Brian Foster
2018-06-14 13:43 ` [PATCH v3 " Brian Foster
2018-06-15 11:24 ` Christoph Hellwig
2018-06-15 11:53 ` Brian Foster
2018-06-13 11:05 ` [PATCH v2 2/2] xfs: use sync buffer I/O for sync delwri queue submission Brian Foster
2018-06-13 22:08 ` Dave Chinner
2018-06-13 23:29 ` Brian Foster
2018-06-13 23:37 ` Dave Chinner
2018-06-15 11:28 ` Christoph Hellwig
2018-06-15 11:53 ` Brian Foster
2018-06-15 12:16 ` Christoph Hellwig
2018-06-15 12:39 ` Brian Foster
2018-06-15 16:31 ` Darrick J. Wong
2018-06-15 17:43 ` Brian Foster
2018-06-18 11:21 ` Christoph Hellwig
2018-06-18 11:47 ` Brian Foster [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180618114744.GA28320@bfoster \
--to=bfoster@redhat.com \
--cc=hch@infradead.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.