minix: convert to iomap and add direct I/O

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 <[email protected]>

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

fs/minix/iomap.c new file +0 -0

new file mode 100644
index 000000000000..7bb0439e3669

fs/minix/iomap.c new file +57 -0

+ */
+
+/*
+ * 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) {

fs/minix/iomap.c new file +34 -0

+	 */
+	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;
+}

fs/minix/itree_v1.c +24 -1

index 1fed906042aa..58c29f4443d3 100644
@@ -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,
+};

fs/minix/itree_v2.c +16 -1

index 9d00f31a2d9d..fc7a5ae8fa1c 100644
@@ -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,
+};

fs/minix/minix.h +21 -2

index f2025c9b5825..77e503cca97f 100644
@@ -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;

Signature · 1 line

2.47.3

Quoted 5 lines

fs/minix/iomap.c new file +0 -0

new file mode 100644
index 000000000000..7bb0439e3669

fs/minix/iomap.c line 5

Quoted 6 lines

fs/minix/iomap.c new file +5 -0

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

Quoted 57 lines

fs/minix/iomap.c new file +57 -0

+ */
+
+/*
+ * 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) {

fs/minix/iomap.c line 72

Quoted 10 lines

fs/minix/iomap.c new file +10 -0

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

fs/minix/iomap.c line 80

Quoted 8 lines

fs/minix/iomap.c new file +8 -0

+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

Quoted 186 lines

fs/minix/iomap.c new file +34 -0

+      */
+     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;
+}

fs/minix/itree_v1.c +24 -1

index 1fed906042aa..58c29f4443d3 100644
@@ -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,
+};

fs/minix/itree_v2.c +16 -1

index 9d00f31a2d9d..fc7a5ae8fa1c 100644
@@ -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,
+};

fs/minix/minix.h +21 -3

index f2025c9b5825..77e503cca97f 100644
@@ -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

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 <[email protected]>

---
 fs/minix/inode.c | 86 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 78 insertions(+), 8 deletions(-)

fs/minix/inode.c +71 -6

index c30cc590698d..2ba6766fce51 100644
@@ -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.

fs/minix/inode.c +2 -1

+	.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);

Signature · 1 line

2.47.3

Quoted 13 lines · 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.

Quoted 1 line

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.

Quoted 122 lines

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 <[email protected]>

---
 fs/minix/inode.c | 86 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 78 insertions(+), 8 deletions(-)

fs/minix/inode.c +71 -6

index c30cc590698d..2ba6766fce51 100644
@@ -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.

fs/minix/inode.c static int minix_write_begin(const struct kiocb *iocb, · line 502

Quoted 10 lines

fs/minix/inode.c +5 -1

+ */
+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

Quoted 17 lines

fs/minix/inode.c +2 -2

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

Quoted 17 lines · Darrick J. Wong

Quoted 14 lines · 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.

Quoted 4 lines

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

Quoted 155 lines

Quoted 122 lines

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 <[email protected]>

---
 fs/minix/inode.c | 86 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 78 insertions(+), 8 deletions(-)

fs/minix/inode.c +71 -6

index c30cc590698d..2ba6766fce51 100644
@@ -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.

fs/minix/inode.c static int minix_write_begin(const struct kiocb *iocb, · line 502

Quoted 10 lines

fs/minix/inode.c +5 -1

+ */
+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

Quoted 17 lines

fs/minix/inode.c +2 -2

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

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

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 <[email protected]>

---
 fs/minix/file.c  | 157 +++++++++++++++++++++++++++++++++++++++++++++--
 fs/minix/minix.h |   2 +
 2 files changed, 153 insertions(+), 6 deletions(-)

fs/minix/file.c +58 -0

index 86e5943cd2ff..b07c853fa43a 100644
@@ -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,

fs/minix/file.c +83 -6

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

fs/minix/minix.h +2 -0

index 77e503cca97f..76718f789369 100644
@@ -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);

Signature · 1 line

2.47.3

Quoted 1 line · Jeremy Bingham

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

...add what?

Quoted 16 lines

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.

Quoted 2 lines

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.

fs/minix/file.c int minix_fsync(struct file *file, loff_t start, loff_t end, int datasync) · line 87

Quoted 7 lines

fs/minix/file.c +7 -0

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

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

Signature · 1 line

2.47.3

Quoted 7 lines · Jeremy Bingham

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... :(

Quoted 5 lines

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

Quoted 5 lines

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.

Quoted 52 lines

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