[PATCH v2 0/4] minix: convert to iomap and add direct I/O

[PATCH v2 0/4] minix: convert to iomap and add direct I/O

[Jeremy Bingham]

This is v2 of the minix iomap conversion series. The original v1
submission (3 patches) was tested by syzbot, which found four issues.
Three were straightforward; the fourth, a null pointer dereference in
page_symlink when creating symlinks, required more substantial changes.

The original description follows:

This series converts the minix filesystem from the buffer_head-based
I/O path to the iomap API, and adds direct I/O support in the process.

The conversion is straightforward: minix's indirect block tree mapping
logic (from itree_common.c) is reworked into an iomap_begin/iomap_end
implementation. The iomap_end callback is a no-op since minix has no
extents or transactions to finalize.

Patch 1 adds the iomap infrastructure: the new iomap.c file, wrapper
functions and iomap_ops structs in itree_v1.c and itree_v2.c, and the
relevant declarations in minix.h.

The iomap.c file is #include'd into itree_v1.c and itree_v2.c rather
than compiled as a standalone translation unit. This is because the
minix filesystem versions (V1 vs V2/V3) have different block_t sizes
(16-bit vs 32-bit) and different indirect tree depths. This follows
the existing pattern in minix where itree_common.c is included into
both itree_v1.c and itree_v2.c. Each version provides a thin wrapper
and a corresponding iomap_ops struct.

Patch 2 converts the regular file address space operations to iomap:
read_folio, readahead, writepages (with a writeback callback), bmap,
and folio lifecycle helpers. Directory inodes continue to use
buffer_head-based operations via a new minix_dir_aops, since directory
handling still relies on buffer head chunks for prepare/write_begin.

Patch 3 converts the file_operations: replacing the generic read/write
iterators with iomap-aware versions, adding direct I/O read/write paths
using iomap_dio_rw, and setting FMODE_CAN_ODIRECT in the open handler.

The minix iomap implementation was adapted from the out-of-tree xiafs
iomap conversion. The xiafs module itself borrowed heavily from the
modernized minix kernel module. The exfat iomap changes were an
additional reference for both conversions.

Changes since v1:

  * Added a fourth patch to fix the symlink and truncate issues:
    - Replaced page_symlink with a custom __page_symlink that writes
      the target directly to a data block via minix_new_block +
      sb_getblk, bypassing the aops write path (which no longer has
      write_begin/write_end). Added a matching custom minix_get_link
      that reads the target from the data block via sb_bread, similar
      to ext4_get_link. No iomap-based filesystem in the kernel uses
      page_symlink; XFS, GFS2, and ext4 all handle symlink storage
      directly. The on-disk format is unchanged.
    - Fixed a buffer_head/iomap type confusion in truncate:
      block_truncate_page attaches buffer_heads to data folios, but
      minix_aops now uses iomap which interprets folio->private as
      struct iomap_folio_state. truncate() now dispatches between
      iomap_truncate_page (for regular files/symlinks) and
      block_truncate_page (for directories) based on the inode's aops.
    - Added .setattr = minix_setattr to minix_symlink_inode_operations
      so symlinks truncate properly through the iomap path.

  * Patch 1 (iomap infrastructure): minix_get_block is now exported
    (non-static) so the directory aops and iomap writeback path can
    use it. Added minix_iomap_ops_ver() inline helper and extern
    declarations for minix_aops and the version-specific iomap_ops.
    Fixed unsigned -> unsigned int in minix_blocks_needed and
    minix_find_first_zero_bit to silence checkpatch warnings.

  * Patch 2 (aops conversion): unchanged in approach; minor cleanup
    of the writeback callback and minix_bmap conversion.

  * Patch 3 (file operations): minix_setattr is now exported for reuse
    by the symlink inode operations in patch 4.

Testing: the full series has been tested with mkfs.minix V1/V2/V3,
exercising file creation, read/write, overwrite, append, binary data,
directories, symlinks (full path, relative, directory symlinks), hard
links, truncation (shrink/grow), large files (1MB, exercising indirect
blocks), deep nesting (20 levels), 100 files in one directory,
deletions, remount persistence, and fsck.minix. All pass cleanly. The
four syzbot-reported issues are resolved.

Jeremy Bingham (4):
  minix: add iomap infrastructure
  minix: convert address space operations to iomap
  minix: convert file operations to iomap and add direct I/O
  minix: fix symlilnk and truncate for iomap compatibility

 fs/minix/file.c         | 157 ++++++++++++++++++++++++++++++++++++++--
 fs/minix/inode.c        |  90 ++++++++++++++++++++---
 fs/minix/iomap.c        | 114 +++++++++++++++++++++++++++++
 fs/minix/itree_common.c |  11 ++-
 fs/minix/itree_v1.c     |  25 ++++++-
 fs/minix/itree_v2.c     |  17 ++++-
 fs/minix/minix.h        |  30 +++++++-
 fs/minix/namei.c        | 137 ++++++++++++++++++++++++++++++++++-
 8 files changed, 558 insertions(+), 23 deletions(-)
 create mode 100644 fs/minix/iomap.c

-- 
2.47.3

[PATCH v2 1/4] minix: add iomap infrastructure

[Jeremy Bingham]

Add an iomap implementation for minix, adapted from the out-of-tree
xiafs module's iomap code. The xiafs module, in turn, borrowed heavily
from minix's own code.

The iomap_begin function replaces get_block for the iomap path:
it walks the indirect block tree, allocates blocks when needed, and
returns the mapping in a struct iomap. The iomap_end function is a
nop since minix has no extents or transactions to finalize.

Because minix has two filesystem versions (V1 with 16-bit block
numbers, V2/V3 with 32-bit) that share itree_common.c via #include,
the iomap.c file follows the same pattern. It is #included into
itree_v1.c and itree_v2.c so it can use the version-specific block_t,
block_to_cpu, and cpu_to_block definitions directly.

Each version gets its own iomap_ops struct (V1_minix_iomap_ops,
V2_minix_iomap_ops) with thin wrappers around the shared
minix_iomap_begin/minix_iomap_end. A minix_iomap_ops_ver() helper
selects the correct ops based on the filesystem version.

minix_get_block is exported from inode.c for use by the iomap
writeback path and the directory aops that still use buffer_heads.

Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
---
 fs/minix/iomap.c    | 114 ++++++++++++++++++++++++++++++++++++++++++++
 fs/minix/itree_v1.c |  25 +++++++++-
 fs/minix/itree_v2.c |  17 ++++++-
 fs/minix/minix.h    |  23 ++++++++-
 4 files changed, 175 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..7bb0439e3669
--- /dev/null
+++ b/fs/minix/iomap.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * iomap functions for minix. At least the first pass of this file was taken
+ * from the xiafs iomap.c, which is fitting since the xiafs module in turn
+ * borrowed heavily from the modernized minix fs kernel module.
+ */
+
+/*
+ * 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. This is taken
+ * directly from the out-of-tree xiafs iomap changes, and the exfat iomap
+ * changes were an inspiration for that.
+ */
+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;
+
+	/* Mostly taken from modern-xiafs itree.c get_block with elements from
+	 * similar exfat operations.
+	 */
+	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)
+		goto out;
+
+	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) {
+		/* Bit of a weird order, but it'll make sense when you get to
+		 * the bottom.
+		 */
+		iomap->flags = IOMAP_F_MERGED;
+got_it:
+		phys = block_to_cpu(chain[depth - 1].key);
+		partial = chain+depth-1;
+		/* Set up the iomap struct before cleaning up */
+		iomap->type = IOMAP_MAPPED;
+		iomap->addr = (u64)phys << blkbits;
+		iomap->length = 1 << blkbits;
+		iomap->offset = (u64)iblock << blkbits;
+		goto cleanup;
+	}
+
+	/* Next simple case - plain lookup or failed read of indirect block */
+	if (!create || err == -EIO) {
+		iomap->type = IOMAP_HOLE;
+		iomap->addr = IOMAP_NULL_ADDR;
+		iomap->length = 1 << blkbits;
+		iomap->offset = (u64)iblock << blkbits;
+		iomap->flags = 0;
+cleanup:
+		while (partial > chain) {
+			brelse(partial->bh);
+			partial--;
+		}
+out:
+		return err;
+	}
+
+	/*
+	 * Indirect block might be removed by truncate while we were
+	 * reading it. Handling of that case (forget what we've got and
+	 * reread) is taken out of the main path.
+	 */
+	if (err == -EAGAIN)
+		goto changed;
+
+	left = (chain + depth) - partial;
+	err = alloc_branch(inode, left, offsets + (partial - chain), partial);
+	if (err)
+		goto cleanup;
+
+	if (splice_branch(inode, chain, partial, left) < 0)
+		goto changed;
+
+	/* Successful allocation, mapping it. */
+	iomap->flags = IOMAP_F_NEW;
+	goto got_it;
+
+changed:
+	while (partial > chain) {
+		brelse(partial->bh);
+		partial--;
+	}
+	goto reread;
+}
+
+/*
+ * 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 f2025c9b5825..77e503cca97f 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 */
@@ -69,6 +70,8 @@ extern int V1_minix_get_block(struct inode *, long, struct buffer_head *, int);
 extern int V2_minix_get_block(struct inode *, long, struct buffer_head *, int);
 extern unsigned V1_minix_blocks(loff_t, struct super_block *);
 extern unsigned V2_minix_blocks(loff_t, struct super_block *);
+extern int minix_get_block(struct inode *inode, sector_t block,
+		    struct buffer_head *bh_result, int create);
 
 struct minix_dir_entry *minix_find_entry(struct dentry *, struct folio **);
 int minix_add_link(struct dentry*, struct inode*);
