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:

quoted

quoted

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>--