Re: [PATCH v4 3/3] minix: finish wiring in iomap functions

From: Jeremy Bingham

Date: Sat Sep 19 2026 - 14:03:15 EST


I've seen all the comments on formatting and will be fixing them, but
I'll just note that once and not keep repeating myself.

On Fri, Sep 18, 2026 at 8:10 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> On Wed, Aug 26, 2026 at 02:41:57PM -0700, Jeremy Bingham wrote:
> > Wire in the new iomap functionality in one pass. Per Christoph Hellwig's
> > feedback, there are no longer direct I/O operations. Without that, only
> > write_iter in minix_file_operations needs a custom function. That and
> > exporting minix_setattr for minix_symlink_inode_operations are the only
> > changes in file.c.
>
> Commit history just goes into the cover letter. In genral you don't
> need to enumerate all low-level changes either. Explain the high-level
> change, what motivated it, and anything that looks a bit unusual and
> unexpected for that high-level change.
>
> Note that you probably want to merge this into the previous patch
> adding the actual iomap ops instead of leaving them dangling between
> thet two patches.

Indeed, and thank you for letting me know that this ought to be merged
into the previous patch. I didn't want to run afoul of making the
patch too big.

> > +
> > + if (ret > 0)
> > + ret = generic_write_sync(iocb, ret);
> > +
> > +unlock:
> > + inode_unlock(inode);
>
> For most file systems we try to have the generic_write_sync outside
> the inode lock to not do the expensive sync with the inode locked.
> generic_file_write_iter also doesn't have the inode locked, so you
> should probably sync after dropping the lock here as swell.

That makes sense. I'll rearrange that.

> If you're looking for another project, we could probably also have
> a iomap version of the directories in pagecache used by minix, ext2
> and co eventually.

That does sound worth working on. I don't love having the directory
operations using a completely different framework than everything
else, but since minix was not the only filesystem afffected by that I
left it be.

Thanks again, and I'll start getting this turned around post-haste.

-j