@@ -80,10 +83,20 @@ int minix_set_link(struct minix_dir_entry *de, struct folio *folio,
 struct minix_dir_entry *minix_dotdot(struct inode*, struct folio **);
 ino_t minix_inode_by_name(struct dentry*);
 
+extern int V1_minix_iomap_begin(struct inode *inode, loff_t offset,
+	loff_t length, unsigned int flags, struct iomap *iomap,
+	struct iomap *srcmap);
+extern 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 address_space_operations minix_aops;
 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)
 {
@@ -95,11 +108,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(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)
 
@@ -129,7 +148,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

[PATCH v2 2/4] minix: convert address space operations to iomap

[Jeremy Bingham]

Convert minix regular file and symlink address space operations from
buffer_head to iomap. The new minix_aops uses iomap_dirty_folio,
iomap_invalidate_folio, iomap_bio_read_folio, iomap_bio_readahead,
iomap_writepages, iomap_bmap, and related iomap helpers.
The write_begin/write_end callbacks are removed since buffered writes
now go through iomap_file_buffered_write in file.c.

Directories keep using buffer_heads via a new minix_dir_aops, which
retains the old block_dirty_folio, block_read_full_folio,
block_write_begin, generic_write_end, and mpage_writepages. This is
necessary because directory entry manipulation (minix_prepare_chunk,
minix_write_begin) still uses the buffer_head chunk protocol.

minix_bmap is converted from generic_block_bmap to iomap_bmap.

The minix_get_block function is exported (non-static) so the
directory aops can still use it for block_write_begin and
mpage_writepages.

Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
---
 fs/minix/inode.c | 86 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 78 insertions(+), 8 deletions(-)

diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index c30cc590698d..2ba6766fce51 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -436,7 +436,32 @@ static int minix_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-static int minix_get_block(struct inode *inode, sector_t block,
+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,
+};
+
+int minix_get_block(struct inode *inode, sector_t block,
 		    struct buffer_head *bh_result, int create)
 {
 	if (INODE_VERSION(inode) == MINIX_V1)
@@ -445,17 +470,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);
@@ -487,19 +540,36 @@ static int minix_write_begin(const struct kiocb *iocb,
 
 static sector_t minix_bmap(struct address_space *mapping, sector_t block)
 {
-	return generic_block_bmap(mapping,block,minix_get_block);
+	const struct iomap_ops *ops = minix_iomap_ops_ver(mapping->host);
+
+	return iomap_bmap(mapping, block, ops);
 }
 
-static const struct address_space_operations minix_aops = {
-	.dirty_folio	= block_dirty_folio,
-	.invalidate_folio = block_invalidate_folio,
+const struct address_space_operations minix_aops = {
+	.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,
+	.bmap = minix_bmap,
+	.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 = {
@@ -516,7 +586,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);
-- 
2.47.3

[PATCH v2 3/4] minix: convert file operations to iomap and add

[Jeremy Bingham]

Replace generic_file_read_iter and generic_file_write_iter with custom
minix_file_read_iter and minix_file_write_iter that dispatch to iomap
for both buffered and direct I/O.

Buffered writes now go through iomap_file_buffered_write instead of
the aops write_begin/write_end path (which no longer exists for
regular files). Buffered reads still use generic_file_read_iter
for the non-DIO case.

Direct I/O is implemented via iomap_dio_rw for both reads and writes.
minix_dio_read_iter takes a shared inode lock; minix_dio_write_iter
takes an exclusive lock, does generic_write_checks, and falls back
to buffered writes via iomap_file_buffered_write for the tail of a
DIO write that is not block-aligned. The minix_dio_write_end_io
callback updates i_size and marks the inode dirty.

minix_file_open sets FMODE_CAN_ODIRECT so the VFS allows O_DIRECT
opens, and splice_write is added to the file operations.

minix_setattr is exported (made non-static) so it can be shared by
the symlink inode operations in a subsequent patch.

Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
---
 fs/minix/file.c  | 157 +++++++++++++++++++++++++++++++++++++++++++++--
 fs/minix/minix.h |   2 +
 2 files changed, 153 insertions(+), 6 deletions(-)

diff --git a/fs/minix/file.c b/fs/minix/file.c
index 86e5943cd2ff..b07c853fa43a 100644
--- a/fs/minix/file.c
+++ b/fs/minix/file.c
@@ -17,21 +17,166 @@ int minix_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 			start, end, datasync);
 }
 
+static ssize_t minix_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
+{
+	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);
+
+	ret = iomap_dio_rw(iocb, to, ops, NULL, 0, NULL, 0);
+	inode_unlock_shared(inode);
+	return ret;
+}
+
+static int minix_dio_write_end_io(struct kiocb *iocb, ssize_t size, int error,
+		unsigned int flags)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+	loff_t pos = iocb->ki_pos;
+
+	if (error)
+		return error;
+
+	pos += size;
+	if (size && pos > i_size_read(inode)) {
+		i_size_write(inode, pos);
+		mark_inode_dirty(inode);
+	}
+	return 0;
+}
+
+static const struct iomap_dio_ops minix_dio_write_ops = {
+	.end_io = minix_dio_write_end_io,
+};
+
+static ssize_t minix_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
+{
+	struct inode *inode = iocb->ki_filp->f_mapping->host;
+	ssize_t ret;
+	unsigned int flags = 0;
+	unsigned long blocksize = inode->i_sb->s_blocksize;
+
+	inode_lock(inode);
+	ret = generic_write_checks(iocb, from);
+	if (ret <= 0)
+		goto out_unlock;
+
+	ret = kiocb_modified(iocb);
+	if (ret)
+		goto out_unlock;
+
+	if (iocb->ki_pos + iov_iter_count(from) > i_size_read(inode) ||
+		!IS_ALIGNED(iocb->ki_pos | iov_iter_alignment(from), blocksize))
+		flags |= IOMAP_DIO_FORCE_WAIT;
+
+	const struct iomap_ops *ops = minix_iomap_ops_ver(inode);
+
+	ret = iomap_dio_rw(iocb, from, ops,
+		&minix_dio_write_ops, flags, NULL, 0);
+	if (ret == -ENOTBLK)
+		ret = 0; /* fallback to buffered */
+
+	if (ret >= 0 && iov_iter_count(from)) {
+		loff_t pos;
+		loff_t endbyte;
+		ssize_t status;
+
+		iocb->ki_flags &= ~IOCB_DIRECT;
+		pos = iocb->ki_pos;
+		status = iomap_file_buffered_write(iocb, from, ops,
+			NULL, NULL);
+		if (unlikely(status < 0)) {
+			ret = status;
+			goto out_unlock;
+		}
+
+		ret += status;
+		endbyte = pos + status - 1;
+		status = filemap_write_and_wait_range(inode->i_mapping, pos, endbyte);
+		if (!status) {
+			invalidate_mapping_pages(inode->i_mapping,
+				pos >> PAGE_SHIFT,
+				endbyte >> PAGE_SHIFT);
+			if (ret > 0)
+				ret = generic_write_sync(iocb, ret);
+		} else {
+			ret = status;
+		}
+	}
+
+out_unlock:
+	inode_unlock(inode);
+	return ret;
+}
+
+static ssize_t minix_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
+{
+	if (iocb->ki_flags & IOCB_DIRECT)
+		return minix_dio_read_iter(iocb, to);
+
+	return generic_file_read_iter(iocb, to);
+}
+
+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;
+
+	/* minix_dio_write_iter also locks the inode and appears to do the same
+	 * general sorts of checks as this, so just return directly from there.
+	 */
+	if (iocb->ki_flags & IOCB_DIRECT)
+		return minix_dio_write_iter(iocb, from);
+
+	inode_lock(inode);
+	ret = generic_write_checks(iocb, from);
+	if (ret <= 0)
+		goto unlock;
+
+	ret = file_modified(iocb->ki_filp);
+	if (ret)
+		goto unlock;
+
+	const struct iomap_ops *ops = minix_iomap_ops_ver(inode);
+
+	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;
+}
+
+static int minix_file_open(struct inode *inode, struct file *filp)
+{
+	filp->f_mode |= FMODE_CAN_ODIRECT;
+	return generic_file_open(inode, filp);
+}
+
 /*
- * We have mostly NULLs here: the current defaults are OK for
- * the minix filesystem.
+ * We still have some NULLs here, but not as many of the current defaults are
+ * still OK for the minix filesystem.
  */
+
 const struct file_operations minix_file_operations = {
 	.llseek		= generic_file_llseek,
-	.read_iter	= generic_file_read_iter,
-	.write_iter	= generic_file_write_iter,
+	.read_iter	= minix_file_read_iter,
+	.write_iter	= minix_file_write_iter,
 	.mmap_prepare	= generic_file_mmap_prepare,
+	.open		= minix_file_open,
 	.fsync		= minix_fsync,
 	.splice_read	= filemap_splice_read,
+	.splice_write	= iter_file_splice_write,
 };
 
-static int minix_setattr(struct mnt_idmap *idmap,
-			 struct dentry *dentry, struct iattr *attr)
+int minix_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
+	struct iattr *attr)
 {
 	struct inode *inode = d_inode(dentry);
 	int error;
diff --git a/fs/minix/minix.h b/fs/minix/minix.h
index 77e503cca97f..76718f789369 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);
 int minix_fsync(struct file *file, loff_t start, loff_t end, int datasync);
-- 
2.47.3

[PATCH v2 4/4] minix: fix symlink and truncate for iomap

[Jeremy Bingham]

The original minix iomap conversion patches had a couple of issues
exposed by syzbot:

1. NULL pointer dereferences in page_symlink. Since the iomap based
minix_aops no longer used write_begin and write_end, when page_link
attempted to call aops->write_begin there was a crash because there was
no longer a pointer assigned to aops->write_begin. This was fixed with a
custom function that writes the symlink target directly to a newly
allocated data block acquired with minix_new_block and sb_getblk, thus
bypassing the aops write path entirely. The symlink read path was in
turn replaced with a new minix_get_link that reads the target from the
first data block directly with sb_bread, similar to how ext4_get_link
does it.

2. Truncate crashed. There was a conflict in minix's truncate() function
between buffer_head and iomap where block_truncate_page was attaching
buffer_heads to folios, but this ran up against how iomap works. Fixed
this by using iomap_truncate_page for inodes using minix_aops and
block_truncate_page for inodes with minix_dir_aops.

Also, minix_symlink_inode_operations now has .setattr = minix_setattr so
symlinks can be truncated more properly through iomap rather than using
the default simple_setattr.

Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
---
 fs/minix/inode.c        |   4 +-
 fs/minix/itree_common.c |  11 +++-
 fs/minix/minix.h        |   5 +-
 fs/minix/namei.c        | 137 +++++++++++++++++++++++++++++++++++++++-
 4 files changed, 152 insertions(+), 5 deletions(-)

diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index 2ba6766fce51..b113c44764ff 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -573,8 +573,9 @@ static const struct address_space_operations minix_dir_aops = {
 };
 
 static const struct inode_operations minix_symlink_inode_operations = {
-	.get_link	= page_get_link,
+	.get_link	= minix_get_link,
 	.getattr	= minix_getattr,
+	.setattr	= minix_setattr,
 };
 
 void minix_set_inode(struct inode *inode, dev_t rdev)
@@ -838,4 +839,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..5a8b73a7beda 100644
--- a/fs/minix/itree_common.c
+++ b/fs/minix/itree_common.c
@@ -311,7 +311,16 @@ 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 what address space operations are being used by the
+	 * inode being truncated, we need to either call iomap_truncate_page or
+	 * block_truncate_page.
+	 */
+	if (inode->i_mapping->a_ops == &minix_aops)
+		iomap_truncate_page(inode, inode->i_size, NULL,
+			minix_iomap_ops_ver(inode), NULL, NULL);
+	else
+		block_truncate_page(inode->i_mapping, inode->i_size, get_block);
 
 	n = block_to_path(inode, iblock, offsets);
 	if (!n)
diff --git a/fs/minix/minix.h b/fs/minix/minix.h
index 76718f789369..d1a890e96abe 100644
--- a/fs/minix/minix.h
+++ b/fs/minix/minix.h
@@ -57,7 +57,7 @@ int minix_new_block(struct inode *inode);
 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);
+		struct kstat *, u32, unsigned);
 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);
@@ -82,6 +82,9 @@ 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);
+extern const char *minix_get_link(struct dentry *dentry, struct inode *inode,
+		struct delayed_call *callback);
+
 struct minix_dir_entry *minix_dotdot(struct inode*, struct folio **);
 ino_t minix_inode_by_name(struct dentry*);
 
diff --git a/fs/minix/namei.c b/fs/minix/namei.c
index 263e4ba8b1c8..e245f55a68ff 100644
--- a/fs/minix/namei.c
+++ b/fs/minix/namei.c
@@ -6,6 +6,8 @@
  */
 
 #include "minix.h"
