From: Matthew Bobrowski <mbobrowski@mbobrowski.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: tytso@mit.edu, jack@suse.cz, adilger.kernel@dilger.ca,
linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
david@fromorbit.com, darrick.wong@oracle.com
Subject: Re: [PATCH v3 5/6] ext4: introduce direct IO write path using iomap infrastructure
Date: Fri, 20 Sep 2019 23:24:36 +1000 [thread overview]
Message-ID: <20190920132436.GA2863@bobrowski> (raw)
In-Reply-To: <20190917090613.GC29487@infradead.org>
On Tue, Sep 17, 2019 at 02:06:13AM -0700, Christoph Hellwig wrote:
> On Tue, Sep 17, 2019 at 08:37:41AM +1000, Matthew Bobrowski wrote:
> > > Independent of the error return issue you probably want to split
> > > modifying ext4_write_checks into a separate preparation patch.
> >
> > Providing that there's no objections to introducing a possible performance
> > change with this separate preparation patch (overhead of calling
> > file_remove_privs/file_update_time twice), then I have no issues in doing so.
>
> Well, we should avoid calling it twice. But what caught my eye is that
> the buffered I/O path also called this function, so we are changing it as
> well here. If that actually is safe (I didn't review these bits carefully
> and don't know ext4 that well) the overall refactoring of the write
> flow might belong into a separate prep patch (that is not relying
> on ->direct_IO, the checks changes, etc).
Yeah, no. Revisiting this again now and trying to implement the
ext4_write_checks() modifications as a pre-patch is a nightmare so to
speak. This is purely due to the way that ext4_file_write_iter() is currently
written and how both the current buffered I/O and direct I/O paths traverse
through and make use of it.
If anything, the changes applied to ext4_write_checks() should be a separate
patch that comes *after* the refactoring of the buffered and direct I/O write
flow. However, even then, there'd be code that we essentially introduce in the
write flow changes and then subsequently removed after the fact. Providing
that's OK, then sure, I can put this within a separate patch.
--<M>--
next prev parent reply other threads:[~2019-09-20 13:24 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-12 11:03 [PATCH v3 0/6] ext4: port direct IO to iomap infrastructure Matthew Bobrowski
2019-09-12 11:03 ` [PATCH v3 1/6] ext4: introduce direct IO read path using " Matthew Bobrowski
2019-09-16 12:00 ` Christoph Hellwig
2019-09-16 13:07 ` Matthew Bobrowski
2019-09-12 11:04 ` [PATCH v3 2/6] ext4: move inode extension/truncate code out from ext4_iomap_end() Matthew Bobrowski
2019-09-23 16:21 ` Jan Kara
2019-09-24 9:50 ` Matthew Bobrowski
2019-09-24 13:13 ` Jan Kara
2019-09-12 11:04 ` [PATCH v3 3/6] iomap: split size and error for iomap_dio_rw ->end_io Matthew Bobrowski
2019-09-12 11:04 ` [PATCH v3 4/6] ext4: reorder map.m_flags checks in ext4_iomap_begin() Matthew Bobrowski
2019-09-16 12:05 ` Christoph Hellwig
2019-09-17 12:48 ` Matthew Bobrowski
2019-09-23 15:08 ` Jan Kara
2019-09-24 9:35 ` Matthew Bobrowski
2019-09-12 11:04 ` [PATCH v3 5/6] ext4: introduce direct IO write path using iomap infrastructure Matthew Bobrowski
2019-09-16 4:37 ` Ritesh Harjani
2019-09-16 10:14 ` Matthew Bobrowski
2019-09-16 12:12 ` Christoph Hellwig
2019-09-16 22:37 ` Matthew Bobrowski
2019-09-17 9:00 ` Ritesh Harjani
2019-09-17 9:02 ` Christoph Hellwig
2019-09-17 10:12 ` Ritesh Harjani
2019-09-17 12:39 ` Matthew Bobrowski
2019-09-24 10:57 ` Jan Kara
2019-09-17 9:06 ` Christoph Hellwig
2019-09-17 11:31 ` Matthew Bobrowski
2019-09-20 13:24 ` Matthew Bobrowski [this message]
2019-09-23 21:10 ` Jan Kara
2019-09-24 10:29 ` Matthew Bobrowski
2019-09-24 14:13 ` Jan Kara
2019-09-25 7:14 ` Matthew Bobrowski
2019-09-25 8:40 ` Jan Kara
2019-09-12 11:05 ` [PATCH v3 6/6] ext4: cleanup legacy buffer_head direct IO code Matthew Bobrowski
2019-09-16 12:06 ` Christoph Hellwig
2019-09-16 12:53 ` Matthew Bobrowski
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=20190920132436.GA2863@bobrowski \
--to=mbobrowski@mbobrowski.org \
--cc=adilger.kernel@dilger.ca \
--cc=darrick.wong@oracle.com \
--cc=david@fromorbit.com \
--cc=hch@infradead.org \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=tytso@mit.edu \
/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.