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