+#include <linux/buffer_head.h>
+#include <linux/namei.h>
 
 static int add_nondir(struct dentry *dentry, struct inode *inode)
 {
@@ -69,6 +71,101 @@ static int minix_create(struct mnt_idmap *idmap, struct inode *dir,
 	return minix_mknod(&nop_mnt_idmap, dir, dentry, mode, 0);
 }
 
+static inline u16 *v1_i_data(struct inode *inode)
+{
+	return (u16 *)minix_i(inode)->u.i1_data;
+}
+
+static inline u32 *v2_i_data(struct inode *inode)
+{
+	return (u32 *)minix_i(inode)->u.i2_data;
+}
+
+static inline u16 cpu_to_v1_block(sector_t n)
+{
+	return n;
+}
+
+static inline u32 cpu_to_v2_block(sector_t n)
+{
+	return n;
+}
+
+static inline sector_t v1_block_to_cpu(u16 n)
+{
+	return n;
+}
+
+static inline sector_t v2_block_to_cpu(u32 n)
+{
+	return n;
+}
+
+/* Reimplement page_symlink's general logic while avoiding using buffer head
+ * based aops operations like aops->write_begin so things behave better with
+ * the new regime of iomap based aops operations. Cribbing from page_symlink in
+ * fs/namei.c and ext4's ext4_init_symlink_block.
+ */
+static int __page_symlink(struct inode *inode, const char *symname, int len)
+{
+	struct super_block *sb = inode->i_sb;
+	struct buffer_head *bh;
+	char *kaddr;
+	int err = 0;
+	u16 *p16; /* v1 16 bit block */
+	u32 *p32; /* v2/3 32 bit block */
+
+	sector_t phys;
+
+	phys = minix_new_block(inode);
+	if (!phys) {
+		err = -ENOSPC;
+		goto ps_out;
+	}
+
+	if (INODE_VERSION(inode) == MINIX_V1) {
+		p16 = v1_i_data(inode);
+		*p16 = cpu_to_v1_block(phys);
+	} else {
+		p32 = v2_i_data(inode);
+		*p32 = cpu_to_v2_block(phys);
+	}
+
+	bh = sb_getblk(sb, phys);
+	if (!bh) {
+		err = -ENOMEM;
+		goto ps_fail;
+	}
+
+	lock_buffer(bh);
+	kaddr = (char *)bh->b_data;
+	memset(kaddr, 0, sb->s_blocksize);
+	memcpy(kaddr, symname, len);
+	inode->i_size = len - 1;
+	set_buffer_uptodate(bh);
+	unlock_buffer(bh);
+
+	mmb_mark_buffer_dirty(bh, &minix_i(inode)->i_metadata_bhs);
+	if (inode_needs_sync(inode)) {
+		sync_dirty_buffer(bh);
+		if (buffer_req(bh) && !buffer_uptodate(bh)) {
+			pr_err("i/o error syncing itable block");
+			err = -EIO;
+		}
+
+	}
+
+	mark_inode_dirty(inode);
+	brelse(bh);
+
+ps_out:
+	return err;
+
+ps_fail:
+	minix_free_block(inode, phys);
+	goto ps_out;
+}
+
 static int minix_symlink(struct mnt_idmap *idmap, struct inode *dir,
 			 struct dentry *dentry, const char *symname)
 {
@@ -84,7 +181,7 @@ 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 = __page_symlink(inode, symname, i);
 	if (unlikely(err)) {
 		inode_dec_link_count(inode);
 		iput(inode);
@@ -273,6 +370,44 @@ static int minix_rename(struct mnt_idmap *idmap,
 	return err;
 }
 
+/* straight up thievery here; stolen verbatim from ext4_get_link */
+static void minix_free_link(void *bh)
+{
+	brelse(bh);
+}
+
+/* Borrowing from ext4_get_link to a degree; since minix inodes and symlinks
+ * are significantly simpler, we don't need to do nearly as much as ext4
+ * requires for old-timey ext4 slow links.
+ */
+const char *minix_get_link(struct dentry *dentry, struct inode *inode,
+		struct delayed_call *callback)
+{
+	struct super_block *sb = inode->i_sb;
+	struct buffer_head *bh;
+	sector_t blk;
+
+	/* Get yon block, depending on what version of the minix fs this is. */
+	if (INODE_VERSION(inode) == MINIX_V1)
+		blk = v1_block_to_cpu(*(v1_i_data(inode)));
+	else
+		blk = v2_block_to_cpu(*(v2_i_data(inode)));
+
+	bh = sb_bread(sb, blk);
+	if (IS_ERR(bh))
+		return ERR_CAST(bh);
+	if (!bh) {
+		pr_err("bad symlink on inode %llu", inode->i_ino);
+		return ERR_PTR(-EFSCORRUPTED);
+	}
+
+	set_delayed_call(callback, minix_free_link, bh);
+	nd_terminate_link(bh->b_data, inode->i_size,
+			inode->i_sb->s_blocksize - 1);
+
+	return bh->b_data;
+}
+
 /*
  * directories can handle most operations...
  */
-- 
2.47.3

[syzbot ci] Re: minix: convert to iomap and add direct I/O

[syzbot ci]

syzbot ci has tested the following series

[v2] minix: convert to iomap and add direct I/O
https://lore.kernel.org/all/cover.1782619718.git.jbingham@gmail.com
* [PATCH v2 1/4] minix: add iomap infrastructure
* [PATCH v2 2/4] minix: convert address space operations to iomap
* [PATCH v2 3/4] minix: convert file operations to iomap and add
* [PATCH v2 4/4] minix: fix symlink and truncate for iomap

and found the following issues:
* BUG: sleeping function called from invalid context in bdev_getblk
* BUG: sleeping function called from invalid context in find_get_block_common

Full report is available here:
https://ci.syzbot.org/series/3b663c96-cf07-46e0-9b8d-785fe615029d

***

BUG: sleeping function called from invalid context in bdev_getblk

tree:      torvalds
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux
base:      780d569e6c4b422290f5cba319eb904b355d64be
arch:      amd64
compiler:  Debian clang version 22.1.6 (++20260514074242+fc4aad7b5db3-1~exp1~20260514074407.73), Debian LLD 22.1.6
config:    https://ci.syzbot.org/builds/61f38821-60a2-4c7c-a39e-27291869d296/config
syz repro: https://ci.syzbot.org/findings/dda4e5c2-7cbc-498a-98ea-83cad3c680e3/syz_repro

         option from the mount to silence this warning.
=======================================================
BUG: sleeping function called from invalid context at ./include/linux/sched/mm.h:323
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5865, name: syz.1.18
preempt_count: 0, expected: 0
RCU nest depth: 1, expected: 0
1 lock held by syz.1.18/5865:
 #0: ffffffff8e959c20 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire include/linux/rcupdate.h:300 [inline]
 #0: ffffffff8e959c20 (rcu_read_lock){....}-{1:3}, at: rcu_read_lock include/linux/rcupdate.h:840 [inline]
 #0: ffffffff8e959c20 (rcu_read_lock){....}-{1:3}, at: path_init+0x124/0x1330 fs/namei.c:2689
CPU: 1 UID: 0 PID: 5865 Comm: syz.1.18 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
 __might_resched+0x378/0x4d0 kernel/sched/core.c:9197
 might_alloc include/linux/sched/mm.h:323 [inline]
 bdev_getblk+0xce/0x6e0 fs/buffer.c:1435
 __bread_gfp+0x89/0x380 fs/buffer.c:1493
 sb_bread include/linux/buffer_head.h:348 [inline]
 minix_get_link+0x143/0x340 fs/minix/namei.c:396
 pick_link+0x782/0xfe0 fs/namei.c:2064
 step_into_slowpath+0x58a/0x820 fs/namei.c:2127
 step_into fs/namei.c:2152 [inline]
 open_last_lookups fs/namei.c:4643 [inline]
 path_openat+0x224e/0x3830 fs/namei.c:4856
 do_file_open+0x23e/0x4a0 fs/namei.c:4888
 do_sys_openat2+0x115/0x200 fs/open.c:1368
 do_sys_open fs/open.c:1374 [inline]
 __do_sys_openat fs/open.c:1390 [inline]
 __se_sys_openat fs/open.c:1385 [inline]
 __x64_sys_openat+0x138/0x170 fs/open.c:1385
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f9618b9ce59
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f9619a29028 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 00007f9618e15fa0 RCX: 00007f9618b9ce59
RDX: 0000000000105042 RSI: 0000200000000080 RDI: ffffffffffffff9c
RBP: 00007f9618c32e6f R08: 0000000000000000 R09: 0000000000000000
R10: 00000000000001ff R11: 0000000000000246 R12: 0000000000000000
R13: 00007f9618e16038 R14: 00007f9618e15fa0 R15: 00007fffce8916b8
 </TASK>


***

BUG: sleeping function called from invalid context in find_get_block_common

tree:      torvalds
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux
base:      780d569e6c4b422290f5cba319eb904b355d64be
arch:      amd64
compiler:  Debian clang version 22.1.6 (++20260514074242+fc4aad7b5db3-1~exp1~20260514074407.73), Debian LLD 22.1.6
config:    https://ci.syzbot.org/builds/61f38821-60a2-4c7c-a39e-27291869d296/config
syz repro: https://ci.syzbot.org/findings/2af8da3b-836a-479e-9af0-fef473c2f315/syz_repro

loop2: detected capacity change from 0 to 64
bad symlink on inode 4
BUG: sleeping function called from invalid context at ./include/linux/pagemap.h:1155
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5813, name: syz.2.19
preempt_count: 0, expected: 0
RCU nest depth: 1, expected: 0
1 lock held by syz.2.19/5813:
 #0: ffffffff8e959c20 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire include/linux/rcupdate.h:300 [inline]
 #0: ffffffff8e959c20 (rcu_read_lock){....}-{1:3}, at: rcu_read_lock include/linux/rcupdate.h:840 [inline]
 #0: ffffffff8e959c20 (rcu_read_lock){....}-{1:3}, at: path_init+0x124/0x1330 fs/namei.c:2689
CPU: 1 UID: 0 PID: 5813 Comm: syz.2.19 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
 __might_resched+0x378/0x4d0 kernel/sched/core.c:9197
 folio_lock include/linux/pagemap.h:1155 [inline]
 __find_get_block_slow fs/buffer.c:242 [inline]
 find_get_block_common+0x2d0/0xe10 fs/buffer.c:1386
 bdev_getblk+0x58/0x6e0 include/linux/gfp.h:-1
 __bread_gfp+0x89/0x380 fs/buffer.c:1493
 sb_bread include/linux/buffer_head.h:348 [inline]
 minix_get_link+0x143/0x340 fs/minix/namei.c:396
 pick_link+0x782/0xfe0 fs/namei.c:2064
 step_into_slowpath+0x58a/0x820 fs/namei.c:2127
 step_into fs/namei.c:2152 [inline]
 open_last_lookups fs/namei.c:4643 [inline]
 path_openat+0x224e/0x3830 fs/namei.c:4856
 do_file_open+0x23e/0x4a0 fs/namei.c:4888
 do_sys_openat2+0x115/0x200 fs/open.c:1368
 do_sys_open fs/open.c:1374 [inline]
 __do_sys_openat fs/open.c:1390 [inline]
 __se_sys_openat fs/open.c:1385 [inline]
 __x64_sys_openat+0x138/0x170 fs/open.c:1385
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fd37ef9ce59
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fd37fe06028 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 00007fd37f215fa0 RCX: 00007fd37ef9ce59
RDX: 0000000000000042 RSI: 0000200000000080 RDI: ffffffffffffff9c
RBP: 00007fd37f032e6f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fd37f216038 R14: 00007fd37f215fa0 R15: 00007ffe5e5f14f8
 </TASK>
bad symlink on inode 4


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a patch for this bug, please reply with `#syz test`
(should be on a separate line).

The patch should be attached to the email.
Note: arguments like custom git repos and branches are not supported.

Re: [syzbot ci] Re: minix: convert to iomap and add direct I/O

[Jeremy Bingham]

#syz test

minix: add the same dentry check as ext4_get_link to minix_get_link

It turns out that minix_get_link does need to have both the sb_bread and
the sb_getblk paths, like ext4_get_link does. It working with just the
sb_bread one initially was deceptive.
---
 fs/minix/namei.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/fs/minix/namei.c b/fs/minix/namei.c
index e245f55a68ff..a4caaa3f7c9a 100644
--- a/fs/minix/namei.c
+++ b/fs/minix/namei.c
@@ -393,12 +393,23 @@ const char *minix_get_link(struct dentry *dentry, struct inode *inode,
 	else
 		blk = v2_block_to_cpu(*(v2_i_data(inode)));
 
-	bh = sb_bread(sb, blk);
-	if (IS_ERR(bh))
-		return ERR_CAST(bh);
-	if (!bh) {
-		pr_err("bad symlink on inode %llu", inode->i_ino);
-		return ERR_PTR(-EFSCORRUPTED);
+	/* This tried dodging the dentry check that ext4 does, but it turns out
+	 * that it's necessary after all.
+	 */
+	if (!dentry) {
+		bh = sb_getblk(sb, blk);
+		if (!bh || !buffer_uptodate(bh)) {
+			brelse(bh);
+			return ERR_PTR(-ECHILD);
+		}
+	} else {
+		bh = sb_bread(sb, blk);
+		if (IS_ERR(bh))
+			return ERR_CAST(bh);
+		if (!bh) {
+			pr_err("bad symlink on inode %llu", inode->i_ino);
+			return ERR_PTR(-EFSCORRUPTED);
+		}
 	}
 
 	set_delayed_call(callback, minix_free_link, bh);
-- 
2.47.3

[syzbot ci] Re: minix: convert to iomap and add direct I/O

[syzbot ci]

syzbot ci has tested the suggested fix patch on top of the following series:

[v2] minix: convert to iomap and add direct I/O
https://lore.kernel.org/all/cover.1782619718.git.jbingham@gmail.com

Patch: https://ci.syzbot.org/jobs/fb0b2cff-4f4e-442b-a6f8-5fa767f200fc/patch
Testing results:
* [build 0] Build Patched: passed
* [build 0] Boot test: Patched: passed

Full report is available here:
https://ci.syzbot.org/session/4d7ba7ce-a1cf-4df0-b856-6be6475a5498

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

Re: [PATCH v2 0/4] minix: convert to iomap and add direct I/O

[Darrick J. Wong]

On Sat, Jun 27, 2026 at 10:15:52PM -0700, Jeremy Bingham wrote:
> This is v2 of the minix iomap conversion series. The original v1
> submission (3 patches) was tested by syzbot, which found four issues.
> Three were straightforward; the fourth, a null pointer dereference in
> page_symlink when creating symlinks, required more substantial changes.
> 
> The original description follows:
> 
> This series converts the minix filesystem from the buffer_head-based
> I/O path to the iomap API, and adds direct I/O support in the process.
> 
> The conversion is straightforward: minix's indirect block tree mapping
> logic (from itree_common.c) is reworked into an iomap_begin/iomap_end
> implementation. The iomap_end callback is a no-op since minix has no
> extents or transactions to finalize.
> 
> Patch 1 adds the iomap infrastructure: the new iomap.c file, wrapper
> functions and iomap_ops structs in itree_v1.c and itree_v2.c, and the
> relevant declarations in minix.h.
> 
> The iomap.c file is #include'd into itree_v1.c and itree_v2.c rather
> than compiled as a standalone translation unit. This is because the
> minix filesystem versions (V1 vs V2/V3) have different block_t sizes
> (16-bit vs 32-bit) and different indirect tree depths. This follows
> the existing pattern in minix where itree_common.c is included into
> both itree_v1.c and itree_v2.c. Each version provides a thin wrapper
> and a corresponding iomap_ops struct.

Yuck.  I guess that's /one/ way to avoid having a geometry struct
capturing those details... :(

> Patch 2 converts the regular file address space operations to iomap:
> read_folio, readahead, writepages (with a writeback callback), bmap,
> and folio lifecycle helpers. Directory inodes continue to use
> buffer_head-based operations via a new minix_dir_aops, since directory
> handling still relies on buffer head chunks for prepare/write_begin.
> 
> Patch 3 converts the file_operations: replacing the generic read/write
> iterators with iomap-aware versions, adding direct I/O read/write paths
> using iomap_dio_rw, and setting FMODE_CAN_ODIRECT in the open handler.
> 
> The minix iomap implementation was adapted from the out-of-tree xiafs
> iomap conversion. The xiafs module itself borrowed heavily from the
> modernized minix kernel module. The exfat iomap changes were an
> additional reference for both conversions.
> 
> Changes since v1:
> 
>   * Added a fourth patch to fix the symlink and truncate issues:
>     - Replaced page_symlink with a custom __page_symlink that writes
>       the target directly to a data block via minix_new_block +

Sounds to me like it's time to write iomap_write_symlink.

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	= target,
		.iov_len	= len,
	};
	struct iomap_iter iter = {
		.inode		= inode,
		.pos		= 0,
		.len		= len,
		.flags		= IOMAP_WRITE,
		.private	= private,
	};
	struct iov_iter iov;
	int ret;

	iov_iter_kvec(&iov, ITER_SRC, &vec, 1, iov.iov_len);

	while ((ret = iomap_iter(&iter, ops)) > 0)
		iter.status = iomap_write_iter(&iter, &iov, write_ops);

	if (unlikely(iter.pos == 0))
		return ret;

	mark_inode_dirty(inode);
	return 0;
}
EXPORT_SYMBOL_GPL(iomap_symlink_write);


>       sb_getblk, bypassing the aops write path (which no longer has
>       write_begin/write_end). Added a matching custom minix_get_link
>       that reads the target from the data block via sb_bread, similar
>       to ext4_get_link. No iomap-based filesystem in the kernel uses
>       page_symlink; XFS, GFS2, and ext4 all handle symlink storage

That's because they embed headers and crcs in the symlink file data
and/or do fancy things with inline targets.  xfs has its own buffer
cache, so there's no need to duplicate it with the pagecache and then
have to interpret ondisk formats.

>       directly. The on-disk format is unchanged.
>     - Fixed a buffer_head/iomap type confusion in truncate:
>       block_truncate_page attaches buffer_heads to data folios, but
>       minix_aops now uses iomap which interprets folio->private as
>       struct iomap_folio_state. truncate() now dispatches between
>       iomap_truncate_page (for regular files/symlinks) and
>       block_truncate_page (for directories) based on the inode's aops.
>     - Added .setattr = minix_setattr to minix_symlink_inode_operations
>       so symlinks truncate properly through the iomap path.
> 
>   * Patch 1 (iomap infrastructure): minix_get_block is now exported
>     (non-static) so the directory aops and iomap writeback path can
>     use it. Added minix_iomap_ops_ver() inline helper and extern
>     declarations for minix_aops and the version-specific iomap_ops.
>     Fixed unsigned -> unsigned int in minix_blocks_needed and
>     minix_find_first_zero_bit to silence checkpatch warnings.
> 
>   * Patch 2 (aops conversion): unchanged in approach; minor cleanup
>     of the writeback callback and minix_bmap conversion.
> 
>   * Patch 3 (file operations): minix_setattr is now exported for reuse
>     by the symlink inode operations in patch 4.
> 
> Testing: the full series has been tested with mkfs.minix V1/V2/V3,
> exercising file creation, read/write, overwrite, append, binary data,
> directories, symlinks (full path, relative, directory symlinks), hard
> links, truncation (shrink/grow), large files (1MB, exercising indirect
> blocks), deep nesting (20 levels), 100 files in one directory,
> deletions, remount persistence, and fsck.minix. All pass cleanly. The
> four syzbot-reported issues are resolved.
> 
> Jeremy Bingham (4):
>   minix: add iomap infrastructure
>   minix: convert address space operations to iomap
>   minix: convert file operations to iomap and add direct I/O
>   minix: fix symlilnk and truncate for iomap compatibility
> 
>  fs/minix/file.c         | 157 ++++++++++++++++++++++++++++++++++++++--
>  fs/minix/inode.c        |  90 ++++++++++++++++++++---
>  fs/minix/iomap.c        | 114 +++++++++++++++++++++++++++++
>  fs/minix/itree_common.c |  11 ++-
>  fs/minix/itree_v1.c     |  25 ++++++-
>  fs/minix/itree_v2.c     |  17 ++++-
>  fs/minix/minix.h        |  30 +++++++-
>  fs/minix/namei.c        | 137 ++++++++++++++++++++++++++++++++++-
>  8 files changed, 558 insertions(+), 23 deletions(-)
>  create mode 100644 fs/minix/iomap.c
> 
> -- 
> 2.47.3
> 
> 

Re: [PATCH v2 1/4] minix: add iomap infrastructure

[Darrick J. Wong]

On Sat, Jun 27, 2026 at 10:15:53PM -0700, Jeremy Bingham wrote:
> Add an iomap implementation for minix, adapted from the out-of-tree
> xiafs module's iomap code. The xiafs module, in turn, borrowed heavily
> from minix's own code.
> 
> The iomap_begin function replaces get_block for the iomap path:
> it walks the indirect block tree, allocates blocks when needed, and
> returns the mapping in a struct iomap. The iomap_end function is a
> nop since minix has no extents or transactions to finalize.
> 
> Because minix has two filesystem versions (V1 with 16-bit block
> numbers, V2/V3 with 32-bit) that share itree_common.c via #include,
> the iomap.c file follows the same pattern. It is #included into
> itree_v1.c and itree_v2.c so it can use the version-specific block_t,
> block_to_cpu, and cpu_to_block definitions directly.
> 
> Each version gets its own iomap_ops struct (V1_minix_iomap_ops,
> V2_minix_iomap_ops) with thin wrappers around the shared
> minix_iomap_begin/minix_iomap_end. A minix_iomap_ops_ver() helper
> selects the correct ops based on the filesystem version.
> 
> minix_get_block is exported from inode.c for use by the iomap
> writeback path and the directory aops that still use buffer_heads.
> 
> Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
> ---
>  fs/minix/iomap.c    | 114 ++++++++++++++++++++++++++++++++++++++++++++
>  fs/minix/itree_v1.c |  25 +++++++++-
>  fs/minix/itree_v2.c |  17 ++++++-
>  fs/minix/minix.h    |  23 ++++++++-
>  4 files changed, 175 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..7bb0439e3669
> --- /dev/null
> +++ b/fs/minix/iomap.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * iomap functions for minix. At least the first pass of this file was taken
> + * from the xiafs iomap.c, which is fitting since the xiafs module in turn
> + * borrowed heavily from the modernized minix fs kernel module.

Is this the "modernized" minix fs kernel module??

xiafs hasn't been in the kernel for years; I see little point in
mentioning an externally maintained driver inside the kernel source.

> + */
> +
> +/*
> + * 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. This is taken
> + * directly from the out-of-tree xiafs iomap changes, and the exfat iomap
> + * changes were an inspiration for that.
> + */
> +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;
> +
> +	/* Mostly taken from modern-xiafs itree.c get_block with elements from
> +	 * similar exfat operations.
> +	 */
> +	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)
> +		goto out;
> +
> +	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) {
> +		/* Bit of a weird order, but it'll make sense when you get to
> +		 * the bottom.
> +		 */
> +		iomap->flags = IOMAP_F_MERGED;
> +got_it:
> +		phys = block_to_cpu(chain[depth - 1].key);
> +		partial = chain+depth-1;
> +		/* Set up the iomap struct before cleaning up */
> +		iomap->type = IOMAP_MAPPED;
> +		iomap->addr = (u64)phys << blkbits;
> +		iomap->length = 1 << blkbits;
> +		iomap->offset = (u64)iblock << blkbits;
> +		goto cleanup;
> +	}
> +
> +	/* Next simple case - plain lookup or failed read of indirect block */
> +	if (!create || err == -EIO) {
> +		iomap->type = IOMAP_HOLE;
> +		iomap->addr = IOMAP_NULL_ADDR;
> +		iomap->length = 1 << blkbits;
> +		iomap->offset = (u64)iblock << blkbits;
> +		iomap->flags = 0;
> +cleanup:
> +		while (partial > chain) {
> +			brelse(partial->bh);
> +			partial--;
> +		}

Uhhh ... these three ought to be factored out into separate helpers to
(a) clean up @chain, (b) set up a IOMAP_MAPPED mapping, and (c) set up a
IOMAP_HOLE mapping.  Don't write a pile of spaghetti goes.

Also kinda surprised that you set IOMAP_F_MERGED but don't try to look
forward to find adjacent blocks?

> +out:
> +		return err;
> +	}
> +
> +	/*
> +	 * Indirect block might be removed by truncate while we were
> +	 * reading it. Handling of that case (forget what we've got and
> +	 * reread) is taken out of the main path.

/me wonders how we'd be racing with setattr?

--D

> +	 */
> +	if (err == -EAGAIN)
> +		goto changed;
> +
> +	left = (chain + depth) - partial;
> +	err = alloc_branch(inode, left, offsets + (partial - chain), partial);
> +	if (err)
> +		goto cleanup;
> +
> +	if (splice_branch(inode, chain, partial, left) < 0)
> +		goto changed;
> +
> +	/* Successful allocation, mapping it. */
> +	iomap->flags = IOMAP_F_NEW;
> +	goto got_it;
> +
> +changed:
> +	while (partial > chain) {
> +		brelse(partial->bh);
> +		partial--;
> +	}
> +	goto reread;
> +}
> +
> +/*
> + * 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 f2025c9b5825..77e503cca97f 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 */
> @@ -69,6 +70,8 @@ extern int V1_minix_get_block(struct inode *, long, struct buffer_head *, int);
>  extern int V2_minix_get_block(struct inode *, long, struct buffer_head *, int);
>  extern unsigned V1_minix_blocks(loff_t, struct super_block *);
>  extern unsigned V2_minix_blocks(loff_t, struct super_block *);
> +extern int minix_get_block(struct inode *inode, sector_t block,
> +		    struct buffer_head *bh_result, int create);
>  
>  struct minix_dir_entry *minix_find_entry(struct dentry *, struct folio **);
>  int minix_add_link(struct dentry*, struct inode*);
> @@ -80,10 +83,20 @@ int minix_set_link(struct minix_dir_entry *de, struct folio *folio,
>  struct minix_dir_entry *minix_dotdot(struct inode*, struct folio **);
>  ino_t minix_inode_by_name(struct dentry*);
>  
> +extern int V1_minix_iomap_begin(struct inode *inode, loff_t offset,
> +	loff_t length, unsigned int flags, struct iomap *iomap,
> +	struct iomap *srcmap);
> +extern 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 address_space_operations minix_aops;
>  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)
>  {
> @@ -95,11 +108,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(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)
>  
> @@ -129,7 +148,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
> 
> 

Re: [PATCH v2 2/4] minix: convert address space operations to iomap

[Darrick J. Wong]

On Sat, Jun 27, 2026 at 10:15:54PM -0700, Jeremy Bingham wrote:
> Convert minix regular file and symlink address space operations from
> buffer_head to iomap. The new minix_aops uses iomap_dirty_folio,
> iomap_invalidate_folio, iomap_bio_read_folio, iomap_bio_readahead,
> iomap_writepages, iomap_bmap, and related iomap helpers.
> The write_begin/write_end callbacks are removed since buffered writes
> now go through iomap_file_buffered_write in file.c.
> 
> Directories keep using buffer_heads via a new minix_dir_aops, which
> retains the old block_dirty_folio, block_read_full_folio,
> block_write_begin, generic_write_end, and mpage_writepages. This is
> necessary because directory entry manipulation (minix_prepare_chunk,
> minix_write_begin) still uses the buffer_head chunk protocol.
> 
> minix_bmap is converted from generic_block_bmap to iomap_bmap.

I'd drop BMAP support entirely, unless you know of people who use LILO
and minixfs.  If all you want is swapfile activation, use
iomap_swapfile_activate... assuming that anyone actually cares about
hosting swapfiles on minixfs.

> The minix_get_block function is exported (non-static) so the
> directory aops can still use it for block_write_begin and
> mpage_writepages.
> 
> Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
> ---
>  fs/minix/inode.c | 86 +++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 78 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> index c30cc590698d..2ba6766fce51 100644
> --- a/fs/minix/inode.c
> +++ b/fs/minix/inode.c
> @@ -436,7 +436,32 @@ static int minix_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	return 0;
>  }
>  
> -static int minix_get_block(struct inode *inode, sector_t block,
> +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,
> +};
> +
> +int minix_get_block(struct inode *inode, sector_t block,
>  		    struct buffer_head *bh_result, int create)
>  {
>  	if (INODE_VERSION(inode) == MINIX_V1)
> @@ -445,17 +470,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);
> @@ -487,19 +540,36 @@ static int minix_write_begin(const struct kiocb *iocb,
>  
>  static sector_t minix_bmap(struct address_space *mapping, sector_t block)
>  {
> -	return generic_block_bmap(mapping,block,minix_get_block);
> +	const struct iomap_ops *ops = minix_iomap_ops_ver(mapping->host);
> +
> +	return iomap_bmap(mapping, block, ops);
>  }
>  
> -static const struct address_space_operations minix_aops = {
> -	.dirty_folio	= block_dirty_folio,
> -	.invalidate_folio = block_invalidate_folio,
> +const struct address_space_operations minix_aops = {
> +	.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,
> +	.bmap = minix_bmap,
> +	.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

I forget, does one have to set FMODE_CAN_ODIRECT if the address_space
operations don't supply a ->direct_IO function?

--D

> +	.writepages = minix_block_writepages,
>  };
>  
>  static const struct inode_operations minix_symlink_inode_operations = {
> @@ -516,7 +586,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);
> -- 
> 2.47.3
> 
> 

Re: [PATCH v2 3/4] minix: convert file operations to iomap and add

[Darrick J. Wong]

On Sat, Jun 27, 2026 at 10:15:55PM -0700, Jeremy Bingham wrote:
> Subject: [PATCH v2 3/4] minix: convert file operations to iomap and add

...add what?

> Replace generic_file_read_iter and generic_file_write_iter with custom
> minix_file_read_iter and minix_file_write_iter that dispatch to iomap
> for both buffered and direct I/O.
> 
> Buffered writes now go through iomap_file_buffered_write instead of
> the aops write_begin/write_end path (which no longer exists for
> regular files). Buffered reads still use generic_file_read_iter
> for the non-DIO case.
> 
> Direct I/O is implemented via iomap_dio_rw for both reads and writes.
> minix_dio_read_iter takes a shared inode lock; minix_dio_write_iter
> takes an exclusive lock, does generic_write_checks, and falls back
> to buffered writes via iomap_file_buffered_write for the tail of a
> DIO write that is not block-aligned. The minix_dio_write_end_io
> callback updates i_size and marks the inode dirty.
> 
> minix_file_open sets FMODE_CAN_ODIRECT so the VFS allows O_DIRECT
> opens, and splice_write is added to the file operations.

Ignore my question about FMODE_CAN_ODIRECT in the previous patch, then.

> minix_setattr is exported (made non-static) so it can be shared by
> the symlink inode operations in a subsequent patch.
> 
> Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
> ---
>  fs/minix/file.c  | 157 +++++++++++++++++++++++++++++++++++++++++++++--
>  fs/minix/minix.h |   2 +
>  2 files changed, 153 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/minix/file.c b/fs/minix/file.c
> index 86e5943cd2ff..b07c853fa43a 100644
> --- a/fs/minix/file.c
> +++ b/fs/minix/file.c
> @@ -17,21 +17,166 @@ int minix_fsync(struct file *file, loff_t start, loff_t end, int datasync)
>  			start, end, datasync);
>  }
>  
> +static ssize_t minix_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
> +{
> +	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);
> +
> +	ret = iomap_dio_rw(iocb, to, ops, NULL, 0, NULL, 0);
> +	inode_unlock_shared(inode);
> +	return ret;
> +}
> +
> +static int minix_dio_write_end_io(struct kiocb *iocb, ssize_t size, int error,
> +		unsigned int flags)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	loff_t pos = iocb->ki_pos;
> +
> +	if (error)
> +		return error;
> +
> +	pos += size;
> +	if (size && pos > i_size_read(inode)) {
> +		i_size_write(inode, pos);
> +		mark_inode_dirty(inode);
> +	}
> +	return 0;
> +}
> +
> +static const struct iomap_dio_ops minix_dio_write_ops = {
> +	.end_io = minix_dio_write_end_io,
> +};
> +
> +static ssize_t minix_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
> +{
> +	struct inode *inode = iocb->ki_filp->f_mapping->host;
> +	ssize_t ret;
> +	unsigned int flags = 0;
> +	unsigned long blocksize = inode->i_sb->s_blocksize;
> +
> +	inode_lock(inode);
> +	ret = generic_write_checks(iocb, from);
> +	if (ret <= 0)
> +		goto out_unlock;
> +
> +	ret = kiocb_modified(iocb);
> +	if (ret)
> +		goto out_unlock;
> +
> +	if (iocb->ki_pos + iov_iter_count(from) > i_size_read(inode) ||
> +		!IS_ALIGNED(iocb->ki_pos | iov_iter_alignment(from), blocksize))
> +		flags |= IOMAP_DIO_FORCE_WAIT;
> +
> +	const struct iomap_ops *ops = minix_iomap_ops_ver(inode);
> +
> +	ret = iomap_dio_rw(iocb, from, ops,
> +		&minix_dio_write_ops, flags, NULL, 0);
> +	if (ret == -ENOTBLK)
> +		ret = 0; /* fallback to buffered */
> +
> +	if (ret >= 0 && iov_iter_count(from)) {
> +		loff_t pos;
> +		loff_t endbyte;
> +		ssize_t status;
> +
> +		iocb->ki_flags &= ~IOCB_DIRECT;

Why not set IOC_DSYNC here and let generic_write_sync do all the
flushing work for you?  There's no requirement to dump the pagecache
after a downgraded direct write, but if you want that, use
IOCB_DONTCACHE.

--D

> +		pos = iocb->ki_pos;
> +		status = iomap_file_buffered_write(iocb, from, ops,
> +			NULL, NULL);
> +		if (unlikely(status < 0)) {
> +			ret = status;
> +			goto out_unlock;
> +		}
> +
> +		ret += status;
> +		endbyte = pos + status - 1;
> +		status = filemap_write_and_wait_range(inode->i_mapping, pos, endbyte);
> +		if (!status) {
> +			invalidate_mapping_pages(inode->i_mapping,
> +				pos >> PAGE_SHIFT,
> +				endbyte >> PAGE_SHIFT);
> +			if (ret > 0)
> +				ret = generic_write_sync(iocb, ret);
> +		} else {
> +			ret = status;
> +		}
> +	}
> +
> +out_unlock:
> +	inode_unlock(inode);
> +	return ret;
> +}
> +
> +static ssize_t minix_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> +{
> +	if (iocb->ki_flags & IOCB_DIRECT)
> +		return minix_dio_read_iter(iocb, to);
> +
> +	return generic_file_read_iter(iocb, to);
> +}
> +
> +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;
> +
> +	/* minix_dio_write_iter also locks the inode and appears to do the same
> +	 * general sorts of checks as this, so just return directly from there.
> +	 */
> +	if (iocb->ki_flags & IOCB_DIRECT)
> +		return minix_dio_write_iter(iocb, from);
> +
> +	inode_lock(inode);
> +	ret = generic_write_checks(iocb, from);
> +	if (ret <= 0)
> +		goto unlock;
> +
> +	ret = file_modified(iocb->ki_filp);
> +	if (ret)
> +		goto unlock;
> +
> +	const struct iomap_ops *ops = minix_iomap_ops_ver(inode);
> +
> +	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;
> +}
> +
> +static int minix_file_open(struct inode *inode, struct file *filp)
> +{
> +	filp->f_mode |= FMODE_CAN_ODIRECT;
> +	return generic_file_open(inode, filp);
> +}
> +
>  /*
> - * We have mostly NULLs here: the current defaults are OK for
> - * the minix filesystem.
> + * We still have some NULLs here, but not as many of the current defaults are
> + * still OK for the minix filesystem.
>   */
> +
>  const struct file_operations minix_file_operations = {
>  	.llseek		= generic_file_llseek,
> -	.read_iter	= generic_file_read_iter,
> -	.write_iter	= generic_file_write_iter,
> +	.read_iter	= minix_file_read_iter,
> +	.write_iter	= minix_file_write_iter,
>  	.mmap_prepare	= generic_file_mmap_prepare,
> +	.open		= minix_file_open,
>  	.fsync		= minix_fsync,
>  	.splice_read	= filemap_splice_read,
> +	.splice_write	= iter_file_splice_write,
>  };
>  
> -static int minix_setattr(struct mnt_idmap *idmap,
> -			 struct dentry *dentry, struct iattr *attr)
> +int minix_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> +	struct iattr *attr)
>  {
>  	struct inode *inode = d_inode(dentry);
>  	int error;
> diff --git a/fs/minix/minix.h b/fs/minix/minix.h
> index 77e503cca97f..76718f789369 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);
>  int minix_fsync(struct file *file, loff_t start, loff_t end, int datasync);
> -- 
> 2.47.3
> 
> 

