Re: [PATCH v3 4/5] minix: add file operation functions

[Jeremy Bingham]

On Mon, Jul 13, 2026 at 2:48 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> On Sat, Jul 11, 2026 at 01:59:08PM -0700, Jeremy Bingham wrote:
> > The previously static minix_setattr function is also now exported; it will
> > be used by minix_symlink_inode_operations.
>
> How does this work without the next patch?

It works as before until the next patch, which is where the symlink functions
are updated to use iomap. I thought it might be better to have it in
place before
rolling the symlink changes out. I'll

> > +     struct inode *inode = iocb->ki_filp->f_mapping->host;
> > +     ssize_t ret;
> > +
> > +     inode_lock_shared(inode);
> > +
> > +     const struct iomap_ops *ops = minix_iomap_ops_ver(inode);
>
> Please keep all variable declarations at the top of the file.

Missed that, apologies.

> > +
> > +     ret = iomap_dio_rw(iocb, to, ops, NULL, 0, NULL, 0);
> > +     inode_unlock_shared(inode);
> > +     return ret;
> > +}
>
> But more importantly please explain the use case for adding direct I/O
> support to minix, as adding new barely tested code to a legacy file
> needs a very good justification.  And it really needs to be split into
> a separate patch.

The direct i/o support was inspired by looking at the exfat and ext4
file.c code.
I honestly don't have a strong justification for it beyond that it seemed like a
nice bonus while I was working with the read/write functions. Since
it's something
that ought to be a separate patch anyway, I'll redo this to not
involve direct i/o.

Thank you for your comments on this patch series,

-j