From: "Darrick J. Wong" <djwong@kernel.org>
To: Jeremy Bingham <jbingham@gmail.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
brauner@kernel.org, jkoolstra@xs4all.nl, jack@suse.cz,
hch@infradead.org, viro@zeniv.linux.org.uk,
syzkaller@googlegroups.com
Subject: Re: [PATCH v2 0/4] minix: convert to iomap and add direct I/O
Date: Wed, 1 Jul 2026 11:00:35 -0700 [thread overview]
Message-ID: <20260701180035.GA6507@frogsfrogsfrogs> (raw)
In-Reply-To: <cover.1782619718.git.jbingham@gmail.com>
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
>
>
next prev parent reply other threads:[~2026-07-01 18:00 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-28 5:15 [PATCH v2 0/4] minix: convert to iomap and add direct I/O Jeremy Bingham
2026-06-28 5:15 ` [PATCH v2 1/4] minix: add iomap infrastructure Jeremy Bingham
2026-07-01 18:06 ` Darrick J. Wong
2026-07-01 18:32 ` Jeremy Bingham
2026-06-28 5:15 ` [PATCH v2 2/4] minix: convert address space operations to iomap Jeremy Bingham
2026-07-01 18:14 ` Darrick J. Wong
2026-07-01 18:37 ` Jeremy Bingham
2026-07-01 18:47 ` Darrick J. Wong
2026-06-28 5:15 ` [PATCH v2 3/4] minix: convert file operations to iomap and add Jeremy Bingham
2026-07-01 18:21 ` Darrick J. Wong
2026-07-01 18:42 ` Jeremy Bingham
2026-06-28 5:15 ` [PATCH v2 4/4] minix: fix symlink and truncate for iomap Jeremy Bingham
2026-06-29 5:27 ` [syzbot ci] Re: minix: convert to iomap and add direct I/O syzbot ci
2026-06-30 3:05 ` Jeremy Bingham
2026-06-30 4:05 ` syzbot ci
2026-07-01 18:00 ` Darrick J. Wong [this message]
2026-07-02 18:42 ` [PATCH v2 0/4] " Jeremy Bingham
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260701180035.GA6507@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=brauner@kernel.org \
--cc=hch@infradead.org \
--cc=jack@suse.cz \
--cc=jbingham@gmail.com \
--cc=jkoolstra@xs4all.nl \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=syzkaller@googlegroups.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.