* [PATCH v4 1/3] iomap: add iomap_symlink_write
2026-08-26 21:41 [PATCH v4 0/3] minix: convert to iomap Jeremy Bingham
@ 2026-08-26 21:41 ` Jeremy Bingham
2026-09-18 13:48 ` Christoph Hellwig
2026-08-26 21:41 ` [PATCH v4 2/3] minix: add iomap functions and definitions Jeremy Bingham
2026-08-26 21:41 ` [PATCH v4 3/3] minix: finish wiring in iomap functions Jeremy Bingham
2 siblings, 1 reply; 7+ messages in thread
From: Jeremy Bingham @ 2026-08-26 21:41 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-kernel, brauner, jkoolstra, jack, djwong, viro, hch,
Jeremy Bingham
Add a new iomap_symlink_write function as an iomap based equivalent to
page_symlink found in fs/namei.c. Part of being that equivalency is
behaving similarly to page_symlink. This function now expects the same
len as page_symlink, where len is the length of the null terminated
target string. The target is still written out without the trailing
null.
Suggested-by: Darrick J. Wong <djwong@kernel.org>
Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
---
fs/iomap/buffered-io.c | 34 ++++++++++++++++++++++++++++++++++
include/linux/iomap.h | 3 +++
2 files changed, 37 insertions(+)
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 0a5ebfda90f1..5e8ac3fad671 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -2100,3 +2100,37 @@ iomap_writepages(struct iomap_writepage_ctx *wpc)
return error;
}
EXPORT_SYMBOL_GPL(iomap_writepages);
+
+int iomap_symlink_write(struct inode *inode, const char *target, int len,
+ const struct iomap_ops *ops,
+ const struct iomap_write_ops *write_ops, void *private)
+{
+ struct kvec vec = {
+ .iov_base = (void *)target,
+ .iov_len = len - 1,
+ };
+ struct iomap_iter iter = {
+ .inode = inode,
+ .pos = 0,
+ .len = len - 1,
+ .flags = IOMAP_WRITE,
+ .private = private,
+ };
+ struct iov_iter iov;
+ int ret;
+
+ iov_iter_kvec(&iov, ITER_SOURCE, &vec, 1, len - 1);
+
+ while ((ret = iomap_iter(&iter, ops)) > 0)
+ iter.status = iomap_write_iter(&iter, &iov, write_ops);
+
+ if (ret < 0)
+ return ret;
+
+ if (unlikely(iter.pos == 0))
+ return -EIO;
+
+ mark_inode_dirty(inode);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iomap_symlink_write);
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 8c754eb974fb..ab27a3a5b8d2 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -471,6 +471,9 @@ loff_t iomap_seek_data(struct inode *inode, loff_t offset,
const struct iomap_ops *ops);
sector_t iomap_bmap(struct address_space *mapping, sector_t bno,
const struct iomap_ops *ops);
+int iomap_symlink_write(struct inode *inode, const char *target, int len,
+ const struct iomap_ops *ops,
+ const struct iomap_write_ops *write_ops, void *private);
/*
* Flags for iomap_ioend->io_flags.
--
2.47.3
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v4 1/3] iomap: add iomap_symlink_write
2026-08-26 21:41 ` [PATCH v4 1/3] iomap: add iomap_symlink_write Jeremy Bingham
@ 2026-09-18 13:48 ` Christoph Hellwig
0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2026-09-18 13:48 UTC (permalink / raw)
To: Jeremy Bingham
Cc: linux-fsdevel, linux-kernel, brauner, jkoolstra, jack, djwong, viro, hch
On Wed, Aug 26, 2026 at 02:41:55PM -0700, Jeremy Bingham wrote:
> Add a new iomap_symlink_write function as an iomap based equivalent to
> page_symlink found in fs/namei.c. Part of being that equivalency is
> behaving similarly to page_symlink. This function now expects the same
> len as page_symlink, where len is the length of the null terminated
> target string. The target is still written out without the trailing
> null.
>
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> Suggested-by: Christoph Hellwig <hch@infradead.org>
>
> Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
No empty line before the signoff, please.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 2/3] minix: add iomap functions and definitions
2026-08-26 21:41 [PATCH v4 0/3] minix: convert to iomap Jeremy Bingham
2026-08-26 21:41 ` [PATCH v4 1/3] iomap: add iomap_symlink_write Jeremy Bingham
@ 2026-08-26 21:41 ` Jeremy Bingham
2026-09-18 14:12 ` Christoph Hellwig
2026-08-26 21:41 ` [PATCH v4 3/3] minix: finish wiring in iomap functions Jeremy Bingham
2 siblings, 1 reply; 7+ messages in thread
From: Jeremy Bingham @ 2026-08-26 21:41 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-kernel, brauner, jkoolstra, jack, djwong, viro, hch,
Jeremy Bingham
Adds a new file, iomap.c, to fs/minix. This provides minix_iomap_begin,
which is the iomap-based version of get_block in itree_common.c. It also
provides minix_iomap_end, which is a no-op function required by the
iomap_ops struct. The minix_iomap_begin function also uses helper
functions to undo the nest of gotos inherited from get_block.
This patch also wires iomap.c into itree_v1.c and itree_v2.c, similarly
to how itree_common.c is included in those files, and exports version
specific versions of minix_iomap_begin and iomap_ops.
Also updates 'unsigned' to 'unsigned int' a few places that got picked
up by checkpatch.pl.
Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
---
fs/minix/iomap.c | 122 ++++++++++++++++++++++++++++++++++++++++++++
fs/minix/itree_v1.c | 25 ++++++++-
fs/minix/itree_v2.c | 17 +++++-
fs/minix/minix.h | 21 +++++++-
4 files changed, 181 insertions(+), 4 deletions(-)
create mode 100644 fs/minix/iomap.c
diff --git a/fs/minix/iomap.c b/fs/minix/iomap.c
new file mode 100644
index 000000000000..90e016aaa01e
--- /dev/null
+++ b/fs/minix/iomap.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * iomap functions for minix.
+ */
+
+static inline void minix_chain_cleanup(Indirect *chain, Indirect *partial)
+{
+ while (partial > chain) {
+ brelse(partial->bh);
+ partial--;
+ }
+}
+
+static inline void minix_iomap_set_mapped(struct iomap *iomap, sector_t phys,
+ unsigned int blkbits, sector_t iblock)
+{
+ iomap->type = IOMAP_MAPPED;
+ iomap->addr = (u64)phys << blkbits;
+ iomap->length = 1 << blkbits;
+ iomap->offset = (u64)iblock << blkbits;
+}
+
+static inline void minix_iomap_set_hole(struct iomap *iomap,
+ unsigned int blkbits, sector_t iblock)
+{
+ iomap->type = IOMAP_HOLE;
+ iomap->addr = IOMAP_NULL_ADDR;
+ iomap->length = 1 << blkbits;
+ iomap->offset = (u64)iblock << blkbits;
+}
+
+/*
+ * minix_iomap_begin - map a file range to disk blocks. It acts as a replacement
+ * for get_block in itree_common.c, at least in the important ways, and is
+ * adapted from it, but it uses iomap instead of buffer_head.
+ */
+static int minix_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+ unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
+{
+ struct super_block *sb = inode->i_sb;
+ unsigned int blkbits = sb->s_blocksize_bits;
+ sector_t iblock = offset >> blkbits;
+ int create = flags & IOMAP_WRITE;
+
+ int offsets[DEPTH];
+ Indirect chain[DEPTH];
+ Indirect *partial;
+ int depth = block_to_path(inode, iblock, offsets);
+ int left;
+ int err = -EIO;
+
+ sector_t phys;
+
+ /* block is beyond max file size */
+ if (depth == 0)
+ return -EINVAL;
+
+ iomap->bdev = inode->i_sb->s_bdev;
+
+reread:
+ partial = get_branch(inode, depth, offsets, chain, &err);
+
+ /* Simplest case - block found, no allocation needed */
+ if (!partial) {
+ iomap->flags = 0;
+ phys = block_to_cpu(chain[depth - 1].key);
+ partial = chain+depth-1;
+ minix_iomap_set_mapped(iomap, phys, blkbits, iblock);
+ minix_chain_cleanup(chain, partial);
+ return err;
+ }
+
+ /* Next simple case - plain lookup or failed read of indirect block */
+ if (!create || err == -EIO) {
+ minix_iomap_set_hole(iomap, blkbits, iblock);
+ minix_chain_cleanup(chain, partial);
+ return err;
+ }
+
+ /*
+ * This is held over from the original get_block logic, where it
+ * acted as a guard in case truncate() deleted blocks from under that
+ * function. There should not be a race with iomap operations, but
+ * we're retaining the defensive coding here to be extra safe just in
+ * case.
+ */
+ if (err == -EAGAIN) {
+ minix_chain_cleanup(chain, partial);
+ goto reread;
+ }
+
+ left = (chain + depth) - partial;
+ err = alloc_branch(inode, left, offsets + (partial - chain), partial);
+ if (err) {
+ minix_chain_cleanup(chain, partial);
+ return err;
+ }
+
+ if (splice_branch(inode, chain, partial, left) < 0) {
+ minix_chain_cleanup(chain, partial);
+ goto reread;
+ }
+
+ /* Successful allocation, mapping it. */
+ iomap->flags = IOMAP_F_NEW;
+ phys = block_to_cpu(chain[depth - 1].key);
+ minix_iomap_set_mapped(iomap, phys, blkbits, iblock);
+ minix_chain_cleanup(chain, partial);
+
+ return err;
+}
+
+/*
+ * minix_iomap_end ends up being a nop; since minix doesn't have any extents or
+ * transactions to worry about, there isn't anything to update here. The on-disk
+ * indirect blocks get dirtied in minix_iomap_begin.
+ */
+static int minix_iomap_end(struct inode *inode, loff_t offset, loff_t length,
+ ssize_t written, unsigned int flags, struct iomap *iomap)
+{
+ return 0;
+}
diff --git a/fs/minix/itree_v1.c b/fs/minix/itree_v1.c
index 1fed906042aa..58c29f4443d3 100644
--- a/fs/minix/itree_v1.c
+++ b/fs/minix/itree_v1.c
@@ -49,6 +49,18 @@ static int block_to_path(struct inode * inode, long block, int offsets[DEPTH])
}
#include "itree_common.c"
+/* NOTA BENE:
+ *
+ * This is icky to me, but at the same time having it be a standalone C file
+ * that's compiled to object form and linked separately like it is in xiafs is
+ * much nastier in minix because of the different versions of the minix fs that
+ * have some very, very different aspects, like the size of block_t. I don't
+ * like it, but since minix already has this pattern where a common itree file
+ * is included in the itree_v1 and itree_v2(and v3) files, I'm including iomap.c
+ * in these files as well. It does at least avoid exporting some currently
+ * static functions that aren't needed anywhere but itree_common.c and iomap.c.
+ */
+#include "iomap.c"
int V1_minix_get_block(struct inode * inode, long block,
struct buffer_head *bh_result, int create)
@@ -61,7 +73,18 @@ void V1_minix_truncate(struct inode * inode)
truncate(inode);
}
-unsigned V1_minix_blocks(loff_t size, struct super_block *sb)
+unsigned int V1_minix_blocks(loff_t size, struct super_block *sb)
{
return nblocks(size, sb);
}
+
+int V1_minix_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+ unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
+{
+ return minix_iomap_begin(inode, offset, length, flags, iomap, srcmap);
+}
+
+const struct iomap_ops V1_minix_iomap_ops = {
+ .iomap_begin = V1_minix_iomap_begin,
+ .iomap_end = minix_iomap_end,
+};
diff --git a/fs/minix/itree_v2.c b/fs/minix/itree_v2.c
index 9d00f31a2d9d..fc7a5ae8fa1c 100644
--- a/fs/minix/itree_v2.c
+++ b/fs/minix/itree_v2.c
@@ -57,6 +57,10 @@ static int block_to_path(struct inode * inode, long block, int offsets[DEPTH])
}
#include "itree_common.c"
+/* See the note in itree_v1 in a comment that starts "NOTA BENE" for an
+ * explanation for why iomap.c is included here.
+ */
+#include "iomap.c"
int V2_minix_get_block(struct inode * inode, long block,
struct buffer_head *bh_result, int create)
@@ -69,7 +73,18 @@ void V2_minix_truncate(struct inode * inode)
truncate(inode);
}
-unsigned V2_minix_blocks(loff_t size, struct super_block *sb)
+unsigned int V2_minix_blocks(loff_t size, struct super_block *sb)
{
return nblocks(size, sb);
}
+
+int V2_minix_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+ unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
+{
+ return minix_iomap_begin(inode, offset, length, flags, iomap, srcmap);
+}
+
+const struct iomap_ops V2_minix_iomap_ops = {
+ .iomap_begin = V2_minix_iomap_begin,
+ .iomap_end = minix_iomap_end,
+};
diff --git a/fs/minix/minix.h b/fs/minix/minix.h
index 78722ce22e1e..f1141d36e3d5 100644
--- a/fs/minix/minix.h
+++ b/fs/minix/minix.h
@@ -5,6 +5,7 @@
#include <linux/fs.h>
#include <linux/pagemap.h>
#include <linux/minix_fs.h>
+#include <linux/iomap.h>
#define INODE_VERSION(inode) minix_sb(inode->i_sb)->s_version
#define MINIX_V1 0x0001 /* original minix fs */
@@ -76,13 +77,23 @@ int minix_make_empty(struct inode*, struct inode*);
int minix_empty_dir(struct inode*);
int minix_set_link(struct minix_dir_entry *de, struct folio *folio,
struct inode *inode);
+
struct minix_dir_entry *minix_dotdot(struct inode*, struct folio **);
ino_t minix_inode_by_name(struct dentry*);
+int V1_minix_iomap_begin(struct inode *inode, loff_t offset,
+ loff_t length, unsigned int flags, struct iomap *iomap,
+ struct iomap *srcmap);
+int V2_minix_iomap_begin(struct inode *inode, loff_t offset,
+ loff_t length, unsigned int flags, struct iomap *iomap,
+ struct iomap *srcmap);
+
extern const struct inode_operations minix_file_inode_operations;
extern const struct inode_operations minix_dir_inode_operations;
extern const struct file_operations minix_file_operations;
extern const struct file_operations minix_dir_operations;
+extern const struct iomap_ops V1_minix_iomap_ops;
+extern const struct iomap_ops V2_minix_iomap_ops;
static inline struct minix_sb_info *minix_sb(struct super_block *sb)
{
@@ -94,11 +105,17 @@ static inline struct minix_inode_info *minix_i(struct inode *inode)
return container_of(inode, struct minix_inode_info, vfs_inode);
}
-static inline unsigned minix_blocks_needed(unsigned bits, unsigned blocksize)
+static inline unsigned int minix_blocks_needed(unsigned int bits, unsigned int blocksize)
{
return DIV_ROUND_UP_POW2(bits, blocksize * 8);
}
+static inline const struct iomap_ops *minix_iomap_ops_ver(struct inode *inode)
+{
+ return (INODE_VERSION(inode) == MINIX_V1) ?
+ &V1_minix_iomap_ops : &V2_minix_iomap_ops;
+}
+
#if defined(CONFIG_MINIX_FS_NATIVE_ENDIAN) && \
defined(CONFIG_MINIX_FS_BIG_ENDIAN_16BIT_INDEXED)
@@ -128,7 +145,7 @@ static inline unsigned minix_blocks_needed(unsigned bits, unsigned blocksize)
* big-endian 16bit indexed bitmaps
*/
-static inline int minix_find_first_zero_bit(const void *vaddr, unsigned size)
+static inline int minix_find_first_zero_bit(const void *vaddr, unsigned int size)
{
const unsigned short *p = vaddr, *addr = vaddr;
unsigned short num;
--
2.47.3
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v4 2/3] minix: add iomap functions and definitions
2026-08-26 21:41 ` [PATCH v4 2/3] minix: add iomap functions and definitions Jeremy Bingham
@ 2026-09-18 14:12 ` Christoph Hellwig
0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2026-09-18 14:12 UTC (permalink / raw)
To: Jeremy Bingham
Cc: linux-fsdevel, linux-kernel, brauner, jkoolstra, jack, djwong, viro, hch
On Wed, Aug 26, 2026 at 02:41:56PM -0700, Jeremy Bingham wrote:
> This patch also wires iomap.c into itree_v1.c and itree_v2.c, similarly
> to how itree_common.c is included in those files, and exports version
> specific versions of minix_iomap_begin and iomap_ops.
Can we keep the iomap code out of this double build mess? Just define
some low-level helper in it, but keep everything else in an only
once build iomap.c
> Also updates 'unsigned' to 'unsigned int' a few places that got picked
> up by checkpatch.pl.
If you want to do these cleanups please split them into a separate
patch.
> +++ b/fs/minix/iomap.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * iomap functions for minix.
> + */
Not really a very useful comment :) OTOH this lacks a copyright
statement for your work.
> +static int minix_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> + unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
This mostly does not need the iomap. It basically returns a physical
block and a trystate of HOLE/FOUND/ALLOCATED. If you change the
interface to that you can keep it in itree_common by directly morphing
the existing block lookup into that, or even do it as a prep patch,
which first refactors get_block to this interface, and then do the
iomap mapping on top of that, and in a place that only gets built
once.
> +/*
> + * minix_iomap_end ends up being a nop; since minix doesn't have any extents or
> + * transactions to worry about, there isn't anything to update here. The on-disk
> + * indirect blocks get dirtied in minix_iomap_begin.
> + */
> +static int minix_iomap_end(struct inode *inode, loff_t offset, loff_t length,
> + ssize_t written, unsigned int flags, struct iomap *iomap)
> +{
> + return 0;
> +}
In which case we can just drop it. Note that for the next version
against 7.3-rc should also move from the begin/end calls to the
single next one using the DEFINE_IOMAP_ITER_NEXT helper macro.
> -static inline unsigned minix_blocks_needed(unsigned bits, unsigned blocksize)
> +static inline unsigned int minix_blocks_needed(unsigned int bits, unsigned int blocksize)
Overly long line. I'd much rather generate the baseless checkpath.pl
complaint and leave the plain unsgineds in place. There is absolutely
no technical downside in using that notation.
> +static inline const struct iomap_ops *minix_iomap_ops_ver(struct inode *inode)
> +{
> + return (INODE_VERSION(inode) == MINIX_V1) ?
No need for the braces.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 3/3] minix: finish wiring in iomap functions
2026-08-26 21:41 [PATCH v4 0/3] minix: convert to iomap Jeremy Bingham
2026-08-26 21:41 ` [PATCH v4 1/3] iomap: add iomap_symlink_write Jeremy Bingham
2026-08-26 21:41 ` [PATCH v4 2/3] minix: add iomap functions and definitions Jeremy Bingham
@ 2026-08-26 21:41 ` Jeremy Bingham
2026-09-18 15:10 ` Christoph Hellwig
2 siblings, 1 reply; 7+ messages in thread
From: Jeremy Bingham @ 2026-08-26 21:41 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-kernel, brauner, jkoolstra, jack, djwong, viro, hch,
Jeremy Bingham
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.
There are two main additions in inode.c. First, minix_writeback_range
and minix_writeback_ops are newly added. Secondly, the old
minix_writepages and minix_read_folio functions were renamed to
minix_block_writepages and minix_block_read_folio respectively while new
functions with those names were created that use iomap. Because
directory operations need to stay using buffer heads, a new set of
address space operations just for directory operations was created while
other file types use the new iomap based address space operations. The
minix_symlink_inode_operations also have setattr set to minix_setattr
now. Support for bmap has been dropped entirely, per Darrick J. Wong's
suggestion.
In itree_common.c, truncate() is updated to use different functions
depending on whether the inode being truncated is a directory or not.
This is because of the changes above where directory operations still
use buffer heads and have their own address operations.
This patch also updates minix_symlink to use the new iomap_symlink_write
function, which brings symlinks under iomap and removes the need to
bypass it with a private custom function. This was suggested by Darrick
J. Wong in an earlier version of this patch series.
Suggested-by: Darrick J. Wong <djwong@kernel.org>
Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
---
fs/minix/file.c | 30 ++++++++++++++-
fs/minix/inode.c | 85 +++++++++++++++++++++++++++++++++++------
fs/minix/itree_common.c | 10 ++++-
fs/minix/minix.h | 2 +
fs/minix/namei.c | 7 +++-
5 files changed, 118 insertions(+), 16 deletions(-)
diff --git a/fs/minix/file.c b/fs/minix/file.c
index 02aabbdb5dea..6765f571bd3f 100644
--- a/fs/minix/file.c
+++ b/fs/minix/file.c
@@ -10,6 +10,32 @@
#include <linux/buffer_head.h>
#include "minix.h"
+static ssize_t minix_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
+{
+ struct inode *inode = iocb->ki_filp->f_mapping->host;
+ ssize_t ret;
+ const struct iomap_ops *ops = minix_iomap_ops_ver(inode);
+
+ inode_lock(inode);
+ ret = generic_write_checks(iocb, from);
+ if (ret <= 0)
+ goto unlock;
+
+ ret = file_modified(iocb->ki_filp);
+ if (ret)
+ goto unlock;
+
+ ret = iomap_file_buffered_write(iocb, from, ops,
+ NULL, NULL);
+
+ if (ret > 0)
+ ret = generic_write_sync(iocb, ret);
+
+unlock:
+ inode_unlock(inode);
+ return ret;
+}
+
/*
* We have mostly NULLs here: the current defaults are OK for
* the minix filesystem.
@@ -17,13 +43,13 @@
const struct file_operations minix_file_operations = {
.llseek = generic_file_llseek,
.read_iter = generic_file_read_iter,
- .write_iter = generic_file_write_iter,
+ .write_iter = minix_file_write_iter,
.mmap_prepare = generic_file_mmap_prepare,
.fsync = simple_fsync,
.splice_read = filemap_splice_read,
};
-static int minix_setattr(struct mnt_idmap *idmap,
+int minix_setattr(struct mnt_idmap *idmap,
struct dentry *dentry, struct iattr *attr)
{
struct inode *inode = d_inode(dentry);
diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index daf83e4ff25c..2bf5ea92360a 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -439,6 +439,31 @@ static int minix_statfs(struct dentry *dentry, struct kstatfs *buf)
return 0;
}
+static ssize_t minix_writeback_range(struct iomap_writepage_ctx *wpc,
+ struct folio *folio, u64 pos, unsigned int len, u64 end_pos)
+{
+ int error;
+
+ if (pos < wpc->iomap.offset ||
+ pos >= wpc->iomap.offset + wpc->iomap.length) {
+ if (INODE_VERSION(wpc->inode) == MINIX_V1)
+ error = V1_minix_iomap_begin(wpc->inode, pos, len, IOMAP_WRITE,
+ &wpc->iomap, NULL);
+ else
+ error = V2_minix_iomap_begin(wpc->inode, pos, len, IOMAP_WRITE,
+ &wpc->iomap, NULL);
+ if (error)
+ return error;
+ }
+
+ return iomap_add_to_ioend(wpc, folio, pos, end_pos, len);
+}
+
+static const struct iomap_writeback_ops minix_writeback_ops = {
+ .writeback_range = minix_writeback_range,
+ .writeback_submit = iomap_ioend_writeback_submit,
+};
+
static int minix_get_block(struct inode *inode, sector_t block,
struct buffer_head *bh_result, int create)
{
@@ -448,17 +473,45 @@ static int minix_get_block(struct inode *inode, sector_t block,
return V2_minix_get_block(inode, block, bh_result, create);
}
-static int minix_writepages(struct address_space *mapping,
+/* The old minix_writepages, preserved for directory operations. */
+static int minix_block_writepages(struct address_space *mapping,
struct writeback_control *wbc)
{
return mpage_writepages(mapping, wbc, minix_get_block);
}
+static int minix_writepages(struct address_space *mapping,
+ struct writeback_control *wbc)
+{
+ struct iomap_writepage_ctx wpc = {
+ .inode = mapping->host,
+ .wbc = wbc,
+ .ops = &minix_writeback_ops,
+ };
+ return iomap_writepages(&wpc);
+}
+
static int minix_read_folio(struct file *file, struct folio *folio)
+{
+ const struct iomap_ops *ops = minix_iomap_ops_ver(folio->mapping->host);
+
+ iomap_bio_read_folio(folio, ops);
+ return 0;
+}
+
+/* The old minix_read_folio, preserved for directory operations. */
+static int minix_block_read_folio(struct file *file, struct folio *folio)
{
return block_read_full_folio(folio, minix_get_block);
}
+static void minix_readahead(struct readahead_control *rac)
+{
+ const struct iomap_ops *ops = minix_iomap_ops_ver(rac->mapping->host);
+
+ iomap_bio_readahead(rac, ops);
+}
+
int minix_prepare_chunk(struct folio *folio, loff_t pos, unsigned len)
{
return __block_write_begin(folio, pos, len, minix_get_block);
@@ -488,26 +541,35 @@ static int minix_write_begin(const struct kiocb *iocb,
return ret;
}
-static sector_t minix_bmap(struct address_space *mapping, sector_t block)
-{
- return generic_block_bmap(mapping,block,minix_get_block);
-}
-
static const struct address_space_operations minix_aops = {
- .dirty_folio = block_dirty_folio,
- .invalidate_folio = block_invalidate_folio,
+ .dirty_folio = iomap_dirty_folio,
+ .invalidate_folio = iomap_invalidate_folio,
.read_folio = minix_read_folio,
+ .readahead = minix_readahead,
.writepages = minix_writepages,
+ .migrate_folio = filemap_migrate_folio,
+ .is_partially_uptodate = iomap_is_partially_uptodate,
+ .release_folio = iomap_release_folio,
+ .error_remove_folio = generic_error_remove_folio,
+};
+
+/* A special aops for directories that keeps using the buffer head chunks, at
+ * least for the time being.
+ */
+static const struct address_space_operations minix_dir_aops = {
+ .dirty_folio = block_dirty_folio,
+ .invalidate_folio = block_invalidate_folio,
+ .read_folio = minix_block_read_folio,
.write_begin = minix_write_begin,
.write_end = generic_write_end,
.migrate_folio = buffer_migrate_folio,
- .bmap = minix_bmap,
- .direct_IO = noop_direct_IO
+ .writepages = minix_block_writepages,
};
static const struct inode_operations minix_symlink_inode_operations = {
.get_link = page_get_link,
.getattr = minix_getattr,
+ .setattr = minix_setattr,
};
void minix_set_inode(struct inode *inode, dev_t rdev)
@@ -519,7 +581,7 @@ void minix_set_inode(struct inode *inode, dev_t rdev)
} else if (S_ISDIR(inode->i_mode)) {
inode->i_op = &minix_dir_inode_operations;
inode->i_fop = &minix_dir_operations;
- inode->i_mapping->a_ops = &minix_aops;
+ inode->i_mapping->a_ops = &minix_dir_aops;
} else if (S_ISLNK(inode->i_mode)) {
inode->i_op = &minix_symlink_inode_operations;
inode_nohighmem(inode);
@@ -786,4 +848,3 @@ module_init(init_minix_fs)
module_exit(exit_minix_fs)
MODULE_DESCRIPTION("Minix file system");
MODULE_LICENSE("GPL");
-
diff --git a/fs/minix/itree_common.c b/fs/minix/itree_common.c
index c3cd2c75af9c..d73ab1caacfe 100644
--- a/fs/minix/itree_common.c
+++ b/fs/minix/itree_common.c
@@ -311,7 +311,15 @@ static inline void truncate (struct inode * inode)
long iblock;
iblock = (inode->i_size + sb->s_blocksize -1) >> sb->s_blocksize_bits;
- block_truncate_page(inode->i_mapping, inode->i_size, get_block);
+
+ /* Depending on whether the inode being truncated is a directory or not,
+ * we need to either call iomap_truncate_page or block_truncate_page.
+ */
+ if (S_ISDIR(inode->i_mode))
+ block_truncate_page(inode->i_mapping, inode->i_size, get_block);
+ else
+ iomap_truncate_page(inode, inode->i_size, NULL,
+ minix_iomap_ops_ver(inode), NULL, NULL);
n = block_to_path(inode, iblock, offsets);
if (!n)
diff --git a/fs/minix/minix.h b/fs/minix/minix.h
index f1141d36e3d5..9035604c68ca 100644
--- a/fs/minix/minix.h
+++ b/fs/minix/minix.h
@@ -58,6 +58,8 @@ void minix_free_block(struct inode *inode, unsigned long block);
unsigned long minix_count_free_blocks(struct super_block *sb);
int minix_getattr(struct mnt_idmap *, const struct path *,
struct kstat *, u32, unsigned int);
+int minix_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
+ struct iattr *attr);
int minix_prepare_chunk(struct folio *folio, loff_t pos, unsigned len);
struct mapping_metadata_bhs *minix_get_metadata_bhs(struct inode *inode);
diff --git a/fs/minix/namei.c b/fs/minix/namei.c
index 5525ba367ed7..52e115013abf 100644
--- a/fs/minix/namei.c
+++ b/fs/minix/namei.c
@@ -6,6 +6,7 @@
*/
#include "minix.h"
+#include <linux/iomap.h>
static int add_nondir(struct dentry *dentry, struct inode *inode)
{
@@ -84,12 +85,16 @@ static int minix_symlink(struct mnt_idmap *idmap, struct inode *dir,
return PTR_ERR(inode);
minix_set_inode(inode, 0);
- err = page_symlink(inode, symname, i);
+ err = iomap_symlink_write(inode, symname, i, minix_iomap_ops_ver(inode), NULL, NULL);
+
if (unlikely(err)) {
inode_dec_link_count(inode);
iput(inode);
return err;
}
+
+ i_size_write(inode, i - 1);
+
return add_nondir(dentry, inode);
}
--
2.47.3
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v4 3/3] minix: finish wiring in iomap functions
2026-08-26 21:41 ` [PATCH v4 3/3] minix: finish wiring in iomap functions Jeremy Bingham
@ 2026-09-18 15:10 ` Christoph Hellwig
0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2026-09-18 15:10 UTC (permalink / raw)
To: Jeremy Bingham
Cc: linux-fsdevel, linux-kernel, brauner, jkoolstra, jack, djwong, viro, hch
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.
> + ret = iomap_file_buffered_write(iocb, from, ops,
> + NULL, NULL);
The two NULL still fit onto the previous line:
ret = iomap_file_buffered_write(iocb, from, ops, NULL, NULL);
> +
> + 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.
> +int minix_setattr(struct mnt_idmap *idmap,
> struct dentry *dentry, struct iattr *attr)
Odd formatting again. The typical style would be:
int minix_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
struct iattr *attr)
> +static ssize_t minix_writeback_range(struct iomap_writepage_ctx *wpc,
> + struct folio *folio, u64 pos, unsigned int len, u64 end_pos)
Two-tab indents please:
static ssize_t minix_writeback_range(struct iomap_writepage_ctx *wpc,
struct folio *folio, u64 pos, unsigned int len, u64 end_pos)
> +{
> + int error;
> +
> + if (pos < wpc->iomap.offset ||
> + pos >= wpc->iomap.offset + wpc->iomap.length) {
if (pos < wpc->iomap.offset ||
pos >= wpc->iomap.offset + wpc->iomap.length) {
> + if (INODE_VERSION(wpc->inode) == MINIX_V1)
> + error = V1_minix_iomap_begin(wpc->inode, pos, len, IOMAP_WRITE,
> + &wpc->iomap, NULL);
> + else
> + error = V2_minix_iomap_begin(wpc->inode, pos, len, IOMAP_WRITE,
> + &wpc->iomap, NULL);
Overly long lines. These should hopefully go away for free with the
common iomap_ops.
> -static int minix_writepages(struct address_space *mapping,
> +/* The old minix_writepages, preserved for directory operations. */
> +static int minix_block_writepages(struct address_space *mapping,
minix_dir_writepages?
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.
> static const struct address_space_operations minix_aops = {
> - .dirty_folio = block_dirty_folio,
> - .invalidate_folio = block_invalidate_folio,
> + .dirty_folio = iomap_dirty_folio,
> + .invalidate_folio = iomap_invalidate_folio,
> .read_folio = minix_read_folio,
> + .readahead = minix_readahead,
> .writepages = minix_writepages,
> + .migrate_folio = filemap_migrate_folio,
> + .is_partially_uptodate = iomap_is_partially_uptodate,
> + .release_folio = iomap_release_folio,
> + .error_remove_folio = generic_error_remove_folio,
> +};
Maybe use tabs to align the initializers if you touch most of them
anyway?
> + /* Depending on whether the inode being truncated is a directory or not,
> + * we need to either call iomap_truncate_page or block_truncate_page.
> + */
The kernel coding style would be:
/*
* Depending on whether the inode being truncated is a directory or not,
* we need to either call iomap_truncate_page or block_truncate_page.
*/
> + err = iomap_symlink_write(inode, symname, i, minix_iomap_ops_ver(inode), NULL, NULL);
Overly long line.
^ permalink raw reply [flat|nested] 7+ messages in thread