Re: [PATCH v2 1/4] minix: add iomap infrastructure

[Jeremy Bingham]

On Wed, Jul 1, 2026 at 11:06 AM Darrick J. Wong <djwong@kernel.org> wrote:
> > diff --git a/fs/minix/iomap.c b/fs/minix/iomap.c
> > new file mode 100644
> > index 000000000000..7bb0439e3669
> > --- /dev/null
> > +++ b/fs/minix/iomap.c
> > @@ -0,0 +1,114 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * iomap functions for minix. At least the first pass of this file was taken
> > + * from the xiafs iomap.c, which is fitting since the xiafs module in turn
> > + * borrowed heavily from the modernized minix fs kernel module.
>
> Is this the "modernized" minix fs kernel module??

I guess it is, yeah, in that it's been updated to use iomap. The rest
of it remains pretty
ancient.

> xiafs hasn't been in the kernel for years; I see little point in
> mentioning an externally maintained driver inside the kernel source.

I simply mentioned it for context on where the iomap conversion originally came
from, that's all.

> > + */
> > +
> > +/*
> > + * 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. This is taken
> > + * directly from the out-of-tree xiafs iomap changes, and the exfat iomap
> > + * changes were an inspiration for that.
> > + */
> > +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;
> > +
> > +     /* Mostly taken from modern-xiafs itree.c get_block with elements from
> > +      * similar exfat operations.
> > +      */
> > +     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)
> > +             goto out;
> > +
> > +     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) {
> > +             /* Bit of a weird order, but it'll make sense when you get to
> > +              * the bottom.
> > +              */
> > +             iomap->flags = IOMAP_F_MERGED;
> > +got_it:
> > +             phys = block_to_cpu(chain[depth - 1].key);
> > +             partial = chain+depth-1;
> > +             /* Set up the iomap struct before cleaning up */
> > +             iomap->type = IOMAP_MAPPED;
> > +             iomap->addr = (u64)phys << blkbits;
> > +             iomap->length = 1 << blkbits;
> > +             iomap->offset = (u64)iblock << blkbits;
> > +             goto cleanup;
> > +     }
> > +
> > +     /* Next simple case - plain lookup or failed read of indirect block */
> > +     if (!create || err == -EIO) {
> > +             iomap->type = IOMAP_HOLE;
> > +             iomap->addr = IOMAP_NULL_ADDR;
> > +             iomap->length = 1 << blkbits;
> > +             iomap->offset = (u64)iblock << blkbits;
> > +             iomap->flags = 0;
> > +cleanup:
> > +             while (partial > chain) {
> > +                     brelse(partial->bh);
> > +                     partial--;
> > +             }
>
> Uhhh ... these three ought to be factored out into separate helpers to
> (a) clean up @chain, (b) set up a IOMAP_MAPPED mapping, and (c) set up a
> IOMAP_HOLE mapping.  Don't write a pile of spaghetti goes.
>
> Also kinda surprised that you set IOMAP_F_MERGED but don't try to look
> forward to find adjacent blocks?

That's fair. I was sticking to the same general pattern used in get_block() in
itree_common.c, but just because it's done that way in the older
function doesn't
mean that it needs to stay that way. I'll split that up.

Thank you for the comments,

-j

> > +out:
> > +             return err;
> > +     }
> > +
> > +     /*
> > +      * Indirect block might be removed by truncate while we were
> > +      * reading it. Handling of that case (forget what we've got and
> > +      * reread) is taken out of the main path.
>
> /me wonders how we'd be racing with setattr?
>
> --D

The same race would be present in get_block(), since that comment and the
following code was lifted & adapted from there because they do the same
things. I'll investigate to see if there are any races with setattr with both
functions, though, and fix them as needed.

> > +      */
> > +     if (err == -EAGAIN)
> > +             goto changed;
> > +
> > +     left = (chain + depth) - partial;
> > +     err = alloc_branch(inode, left, offsets + (partial - chain), partial);
> > +     if (err)
> > +             goto cleanup;
> > +
> > +     if (splice_branch(inode, chain, partial, left) < 0)
> > +             goto changed;
> > +
> > +     /* Successful allocation, mapping it. */
> > +     iomap->flags = IOMAP_F_NEW;
> > +     goto got_it;
> > +
> > +changed:
> > +     while (partial > chain) {
> > +             brelse(partial->bh);
> > +             partial--;
> > +     }
> > +     goto reread;
> > +}
> > +
> > +/*
> > + * 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 f2025c9b5825..77e503cca97f 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 */
> > @@ -69,6 +70,8 @@ extern int V1_minix_get_block(struct inode *, long, struct buffer_head *, int);
> >  extern int V2_minix_get_block(struct inode *, long, struct buffer_head *, int);
> >  extern unsigned V1_minix_blocks(loff_t, struct super_block *);
> >  extern unsigned V2_minix_blocks(loff_t, struct super_block *);
> > +extern int minix_get_block(struct inode *inode, sector_t block,
> > +                 struct buffer_head *bh_result, int create);
> >
> >  struct minix_dir_entry *minix_find_entry(struct dentry *, struct folio **);
> >  int minix_add_link(struct dentry*, struct inode*);
> > @@ -80,10 +83,20 @@ int minix_set_link(struct minix_dir_entry *de, struct folio *folio,
> >  struct minix_dir_entry *minix_dotdot(struct inode*, struct folio **);
> >  ino_t minix_inode_by_name(struct dentry*);
> >
> > +extern int V1_minix_iomap_begin(struct inode *inode, loff_t offset,
> > +     loff_t length, unsigned int flags, struct iomap *iomap,
> > +     struct iomap *srcmap);
> > +extern 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 address_space_operations minix_aops;
> >  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)
> >  {
> > @@ -95,11 +108,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(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)
> >
> > @@ -129,7 +148,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
> >
> >

Re: [PATCH v2 2/4] minix: convert address space operations to iomap

[Jeremy Bingham]

On Wed, Jul 1, 2026 at 11:14 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Sat, Jun 27, 2026 at 10:15:54PM -0700, Jeremy Bingham wrote:
> > Convert minix regular file and symlink address space operations from
> > buffer_head to iomap. The new minix_aops uses iomap_dirty_folio,
> > iomap_invalidate_folio, iomap_bio_read_folio, iomap_bio_readahead,
> > iomap_writepages, iomap_bmap, and related iomap helpers.
> > The write_begin/write_end callbacks are removed since buffered writes
> > now go through iomap_file_buffered_write in file.c.
> >
> > Directories keep using buffer_heads via a new minix_dir_aops, which
> > retains the old block_dirty_folio, block_read_full_folio,
> > block_write_begin, generic_write_end, and mpage_writepages. This is
> > necessary because directory entry manipulation (minix_prepare_chunk,
> > minix_write_begin) still uses the buffer_head chunk protocol.
> >
> > minix_bmap is converted from generic_block_bmap to iomap_bmap.
>
> I'd drop BMAP support entirely, unless you know of people who use LILO
> and minixfs.  If all you want is swapfile activation, use
> iomap_swapfile_activate... assuming that anyone actually cares about
> hosting swapfiles on minixfs.

I'm not dead set against dropping BMAP support entirely, but is there a
compelling reason to drop it?

Thanks,

-j

> > The minix_get_block function is exported (non-static) so the
> > directory aops can still use it for block_write_begin and
> > mpage_writepages.
> >
> > Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
> > ---
> >  fs/minix/inode.c | 86 +++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 78 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> > index c30cc590698d..2ba6766fce51 100644
> > --- a/fs/minix/inode.c
> > +++ b/fs/minix/inode.c
> > @@ -436,7 +436,32 @@ static int minix_statfs(struct dentry *dentry, struct kstatfs *buf)
> >       return 0;
> >  }
> >
> > -static int minix_get_block(struct inode *inode, sector_t block,
> > +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,
> > +};
> > +
> > +int minix_get_block(struct inode *inode, sector_t block,
> >                   struct buffer_head *bh_result, int create)
> >  {
> >       if (INODE_VERSION(inode) == MINIX_V1)
> > @@ -445,17 +470,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);
> > @@ -487,19 +540,36 @@ static int minix_write_begin(const struct kiocb *iocb,
> >
> >  static sector_t minix_bmap(struct address_space *mapping, sector_t block)
> >  {
> > -     return generic_block_bmap(mapping,block,minix_get_block);
> > +     const struct iomap_ops *ops = minix_iomap_ops_ver(mapping->host);
> > +
> > +     return iomap_bmap(mapping, block, ops);
> >  }
> >
> > -static const struct address_space_operations minix_aops = {
> > -     .dirty_folio    = block_dirty_folio,
> > -     .invalidate_folio = block_invalidate_folio,
> > +const struct address_space_operations minix_aops = {
> > +     .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,
> > +     .bmap = minix_bmap,
> > +     .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
>
> I forget, does one have to set FMODE_CAN_ODIRECT if the address_space
> operations don't supply a ->direct_IO function?
>
> --D
>
> > +     .writepages = minix_block_writepages,
> >  };
> >
> >  static const struct inode_operations minix_symlink_inode_operations = {
> > @@ -516,7 +586,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);
> > --
> > 2.47.3
> >
> >

Re: [PATCH v2 3/4] minix: convert file operations to iomap and add

[Jeremy Bingham]

On Wed, Jul 1, 2026 at 11:21 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Sat, Jun 27, 2026 at 10:15:55PM -0700, Jeremy Bingham wrote:
> > Subject: [PATCH v2 3/4] minix: convert file operations to iomap and add
>
> ...add what?

Ugh, "add direct I/O". When I was preparing the second version of the patches,
I screwed up.

> > Replace generic_file_read_iter and generic_file_write_iter with custom
> > minix_file_read_iter and minix_file_write_iter that dispatch to iomap
> > for both buffered and direct I/O.
> >
> > Buffered writes now go through iomap_file_buffered_write instead of
> > the aops write_begin/write_end path (which no longer exists for
> > regular files). Buffered reads still use generic_file_read_iter
> > for the non-DIO case.
> >
> > Direct I/O is implemented via iomap_dio_rw for both reads and writes.
> > minix_dio_read_iter takes a shared inode lock; minix_dio_write_iter
> > takes an exclusive lock, does generic_write_checks, and falls back
> > to buffered writes via iomap_file_buffered_write for the tail of a
> > DIO write that is not block-aligned. The minix_dio_write_end_io
> > callback updates i_size and marks the inode dirty.
> >
> > minix_file_open sets FMODE_CAN_ODIRECT so the VFS allows O_DIRECT
> > opens, and splice_write is added to the file operations.
>
> Ignore my question about FMODE_CAN_ODIRECT in the previous patch, then.

Noted.

> > +
> > +     if (ret >= 0 && iov_iter_count(from)) {
> > +             loff_t pos;
> > +             loff_t endbyte;
> > +             ssize_t status;
> > +
> > +             iocb->ki_flags &= ~IOCB_DIRECT;
>
> Why not set IOC_DSYNC here and let generic_write_sync do all the
> flushing work for you?  There's no requirement to dump the pagecache
> after a downgraded direct write, but if you want that, use
> IOCB_DONTCACHE.
>
> --D

*nod* I will also try this when I'm updating the minix patches.

Thank you again for your comments,

-j

Re: [PATCH v2 2/4] minix: convert address space operations to iomap

[Darrick J. Wong]

On Wed, Jul 01, 2026 at 11:37:10AM -0700, Jeremy Bingham wrote:
> On Wed, Jul 1, 2026 at 11:14 AM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Sat, Jun 27, 2026 at 10:15:54PM -0700, Jeremy Bingham wrote:
> > > Convert minix regular file and symlink address space operations from
> > > buffer_head to iomap. The new minix_aops uses iomap_dirty_folio,
> > > iomap_invalidate_folio, iomap_bio_read_folio, iomap_bio_readahead,
> > > iomap_writepages, iomap_bmap, and related iomap helpers.
> > > The write_begin/write_end callbacks are removed since buffered writes
> > > now go through iomap_file_buffered_write in file.c.
> > >
> > > Directories keep using buffer_heads via a new minix_dir_aops, which
> > > retains the old block_dirty_folio, block_read_full_folio,
> > > block_write_begin, generic_write_end, and mpage_writepages. This is
> > > necessary because directory entry manipulation (minix_prepare_chunk,
> > > minix_write_begin) still uses the buffer_head chunk protocol.
> > >
> > > minix_bmap is converted from generic_block_bmap to iomap_bmap.
> >
> > I'd drop BMAP support entirely, unless you know of people who use LILO
> > and minixfs.  If all you want is swapfile activation, use
> > iomap_swapfile_activate... assuming that anyone actually cares about
> > hosting swapfiles on minixfs.
> 
> I'm not dead set against dropping BMAP support entirely, but is there a
> compelling reason to drop it?

It's a terrible legacy interface -- you can only ask about a single
block, the block and sector numbers are limited to u32, the magic value
0 means no mapping, and it doesn't actually tell you which device.

Anyone who really wants to find the sparse areas of a file would be
better off calling SEEK_{DATA,HOLE} ... which I guess is something you
could implement as part of this patchset.

--D

> Thanks,
> 
> -j
> 
> > > The minix_get_block function is exported (non-static) so the
> > > directory aops can still use it for block_write_begin and
> > > mpage_writepages.
> > >
> > > Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
> > > ---
> > >  fs/minix/inode.c | 86 +++++++++++++++++++++++++++++++++++++++++++-----
> > >  1 file changed, 78 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> > > index c30cc590698d..2ba6766fce51 100644
> > > --- a/fs/minix/inode.c
> > > +++ b/fs/minix/inode.c
> > > @@ -436,7 +436,32 @@ static int minix_statfs(struct dentry *dentry, struct kstatfs *buf)
> > >       return 0;
> > >  }
> > >
> > > -static int minix_get_block(struct inode *inode, sector_t block,
> > > +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,
> > > +};
> > > +
> > > +int minix_get_block(struct inode *inode, sector_t block,
> > >                   struct buffer_head *bh_result, int create)
> > >  {
> > >       if (INODE_VERSION(inode) == MINIX_V1)
> > > @@ -445,17 +470,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);
> > > @@ -487,19 +540,36 @@ static int minix_write_begin(const struct kiocb *iocb,
> > >
> > >  static sector_t minix_bmap(struct address_space *mapping, sector_t block)
> > >  {
> > > -     return generic_block_bmap(mapping,block,minix_get_block);
> > > +     const struct iomap_ops *ops = minix_iomap_ops_ver(mapping->host);
> > > +
> > > +     return iomap_bmap(mapping, block, ops);
> > >  }
> > >
> > > -static const struct address_space_operations minix_aops = {
> > > -     .dirty_folio    = block_dirty_folio,
> > > -     .invalidate_folio = block_invalidate_folio,
> > > +const struct address_space_operations minix_aops = {
> > > +     .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,
> > > +     .bmap = minix_bmap,
> > > +     .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
> >
> > I forget, does one have to set FMODE_CAN_ODIRECT if the address_space
> > operations don't supply a ->direct_IO function?
> >
> > --D
> >
> > > +     .writepages = minix_block_writepages,
> > >  };
> > >
> > >  static const struct inode_operations minix_symlink_inode_operations = {
> > > @@ -516,7 +586,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);
> > > --
> > > 2.47.3
> > >
> > >
> 

Re: [PATCH v2 0/4] minix: convert to iomap and add direct I/O

[Jeremy Bingham]

On Wed, Jul 1, 2026 at 11:00 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Sat, Jun 27, 2026 at 10:15:52PM -0700, Jeremy Bingham wrote:
> > The iomap.c file is #include'd into itree_v1.c and itree_v2.c rather
> > than compiled as a standalone translation unit. This is because the
> > minix filesystem versions (V1 vs V2/V3) have different block_t sizes
> > (16-bit vs 32-bit) and different indirect tree depths. This follows
> > the existing pattern in minix where itree_common.c is included into
> > both itree_v1.c and itree_v2.c. Each version provides a thin wrapper
> > and a corresponding iomap_ops struct.
>
> Yuck.  I guess that's /one/ way to avoid having a geometry struct
> capturing those details... :(

It's all absolutely hideous. I've hated the way the minix module #includes
itree_common.c like that with the separate itree_v1.c and itree_v2.c for
years, but it's also been like that for so long that changing it is
intimidating.
I could address it in this patch, if it's not out of scope. (Also I
thought I had
already responded to this, my apologies.)

> > Changes since v1:
> >
> >   * Added a fourth patch to fix the symlink and truncate issues:
> >     - Replaced page_symlink with a custom __page_symlink that writes
> >       the target directly to a data block via minix_new_block +
>
> Sounds to me like it's time to write iomap_write_symlink.
>
> 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       = target,
>                 .iov_len        = len,
>         };
>         struct iomap_iter iter = {
>                 .inode          = inode,
>                 .pos            = 0,
>                 .len            = len,
>                 .flags          = IOMAP_WRITE,
>                 .private        = private,
>         };
>         struct iov_iter iov;
>         int ret;
>
>         iov_iter_kvec(&iov, ITER_SRC, &vec, 1, iov.iov_len);
>
>         while ((ret = iomap_iter(&iter, ops)) > 0)
>                 iter.status = iomap_write_iter(&iter, &iov, write_ops);
>
>         if (unlikely(iter.pos == 0))
>                 return ret;
>
>         mark_inode_dirty(inode);
>         return 0;
> }
> EXPORT_SYMBOL_GPL(iomap_symlink_write);

Noted. I'll take this and the __page_symlink I originally made, smoosh
them together as or if needed, and put it in fs/iomap.c.

> >       sb_getblk, bypassing the aops write path (which no longer has
> >       write_begin/write_end). Added a matching custom minix_get_link
> >       that reads the target from the data block via sb_bread, similar
> >       to ext4_get_link. No iomap-based filesystem in the kernel uses
> >       page_symlink; XFS, GFS2, and ext4 all handle symlink storage
>
> That's because they embed headers and crcs in the symlink file data
> and/or do fancy things with inline targets.  xfs has its own buffer
> cache, so there's no need to duplicate it with the pagecache and then
> have to interpret ondisk formats.

Also noted.

Thanks again,

-j

> >       directly. The on-disk format is unchanged.
> >     - Fixed a buffer_head/iomap type confusion in truncate:
> >       block_truncate_page attaches buffer_heads to data folios, but
> >       minix_aops now uses iomap which interprets folio->private as
> >       struct iomap_folio_state. truncate() now dispatches between
> >       iomap_truncate_page (for regular files/symlinks) and
> >       block_truncate_page (for directories) based on the inode's aops.
> >     - Added .setattr = minix_setattr to minix_symlink_inode_operations
> >       so symlinks truncate properly through the iomap path.
> >
> >   * Patch 1 (iomap infrastructure): minix_get_block is now exported
> >     (non-static) so the directory aops and iomap writeback path can
> >     use it. Added minix_iomap_ops_ver() inline helper and extern
> >     declarations for minix_aops and the version-specific iomap_ops.
> >     Fixed unsigned -> unsigned int in minix_blocks_needed and
> >     minix_find_first_zero_bit to silence checkpatch warnings.
> >
> >   * Patch 2 (aops conversion): unchanged in approach; minor cleanup
> >     of the writeback callback and minix_bmap conversion.
> >
> >   * Patch 3 (file operations): minix_setattr is now exported for reuse
> >     by the symlink inode operations in patch 4.
> >
> > Testing: the full series has been tested with mkfs.minix V1/V2/V3,
> > exercising file creation, read/write, overwrite, append, binary data,
> > directories, symlinks (full path, relative, directory symlinks), hard
> > links, truncation (shrink/grow), large files (1MB, exercising indirect
> > blocks), deep nesting (20 levels), 100 files in one directory,
> > deletions, remount persistence, and fsck.minix. All pass cleanly. The
> > four syzbot-reported issues are resolved.
> >
> > Jeremy Bingham (4):
> >   minix: add iomap infrastructure
> >   minix: convert address space operations to iomap
> >   minix: convert file operations to iomap and add direct I/O
> >   minix: fix symlilnk and truncate for iomap compatibility
> >
> >  fs/minix/file.c         | 157 ++++++++++++++++++++++++++++++++++++++--
> >  fs/minix/inode.c        |  90 ++++++++++++++++++++---
> >  fs/minix/iomap.c        | 114 +++++++++++++++++++++++++++++
> >  fs/minix/itree_common.c |  11 ++-
> >  fs/minix/itree_v1.c     |  25 ++++++-
> >  fs/minix/itree_v2.c     |  17 ++++-
> >  fs/minix/minix.h        |  30 +++++++-
> >  fs/minix/namei.c        | 137 ++++++++++++++++++++++++++++++++++-
> >  8 files changed, 558 insertions(+), 23 deletions(-)
> >  create mode 100644 fs/minix/iomap.c
> >
> > --
> > 2.47.3
> >
> >