* [PATCH v4 1/7] hfs/hfsplus: exchange hardcoded number of extents on named constants
2026-09-14 23:39 [PATCH v4 0/7] hfsplus: convert regular file I/O to iomap-based operations Viacheslav Dubeyko
@ 2026-09-14 23:39 ` Viacheslav Dubeyko
2026-09-18 13:28 ` Christoph Hellwig
2026-09-14 23:39 ` [PATCH v4 2/7] hfsplus: rework hfsplus_get_block() logic Viacheslav Dubeyko
` (5 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-14 23:39 UTC (permalink / raw)
To: glaubitz, frank.li, hch
Cc: linux-fsdevel, linux-kernel, vdubeyko, willy, brauner, djwong,
Viacheslav Dubeyko
Replace the magic 8/3 fork extent-count literals with named
constants (HFS_FORK_EXTENT_COUNT and HFSPLUS_FORK_EXTENT_COUNT).
No functional change.
cc: Christoph Hellwig <hch@lst.de>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
---
include/linux/hfs_common.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
index d6a615e74b26..8673933bbee5 100644
--- a/include/linux/hfs_common.h
+++ b/include/linux/hfs_common.h
@@ -171,18 +171,22 @@ enum {
HFS_XATTR_NAME,
};
+#define HFS_FORK_EXTENT_COUNT (3)
+
struct hfs_extent {
__be16 block;
__be16 count;
};
-typedef struct hfs_extent hfs_extent_rec[3];
+typedef struct hfs_extent hfs_extent_rec[HFS_FORK_EXTENT_COUNT];
+
+#define HFSPLUS_FORK_EXTENT_COUNT (8)
/* A single contiguous area of a file */
struct hfsplus_extent {
__be32 start_block;
__be32 block_count;
} __packed;
-typedef struct hfsplus_extent hfsplus_extent_rec[8];
+typedef struct hfsplus_extent hfsplus_extent_rec[HFSPLUS_FORK_EXTENT_COUNT];
/* Information for a "Fork" in a file */
struct hfsplus_fork_raw {
--
2.43.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 2/7] hfsplus: rework hfsplus_get_block() logic
2026-09-14 23:39 [PATCH v4 0/7] hfsplus: convert regular file I/O to iomap-based operations Viacheslav Dubeyko
2026-09-14 23:39 ` [PATCH v4 1/7] hfs/hfsplus: exchange hardcoded number of extents on named constants Viacheslav Dubeyko
@ 2026-09-14 23:39 ` Viacheslav Dubeyko
2026-09-15 2:34 ` Darrick J. Wong
2026-09-18 13:29 ` Christoph Hellwig
2026-09-14 23:39 ` [PATCH v4 3/7] hfsplus: take the bitmap page lock for allocate/free Viacheslav Dubeyko
` (4 subsequent siblings)
6 siblings, 2 replies; 20+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-14 23:39 UTC (permalink / raw)
To: glaubitz, frank.li, hch
Cc: linux-fsdevel, linux-kernel, vdubeyko, willy, brauner, djwong,
Viacheslav Dubeyko
Split the extent-lookup/allocate logic out of hfsplus_get_block() into
a new hfsplus_map_extent(), which reports the mapping as
(dblock, max_blocks, balloc) instead of filling in a buffer_head.
hfsplus_get_block() becomes a thin wrapper around it for the
buffer_head-based callers (B-tree metadata, symlinks).
No functional change to the existing buffer_head path. This is
preparation for the iomap-based regular file I/O path added in a
later patch, which will call hfsplus_map_extent() directly.
cc: Christoph Hellwig <hch@lst.de>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
---
fs/hfsplus/extents.c | 118 +++++++++++++++++++++++++++++-----------
fs/hfsplus/hfsplus_fs.h | 2 +
2 files changed, 87 insertions(+), 33 deletions(-)
diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index eb7c11524d18..ffd52ad8867c 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -48,18 +48,29 @@ static void hfsplus_ext_build_key(hfsplus_btree_key *key, u32 cnid,
key->ext.pad = 0;
}
-static u32 hfsplus_ext_find_block(struct hfsplus_extent *ext, u32 off)
+/*
+ * hfsplus_ext_find_block() - find contiguous sequence of block
+ *
+ * Find the disk allocation block for 'off' within an 8-entry
+ * extent record, and the number of further allocation blocks
+ * that are contiguous with it in the same extent entry.
+ */
+static u32 hfsplus_ext_find_block(struct hfsplus_extent *ext, u32 off,
+ u32 *dblock)
{
int i;
u32 count;
- for (i = 0; i < 8; ext++, i++) {
+ for (i = 0; i < HFSPLUS_FORK_EXTENT_COUNT; ext++, i++) {
count = be32_to_cpu(ext->block_count);
- if (off < count)
- return be32_to_cpu(ext->start_block) + off;
+ if (off < count) {
+ *dblock = be32_to_cpu(ext->start_block) + off;
+ return count - off;
+ }
off -= count;
}
/* panic? */
+ *dblock = 0;
return 0;
}
@@ -68,7 +79,7 @@ static int hfsplus_ext_block_count(struct hfsplus_extent *ext)
int i;
u32 count = 0;
- for (i = 0; i < 8; ext++, i++)
+ for (i = 0; i < HFSPLUS_FORK_EXTENT_COUNT; ext++, i++)
count += be32_to_cpu(ext->block_count);
return count;
}
@@ -225,37 +236,46 @@ static int hfsplus_ext_read_extent(struct inode *inode, u32 block)
return res;
}
-/* Get a block at iblock for inode, possibly allocating if create */
-int hfsplus_get_block(struct inode *inode, sector_t iblock,
- struct buffer_head *bh_result, int create)
+/*
+ * hfsplus_map_extent() - find or allocate a sequence of allocation blocks
+ *
+ * Looks up the allocation block at 'ablock' for inode, extending the
+ * file (via hfsplus_file_extend(), in clump_blocks-sized chunks) when
+ * 'create' is set and 'ablock' lies beyond the current allocation.
+ *
+ * On success, *dblock is the disk allocation block backing 'ablock',
+ * and *max_blocks is the number of further allocation blocks that are
+ * contiguous with it (i.e. the remaining length of the extent entry
+ * that contains 'ablock'), which may be smaller than the whole file's
+ * remaining allocation when the fork is fragmented across several
+ * extent entries. If a new extent had to be allocated to satisfy the
+ * request, *balloc (when non-NULL) is set to true.
+ */
+int hfsplus_map_extent(struct inode *inode, u32 ablock, int create,
+ u32 *dblock, u32 *max_blocks, bool *balloc)
{
- struct super_block *sb = inode->i_sb;
- struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
- int res = -EIO;
- u32 ablock, dblock, mask;
- sector_t sector;
- int was_dirty = 0;
+ int was_dirty;
+ int res;
- /* Convert inode block to disk allocation block */
- ablock = iblock >> sbi->fs_shift;
+ if (balloc)
+ *balloc = false;
- if (iblock >= hip->fs_blocks) {
+ if (ablock >= hip->alloc_blocks) {
if (!create)
- return 0;
- if (iblock > hip->fs_blocks)
return -EIO;
- if (ablock >= hip->alloc_blocks) {
- res = hfsplus_file_extend(inode, false);
- if (res)
- return res;
- }
- } else
- create = 0;
+ res = hfsplus_file_extend(inode, false);
+ if (res)
+ return res;
+ if (balloc)
+ *balloc = true;
+ }
if (ablock < hip->first_blocks) {
- dblock = hfsplus_ext_find_block(hip->first_extents, ablock);
- goto done;
+ *max_blocks = hfsplus_ext_find_block(hip->first_extents,
+ ablock,
+ dblock);
+ return 0;
}
if (inode->i_ino == HFSPLUS_EXT_CNID)
@@ -274,11 +294,44 @@ int hfsplus_get_block(struct inode *inode, sector_t iblock,
mutex_unlock(&hip->extents_lock);
return -EIO;
}
- dblock = hfsplus_ext_find_block(hip->cached_extents,
- ablock - hip->cached_start);
+ *max_blocks = hfsplus_ext_find_block(hip->cached_extents,
+ ablock - hip->cached_start,
+ dblock);
mutex_unlock(&hip->extents_lock);
-done:
+ if (was_dirty)
+ mark_inode_dirty(inode);
+
+ return 0;
+}
+
+/* Get a block at iblock for inode, possibly allocating if create */
+int hfsplus_get_block(struct inode *inode, sector_t iblock,
+ struct buffer_head *bh_result, int create)
+{
+ struct super_block *sb = inode->i_sb;
+ struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+ struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
+ u32 ablock, dblock, mask, max_blocks;
+ sector_t sector;
+ int res;
+
+ /* Convert inode block to disk allocation block */
+ ablock = iblock >> sbi->fs_shift;
+
+ if (iblock >= hip->fs_blocks) {
+ if (!create)
+ return 0;
+ if (iblock > hip->fs_blocks)
+ return -EIO;
+ } else
+ create = 0;
+
+ res = hfsplus_map_extent(inode, ablock, create, &dblock, &max_blocks,
+ NULL);
+ if (res)
+ return res;
+
hfs_dbg("ino %llu, iblock %llu - dblock %u\n",
inode->i_ino, (long long)iblock, dblock);
@@ -292,9 +345,8 @@ int hfsplus_get_block(struct inode *inode, sector_t iblock,
hip->phys_size += sb->s_blocksize;
hip->fs_blocks++;
inode_add_bytes(inode, sb->s_blocksize);
- }
- if (create || was_dirty)
mark_inode_dirty(inode);
+ }
return 0;
}
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 1e5b58e6a13f..67586382269b 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -436,6 +436,8 @@ int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
int hfsplus_ext_write_extent(struct inode *inode);
int hfsplus_get_block(struct inode *inode, sector_t iblock,
struct buffer_head *bh_result, int create);
+int hfsplus_map_extent(struct inode *inode, u32 ablock, int create,
+ u32 *dblock, u32 *max_blocks, bool *balloc);
int hfsplus_free_fork(struct super_block *sb, u32 cnid,
struct hfsplus_fork_raw *fork, int type);
int hfsplus_file_extend(struct inode *inode, bool zeroout);
--
2.43.0
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 2/7] hfsplus: rework hfsplus_get_block() logic
2026-09-14 23:39 ` [PATCH v4 2/7] hfsplus: rework hfsplus_get_block() logic Viacheslav Dubeyko
@ 2026-09-15 2:34 ` Darrick J. Wong
2026-09-15 18:34 ` Viacheslav Dubeyko
2026-09-18 13:29 ` Christoph Hellwig
1 sibling, 1 reply; 20+ messages in thread
From: Darrick J. Wong @ 2026-09-15 2:34 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner
On Mon, Sep 14, 2026 at 04:39:36PM -0700, Viacheslav Dubeyko wrote:
> Split the extent-lookup/allocate logic out of hfsplus_get_block() into
> a new hfsplus_map_extent(), which reports the mapping as
> (dblock, max_blocks, balloc) instead of filling in a buffer_head.
> hfsplus_get_block() becomes a thin wrapper around it for the
> buffer_head-based callers (B-tree metadata, symlinks).
>
> No functional change to the existing buffer_head path. This is
> preparation for the iomap-based regular file I/O path added in a
> later patch, which will call hfsplus_map_extent() directly.
>
> cc: Christoph Hellwig <hch@lst.de>
> cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> cc: Yangtao Li <frank.li@vivo.com>
> cc: linux-fsdevel@vger.kernel.org
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
> ---
> fs/hfsplus/extents.c | 118 +++++++++++++++++++++++++++++-----------
> fs/hfsplus/hfsplus_fs.h | 2 +
> 2 files changed, 87 insertions(+), 33 deletions(-)
>
> diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
> index eb7c11524d18..ffd52ad8867c 100644
> --- a/fs/hfsplus/extents.c
> +++ b/fs/hfsplus/extents.c
> @@ -48,18 +48,29 @@ static void hfsplus_ext_build_key(hfsplus_btree_key *key, u32 cnid,
> key->ext.pad = 0;
> }
>
> -static u32 hfsplus_ext_find_block(struct hfsplus_extent *ext, u32 off)
> +/*
> + * hfsplus_ext_find_block() - find contiguous sequence of block
> + *
> + * Find the disk allocation block for 'off' within an 8-entry
> + * extent record, and the number of further allocation blocks
> + * that are contiguous with it in the same extent entry.
> + */
> +static u32 hfsplus_ext_find_block(struct hfsplus_extent *ext, u32 off,
> + u32 *dblock)
Hmm. Does this function take an hfs+ extent and a file block offset
within that extent as inputs? And are its outputs the physical block
number in @dblock, and the block count as the return value?
(Wow, TN1150 is still available!)
It's sorta too bad that this code doesn't just return a mapping
structure with some nice names to make the code easier to understand,
but, eh, whatever. Old fs code isn't always pretty. :)
This took me a while to understand, but it seems reasonable to me so
Acked-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> {
> int i;
> u32 count;
>
> - for (i = 0; i < 8; ext++, i++) {
> + for (i = 0; i < HFSPLUS_FORK_EXTENT_COUNT; ext++, i++) {
> count = be32_to_cpu(ext->block_count);
> - if (off < count)
> - return be32_to_cpu(ext->start_block) + off;
> + if (off < count) {
> + *dblock = be32_to_cpu(ext->start_block) + off;
> + return count - off;
> + }
> off -= count;
> }
> /* panic? */
> + *dblock = 0;
> return 0;
> }
>
> @@ -68,7 +79,7 @@ static int hfsplus_ext_block_count(struct hfsplus_extent *ext)
> int i;
> u32 count = 0;
>
> - for (i = 0; i < 8; ext++, i++)
> + for (i = 0; i < HFSPLUS_FORK_EXTENT_COUNT; ext++, i++)
> count += be32_to_cpu(ext->block_count);
> return count;
> }
> @@ -225,37 +236,46 @@ static int hfsplus_ext_read_extent(struct inode *inode, u32 block)
> return res;
> }
>
> -/* Get a block at iblock for inode, possibly allocating if create */
> -int hfsplus_get_block(struct inode *inode, sector_t iblock,
> - struct buffer_head *bh_result, int create)
> +/*
> + * hfsplus_map_extent() - find or allocate a sequence of allocation blocks
> + *
> + * Looks up the allocation block at 'ablock' for inode, extending the
> + * file (via hfsplus_file_extend(), in clump_blocks-sized chunks) when
> + * 'create' is set and 'ablock' lies beyond the current allocation.
> + *
> + * On success, *dblock is the disk allocation block backing 'ablock',
> + * and *max_blocks is the number of further allocation blocks that are
> + * contiguous with it (i.e. the remaining length of the extent entry
> + * that contains 'ablock'), which may be smaller than the whole file's
> + * remaining allocation when the fork is fragmented across several
> + * extent entries. If a new extent had to be allocated to satisfy the
> + * request, *balloc (when non-NULL) is set to true.
> + */
> +int hfsplus_map_extent(struct inode *inode, u32 ablock, int create,
> + u32 *dblock, u32 *max_blocks, bool *balloc)
> {
> - struct super_block *sb = inode->i_sb;
> - struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
> struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
> - int res = -EIO;
> - u32 ablock, dblock, mask;
> - sector_t sector;
> - int was_dirty = 0;
> + int was_dirty;
> + int res;
>
> - /* Convert inode block to disk allocation block */
> - ablock = iblock >> sbi->fs_shift;
> + if (balloc)
> + *balloc = false;
>
> - if (iblock >= hip->fs_blocks) {
> + if (ablock >= hip->alloc_blocks) {
> if (!create)
> - return 0;
> - if (iblock > hip->fs_blocks)
> return -EIO;
> - if (ablock >= hip->alloc_blocks) {
> - res = hfsplus_file_extend(inode, false);
> - if (res)
> - return res;
> - }
> - } else
> - create = 0;
> + res = hfsplus_file_extend(inode, false);
> + if (res)
> + return res;
> + if (balloc)
> + *balloc = true;
> + }
>
> if (ablock < hip->first_blocks) {
> - dblock = hfsplus_ext_find_block(hip->first_extents, ablock);
> - goto done;
> + *max_blocks = hfsplus_ext_find_block(hip->first_extents,
> + ablock,
> + dblock);
> + return 0;
> }
>
> if (inode->i_ino == HFSPLUS_EXT_CNID)
> @@ -274,11 +294,44 @@ int hfsplus_get_block(struct inode *inode, sector_t iblock,
> mutex_unlock(&hip->extents_lock);
> return -EIO;
> }
> - dblock = hfsplus_ext_find_block(hip->cached_extents,
> - ablock - hip->cached_start);
> + *max_blocks = hfsplus_ext_find_block(hip->cached_extents,
> + ablock - hip->cached_start,
> + dblock);
> mutex_unlock(&hip->extents_lock);
>
> -done:
> + if (was_dirty)
> + mark_inode_dirty(inode);
> +
> + return 0;
> +}
> +
> +/* Get a block at iblock for inode, possibly allocating if create */
> +int hfsplus_get_block(struct inode *inode, sector_t iblock,
> + struct buffer_head *bh_result, int create)
> +{
> + struct super_block *sb = inode->i_sb;
> + struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
> + struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
> + u32 ablock, dblock, mask, max_blocks;
> + sector_t sector;
> + int res;
> +
> + /* Convert inode block to disk allocation block */
> + ablock = iblock >> sbi->fs_shift;
> +
> + if (iblock >= hip->fs_blocks) {
> + if (!create)
> + return 0;
> + if (iblock > hip->fs_blocks)
> + return -EIO;
> + } else
> + create = 0;
> +
> + res = hfsplus_map_extent(inode, ablock, create, &dblock, &max_blocks,
> + NULL);
> + if (res)
> + return res;
> +
> hfs_dbg("ino %llu, iblock %llu - dblock %u\n",
> inode->i_ino, (long long)iblock, dblock);
>
> @@ -292,9 +345,8 @@ int hfsplus_get_block(struct inode *inode, sector_t iblock,
> hip->phys_size += sb->s_blocksize;
> hip->fs_blocks++;
> inode_add_bytes(inode, sb->s_blocksize);
> - }
> - if (create || was_dirty)
> mark_inode_dirty(inode);
> + }
> return 0;
> }
>
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 1e5b58e6a13f..67586382269b 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -436,6 +436,8 @@ int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
> int hfsplus_ext_write_extent(struct inode *inode);
> int hfsplus_get_block(struct inode *inode, sector_t iblock,
> struct buffer_head *bh_result, int create);
> +int hfsplus_map_extent(struct inode *inode, u32 ablock, int create,
> + u32 *dblock, u32 *max_blocks, bool *balloc);
> int hfsplus_free_fork(struct super_block *sb, u32 cnid,
> struct hfsplus_fork_raw *fork, int type);
> int hfsplus_file_extend(struct inode *inode, bool zeroout);
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 2/7] hfsplus: rework hfsplus_get_block() logic
2026-09-15 2:34 ` Darrick J. Wong
@ 2026-09-15 18:34 ` Viacheslav Dubeyko
0 siblings, 0 replies; 20+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-15 18:34 UTC (permalink / raw)
To: Darrick J. Wong
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner
On Mon, 2026-09-14 at 19:34 -0700, Darrick J. Wong wrote:
> On Mon, Sep 14, 2026 at 04:39:36PM -0700, Viacheslav Dubeyko wrote:
> > Split the extent-lookup/allocate logic out of hfsplus_get_block()
> > into
> > a new hfsplus_map_extent(), which reports the mapping as
> > (dblock, max_blocks, balloc) instead of filling in a buffer_head.
> > hfsplus_get_block() becomes a thin wrapper around it for the
> > buffer_head-based callers (B-tree metadata, symlinks).
> >
> > No functional change to the existing buffer_head path. This is
> > preparation for the iomap-based regular file I/O path added in a
> > later patch, which will call hfsplus_map_extent() directly.
> >
> > cc: Christoph Hellwig <hch@lst.de>
> > cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> > cc: Yangtao Li <frank.li@vivo.com>
> > cc: linux-fsdevel@vger.kernel.org
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
> > ---
> > fs/hfsplus/extents.c | 118 +++++++++++++++++++++++++++++-------
> > ----
> > fs/hfsplus/hfsplus_fs.h | 2 +
> > 2 files changed, 87 insertions(+), 33 deletions(-)
> >
> > diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
> > index eb7c11524d18..ffd52ad8867c 100644
> > --- a/fs/hfsplus/extents.c
> > +++ b/fs/hfsplus/extents.c
> > @@ -48,18 +48,29 @@ static void
> > hfsplus_ext_build_key(hfsplus_btree_key *key, u32 cnid,
> > key->ext.pad = 0;
> > }
> >
> > -static u32 hfsplus_ext_find_block(struct hfsplus_extent *ext, u32
> > off)
> > +/*
> > + * hfsplus_ext_find_block() - find contiguous sequence of block
> > + *
> > + * Find the disk allocation block for 'off' within an 8-entry
> > + * extent record, and the number of further allocation blocks
> > + * that are contiguous with it in the same extent entry.
> > + */
> > +static u32 hfsplus_ext_find_block(struct hfsplus_extent *ext, u32
> > off,
> > + u32 *dblock)
>
> Hmm. Does this function take an hfs+ extent and a file block offset
> within that extent as inputs? And are its outputs the physical block
> number in @dblock, and the block count as the return value?
>
> (Wow, TN1150 is still available!)
>
> It's sorta too bad that this code doesn't just return a mapping
> structure with some nice names to make the code easier to understand,
> but, eh, whatever. Old fs code isn't always pretty. :)
>
> This took me a while to understand, but it seems reasonable to me so
> Acked-by: "Darrick J. Wong" <djwong@kernel.org>
>
Yeah, I agree that it requires to have more nice refactoring. :) But
it's too many places in HFS+ that requires of this. So, small step at
once. :)
Thanks,
Slava.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 2/7] hfsplus: rework hfsplus_get_block() logic
2026-09-14 23:39 ` [PATCH v4 2/7] hfsplus: rework hfsplus_get_block() logic Viacheslav Dubeyko
2026-09-15 2:34 ` Darrick J. Wong
@ 2026-09-18 13:29 ` Christoph Hellwig
1 sibling, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2026-09-18 13:29 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner, djwong
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 3/7] hfsplus: take the bitmap page lock for allocate/free
2026-09-14 23:39 [PATCH v4 0/7] hfsplus: convert regular file I/O to iomap-based operations Viacheslav Dubeyko
2026-09-14 23:39 ` [PATCH v4 1/7] hfs/hfsplus: exchange hardcoded number of extents on named constants Viacheslav Dubeyko
2026-09-14 23:39 ` [PATCH v4 2/7] hfsplus: rework hfsplus_get_block() logic Viacheslav Dubeyko
@ 2026-09-14 23:39 ` Viacheslav Dubeyko
2026-09-18 13:30 ` Christoph Hellwig
2026-09-14 23:39 ` [PATCH v4 4/7] hfsplus: add iomap operations for regular file data Viacheslav Dubeyko
` (3 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-14 23:39 UTC (permalink / raw)
To: glaubitz, frank.li, hch
Cc: linux-fsdevel, linux-kernel, vdubeyko, willy, brauner, djwong,
Viacheslav Dubeyko
The hfsplus_block_allocate() and hfsplus_block_free() kmap
the allocation bitmap's pages and modify their bits in place
under sbi->alloc_mutex, but without holding the page lock.
That leaves the read-modify-write of the bitmap bits
unprotected against a concurrent writeback of the same page,
which can read a partially-updated bitmap word or race with
the dirty-bit update.
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
cc: Christoph Hellwig <hch@lst.de>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
---
fs/hfsplus/bitmap.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/fs/hfsplus/bitmap.c b/fs/hfsplus/bitmap.c
index 1b3af8c87cad..30178ea47362 100644
--- a/fs/hfsplus/bitmap.c
+++ b/fs/hfsplus/bitmap.c
@@ -39,6 +39,8 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size,
start = size;
goto out;
}
+ lock_page(page);
+ folio_wait_writeback(page_folio(page));
pptr = kmap_local_page(page);
curr = pptr + (offset & (PAGE_CACHE_BITS - 1)) / 32;
i = offset % 32;
@@ -75,6 +77,7 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size,
curr++;
}
kunmap_local(pptr);
+ unlock_page(page);
offset += PAGE_CACHE_BITS;
if (offset >= size)
break;
@@ -84,6 +87,8 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size,
start = size;
goto out;
}
+ lock_page(page);
+ folio_wait_writeback(page_folio(page));
curr = pptr = kmap_local_page(page);
if ((size ^ offset) / PAGE_CACHE_BITS)
end = pptr + PAGE_CACHE_BITS / 32;
@@ -98,6 +103,9 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size,
start = offset + (curr - pptr) * 32 + i;
if (start >= size) {
hfs_dbg("bitmap full\n");
+ kunmap_local(pptr);
+ unlock_page(page);
+ start = size;
goto out;
}
/* do any partial u32 at the start */
@@ -128,6 +136,7 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size,
}
set_page_dirty(page);
kunmap_local(pptr);
+ unlock_page(page);
offset += PAGE_CACHE_BITS;
page = read_mapping_page(mapping, offset / PAGE_CACHE_BITS,
NULL);
@@ -135,6 +144,8 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size,
start = size;
goto out;
}
+ lock_page(page);
+ folio_wait_writeback(page_folio(page));
pptr = kmap_local_page(page);
curr = pptr;
end = pptr + PAGE_CACHE_BITS / 32;
@@ -152,6 +163,7 @@ int hfsplus_block_allocate(struct super_block *sb, u32 size,
*curr = cpu_to_be32(n);
set_page_dirty(page);
kunmap_local(pptr);
+ unlock_page(page);
*max = offset + (curr - pptr) * 32 + i - start;
sbi->free_blocks -= *max;
hfsplus_mark_mdb_dirty(sb);
@@ -185,6 +197,8 @@ int hfsplus_block_free(struct super_block *sb, u32 offset, u32 count)
page = read_mapping_page(mapping, pnr, NULL);
if (IS_ERR(page))
goto kaboom;
+ lock_page(page);
+ folio_wait_writeback(page_folio(page));
pptr = kmap_local_page(page);
curr = pptr + (offset & (PAGE_CACHE_BITS - 1)) / 32;
end = pptr + PAGE_CACHE_BITS / 32;
@@ -216,9 +230,12 @@ int hfsplus_block_free(struct super_block *sb, u32 offset, u32 count)
break;
set_page_dirty(page);
kunmap_local(pptr);
+ unlock_page(page);
page = read_mapping_page(mapping, ++pnr, NULL);
if (IS_ERR(page))
goto kaboom;
+ lock_page(page);
+ folio_wait_writeback(page_folio(page));
pptr = kmap_local_page(page);
curr = pptr;
end = pptr + PAGE_CACHE_BITS / 32;
@@ -232,6 +249,7 @@ int hfsplus_block_free(struct super_block *sb, u32 offset, u32 count)
out:
set_page_dirty(page);
kunmap_local(pptr);
+ unlock_page(page);
sbi->free_blocks += len;
hfsplus_mark_mdb_dirty(sb);
mutex_unlock(&sbi->alloc_mutex);
--
2.43.0
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 3/7] hfsplus: take the bitmap page lock for allocate/free
2026-09-14 23:39 ` [PATCH v4 3/7] hfsplus: take the bitmap page lock for allocate/free Viacheslav Dubeyko
@ 2026-09-18 13:30 ` Christoph Hellwig
0 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2026-09-18 13:30 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner, djwong
On Mon, Sep 14, 2026 at 04:39:37PM -0700, Viacheslav Dubeyko wrote:
> The hfsplus_block_allocate() and hfsplus_block_free() kmap
> the allocation bitmap's pages and modify their bits in place
> under sbi->alloc_mutex, but without holding the page lock.
> That leaves the read-modify-write of the bitmap bits
> unprotected against a concurrent writeback of the same page,
> which can read a partially-updated bitmap word or race with
> the dirty-bit update.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
> + lock_page(page);
> + folio_wait_writeback(page_folio(page));
> pptr = kmap_local_page(page);
Although this really begs for a folio conversion rather sooner than
later.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 4/7] hfsplus: add iomap operations for regular file data
2026-09-14 23:39 [PATCH v4 0/7] hfsplus: convert regular file I/O to iomap-based operations Viacheslav Dubeyko
` (2 preceding siblings ...)
2026-09-14 23:39 ` [PATCH v4 3/7] hfsplus: take the bitmap page lock for allocate/free Viacheslav Dubeyko
@ 2026-09-14 23:39 ` Viacheslav Dubeyko
2026-09-18 13:36 ` Christoph Hellwig
2026-09-14 23:39 ` [PATCH v4 5/7] hfsplus: move file related operations to file.c Viacheslav Dubeyko
` (2 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-14 23:39 UTC (permalink / raw)
To: glaubitz, frank.li, hch
Cc: linux-fsdevel, linux-kernel, vdubeyko, willy, brauner, djwong,
Viacheslav Dubeyko
This patch adds iomap.h and iomap.c files. The iomap.h contains
declarations of hfsplus_iomap_ops, hfsplus_write_iomap_ops,
hfsplus_writeback_ops, and hfsplus_write_dio_ops operations.
The iomap.c implements __hfsplus_iomap_begin(),
hfsplus_write_iomap_end(), hfsplus_iomap_cont_expand(),
hfsplus_writeback_range() methods that become the basis of
HFS+ iomap operations.
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
cc: Christoph Hellwig <hch@lst.de>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Acked-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/hfsplus/Kconfig | 1 +
fs/hfsplus/Makefile | 5 +-
fs/hfsplus/hfsplus_fs.h | 16 ++++
fs/hfsplus/iomap.c | 192 ++++++++++++++++++++++++++++++++++++++++
fs/hfsplus/iomap.h | 18 ++++
include/linux/iomap.h | 20 +++++
6 files changed, 250 insertions(+), 2 deletions(-)
create mode 100644 fs/hfsplus/iomap.c
create mode 100644 fs/hfsplus/iomap.h
diff --git a/fs/hfsplus/Kconfig b/fs/hfsplus/Kconfig
index ca8401cb6954..865a1966f395 100644
--- a/fs/hfsplus/Kconfig
+++ b/fs/hfsplus/Kconfig
@@ -6,6 +6,7 @@ config HFSPLUS_FS
select NLS
select NLS_UTF8
select LEGACY_DIRECT_IO
+ select FS_IOMAP
help
If you say Y here, you will be able to mount extended format
Macintosh-formatted hard drive partitions with full read-write access.
diff --git a/fs/hfsplus/Makefile b/fs/hfsplus/Makefile
index f2a9ae697e81..2416dfdc3190 100644
--- a/fs/hfsplus/Makefile
+++ b/fs/hfsplus/Makefile
@@ -5,8 +5,9 @@
obj-$(CONFIG_HFSPLUS_FS) += hfsplus.o
-hfsplus-objs := super.o options.o inode.o ioctl.o extents.o catalog.o dir.o btree.o \
- bnode.o brec.o bfind.o tables.o unicode.o wrapper.o bitmap.o part_tbl.o \
+hfsplus-objs := super.o options.o inode.o iomap.o ioctl.o extents.o catalog.o \
+ dir.o btree.o bnode.o brec.o bfind.o tables.o unicode.o \
+ wrapper.o bitmap.o part_tbl.o \
attributes.o xattr.o xattr_user.o xattr_security.o xattr_trusted.o
# KUnit tests
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 67586382269b..0a0df0388e7b 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -175,6 +175,22 @@ static inline struct hfsplus_sb_info *HFSPLUS_SB(struct super_block *sb)
return sb->s_fs_info;
}
+/*
+ * Physical byte offset of allocation block 'dblock' on the volume.
+ */
+static inline loff_t hfsplus_ablock_to_phys_bytes(struct super_block *sb,
+ u32 dblock)
+{
+ struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+ loff_t phys_bytes;
+
+ phys_bytes = dblock;
+ phys_bytes <<= sbi->fs_shift;
+ phys_bytes += sbi->blockoffset;
+ phys_bytes <<= sb->s_blocksize_bits;
+
+ return phys_bytes;
+}
struct hfsplus_inode_info {
atomic_t opencnt;
diff --git a/fs/hfsplus/iomap.c b/fs/hfsplus/iomap.c
new file mode 100644
index 000000000000..5723e854e58e
--- /dev/null
+++ b/fs/hfsplus/iomap.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * iomap callback functions for the hfsplus filesystem
+ */
+
+#include <linux/iomap.h>
+#include <linux/pagemap.h>
+
+#include "hfsplus_fs.h"
+#include "hfsplus_raw.h"
+#include "iomap.h"
+
+static int __hfsplus_iomap_begin(struct inode *inode, loff_t offset,
+ loff_t length, unsigned int flags,
+ struct iomap *iomap, bool may_alloc)
+{
+ struct super_block *sb = inode->i_sb;
+ struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+ struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
+ u32 ablock, dblock, max_blocks;
+ loff_t ablock_offset, ablock_bytes;
+ loff_t block_start;
+ bool is_new;
+ int err;
+
+ if (!may_alloc) {
+ /* Completely beyond EOF. Treat as hole */
+ if (i_size_read(inode) <= offset) {
+ iomap->type = IOMAP_HOLE;
+ iomap->addr = IOMAP_NULL_ADDR;
+ iomap->offset = offset;
+ iomap->length = length;
+ return 0;
+ }
+
+ /* Clamp length if the requested range goes beyond i_size */
+ if (offset + length > i_size_read(inode)) {
+ loff_t i_size = i_size_read(inode);
+ unsigned int blocksize = i_blocksize(inode);
+
+ length = round_up(i_size, blocksize) - offset;
+ }
+ }
+
+ ablock = offset >> sbi->alloc_blksz_shift;
+
+ err = hfsplus_map_extent(inode, ablock, may_alloc, &dblock,
+ &max_blocks, NULL);
+ if (err)
+ return err;
+
+ ablock_offset = offset & (sbi->alloc_blksz - 1);
+ ablock_bytes = (loff_t)max_blocks << sbi->alloc_blksz_shift;
+
+ length = min_t(loff_t, length, ablock_bytes - ablock_offset);
+ block_start = round_down(offset, i_blocksize(inode));
+ is_new = may_alloc && block_start >= hip->phys_size;
+ if (may_alloc && !is_new && offset < hip->phys_size)
+ length = min_t(loff_t, length, hip->phys_size - offset);
+
+ iomap->bdev = sb->s_bdev;
+ iomap->offset = offset;
+ iomap->length = length;
+ iomap->addr = hfsplus_ablock_to_phys_bytes(sb, dblock) + ablock_offset;
+ iomap->type = IOMAP_MAPPED;
+ iomap->flags = IOMAP_F_MERGED;
+
+ if (is_new)
+ iomap->flags |= IOMAP_F_NEW;
+
+ return 0;
+}
+
+static int hfsplus_iomap_begin(struct inode *inode, loff_t offset,
+ loff_t length, unsigned int flags,
+ struct iomap *iomap, struct iomap *srcmap)
+{
+ return __hfsplus_iomap_begin(inode,
+ offset, length, flags,
+ iomap, false);
+}
+
+static int hfsplus_write_iomap_begin(struct inode *inode, loff_t offset,
+ loff_t length, unsigned int flags,
+ struct iomap *iomap, struct iomap *srcmap)
+{
+ return __hfsplus_iomap_begin(inode,
+ offset, length, flags,
+ iomap, true);
+}
+
+const struct iomap_ops hfsplus_iomap_ops = {
+ .iomap_begin = hfsplus_iomap_begin,
+};
+
+/*
+ * hfsplus_write_iomap_end()
+ *
+ * Advance the allocated-and-zeroed high-water mark
+ * (hip->phys_size / hip->fs_blocks) to cover the newly written range.
+ */
+static int hfsplus_write_iomap_end(struct inode *inode, loff_t pos,
+ loff_t length, ssize_t written,
+ unsigned int flags, struct iomap *iomap)
+{
+ struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
+ struct super_block *sb = inode->i_sb;
+ loff_t end;
+ bool dirtied = false;
+
+ if (!written)
+ return 0;
+
+ end = pos + written;
+
+ if (iomap->flags & IOMAP_F_NEW)
+ end = round_up(end, sb->s_blocksize);
+
+ if (hip->phys_size < end) {
+ inode_add_bytes(inode, end - hip->phys_size);
+ hip->phys_size = end;
+ hip->fs_blocks = end >> sb->s_blocksize_bits;
+ dirtied = true;
+ }
+
+ if (dirtied)
+ mark_inode_dirty(inode);
+
+ return written;
+}
+
+const struct iomap_ops hfsplus_write_iomap_ops = {
+ .iomap_begin = hfsplus_write_iomap_begin,
+ .iomap_end = hfsplus_write_iomap_end,
+};
+
+/*
+ * hfsplus_iomap_cont_expand()
+ *
+ * Zero-extend the backing store from the current phys_size up to 'size'.
+ * Used both by hfsplus_setattr() and by hfsplus_file_truncate().
+ */
+int hfsplus_iomap_cont_expand(struct inode *inode, loff_t size)
+{
+ struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
+ loff_t start = hip->phys_size;
+
+ if (size <= start)
+ return 0;
+
+ return iomap_zero_range(inode, start, size - start, NULL,
+ &hfsplus_write_iomap_ops, NULL, NULL);
+}
+
+/*
+ * hfsplus_writeback_range() - map folio during writeback
+ *
+ * Called for each folio during writeback. If the folio falls outside
+ * the current iomap, remaps by calling __hfsplus_iomap_begin() again.
+ */
+static ssize_t hfsplus_writeback_range(struct iomap_writepage_ctx *wpc,
+ struct folio *folio, u64 offset,
+ unsigned int len, u64 end_pos)
+{
+ int err;
+
+ if (offset < wpc->iomap.offset ||
+ offset >= wpc->iomap.offset + wpc->iomap.length) {
+ err = __hfsplus_iomap_begin(wpc->inode,
+ offset, len, 0,
+ &wpc->iomap, false);
+ if (err)
+ return err;
+ }
+
+ return iomap_add_to_ioend(wpc, folio, offset, end_pos, len);
+}
+
+const struct iomap_writeback_ops hfsplus_writeback_ops = {
+ .writeback_range = hfsplus_writeback_range,
+ .writeback_submit = iomap_ioend_writeback_submit,
+};
+
+const struct iomap_dio_ops hfsplus_write_dio_ops = {
+ .end_io = iomap_dio_end_io,
+};
+
+int hfsplus_iomap_swap_activate(struct swap_info_struct *sis,
+ struct file *file, sector_t *span)
+{
+ return iomap_swapfile_activate(sis, file, span, &hfsplus_iomap_ops);
+}
diff --git a/fs/hfsplus/iomap.h b/fs/hfsplus/iomap.h
new file mode 100644
index 000000000000..dac07a9d25f8
--- /dev/null
+++ b/fs/hfsplus/iomap.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * iomap callback declarations for the hfsplus filesystem
+ */
+
+#ifndef _LINUX_HFSPLUS_IOMAP_H
+#define _LINUX_HFSPLUS_IOMAP_H
+
+extern const struct iomap_ops hfsplus_iomap_ops;
+extern const struct iomap_ops hfsplus_write_iomap_ops;
+extern const struct iomap_writeback_ops hfsplus_writeback_ops;
+extern const struct iomap_dio_ops hfsplus_write_dio_ops;
+
+int hfsplus_iomap_cont_expand(struct inode *inode, loff_t size);
+int hfsplus_iomap_swap_activate(struct swap_info_struct *sis,
+ struct file *file, sector_t *span);
+
+#endif /* _LINUX_HFSPLUS_IOMAP_H */
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index bc7ae6327dbf..66a6389c9c74 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -636,6 +636,26 @@ struct iomap_dio_ops {
struct bio_set *bio_set;
};
+/*
+ * Direct I/O completion handler
+ */
+static inline
+int iomap_dio_end_io(struct kiocb *iocb, ssize_t size,
+ int error, unsigned int flags)
+{
+ struct inode *inode = file_inode(iocb->ki_filp);
+
+ if (error)
+ return error;
+
+ if (size && i_size_read(inode) < iocb->ki_pos + size) {
+ i_size_write(inode, iocb->ki_pos + size);
+ mark_inode_dirty(inode);
+ }
+
+ return 0;
+}
+
/*
* Wait for the I/O to complete in iomap_dio_rw even if the kiocb is not
* synchronous.
--
2.43.0
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 4/7] hfsplus: add iomap operations for regular file data
2026-09-14 23:39 ` [PATCH v4 4/7] hfsplus: add iomap operations for regular file data Viacheslav Dubeyko
@ 2026-09-18 13:36 ` Christoph Hellwig
0 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2026-09-18 13:36 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner, djwong
On Mon, Sep 14, 2026 at 04:39:38PM -0700, Viacheslav Dubeyko wrote:
> This patch adds iomap.h and iomap.c files. The iomap.h contains
> declarations of hfsplus_iomap_ops, hfsplus_write_iomap_ops,
> hfsplus_writeback_ops, and hfsplus_write_dio_ops operations.
> The iomap.c implements __hfsplus_iomap_begin(),
> hfsplus_write_iomap_end(), hfsplus_iomap_cont_expand(),
> hfsplus_writeback_range() methods that become the basis of
> HFS+ iomap operations.
Adding this without the users in the last two patches is odd, as this
isn't really the kind of atomic change we're usually doing in Linux.
I'd vote for merging this and the last two patches into one.
> index 000000000000..5723e854e58e
> --- /dev/null
> +++ b/fs/hfsplus/iomap.c
> @@ -0,0 +1,192 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * iomap callback functions for the hfsplus filesystem
In Linux terminology these are methods, not callbacks. I'd probably just
drop this comment entirely, as it doesn't really add value, though.
> +static int hfsplus_iomap_begin(struct inode *inode, loff_t offset,
> + loff_t length, unsigned int flags,
> + struct iomap *iomap, struct iomap *srcmap)
> +{
> + return __hfsplus_iomap_begin(inode,
> + offset, length, flags,
> + iomap, false);
Odd formatting. Why not:
return __hfsplus_iomap_begin(inode, offset, length, flags, iomap,
false);
?
Also may_alloc as flags with a readable flag name might be nicer here.
> +}
> +
> +static int hfsplus_write_iomap_begin(struct inode *inode, loff_t offset,
> + loff_t length, unsigned int flags,
> + struct iomap *iomap, struct iomap *srcmap)
> +{
> + return __hfsplus_iomap_begin(inode,
> + offset, length, flags,
> + iomap, true);
> +}
Same.
> +const struct iomap_ops hfsplus_iomap_ops = {
> + .iomap_begin = hfsplus_iomap_begin,
> +};
Please use the new iomap next scheme merged in 7.3-rc. In fact
the old begin/end methods were supposed to be removed after -rc1,
but someone they managed to still stay around.
> +
> +/*
> + * hfsplus_write_iomap_end()
Duplicating the function name adds no value.
Same for the other functions.
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * iomap callback declarations for the hfsplus filesystem
> + */
Same comment as for iomap.c.
> +static inline
> +int iomap_dio_end_io(struct kiocb *iocb, ssize_t size,
> + int error, unsigned int flags)
Odd formatting, normally we'd do:
static inline int iomap_dio_end_io(struct kiocb *iocb, ssize_t size, int error,
unsigned int flags)
or:
static inline int
iomap_dio_end_io(struct kiocb *iocb, ssize_t size, int error,
unsigned int flags)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 5/7] hfsplus: move file related operations to file.c
2026-09-14 23:39 [PATCH v4 0/7] hfsplus: convert regular file I/O to iomap-based operations Viacheslav Dubeyko
` (3 preceding siblings ...)
2026-09-14 23:39 ` [PATCH v4 4/7] hfsplus: add iomap operations for regular file data Viacheslav Dubeyko
@ 2026-09-14 23:39 ` Viacheslav Dubeyko
2026-09-15 2:22 ` Darrick J. Wong
2026-09-18 13:31 ` Christoph Hellwig
2026-09-14 23:39 ` [PATCH v4 6/7] hfsplus: introduce iomap-based file_operations Viacheslav Dubeyko
2026-09-14 23:39 ` [PATCH v4 7/7] hfsplus: switch address_space_operations on iomap-based support Viacheslav Dubeyko
6 siblings, 2 replies; 20+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-14 23:39 UTC (permalink / raw)
To: glaubitz, frank.li, hch
Cc: linux-fsdevel, linux-kernel, vdubeyko, willy, brauner, djwong,
Viacheslav Dubeyko
This patch introduces fs/hfsplus/file.c and moves
file related operations from fs/hfsplus/inode.c
into the new file.
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
cc: Christoph Hellwig <hch@lst.de>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
---
fs/hfsplus/Makefile | 6 +-
fs/hfsplus/file.c | 133 ++++++++++++++++++++++++++++++++++++++++
fs/hfsplus/hfsplus_fs.h | 7 ++-
fs/hfsplus/inode.c | 122 ------------------------------------
4 files changed, 141 insertions(+), 127 deletions(-)
create mode 100644 fs/hfsplus/file.c
diff --git a/fs/hfsplus/Makefile b/fs/hfsplus/Makefile
index 2416dfdc3190..3ddea69a9c69 100644
--- a/fs/hfsplus/Makefile
+++ b/fs/hfsplus/Makefile
@@ -5,9 +5,9 @@
obj-$(CONFIG_HFSPLUS_FS) += hfsplus.o
-hfsplus-objs := super.o options.o inode.o iomap.o ioctl.o extents.o catalog.o \
- dir.o btree.o bnode.o brec.o bfind.o tables.o unicode.o \
- wrapper.o bitmap.o part_tbl.o \
+hfsplus-objs := super.o options.o inode.o file.o iomap.o ioctl.o extents.o \
+ catalog.o dir.o btree.o bnode.o brec.o bfind.o tables.o \
+ unicode.o wrapper.o bitmap.o part_tbl.o \
attributes.o xattr.o xattr_user.o xattr_security.o xattr_trusted.o
# KUnit tests
diff --git a/fs/hfsplus/file.c b/fs/hfsplus/file.c
new file mode 100644
index 000000000000..509046aad0c6
--- /dev/null
+++ b/fs/hfsplus/file.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * File operations: open/release/fsync and iomap-based read/write/seek
+ */
+
+#include <linux/fs.h>
+#include <linux/uio.h>
+#include <linux/mount.h>
+
+#include "hfsplus_fs.h"
+#include "hfsplus_raw.h"
+
+static int hfsplus_file_open(struct inode *inode, struct file *file)
+{
+ if (HFSPLUS_IS_RSRC(inode))
+ inode = HFSPLUS_I(inode)->rsrc_inode;
+ if (!(file->f_flags & O_LARGEFILE) && i_size_read(inode) > MAX_NON_LFS)
+ return -EOVERFLOW;
+ atomic_inc(&HFSPLUS_I(inode)->opencnt);
+ return 0;
+}
+
+static int hfsplus_file_release(struct inode *inode, struct file *file)
+{
+ struct super_block *sb = inode->i_sb;
+
+ if (HFSPLUS_IS_RSRC(inode))
+ inode = HFSPLUS_I(inode)->rsrc_inode;
+ if (atomic_dec_and_test(&HFSPLUS_I(inode)->opencnt)) {
+ inode_lock(inode);
+ hfsplus_file_truncate(inode);
+ if (inode->i_flags & S_DEAD) {
+ hfsplus_delete_cat(inode->i_ino,
+ HFSPLUS_SB(sb)->hidden_dir, NULL);
+ hfsplus_delete_inode(inode);
+ }
+ inode_unlock(inode);
+ }
+ return 0;
+}
+
+int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
+ int datasync)
+{
+ struct inode *inode = file->f_mapping->host;
+ struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
+ struct super_block *sb = inode->i_sb;
+ struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb);
+ struct hfsplus_vh *vhdr = sbi->s_vhdr;
+ int error = 0, error2;
+
+ hfs_dbg("inode->i_ino %llu, start %llu, end %llu\n",
+ inode->i_ino, start, end);
+
+ error = file_write_and_wait_range(file, start, end);
+ if (error)
+ return error;
+ inode_lock(inode);
+
+ /*
+ * Sync inode metadata into the catalog and extent trees.
+ */
+ sync_inode_metadata(inode, 1);
+
+ /*
+ * And explicitly write out the btrees.
+ */
+ if (test_and_clear_bit(HFSPLUS_I_CAT_DIRTY,
+ &HFSPLUS_I(HFSPLUS_CAT_TREE_I(sb))->flags)) {
+ clear_bit(HFSPLUS_I_CAT_DIRTY, &hip->flags);
+ error = filemap_write_and_wait(sbi->cat_tree->inode->i_mapping);
+ }
+
+ if (test_and_clear_bit(HFSPLUS_I_EXT_DIRTY,
+ &HFSPLUS_I(HFSPLUS_EXT_TREE_I(sb))->flags)) {
+ clear_bit(HFSPLUS_I_EXT_DIRTY, &hip->flags);
+ error2 =
+ filemap_write_and_wait(sbi->ext_tree->inode->i_mapping);
+ if (!error)
+ error = error2;
+ }
+
+ if (sbi->attr_tree) {
+ if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY,
+ &HFSPLUS_I(HFSPLUS_ATTR_TREE_I(sb))->flags)) {
+ clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags);
+ error2 =
+ filemap_write_and_wait(
+ sbi->attr_tree->inode->i_mapping);
+ if (!error)
+ error = error2;
+ }
+ } else {
+ if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags))
+ pr_err("sync non-existent attributes tree\n");
+ }
+
+ if (test_and_clear_bit(HFSPLUS_I_ALLOC_DIRTY,
+ &HFSPLUS_I(sbi->alloc_file)->flags)) {
+ clear_bit(HFSPLUS_I_ALLOC_DIRTY, &hip->flags);
+ error2 = filemap_write_and_wait(sbi->alloc_file->i_mapping);
+ if (!error)
+ error = error2;
+ }
+
+ mutex_lock(&sbi->vh_mutex);
+ hfsplus_prepare_volume_header_for_commit(vhdr);
+ mutex_unlock(&sbi->vh_mutex);
+
+ error2 = hfsplus_commit_superblock(inode->i_sb);
+ if (!error)
+ error = error2;
+
+ if (!test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
+ blkdev_issue_flush(inode->i_sb->s_bdev);
+
+ inode_unlock(inode);
+
+ return error;
+}
+
+const struct file_operations hfsplus_file_operations = {
+ .llseek = generic_file_llseek,
+ .read_iter = generic_file_read_iter,
+ .write_iter = generic_file_write_iter,
+ .mmap_prepare = generic_file_mmap_prepare,
+ .splice_read = filemap_splice_read,
+ .splice_write = iter_file_splice_write,
+ .fsync = hfsplus_file_fsync,
+ .open = hfsplus_file_open,
+ .release = hfsplus_file_release,
+ .unlocked_ioctl = hfsplus_ioctl,
+};
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 0a0df0388e7b..190c7de704fd 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -446,6 +446,11 @@ int hfsplus_rename_cat(u32 cnid, struct inode *src_dir, const struct qstr *src_n
extern const struct inode_operations hfsplus_dir_inode_operations;
extern const struct file_operations hfsplus_dir_operations;
+/* file.c */
+extern const struct file_operations hfsplus_file_operations;
+int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
+ int datasync);
+
/* extents.c */
int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
const hfsplus_btree_key *k2);
@@ -480,8 +485,6 @@ int hfsplus_cat_write_inode(struct inode *inode);
int hfsplus_getattr(struct mnt_idmap *idmap, const struct path *path,
struct kstat *stat, u32 request_mask,
unsigned int query_flags);
-int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
- int datasync);
int hfsplus_fileattr_get(struct dentry *dentry, struct file_kattr *fa);
int hfsplus_fileattr_set(struct mnt_idmap *idmap,
struct dentry *dentry, struct file_kattr *fa);
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 2ce6de574fa6..9d25e6224ee5 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -276,35 +276,6 @@ static int hfsplus_get_perms(struct inode *inode,
return -EIO;
}
-static int hfsplus_file_open(struct inode *inode, struct file *file)
-{
- if (HFSPLUS_IS_RSRC(inode))
- inode = HFSPLUS_I(inode)->rsrc_inode;
- if (!(file->f_flags & O_LARGEFILE) && i_size_read(inode) > MAX_NON_LFS)
- return -EOVERFLOW;
- atomic_inc(&HFSPLUS_I(inode)->opencnt);
- return 0;
-}
-
-static int hfsplus_file_release(struct inode *inode, struct file *file)
-{
- struct super_block *sb = inode->i_sb;
-
- if (HFSPLUS_IS_RSRC(inode))
- inode = HFSPLUS_I(inode)->rsrc_inode;
- if (atomic_dec_and_test(&HFSPLUS_I(inode)->opencnt)) {
- inode_lock(inode);
- hfsplus_file_truncate(inode);
- if (inode->i_flags & S_DEAD) {
- hfsplus_delete_cat(inode->i_ino,
- HFSPLUS_SB(sb)->hidden_dir, NULL);
- hfsplus_delete_inode(inode);
- }
- inode_unlock(inode);
- }
- return 0;
-}
-
static int hfsplus_setattr(struct mnt_idmap *idmap,
struct dentry *dentry, struct iattr *attr)
{
@@ -361,86 +332,6 @@ int hfsplus_getattr(struct mnt_idmap *idmap, const struct path *path,
return 0;
}
-int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
- int datasync)
-{
- struct inode *inode = file->f_mapping->host;
- struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
- struct super_block *sb = inode->i_sb;
- struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb);
- struct hfsplus_vh *vhdr = sbi->s_vhdr;
- int error = 0, error2;
-
- hfs_dbg("inode->i_ino %llu, start %llu, end %llu\n",
- inode->i_ino, start, end);
-
- error = file_write_and_wait_range(file, start, end);
- if (error)
- return error;
- inode_lock(inode);
-
- /*
- * Sync inode metadata into the catalog and extent trees.
- */
- sync_inode_metadata(inode, 1);
-
- /*
- * And explicitly write out the btrees.
- */
- if (test_and_clear_bit(HFSPLUS_I_CAT_DIRTY,
- &HFSPLUS_I(HFSPLUS_CAT_TREE_I(sb))->flags)) {
- clear_bit(HFSPLUS_I_CAT_DIRTY, &hip->flags);
- error = filemap_write_and_wait(sbi->cat_tree->inode->i_mapping);
- }
-
- if (test_and_clear_bit(HFSPLUS_I_EXT_DIRTY,
- &HFSPLUS_I(HFSPLUS_EXT_TREE_I(sb))->flags)) {
- clear_bit(HFSPLUS_I_EXT_DIRTY, &hip->flags);
- error2 =
- filemap_write_and_wait(sbi->ext_tree->inode->i_mapping);
- if (!error)
- error = error2;
- }
-
- if (sbi->attr_tree) {
- if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY,
- &HFSPLUS_I(HFSPLUS_ATTR_TREE_I(sb))->flags)) {
- clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags);
- error2 =
- filemap_write_and_wait(
- sbi->attr_tree->inode->i_mapping);
- if (!error)
- error = error2;
- }
- } else {
- if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags))
- pr_err("sync non-existent attributes tree\n");
- }
-
- if (test_and_clear_bit(HFSPLUS_I_ALLOC_DIRTY,
- &HFSPLUS_I(sbi->alloc_file)->flags)) {
- clear_bit(HFSPLUS_I_ALLOC_DIRTY, &hip->flags);
- error2 = filemap_write_and_wait(sbi->alloc_file->i_mapping);
- if (!error)
- error = error2;
- }
-
- mutex_lock(&sbi->vh_mutex);
- hfsplus_prepare_volume_header_for_commit(vhdr);
- mutex_unlock(&sbi->vh_mutex);
-
- error2 = hfsplus_commit_superblock(inode->i_sb);
- if (!error)
- error = error2;
-
- if (!test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
- blkdev_issue_flush(inode->i_sb->s_bdev);
-
- inode_unlock(inode);
-
- return error;
-}
-
static const struct inode_operations hfsplus_file_inode_operations = {
.setattr = hfsplus_setattr,
.getattr = hfsplus_getattr,
@@ -462,19 +353,6 @@ static const struct inode_operations hfsplus_special_inode_operations = {
.listxattr = hfsplus_listxattr,
};
-static const struct file_operations hfsplus_file_operations = {
- .llseek = generic_file_llseek,
- .read_iter = generic_file_read_iter,
- .write_iter = generic_file_write_iter,
- .mmap_prepare = generic_file_mmap_prepare,
- .splice_read = filemap_splice_read,
- .splice_write = iter_file_splice_write,
- .fsync = hfsplus_file_fsync,
- .open = hfsplus_file_open,
- .release = hfsplus_file_release,
- .unlocked_ioctl = hfsplus_ioctl,
-};
-
struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
umode_t mode)
{
--
2.43.0
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/7] hfsplus: move file related operations to file.c
2026-09-14 23:39 ` [PATCH v4 5/7] hfsplus: move file related operations to file.c Viacheslav Dubeyko
@ 2026-09-15 2:22 ` Darrick J. Wong
2026-09-18 13:31 ` Christoph Hellwig
1 sibling, 0 replies; 20+ messages in thread
From: Darrick J. Wong @ 2026-09-15 2:22 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner
On Mon, Sep 14, 2026 at 04:39:39PM -0700, Viacheslav Dubeyko wrote:
> This patch introduces fs/hfsplus/file.c and moves
> file related operations from fs/hfsplus/inode.c
> into the new file.
>
> Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
> cc: Christoph Hellwig <hch@lst.de>
> cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> cc: Yangtao Li <frank.li@vivo.com>
> cc: linux-fsdevel@vger.kernel.org
Makes sense to me to split up file vs. inode code,
Acked-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/hfsplus/Makefile | 6 +-
> fs/hfsplus/file.c | 133 ++++++++++++++++++++++++++++++++++++++++
> fs/hfsplus/hfsplus_fs.h | 7 ++-
> fs/hfsplus/inode.c | 122 ------------------------------------
> 4 files changed, 141 insertions(+), 127 deletions(-)
> create mode 100644 fs/hfsplus/file.c
>
> diff --git a/fs/hfsplus/Makefile b/fs/hfsplus/Makefile
> index 2416dfdc3190..3ddea69a9c69 100644
> --- a/fs/hfsplus/Makefile
> +++ b/fs/hfsplus/Makefile
> @@ -5,9 +5,9 @@
>
> obj-$(CONFIG_HFSPLUS_FS) += hfsplus.o
>
> -hfsplus-objs := super.o options.o inode.o iomap.o ioctl.o extents.o catalog.o \
> - dir.o btree.o bnode.o brec.o bfind.o tables.o unicode.o \
> - wrapper.o bitmap.o part_tbl.o \
> +hfsplus-objs := super.o options.o inode.o file.o iomap.o ioctl.o extents.o \
> + catalog.o dir.o btree.o bnode.o brec.o bfind.o tables.o \
> + unicode.o wrapper.o bitmap.o part_tbl.o \
> attributes.o xattr.o xattr_user.o xattr_security.o xattr_trusted.o
>
> # KUnit tests
> diff --git a/fs/hfsplus/file.c b/fs/hfsplus/file.c
> new file mode 100644
> index 000000000000..509046aad0c6
> --- /dev/null
> +++ b/fs/hfsplus/file.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * File operations: open/release/fsync and iomap-based read/write/seek
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/uio.h>
> +#include <linux/mount.h>
> +
> +#include "hfsplus_fs.h"
> +#include "hfsplus_raw.h"
> +
> +static int hfsplus_file_open(struct inode *inode, struct file *file)
> +{
> + if (HFSPLUS_IS_RSRC(inode))
> + inode = HFSPLUS_I(inode)->rsrc_inode;
> + if (!(file->f_flags & O_LARGEFILE) && i_size_read(inode) > MAX_NON_LFS)
> + return -EOVERFLOW;
> + atomic_inc(&HFSPLUS_I(inode)->opencnt);
> + return 0;
> +}
> +
> +static int hfsplus_file_release(struct inode *inode, struct file *file)
> +{
> + struct super_block *sb = inode->i_sb;
> +
> + if (HFSPLUS_IS_RSRC(inode))
> + inode = HFSPLUS_I(inode)->rsrc_inode;
> + if (atomic_dec_and_test(&HFSPLUS_I(inode)->opencnt)) {
> + inode_lock(inode);
> + hfsplus_file_truncate(inode);
> + if (inode->i_flags & S_DEAD) {
> + hfsplus_delete_cat(inode->i_ino,
> + HFSPLUS_SB(sb)->hidden_dir, NULL);
> + hfsplus_delete_inode(inode);
> + }
> + inode_unlock(inode);
> + }
> + return 0;
> +}
> +
> +int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
> + int datasync)
> +{
> + struct inode *inode = file->f_mapping->host;
> + struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
> + struct super_block *sb = inode->i_sb;
> + struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb);
> + struct hfsplus_vh *vhdr = sbi->s_vhdr;
> + int error = 0, error2;
> +
> + hfs_dbg("inode->i_ino %llu, start %llu, end %llu\n",
> + inode->i_ino, start, end);
> +
> + error = file_write_and_wait_range(file, start, end);
> + if (error)
> + return error;
> + inode_lock(inode);
> +
> + /*
> + * Sync inode metadata into the catalog and extent trees.
> + */
> + sync_inode_metadata(inode, 1);
> +
> + /*
> + * And explicitly write out the btrees.
> + */
> + if (test_and_clear_bit(HFSPLUS_I_CAT_DIRTY,
> + &HFSPLUS_I(HFSPLUS_CAT_TREE_I(sb))->flags)) {
> + clear_bit(HFSPLUS_I_CAT_DIRTY, &hip->flags);
> + error = filemap_write_and_wait(sbi->cat_tree->inode->i_mapping);
> + }
> +
> + if (test_and_clear_bit(HFSPLUS_I_EXT_DIRTY,
> + &HFSPLUS_I(HFSPLUS_EXT_TREE_I(sb))->flags)) {
> + clear_bit(HFSPLUS_I_EXT_DIRTY, &hip->flags);
> + error2 =
> + filemap_write_and_wait(sbi->ext_tree->inode->i_mapping);
> + if (!error)
> + error = error2;
> + }
> +
> + if (sbi->attr_tree) {
> + if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY,
> + &HFSPLUS_I(HFSPLUS_ATTR_TREE_I(sb))->flags)) {
> + clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags);
> + error2 =
> + filemap_write_and_wait(
> + sbi->attr_tree->inode->i_mapping);
> + if (!error)
> + error = error2;
> + }
> + } else {
> + if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags))
> + pr_err("sync non-existent attributes tree\n");
> + }
> +
> + if (test_and_clear_bit(HFSPLUS_I_ALLOC_DIRTY,
> + &HFSPLUS_I(sbi->alloc_file)->flags)) {
> + clear_bit(HFSPLUS_I_ALLOC_DIRTY, &hip->flags);
> + error2 = filemap_write_and_wait(sbi->alloc_file->i_mapping);
> + if (!error)
> + error = error2;
> + }
> +
> + mutex_lock(&sbi->vh_mutex);
> + hfsplus_prepare_volume_header_for_commit(vhdr);
> + mutex_unlock(&sbi->vh_mutex);
> +
> + error2 = hfsplus_commit_superblock(inode->i_sb);
> + if (!error)
> + error = error2;
> +
> + if (!test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
> + blkdev_issue_flush(inode->i_sb->s_bdev);
> +
> + inode_unlock(inode);
> +
> + return error;
> +}
> +
> +const struct file_operations hfsplus_file_operations = {
> + .llseek = generic_file_llseek,
> + .read_iter = generic_file_read_iter,
> + .write_iter = generic_file_write_iter,
> + .mmap_prepare = generic_file_mmap_prepare,
> + .splice_read = filemap_splice_read,
> + .splice_write = iter_file_splice_write,
> + .fsync = hfsplus_file_fsync,
> + .open = hfsplus_file_open,
> + .release = hfsplus_file_release,
> + .unlocked_ioctl = hfsplus_ioctl,
> +};
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 0a0df0388e7b..190c7de704fd 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -446,6 +446,11 @@ int hfsplus_rename_cat(u32 cnid, struct inode *src_dir, const struct qstr *src_n
> extern const struct inode_operations hfsplus_dir_inode_operations;
> extern const struct file_operations hfsplus_dir_operations;
>
> +/* file.c */
> +extern const struct file_operations hfsplus_file_operations;
> +int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
> + int datasync);
> +
> /* extents.c */
> int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
> const hfsplus_btree_key *k2);
> @@ -480,8 +485,6 @@ int hfsplus_cat_write_inode(struct inode *inode);
> int hfsplus_getattr(struct mnt_idmap *idmap, const struct path *path,
> struct kstat *stat, u32 request_mask,
> unsigned int query_flags);
> -int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
> - int datasync);
> int hfsplus_fileattr_get(struct dentry *dentry, struct file_kattr *fa);
> int hfsplus_fileattr_set(struct mnt_idmap *idmap,
> struct dentry *dentry, struct file_kattr *fa);
> diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
> index 2ce6de574fa6..9d25e6224ee5 100644
> --- a/fs/hfsplus/inode.c
> +++ b/fs/hfsplus/inode.c
> @@ -276,35 +276,6 @@ static int hfsplus_get_perms(struct inode *inode,
> return -EIO;
> }
>
> -static int hfsplus_file_open(struct inode *inode, struct file *file)
> -{
> - if (HFSPLUS_IS_RSRC(inode))
> - inode = HFSPLUS_I(inode)->rsrc_inode;
> - if (!(file->f_flags & O_LARGEFILE) && i_size_read(inode) > MAX_NON_LFS)
> - return -EOVERFLOW;
> - atomic_inc(&HFSPLUS_I(inode)->opencnt);
> - return 0;
> -}
> -
> -static int hfsplus_file_release(struct inode *inode, struct file *file)
> -{
> - struct super_block *sb = inode->i_sb;
> -
> - if (HFSPLUS_IS_RSRC(inode))
> - inode = HFSPLUS_I(inode)->rsrc_inode;
> - if (atomic_dec_and_test(&HFSPLUS_I(inode)->opencnt)) {
> - inode_lock(inode);
> - hfsplus_file_truncate(inode);
> - if (inode->i_flags & S_DEAD) {
> - hfsplus_delete_cat(inode->i_ino,
> - HFSPLUS_SB(sb)->hidden_dir, NULL);
> - hfsplus_delete_inode(inode);
> - }
> - inode_unlock(inode);
> - }
> - return 0;
> -}
> -
> static int hfsplus_setattr(struct mnt_idmap *idmap,
> struct dentry *dentry, struct iattr *attr)
> {
> @@ -361,86 +332,6 @@ int hfsplus_getattr(struct mnt_idmap *idmap, const struct path *path,
> return 0;
> }
>
> -int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
> - int datasync)
> -{
> - struct inode *inode = file->f_mapping->host;
> - struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
> - struct super_block *sb = inode->i_sb;
> - struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb);
> - struct hfsplus_vh *vhdr = sbi->s_vhdr;
> - int error = 0, error2;
> -
> - hfs_dbg("inode->i_ino %llu, start %llu, end %llu\n",
> - inode->i_ino, start, end);
> -
> - error = file_write_and_wait_range(file, start, end);
> - if (error)
> - return error;
> - inode_lock(inode);
> -
> - /*
> - * Sync inode metadata into the catalog and extent trees.
> - */
> - sync_inode_metadata(inode, 1);
> -
> - /*
> - * And explicitly write out the btrees.
> - */
> - if (test_and_clear_bit(HFSPLUS_I_CAT_DIRTY,
> - &HFSPLUS_I(HFSPLUS_CAT_TREE_I(sb))->flags)) {
> - clear_bit(HFSPLUS_I_CAT_DIRTY, &hip->flags);
> - error = filemap_write_and_wait(sbi->cat_tree->inode->i_mapping);
> - }
> -
> - if (test_and_clear_bit(HFSPLUS_I_EXT_DIRTY,
> - &HFSPLUS_I(HFSPLUS_EXT_TREE_I(sb))->flags)) {
> - clear_bit(HFSPLUS_I_EXT_DIRTY, &hip->flags);
> - error2 =
> - filemap_write_and_wait(sbi->ext_tree->inode->i_mapping);
> - if (!error)
> - error = error2;
> - }
> -
> - if (sbi->attr_tree) {
> - if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY,
> - &HFSPLUS_I(HFSPLUS_ATTR_TREE_I(sb))->flags)) {
> - clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags);
> - error2 =
> - filemap_write_and_wait(
> - sbi->attr_tree->inode->i_mapping);
> - if (!error)
> - error = error2;
> - }
> - } else {
> - if (test_and_clear_bit(HFSPLUS_I_ATTR_DIRTY, &hip->flags))
> - pr_err("sync non-existent attributes tree\n");
> - }
> -
> - if (test_and_clear_bit(HFSPLUS_I_ALLOC_DIRTY,
> - &HFSPLUS_I(sbi->alloc_file)->flags)) {
> - clear_bit(HFSPLUS_I_ALLOC_DIRTY, &hip->flags);
> - error2 = filemap_write_and_wait(sbi->alloc_file->i_mapping);
> - if (!error)
> - error = error2;
> - }
> -
> - mutex_lock(&sbi->vh_mutex);
> - hfsplus_prepare_volume_header_for_commit(vhdr);
> - mutex_unlock(&sbi->vh_mutex);
> -
> - error2 = hfsplus_commit_superblock(inode->i_sb);
> - if (!error)
> - error = error2;
> -
> - if (!test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
> - blkdev_issue_flush(inode->i_sb->s_bdev);
> -
> - inode_unlock(inode);
> -
> - return error;
> -}
> -
> static const struct inode_operations hfsplus_file_inode_operations = {
> .setattr = hfsplus_setattr,
> .getattr = hfsplus_getattr,
> @@ -462,19 +353,6 @@ static const struct inode_operations hfsplus_special_inode_operations = {
> .listxattr = hfsplus_listxattr,
> };
>
> -static const struct file_operations hfsplus_file_operations = {
> - .llseek = generic_file_llseek,
> - .read_iter = generic_file_read_iter,
> - .write_iter = generic_file_write_iter,
> - .mmap_prepare = generic_file_mmap_prepare,
> - .splice_read = filemap_splice_read,
> - .splice_write = iter_file_splice_write,
> - .fsync = hfsplus_file_fsync,
> - .open = hfsplus_file_open,
> - .release = hfsplus_file_release,
> - .unlocked_ioctl = hfsplus_ioctl,
> -};
> -
> struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
> umode_t mode)
> {
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/7] hfsplus: move file related operations to file.c
2026-09-14 23:39 ` [PATCH v4 5/7] hfsplus: move file related operations to file.c Viacheslav Dubeyko
2026-09-15 2:22 ` Darrick J. Wong
@ 2026-09-18 13:31 ` Christoph Hellwig
1 sibling, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2026-09-18 13:31 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner, djwong
On Mon, Sep 14, 2026 at 04:39:39PM -0700, Viacheslav Dubeyko wrote:
> This patch introduces fs/hfsplus/file.c and moves
> file related operations from fs/hfsplus/inode.c
> into the new file.
Looks good, but I think it would be much better to keep this before
adding any iomap code.
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 6/7] hfsplus: introduce iomap-based file_operations
2026-09-14 23:39 [PATCH v4 0/7] hfsplus: convert regular file I/O to iomap-based operations Viacheslav Dubeyko
` (4 preceding siblings ...)
2026-09-14 23:39 ` [PATCH v4 5/7] hfsplus: move file related operations to file.c Viacheslav Dubeyko
@ 2026-09-14 23:39 ` Viacheslav Dubeyko
2026-09-18 13:39 ` Christoph Hellwig
2026-09-14 23:39 ` [PATCH v4 7/7] hfsplus: switch address_space_operations on iomap-based support Viacheslav Dubeyko
6 siblings, 1 reply; 20+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-14 23:39 UTC (permalink / raw)
To: glaubitz, frank.li, hch
Cc: linux-fsdevel, linux-kernel, vdubeyko, willy, brauner, djwong,
Viacheslav Dubeyko
This patch implements specialized iomap-based
hfsplus_file_llseek(), hfsplus_file_read_iter(),
and hfsplus_file_write_iter() methods.
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
cc: Christoph Hellwig <hch@lst.de>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Acked-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/hfsplus/file.c | 181 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 176 insertions(+), 5 deletions(-)
diff --git a/fs/hfsplus/file.c b/fs/hfsplus/file.c
index 509046aad0c6..5a5776881c3e 100644
--- a/fs/hfsplus/file.c
+++ b/fs/hfsplus/file.c
@@ -6,9 +6,11 @@
#include <linux/fs.h>
#include <linux/uio.h>
#include <linux/mount.h>
+#include <linux/iomap.h>
#include "hfsplus_fs.h"
#include "hfsplus_raw.h"
+#include "iomap.h"
static int hfsplus_file_open(struct inode *inode, struct file *file)
{
@@ -17,6 +19,7 @@ static int hfsplus_file_open(struct inode *inode, struct file *file)
if (!(file->f_flags & O_LARGEFILE) && i_size_read(inode) > MAX_NON_LFS)
return -EOVERFLOW;
atomic_inc(&HFSPLUS_I(inode)->opencnt);
+ file->f_mode |= FMODE_CAN_ODIRECT;
return 0;
}
@@ -55,7 +58,6 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
error = file_write_and_wait_range(file, start, end);
if (error)
return error;
- inode_lock(inode);
/*
* Sync inode metadata into the catalog and extent trees.
@@ -114,15 +116,184 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
if (!test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
blkdev_issue_flush(inode->i_sb->s_bdev);
+ return error;
+}
+
+/*
+ * hfsplus_fallback_buffered_write() - fall back to buffered I/O for the
+ * tail of a write that iomap_dio_rw() could not perform directly
+ * (unaligned tail, or no blocks could be mapped without allocation
+ * outside the direct path).
+ */
+static ssize_t hfsplus_fallback_buffered_write(struct kiocb *iocb,
+ struct iov_iter *from)
+{
+ loff_t offset = iocb->ki_pos, end;
+ ssize_t written;
+ int ret;
+
+ iocb->ki_flags &= ~IOCB_DIRECT;
+
+ written = iomap_file_buffered_write(iocb, from,
+ &hfsplus_write_iomap_ops,
+ NULL, NULL);
+ if (written < 0)
+ return written;
+
+ end = iocb->ki_pos + written - 1;
+ ret = filemap_write_and_wait_range(iocb->ki_filp->f_mapping,
+ offset, end);
+ if (ret)
+ return -EIO;
+
+ invalidate_mapping_pages(iocb->ki_filp->f_mapping,
+ offset >> PAGE_SHIFT,
+ end >> PAGE_SHIFT);
+
+ return written;
+}
+
+static ssize_t hfsplus_dio_write_iter(struct kiocb *iocb,
+ struct iov_iter *from,
+ unsigned int dio_flags)
+{
+ ssize_t ret;
+
+ ret = iomap_dio_rw(iocb, from,
+ &hfsplus_write_iomap_ops,
+ &hfsplus_write_dio_ops,
+ dio_flags, NULL, 0);
+ if (ret == -ENOTBLK)
+ ret = 0;
+ else if (ret < 0)
+ return ret;
+
+ if (iov_iter_count(from)) {
+ ssize_t written;
+
+ written = hfsplus_fallback_buffered_write(iocb, from);
+ if (written < 0)
+ return written;
+ ret += written;
+ }
+
+ return ret;
+}
+
+static ssize_t hfsplus_file_write_iter(struct kiocb *iocb,
+ struct iov_iter *iter)
+{
+ struct file *file = iocb->ki_filp;
+ struct inode *inode = file_inode(file);
+ struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb);
+ loff_t total_capacity;
+ ssize_t ret;
+ int err;
+
+ inode_lock(inode);
+
+ ret = generic_write_checks(iocb, iter);
+ if (ret <= 0)
+ goto unlock;
+
+ total_capacity = (loff_t)sbi->total_blocks << sbi->alloc_blksz_shift;
+ if (iocb->ki_pos >= total_capacity) {
+ ret = -EFBIG;
+ goto unlock;
+ }
+
+ err = file_modified(file);
+ if (err) {
+ ret = err;
+ goto unlock;
+ }
+
+ if (iocb->ki_pos > i_size_read(inode)) {
+ loff_t old_size = i_size_read(inode);
+
+ i_size_write(inode, iocb->ki_pos);
+ err = hfsplus_iomap_cont_expand(inode, iocb->ki_pos);
+ if (err) {
+ i_size_write(inode, old_size);
+ ret = err;
+ goto unlock;
+ }
+ mark_inode_dirty(inode);
+ }
+
+ if (iocb->ki_flags & IOCB_DIRECT) {
+ unsigned int dio_flags = 0;
+
+ if (iocb->ki_pos + iov_iter_count(iter) >
+ HFSPLUS_I(inode)->phys_size)
+ dio_flags |= IOMAP_DIO_FORCE_WAIT;
+
+ ret = hfsplus_dio_write_iter(iocb, iter, dio_flags);
+ } else {
+ ret = iomap_file_buffered_write(iocb, iter,
+ &hfsplus_write_iomap_ops,
+ NULL, NULL);
+ }
+
+unlock:
inode_unlock(inode);
- return error;
+ if (ret > 0)
+ ret = generic_write_sync(iocb, ret);
+
+ return ret;
+}
+
+static ssize_t hfsplus_file_read_iter(struct kiocb *iocb,
+ struct iov_iter *iter)
+{
+ struct inode *inode = file_inode(iocb->ki_filp);
+ ssize_t ret;
+
+ inode_lock_shared(inode);
+
+ if (iocb->ki_flags & IOCB_DIRECT) {
+ file_accessed(iocb->ki_filp);
+ ret = iomap_dio_rw(iocb, iter,
+ &hfsplus_iomap_ops,
+ NULL, 0, NULL, 0);
+ } else
+ ret = generic_file_read_iter(iocb, iter);
+
+ inode_unlock_shared(inode);
+
+ return ret;
+}
+
+static loff_t hfsplus_file_llseek(struct file *file, loff_t offset, int whence)
+{
+ struct inode *inode = file->f_mapping->host;
+
+ switch (whence) {
+ case SEEK_HOLE:
+ inode_lock_shared(inode);
+ offset = iomap_seek_hole(inode, offset, &hfsplus_iomap_ops);
+ inode_unlock_shared(inode);
+ break;
+ case SEEK_DATA:
+ inode_lock_shared(inode);
+ offset = iomap_seek_data(inode, offset, &hfsplus_iomap_ops);
+ inode_unlock_shared(inode);
+ break;
+ default:
+ return generic_file_llseek(file, offset, whence);
+ }
+
+ if (offset < 0)
+ return offset;
+
+ return vfs_setpos(file, offset, inode->i_sb->s_maxbytes);
}
const struct file_operations hfsplus_file_operations = {
- .llseek = generic_file_llseek,
- .read_iter = generic_file_read_iter,
- .write_iter = generic_file_write_iter,
+ .llseek = hfsplus_file_llseek,
+ .read_iter = hfsplus_file_read_iter,
+ .write_iter = hfsplus_file_write_iter,
.mmap_prepare = generic_file_mmap_prepare,
.splice_read = filemap_splice_read,
.splice_write = iter_file_splice_write,
--
2.43.0
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 6/7] hfsplus: introduce iomap-based file_operations
2026-09-14 23:39 ` [PATCH v4 6/7] hfsplus: introduce iomap-based file_operations Viacheslav Dubeyko
@ 2026-09-18 13:39 ` Christoph Hellwig
0 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2026-09-18 13:39 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner, djwong
On Mon, Sep 14, 2026 at 04:39:40PM -0700, Viacheslav Dubeyko wrote:
> This patch
Patch descriptions are not supposed to start with "this patch".
By the time this fets into git, it isn't a patch any more :)
> @@ -55,7 +58,6 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
> error = file_write_and_wait_range(file, start, end);
> if (error)
> return error;
> - inode_lock(inode);
>
> /*
> * Sync inode metadata into the catalog and extent trees.
> @@ -114,15 +116,184 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
> if (!test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
> blkdev_issue_flush(inode->i_sb->s_bdev);
>
> + return error;
> +}
Changing fsync locking feels like something that should move
into a well-documented prep patch.
> +/*
> + * hfsplus_fallback_buffered_write() - fall back to buffered I/O for the
Pointless function name in the comment.
> + * tail of a write that iomap_dio_rw() could not perform directly
> + * (unaligned tail, or no blocks could be mapped without allocation
> + * outside the direct path).
> + */
> +static ssize_t hfsplus_fallback_buffered_write(struct kiocb *iocb,
> + struct iov_iter *from)
> +{
Can this use direct_write_fallback()?
> + struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb);
> + loff_t total_capacity;
> + ssize_t ret;
> + int err;
> +
> + inode_lock(inode);
> +
> + ret = generic_write_checks(iocb, iter);
> + if (ret <= 0)
> + goto unlock;
> +
> + total_capacity = (loff_t)sbi->total_blocks << sbi->alloc_blksz_shift;
> + if (iocb->ki_pos >= total_capacity) {
This is weird. The total capacity should not matter for a write.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 7/7] hfsplus: switch address_space_operations on iomap-based support
2026-09-14 23:39 [PATCH v4 0/7] hfsplus: convert regular file I/O to iomap-based operations Viacheslav Dubeyko
` (5 preceding siblings ...)
2026-09-14 23:39 ` [PATCH v4 6/7] hfsplus: introduce iomap-based file_operations Viacheslav Dubeyko
@ 2026-09-14 23:39 ` Viacheslav Dubeyko
2026-09-15 2:21 ` Darrick J. Wong
2026-09-18 13:57 ` Christoph Hellwig
6 siblings, 2 replies; 20+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-14 23:39 UTC (permalink / raw)
To: glaubitz, frank.li, hch
Cc: linux-fsdevel, linux-kernel, vdubeyko, willy, brauner, djwong,
Viacheslav Dubeyko
This patch switches the regular file operations on iomap-based
ones. The hfsplus_aops is redefined as the iomap-based
operations. As a result, hfsplus_direct_IO() has been completely
removed as a user of blockdev_direct_IO(). Also, unnecessary
LEGACY_DIRECT_IO dependency has been removed from Kconfig.
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
cc: Christoph Hellwig <hch@lst.de>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
---
fs/hfsplus/Kconfig | 1 -
fs/hfsplus/extents.c | 47 ++++++++----
fs/hfsplus/file.c | 2 +-
fs/hfsplus/hfsplus_fs.h | 1 +
fs/hfsplus/inode.c | 160 +++++++++++++++++++++-------------------
fs/hfsplus/iomap.c | 14 ++--
fs/hfsplus/iomap.h | 2 +-
7 files changed, 127 insertions(+), 100 deletions(-)
diff --git a/fs/hfsplus/Kconfig b/fs/hfsplus/Kconfig
index 865a1966f395..b4432c64db3b 100644
--- a/fs/hfsplus/Kconfig
+++ b/fs/hfsplus/Kconfig
@@ -5,7 +5,6 @@ config HFSPLUS_FS
select BUFFER_HEAD
select NLS
select NLS_UTF8
- select LEGACY_DIRECT_IO
select FS_IOMAP
help
If you say Y here, you will be able to mount extended format
diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index ffd52ad8867c..b724cc1ca3e1 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -15,6 +15,7 @@
#include "hfsplus_fs.h"
#include "hfsplus_raw.h"
+#include "iomap.h"
/* Compare two extents keys, returns 0 on same, pos/neg for difference */
int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
@@ -275,6 +276,8 @@ int hfsplus_map_extent(struct inode *inode, u32 ablock, int create,
*max_blocks = hfsplus_ext_find_block(hip->first_extents,
ablock,
dblock);
+ if (!*max_blocks)
+ return -EIO;
return 0;
}
@@ -302,6 +305,9 @@ int hfsplus_map_extent(struct inode *inode, u32 ablock, int create,
if (was_dirty)
mark_inode_dirty(inode);
+ if (!*max_blocks)
+ return -EIO;
+
return 0;
}
@@ -342,7 +348,7 @@ int hfsplus_get_block(struct inode *inode, sector_t iblock,
if (create) {
set_buffer_new(bh_result);
- hip->phys_size += sb->s_blocksize;
+ hip->phys_size = (loff_t)(iblock + 1) << sb->s_blocksize_bits;
hip->fs_blocks++;
inode_add_bytes(inode, sb->s_blocksize);
mark_inode_dirty(inode);
@@ -607,20 +613,33 @@ void hfsplus_file_truncate(struct inode *inode)
inode->i_ino, (long long)hip->phys_size, inode->i_size);
if (inode->i_size > hip->phys_size) {
- struct address_space *mapping = inode->i_mapping;
- struct folio *folio;
- void *fsdata = NULL;
- loff_t size = inode->i_size;
+ if (S_ISREG(inode->i_mode)) {
+ res = hfsplus_iomap_cont_expand(inode, hip->phys_size,
+ inode->i_size);
+ if (res)
+ return;
+
+ mark_inode_dirty(inode);
+ } else {
+ struct address_space *mapping = inode->i_mapping;
+ struct folio *folio;
+ void *fsdata = NULL;
+
+ res = hfsplus_write_begin(NULL, mapping,
+ inode->i_size, 0,
+ &folio, &fsdata);
+ if (res)
+ return;
+
+ res = generic_write_end(NULL, mapping,
+ inode->i_size, 0, 0,
+ folio, fsdata);
+ if (res < 0)
+ return;
+
+ mark_inode_dirty(inode);
+ }
- res = hfsplus_write_begin(NULL, mapping, size, 0,
- &folio, &fsdata);
- if (res)
- return;
- res = generic_write_end(NULL, mapping, size, 0, 0,
- folio, fsdata);
- if (res < 0)
- return;
- mark_inode_dirty(inode);
return;
} else if (inode->i_size == hip->phys_size)
return;
diff --git a/fs/hfsplus/file.c b/fs/hfsplus/file.c
index 5a5776881c3e..0b0ae6b6e307 100644
--- a/fs/hfsplus/file.c
+++ b/fs/hfsplus/file.c
@@ -212,7 +212,7 @@ static ssize_t hfsplus_file_write_iter(struct kiocb *iocb,
loff_t old_size = i_size_read(inode);
i_size_write(inode, iocb->ki_pos);
- err = hfsplus_iomap_cont_expand(inode, iocb->ki_pos);
+ err = hfsplus_iomap_cont_expand(inode, old_size, iocb->ki_pos);
if (err) {
i_size_write(inode, old_size);
ret = err;
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 190c7de704fd..844027679a75 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -466,6 +466,7 @@ void hfsplus_file_truncate(struct inode *inode);
/* inode.c */
extern const struct address_space_operations hfsplus_aops;
+extern const struct address_space_operations hfsplus_symlink_aops;
extern const struct address_space_operations hfsplus_btree_aops;
extern const struct dentry_operations hfsplus_dentry_operations;
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 9d25e6224ee5..1779061dcad5 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -18,15 +18,12 @@
#include <linux/cred.h>
#include <linux/uio.h>
#include <linux/fileattr.h>
+#include <linux/iomap.h>
#include "hfsplus_fs.h"
#include "hfsplus_raw.h"
#include "xattr.h"
-
-static int hfsplus_read_folio(struct file *file, struct folio *folio)
-{
- return block_read_full_folio(folio, hfsplus_get_block);
-}
+#include "iomap.h"
static void hfsplus_write_failed(struct address_space *mapping, loff_t to)
{
@@ -128,67 +125,13 @@ static bool hfsplus_release_folio(struct folio *folio, gfp_t mask)
return res ? try_to_free_buffers(folio) : false;
}
-static ssize_t hfsplus_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
+static int hfsplus_btree_read_folio(struct file *file, struct folio *folio)
{
- struct file *file = iocb->ki_filp;
- struct address_space *mapping = file->f_mapping;
- struct inode *inode = mapping->host;
- loff_t isize;
- size_t count = iov_iter_count(iter);
- loff_t end = iocb->ki_pos + count;
- ssize_t ret;
-
- /*
- * The hfsplus_get_block() only allows creating the next sequential block.
- * For direct writes beyond EOF, expand the file first.
- */
- if (iov_iter_rw(iter) == WRITE && iocb->ki_pos > i_size_read(inode)) {
- loff_t start_off, end_off;
- loff_t start_page, end_page;
-
- isize = i_size_read(inode);
-
- /*
- * Wait for any in-flight DIO on this inode to finish before
- * calling generic_cont_expand_simple().
- */
- inode_dio_wait(inode);
-
- ret = generic_cont_expand_simple(inode, iocb->ki_pos);
- if (ret)
- return ret;
-
- start_off = isize;
- end_off = (end > 0) ? end - 1 : end;
-
- ret = filemap_write_and_wait_range(mapping, start_off, end_off);
- if (ret)
- return ret;
-
- start_page = start_off >> PAGE_SHIFT;
- end_page = end_off >> PAGE_SHIFT;
-
- invalidate_inode_pages2_range(mapping, start_page, end_page);
- }
-
- ret = blockdev_direct_IO(iocb, inode, iter, hfsplus_get_block);
-
- /*
- * In case of error extending write may have instantiated a few
- * blocks outside i_size. Trim these off again.
- */
- if (unlikely(iov_iter_rw(iter) == WRITE && ret < 0)) {
- isize = i_size_read(inode);
-
- if (end > isize)
- hfsplus_write_failed(mapping, end);
- }
-
- return ret;
+ return block_read_full_folio(folio, hfsplus_get_block);
}
-static int hfsplus_writepages(struct address_space *mapping,
- struct writeback_control *wbc)
+static int hfsplus_btree_writepages(struct address_space *mapping,
+ struct writeback_control *wbc)
{
return mpage_writepages(mapping, wbc, hfsplus_get_block);
}
@@ -196,8 +139,8 @@ static int hfsplus_writepages(struct address_space *mapping,
const struct address_space_operations hfsplus_btree_aops = {
.dirty_folio = block_dirty_folio,
.invalidate_folio = block_invalidate_folio,
- .read_folio = hfsplus_read_folio,
- .writepages = hfsplus_writepages,
+ .read_folio = hfsplus_btree_read_folio,
+ .writepages = hfsplus_btree_writepages,
.write_begin = hfsplus_write_begin,
.write_end = generic_write_end,
.migrate_folio = buffer_migrate_folio,
@@ -205,18 +148,70 @@ const struct address_space_operations hfsplus_btree_aops = {
.release_folio = hfsplus_release_folio,
};
-const struct address_space_operations hfsplus_aops = {
+static int hfsplus_symlink_read_folio(struct file *file, struct folio *folio)
+{
+ return block_read_full_folio(folio, hfsplus_get_block);
+}
+
+static int hfsplus_symlink_writepages(struct address_space *mapping,
+ struct writeback_control *wbc)
+{
+ return mpage_writepages(mapping, wbc, hfsplus_get_block);
+}
+
+const struct address_space_operations hfsplus_symlink_aops = {
.dirty_folio = block_dirty_folio,
.invalidate_folio = block_invalidate_folio,
- .read_folio = hfsplus_read_folio,
+ .read_folio = hfsplus_symlink_read_folio,
.write_begin = hfsplus_write_begin,
.write_end = generic_write_end,
.bmap = hfsplus_bmap,
- .direct_IO = hfsplus_direct_IO,
- .writepages = hfsplus_writepages,
+ .writepages = hfsplus_symlink_writepages,
.migrate_folio = buffer_migrate_folio,
};
+static int hfsplus_read_folio(struct file *file, struct folio *folio)
+{
+ iomap_bio_read_folio(folio, &hfsplus_iomap_ops);
+ return 0;
+}
+
+static void hfsplus_readahead(struct readahead_control *rac)
+{
+ iomap_bio_readahead(rac, &hfsplus_iomap_ops);
+}
+
+static int hfsplus_writepages(struct address_space *mapping,
+ struct writeback_control *wbc)
+{
+ struct iomap_writepage_ctx wpc = {
+ .inode = mapping->host,
+ .wbc = wbc,
+ .ops = &hfsplus_writeback_ops,
+ };
+
+ return iomap_writepages(&wpc);
+}
+
+static sector_t hfsplus_aop_bmap(struct address_space *mapping, sector_t block)
+{
+ return iomap_bmap(mapping, block, &hfsplus_iomap_ops);
+}
+
+const struct address_space_operations hfsplus_aops = {
+ .read_folio = hfsplus_read_folio,
+ .readahead = hfsplus_readahead,
+ .writepages = hfsplus_writepages,
+ .dirty_folio = iomap_dirty_folio,
+ .bmap = hfsplus_aop_bmap,
+ .migrate_folio = filemap_migrate_folio,
+ .is_partially_uptodate = iomap_is_partially_uptodate,
+ .error_remove_folio = generic_error_remove_folio,
+ .release_folio = iomap_release_folio,
+ .invalidate_folio = iomap_invalidate_folio,
+ .swap_activate = hfsplus_iomap_swap_activate,
+};
+
const struct dentry_operations hfsplus_dentry_operations = {
.d_hash = hfsplus_hash_dentry,
.d_compare = hfsplus_compare_dentry,
@@ -290,13 +285,28 @@ static int hfsplus_setattr(struct mnt_idmap *idmap,
attr->ia_size != i_size_read(inode)) {
inode_dio_wait(inode);
if (attr->ia_size > inode->i_size) {
- error = generic_cont_expand_simple(inode,
- attr->ia_size);
+ loff_t old_size = inode->i_size;
+
+ i_size_write(inode, attr->ia_size);
+ error = hfsplus_iomap_cont_expand(inode, old_size,
+ attr->ia_size);
+ if (error) {
+ i_size_write(inode, old_size);
+ return error;
+ }
+ truncate_setsize(inode, attr->ia_size);
+ } else {
+ bool did_zero = false;
+
+ error = iomap_truncate_page(inode, attr->ia_size,
+ &did_zero,
+ &hfsplus_write_iomap_ops,
+ NULL, NULL);
if (error)
return error;
+ truncate_setsize(inode, attr->ia_size);
+ hfsplus_file_truncate(inode);
}
- truncate_setsize(inode, attr->ia_size);
- hfsplus_file_truncate(inode);
inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
}
@@ -399,7 +409,7 @@ struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
sbi->file_count++;
inode->i_op = &hfsplus_symlink_inode_operations;
inode_nohighmem(inode);
- inode->i_mapping->a_ops = &hfsplus_aops;
+ inode->i_mapping->a_ops = &hfsplus_symlink_aops;
hip->clump_blocks = 1;
} else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode) ||
S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
@@ -540,7 +550,7 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
} else if (S_ISLNK(inode->i_mode)) {
inode->i_op = &hfsplus_symlink_inode_operations;
inode_nohighmem(inode);
- inode->i_mapping->a_ops = &hfsplus_aops;
+ inode->i_mapping->a_ops = &hfsplus_symlink_aops;
} else {
inode->i_op = &hfsplus_special_inode_operations;
init_special_inode(inode, inode->i_mode,
diff --git a/fs/hfsplus/iomap.c b/fs/hfsplus/iomap.c
index 5723e854e58e..0eb392789126 100644
--- a/fs/hfsplus/iomap.c
+++ b/fs/hfsplus/iomap.c
@@ -137,18 +137,16 @@ const struct iomap_ops hfsplus_write_iomap_ops = {
/*
* hfsplus_iomap_cont_expand()
*
- * Zero-extend the backing store from the current phys_size up to 'size'.
- * Used both by hfsplus_setattr() and by hfsplus_file_truncate().
+ * Zero the byte range [from, to) of a file that is being extended, where
+ * 'from' is the old end-of-file and 'to' the new one. Used by the extending
+ * write path, hfsplus_setattr() (truncate up) and hfsplus_file_truncate().
*/
-int hfsplus_iomap_cont_expand(struct inode *inode, loff_t size)
+int hfsplus_iomap_cont_expand(struct inode *inode, loff_t from, loff_t to)
{
- struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
- loff_t start = hip->phys_size;
-
- if (size <= start)
+ if (to <= from)
return 0;
- return iomap_zero_range(inode, start, size - start, NULL,
+ return iomap_zero_range(inode, from, to - from, NULL,
&hfsplus_write_iomap_ops, NULL, NULL);
}
diff --git a/fs/hfsplus/iomap.h b/fs/hfsplus/iomap.h
index dac07a9d25f8..4dd4aca1804f 100644
--- a/fs/hfsplus/iomap.h
+++ b/fs/hfsplus/iomap.h
@@ -11,7 +11,7 @@ extern const struct iomap_ops hfsplus_write_iomap_ops;
extern const struct iomap_writeback_ops hfsplus_writeback_ops;
extern const struct iomap_dio_ops hfsplus_write_dio_ops;
-int hfsplus_iomap_cont_expand(struct inode *inode, loff_t size);
+int hfsplus_iomap_cont_expand(struct inode *inode, loff_t from, loff_t to);
int hfsplus_iomap_swap_activate(struct swap_info_struct *sis,
struct file *file, sector_t *span);
--
2.43.0
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 7/7] hfsplus: switch address_space_operations on iomap-based support
2026-09-14 23:39 ` [PATCH v4 7/7] hfsplus: switch address_space_operations on iomap-based support Viacheslav Dubeyko
@ 2026-09-15 2:21 ` Darrick J. Wong
2026-09-15 18:37 ` Viacheslav Dubeyko
2026-09-18 13:57 ` Christoph Hellwig
1 sibling, 1 reply; 20+ messages in thread
From: Darrick J. Wong @ 2026-09-15 2:21 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner
On Mon, Sep 14, 2026 at 04:39:41PM -0700, Viacheslav Dubeyko wrote:
> This patch switches the regular file operations on iomap-based
> ones. The hfsplus_aops is redefined as the iomap-based
> operations. As a result, hfsplus_direct_IO() has been completely
> removed as a user of blockdev_direct_IO(). Also, unnecessary
> LEGACY_DIRECT_IO dependency has been removed from Kconfig.
>
> Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
> cc: Christoph Hellwig <hch@lst.de>
> cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> cc: Yangtao Li <frank.li@vivo.com>
> cc: linux-fsdevel@vger.kernel.org
> ---
> fs/hfsplus/Kconfig | 1 -
> fs/hfsplus/extents.c | 47 ++++++++----
> fs/hfsplus/file.c | 2 +-
> fs/hfsplus/hfsplus_fs.h | 1 +
> fs/hfsplus/inode.c | 160 +++++++++++++++++++++-------------------
> fs/hfsplus/iomap.c | 14 ++--
> fs/hfsplus/iomap.h | 2 +-
> 7 files changed, 127 insertions(+), 100 deletions(-)
>
> diff --git a/fs/hfsplus/Kconfig b/fs/hfsplus/Kconfig
> index 865a1966f395..b4432c64db3b 100644
> --- a/fs/hfsplus/Kconfig
> +++ b/fs/hfsplus/Kconfig
> @@ -5,7 +5,6 @@ config HFSPLUS_FS
> select BUFFER_HEAD
> select NLS
> select NLS_UTF8
> - select LEGACY_DIRECT_IO
> select FS_IOMAP
> help
> If you say Y here, you will be able to mount extended format
> diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
> index ffd52ad8867c..b724cc1ca3e1 100644
> --- a/fs/hfsplus/extents.c
> +++ b/fs/hfsplus/extents.c
> @@ -15,6 +15,7 @@
>
> #include "hfsplus_fs.h"
> #include "hfsplus_raw.h"
> +#include "iomap.h"
>
> /* Compare two extents keys, returns 0 on same, pos/neg for difference */
> int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
> @@ -275,6 +276,8 @@ int hfsplus_map_extent(struct inode *inode, u32 ablock, int create,
> *max_blocks = hfsplus_ext_find_block(hip->first_extents,
> ablock,
> dblock);
> + if (!*max_blocks)
> + return -EIO;
> return 0;
> }
>
> @@ -302,6 +305,9 @@ int hfsplus_map_extent(struct inode *inode, u32 ablock, int create,
> if (was_dirty)
> mark_inode_dirty(inode);
>
> + if (!*max_blocks)
> + return -EIO;
> +
> return 0;
> }
>
> @@ -342,7 +348,7 @@ int hfsplus_get_block(struct inode *inode, sector_t iblock,
>
> if (create) {
> set_buffer_new(bh_result);
> - hip->phys_size += sb->s_blocksize;
> + hip->phys_size = (loff_t)(iblock + 1) << sb->s_blocksize_bits;
> hip->fs_blocks++;
> inode_add_bytes(inode, sb->s_blocksize);
> mark_inode_dirty(inode);
> @@ -607,20 +613,33 @@ void hfsplus_file_truncate(struct inode *inode)
> inode->i_ino, (long long)hip->phys_size, inode->i_size);
>
> if (inode->i_size > hip->phys_size) {
> - struct address_space *mapping = inode->i_mapping;
> - struct folio *folio;
> - void *fsdata = NULL;
> - loff_t size = inode->i_size;
> + if (S_ISREG(inode->i_mode)) {
> + res = hfsplus_iomap_cont_expand(inode, hip->phys_size,
> + inode->i_size);
> + if (res)
> + return;
> +
> + mark_inode_dirty(inode);
> + } else {
> + struct address_space *mapping = inode->i_mapping;
> + struct folio *folio;
> + void *fsdata = NULL;
> +
> + res = hfsplus_write_begin(NULL, mapping,
> + inode->i_size, 0,
> + &folio, &fsdata);
> + if (res)
> + return;
> +
> + res = generic_write_end(NULL, mapping,
> + inode->i_size, 0, 0,
> + folio, fsdata);
> + if (res < 0)
> + return;
> +
> + mark_inode_dirty(inode);
> + }
>
> - res = hfsplus_write_begin(NULL, mapping, size, 0,
> - &folio, &fsdata);
> - if (res)
> - return;
> - res = generic_write_end(NULL, mapping, size, 0, 0,
> - folio, fsdata);
> - if (res < 0)
> - return;
> - mark_inode_dirty(inode);
> return;
> } else if (inode->i_size == hip->phys_size)
> return;
> diff --git a/fs/hfsplus/file.c b/fs/hfsplus/file.c
> index 5a5776881c3e..0b0ae6b6e307 100644
> --- a/fs/hfsplus/file.c
> +++ b/fs/hfsplus/file.c
> @@ -212,7 +212,7 @@ static ssize_t hfsplus_file_write_iter(struct kiocb *iocb,
> loff_t old_size = i_size_read(inode);
>
> i_size_write(inode, iocb->ki_pos);
> - err = hfsplus_iomap_cont_expand(inode, iocb->ki_pos);
> + err = hfsplus_iomap_cont_expand(inode, old_size, iocb->ki_pos);
Hmm, I guess this was the change that generic/363 required? Which is to
say, the ability to zero specific EOF ranges rather than just phys_size
to the new EOF?
If so, then I think I understand this well enough to
Acked-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> if (err) {
> i_size_write(inode, old_size);
> ret = err;
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 190c7de704fd..844027679a75 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -466,6 +466,7 @@ void hfsplus_file_truncate(struct inode *inode);
>
> /* inode.c */
> extern const struct address_space_operations hfsplus_aops;
> +extern const struct address_space_operations hfsplus_symlink_aops;
> extern const struct address_space_operations hfsplus_btree_aops;
> extern const struct dentry_operations hfsplus_dentry_operations;
>
> diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
> index 9d25e6224ee5..1779061dcad5 100644
> --- a/fs/hfsplus/inode.c
> +++ b/fs/hfsplus/inode.c
> @@ -18,15 +18,12 @@
> #include <linux/cred.h>
> #include <linux/uio.h>
> #include <linux/fileattr.h>
> +#include <linux/iomap.h>
>
> #include "hfsplus_fs.h"
> #include "hfsplus_raw.h"
> #include "xattr.h"
> -
> -static int hfsplus_read_folio(struct file *file, struct folio *folio)
> -{
> - return block_read_full_folio(folio, hfsplus_get_block);
> -}
> +#include "iomap.h"
>
> static void hfsplus_write_failed(struct address_space *mapping, loff_t to)
> {
> @@ -128,67 +125,13 @@ static bool hfsplus_release_folio(struct folio *folio, gfp_t mask)
> return res ? try_to_free_buffers(folio) : false;
> }
>
> -static ssize_t hfsplus_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
> +static int hfsplus_btree_read_folio(struct file *file, struct folio *folio)
> {
> - struct file *file = iocb->ki_filp;
> - struct address_space *mapping = file->f_mapping;
> - struct inode *inode = mapping->host;
> - loff_t isize;
> - size_t count = iov_iter_count(iter);
> - loff_t end = iocb->ki_pos + count;
> - ssize_t ret;
> -
> - /*
> - * The hfsplus_get_block() only allows creating the next sequential block.
> - * For direct writes beyond EOF, expand the file first.
> - */
> - if (iov_iter_rw(iter) == WRITE && iocb->ki_pos > i_size_read(inode)) {
> - loff_t start_off, end_off;
> - loff_t start_page, end_page;
> -
> - isize = i_size_read(inode);
> -
> - /*
> - * Wait for any in-flight DIO on this inode to finish before
> - * calling generic_cont_expand_simple().
> - */
> - inode_dio_wait(inode);
> -
> - ret = generic_cont_expand_simple(inode, iocb->ki_pos);
> - if (ret)
> - return ret;
> -
> - start_off = isize;
> - end_off = (end > 0) ? end - 1 : end;
> -
> - ret = filemap_write_and_wait_range(mapping, start_off, end_off);
> - if (ret)
> - return ret;
> -
> - start_page = start_off >> PAGE_SHIFT;
> - end_page = end_off >> PAGE_SHIFT;
> -
> - invalidate_inode_pages2_range(mapping, start_page, end_page);
> - }
> -
> - ret = blockdev_direct_IO(iocb, inode, iter, hfsplus_get_block);
> -
> - /*
> - * In case of error extending write may have instantiated a few
> - * blocks outside i_size. Trim these off again.
> - */
> - if (unlikely(iov_iter_rw(iter) == WRITE && ret < 0)) {
> - isize = i_size_read(inode);
> -
> - if (end > isize)
> - hfsplus_write_failed(mapping, end);
> - }
> -
> - return ret;
> + return block_read_full_folio(folio, hfsplus_get_block);
> }
>
> -static int hfsplus_writepages(struct address_space *mapping,
> - struct writeback_control *wbc)
> +static int hfsplus_btree_writepages(struct address_space *mapping,
> + struct writeback_control *wbc)
> {
> return mpage_writepages(mapping, wbc, hfsplus_get_block);
> }
> @@ -196,8 +139,8 @@ static int hfsplus_writepages(struct address_space *mapping,
> const struct address_space_operations hfsplus_btree_aops = {
> .dirty_folio = block_dirty_folio,
> .invalidate_folio = block_invalidate_folio,
> - .read_folio = hfsplus_read_folio,
> - .writepages = hfsplus_writepages,
> + .read_folio = hfsplus_btree_read_folio,
> + .writepages = hfsplus_btree_writepages,
> .write_begin = hfsplus_write_begin,
> .write_end = generic_write_end,
> .migrate_folio = buffer_migrate_folio,
> @@ -205,18 +148,70 @@ const struct address_space_operations hfsplus_btree_aops = {
> .release_folio = hfsplus_release_folio,
> };
>
> -const struct address_space_operations hfsplus_aops = {
> +static int hfsplus_symlink_read_folio(struct file *file, struct folio *folio)
> +{
> + return block_read_full_folio(folio, hfsplus_get_block);
> +}
> +
> +static int hfsplus_symlink_writepages(struct address_space *mapping,
> + struct writeback_control *wbc)
> +{
> + return mpage_writepages(mapping, wbc, hfsplus_get_block);
> +}
> +
> +const struct address_space_operations hfsplus_symlink_aops = {
> .dirty_folio = block_dirty_folio,
> .invalidate_folio = block_invalidate_folio,
> - .read_folio = hfsplus_read_folio,
> + .read_folio = hfsplus_symlink_read_folio,
> .write_begin = hfsplus_write_begin,
> .write_end = generic_write_end,
> .bmap = hfsplus_bmap,
> - .direct_IO = hfsplus_direct_IO,
> - .writepages = hfsplus_writepages,
> + .writepages = hfsplus_symlink_writepages,
> .migrate_folio = buffer_migrate_folio,
> };
>
> +static int hfsplus_read_folio(struct file *file, struct folio *folio)
> +{
> + iomap_bio_read_folio(folio, &hfsplus_iomap_ops);
> + return 0;
> +}
> +
> +static void hfsplus_readahead(struct readahead_control *rac)
> +{
> + iomap_bio_readahead(rac, &hfsplus_iomap_ops);
> +}
> +
> +static int hfsplus_writepages(struct address_space *mapping,
> + struct writeback_control *wbc)
> +{
> + struct iomap_writepage_ctx wpc = {
> + .inode = mapping->host,
> + .wbc = wbc,
> + .ops = &hfsplus_writeback_ops,
> + };
> +
> + return iomap_writepages(&wpc);
> +}
> +
> +static sector_t hfsplus_aop_bmap(struct address_space *mapping, sector_t block)
> +{
> + return iomap_bmap(mapping, block, &hfsplus_iomap_ops);
> +}
> +
> +const struct address_space_operations hfsplus_aops = {
> + .read_folio = hfsplus_read_folio,
> + .readahead = hfsplus_readahead,
> + .writepages = hfsplus_writepages,
> + .dirty_folio = iomap_dirty_folio,
> + .bmap = hfsplus_aop_bmap,
> + .migrate_folio = filemap_migrate_folio,
> + .is_partially_uptodate = iomap_is_partially_uptodate,
> + .error_remove_folio = generic_error_remove_folio,
> + .release_folio = iomap_release_folio,
> + .invalidate_folio = iomap_invalidate_folio,
> + .swap_activate = hfsplus_iomap_swap_activate,
> +};
> +
> const struct dentry_operations hfsplus_dentry_operations = {
> .d_hash = hfsplus_hash_dentry,
> .d_compare = hfsplus_compare_dentry,
> @@ -290,13 +285,28 @@ static int hfsplus_setattr(struct mnt_idmap *idmap,
> attr->ia_size != i_size_read(inode)) {
> inode_dio_wait(inode);
> if (attr->ia_size > inode->i_size) {
> - error = generic_cont_expand_simple(inode,
> - attr->ia_size);
> + loff_t old_size = inode->i_size;
> +
> + i_size_write(inode, attr->ia_size);
> + error = hfsplus_iomap_cont_expand(inode, old_size,
> + attr->ia_size);
> + if (error) {
> + i_size_write(inode, old_size);
> + return error;
> + }
> + truncate_setsize(inode, attr->ia_size);
> + } else {
> + bool did_zero = false;
> +
> + error = iomap_truncate_page(inode, attr->ia_size,
> + &did_zero,
> + &hfsplus_write_iomap_ops,
> + NULL, NULL);
> if (error)
> return error;
> + truncate_setsize(inode, attr->ia_size);
> + hfsplus_file_truncate(inode);
> }
> - truncate_setsize(inode, attr->ia_size);
> - hfsplus_file_truncate(inode);
> inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
> }
>
> @@ -399,7 +409,7 @@ struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
> sbi->file_count++;
> inode->i_op = &hfsplus_symlink_inode_operations;
> inode_nohighmem(inode);
> - inode->i_mapping->a_ops = &hfsplus_aops;
> + inode->i_mapping->a_ops = &hfsplus_symlink_aops;
> hip->clump_blocks = 1;
> } else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode) ||
> S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
> @@ -540,7 +550,7 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
> } else if (S_ISLNK(inode->i_mode)) {
> inode->i_op = &hfsplus_symlink_inode_operations;
> inode_nohighmem(inode);
> - inode->i_mapping->a_ops = &hfsplus_aops;
> + inode->i_mapping->a_ops = &hfsplus_symlink_aops;
> } else {
> inode->i_op = &hfsplus_special_inode_operations;
> init_special_inode(inode, inode->i_mode,
> diff --git a/fs/hfsplus/iomap.c b/fs/hfsplus/iomap.c
> index 5723e854e58e..0eb392789126 100644
> --- a/fs/hfsplus/iomap.c
> +++ b/fs/hfsplus/iomap.c
> @@ -137,18 +137,16 @@ const struct iomap_ops hfsplus_write_iomap_ops = {
> /*
> * hfsplus_iomap_cont_expand()
> *
> - * Zero-extend the backing store from the current phys_size up to 'size'.
> - * Used both by hfsplus_setattr() and by hfsplus_file_truncate().
> + * Zero the byte range [from, to) of a file that is being extended, where
> + * 'from' is the old end-of-file and 'to' the new one. Used by the extending
> + * write path, hfsplus_setattr() (truncate up) and hfsplus_file_truncate().
> */
> -int hfsplus_iomap_cont_expand(struct inode *inode, loff_t size)
> +int hfsplus_iomap_cont_expand(struct inode *inode, loff_t from, loff_t to)
> {
> - struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
> - loff_t start = hip->phys_size;
> -
> - if (size <= start)
> + if (to <= from)
> return 0;
>
> - return iomap_zero_range(inode, start, size - start, NULL,
> + return iomap_zero_range(inode, from, to - from, NULL,
> &hfsplus_write_iomap_ops, NULL, NULL);
> }
>
> diff --git a/fs/hfsplus/iomap.h b/fs/hfsplus/iomap.h
> index dac07a9d25f8..4dd4aca1804f 100644
> --- a/fs/hfsplus/iomap.h
> +++ b/fs/hfsplus/iomap.h
> @@ -11,7 +11,7 @@ extern const struct iomap_ops hfsplus_write_iomap_ops;
> extern const struct iomap_writeback_ops hfsplus_writeback_ops;
> extern const struct iomap_dio_ops hfsplus_write_dio_ops;
>
> -int hfsplus_iomap_cont_expand(struct inode *inode, loff_t size);
> +int hfsplus_iomap_cont_expand(struct inode *inode, loff_t from, loff_t to);
> int hfsplus_iomap_swap_activate(struct swap_info_struct *sis,
> struct file *file, sector_t *span);
>
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 7/7] hfsplus: switch address_space_operations on iomap-based support
2026-09-15 2:21 ` Darrick J. Wong
@ 2026-09-15 18:37 ` Viacheslav Dubeyko
0 siblings, 0 replies; 20+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-15 18:37 UTC (permalink / raw)
To: Darrick J. Wong
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner
On Mon, 2026-09-14 at 19:21 -0700, Darrick J. Wong wrote:
> On Mon, Sep 14, 2026 at 04:39:41PM -0700, Viacheslav Dubeyko wrote:
> > This patch switches the regular file operations on iomap-based
> > ones. The hfsplus_aops is redefined as the iomap-based
> > operations. As a result, hfsplus_direct_IO() has been completely
> > removed as a user of blockdev_direct_IO(). Also, unnecessary
> > LEGACY_DIRECT_IO dependency has been removed from Kconfig.
> >
> > Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
> > cc: Christoph Hellwig <hch@lst.de>
> > cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> > cc: Yangtao Li <frank.li@vivo.com>
> > cc: linux-fsdevel@vger.kernel.org
> > ---
> > fs/hfsplus/Kconfig | 1 -
> > fs/hfsplus/extents.c | 47 ++++++++----
> > fs/hfsplus/file.c | 2 +-
> > fs/hfsplus/hfsplus_fs.h | 1 +
> > fs/hfsplus/inode.c | 160 +++++++++++++++++++++---------------
> > ----
> > fs/hfsplus/iomap.c | 14 ++--
> > fs/hfsplus/iomap.h | 2 +-
> > 7 files changed, 127 insertions(+), 100 deletions(-)
> >
> > diff --git a/fs/hfsplus/Kconfig b/fs/hfsplus/Kconfig
> > index 865a1966f395..b4432c64db3b 100644
> > --- a/fs/hfsplus/Kconfig
> > +++ b/fs/hfsplus/Kconfig
> > @@ -5,7 +5,6 @@ config HFSPLUS_FS
> > select BUFFER_HEAD
> > select NLS
> > select NLS_UTF8
> > - select LEGACY_DIRECT_IO
> > select FS_IOMAP
> > help
> > If you say Y here, you will be able to mount extended
> > format
> > diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
> > index ffd52ad8867c..b724cc1ca3e1 100644
> > --- a/fs/hfsplus/extents.c
> > +++ b/fs/hfsplus/extents.c
> > @@ -15,6 +15,7 @@
> >
> > #include "hfsplus_fs.h"
> > #include "hfsplus_raw.h"
> > +#include "iomap.h"
> >
> > /* Compare two extents keys, returns 0 on same, pos/neg for
> > difference */
> > int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
> > @@ -275,6 +276,8 @@ int hfsplus_map_extent(struct inode *inode, u32
> > ablock, int create,
> > *max_blocks = hfsplus_ext_find_block(hip-
> > >first_extents,
> > ablock,
> > dblock);
> > + if (!*max_blocks)
> > + return -EIO;
> > return 0;
> > }
> >
> > @@ -302,6 +305,9 @@ int hfsplus_map_extent(struct inode *inode, u32
> > ablock, int create,
> > if (was_dirty)
> > mark_inode_dirty(inode);
> >
> > + if (!*max_blocks)
> > + return -EIO;
> > +
> > return 0;
> > }
> >
> > @@ -342,7 +348,7 @@ int hfsplus_get_block(struct inode *inode,
> > sector_t iblock,
> >
> > if (create) {
> > set_buffer_new(bh_result);
> > - hip->phys_size += sb->s_blocksize;
> > + hip->phys_size = (loff_t)(iblock + 1) << sb-
> > >s_blocksize_bits;
> > hip->fs_blocks++;
> > inode_add_bytes(inode, sb->s_blocksize);
> > mark_inode_dirty(inode);
> > @@ -607,20 +613,33 @@ void hfsplus_file_truncate(struct inode
> > *inode)
> > inode->i_ino, (long long)hip->phys_size, inode-
> > >i_size);
> >
> > if (inode->i_size > hip->phys_size) {
> > - struct address_space *mapping = inode->i_mapping;
> > - struct folio *folio;
> > - void *fsdata = NULL;
> > - loff_t size = inode->i_size;
> > + if (S_ISREG(inode->i_mode)) {
> > + res = hfsplus_iomap_cont_expand(inode,
> > hip->phys_size,
> > + inode-
> > >i_size);
> > + if (res)
> > + return;
> > +
> > + mark_inode_dirty(inode);
> > + } else {
> > + struct address_space *mapping = inode-
> > >i_mapping;
> > + struct folio *folio;
> > + void *fsdata = NULL;
> > +
> > + res = hfsplus_write_begin(NULL, mapping,
> > + inode->i_size,
> > 0,
> > + &folio,
> > &fsdata);
> > + if (res)
> > + return;
> > +
> > + res = generic_write_end(NULL, mapping,
> > + inode->i_size, 0,
> > 0,
> > + folio, fsdata);
> > + if (res < 0)
> > + return;
> > +
> > + mark_inode_dirty(inode);
> > + }
> >
> > - res = hfsplus_write_begin(NULL, mapping, size, 0,
> > - &folio, &fsdata);
> > - if (res)
> > - return;
> > - res = generic_write_end(NULL, mapping, size, 0, 0,
> > - folio, fsdata);
> > - if (res < 0)
> > - return;
> > - mark_inode_dirty(inode);
> > return;
> > } else if (inode->i_size == hip->phys_size)
> > return;
> > diff --git a/fs/hfsplus/file.c b/fs/hfsplus/file.c
> > index 5a5776881c3e..0b0ae6b6e307 100644
> > --- a/fs/hfsplus/file.c
> > +++ b/fs/hfsplus/file.c
> > @@ -212,7 +212,7 @@ static ssize_t hfsplus_file_write_iter(struct
> > kiocb *iocb,
> > loff_t old_size = i_size_read(inode);
> >
> > i_size_write(inode, iocb->ki_pos);
> > - err = hfsplus_iomap_cont_expand(inode, iocb-
> > >ki_pos);
> > + err = hfsplus_iomap_cont_expand(inode, old_size,
> > iocb->ki_pos);
>
> Hmm, I guess this was the change that generic/363 required? Which is
> to
> say, the ability to zero specific EOF ranges rather than just
> phys_size
> to the new EOF?
Yes, exactly. This was the key issue in the previous versions of the
pathset.
Thanks,
Slava.
>
> If so, then I think I understand this well enough to
> Acked-by: "Darrick J. Wong" <djwong@kernel.org>
>
> --D
>
> > if (err) {
> > i_size_write(inode, old_size);
> > ret = err;
> > diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> > index 190c7de704fd..844027679a75 100644
> > --- a/fs/hfsplus/hfsplus_fs.h
> > +++ b/fs/hfsplus/hfsplus_fs.h
> > @@ -466,6 +466,7 @@ void hfsplus_file_truncate(struct inode
> > *inode);
> >
> > /* inode.c */
> > extern const struct address_space_operations hfsplus_aops;
> > +extern const struct address_space_operations hfsplus_symlink_aops;
> > extern const struct address_space_operations hfsplus_btree_aops;
> > extern const struct dentry_operations hfsplus_dentry_operations;
> >
> > diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
> > index 9d25e6224ee5..1779061dcad5 100644
> > --- a/fs/hfsplus/inode.c
> > +++ b/fs/hfsplus/inode.c
> > @@ -18,15 +18,12 @@
> > #include <linux/cred.h>
> > #include <linux/uio.h>
> > #include <linux/fileattr.h>
> > +#include <linux/iomap.h>
> >
> > #include "hfsplus_fs.h"
> > #include "hfsplus_raw.h"
> > #include "xattr.h"
> > -
> > -static int hfsplus_read_folio(struct file *file, struct folio
> > *folio)
> > -{
> > - return block_read_full_folio(folio, hfsplus_get_block);
> > -}
> > +#include "iomap.h"
> >
> > static void hfsplus_write_failed(struct address_space *mapping,
> > loff_t to)
> > {
> > @@ -128,67 +125,13 @@ static bool hfsplus_release_folio(struct
> > folio *folio, gfp_t mask)
> > return res ? try_to_free_buffers(folio) : false;
> > }
> >
> > -static ssize_t hfsplus_direct_IO(struct kiocb *iocb, struct
> > iov_iter *iter)
> > +static int hfsplus_btree_read_folio(struct file *file, struct
> > folio *folio)
> > {
> > - struct file *file = iocb->ki_filp;
> > - struct address_space *mapping = file->f_mapping;
> > - struct inode *inode = mapping->host;
> > - loff_t isize;
> > - size_t count = iov_iter_count(iter);
> > - loff_t end = iocb->ki_pos + count;
> > - ssize_t ret;
> > -
> > - /*
> > - * The hfsplus_get_block() only allows creating the next
> > sequential block.
> > - * For direct writes beyond EOF, expand the file first.
> > - */
> > - if (iov_iter_rw(iter) == WRITE && iocb->ki_pos >
> > i_size_read(inode)) {
> > - loff_t start_off, end_off;
> > - loff_t start_page, end_page;
> > -
> > - isize = i_size_read(inode);
> > -
> > - /*
> > - * Wait for any in-flight DIO on this inode to
> > finish before
> > - * calling generic_cont_expand_simple().
> > - */
> > - inode_dio_wait(inode);
> > -
> > - ret = generic_cont_expand_simple(inode, iocb-
> > >ki_pos);
> > - if (ret)
> > - return ret;
> > -
> > - start_off = isize;
> > - end_off = (end > 0) ? end - 1 : end;
> > -
> > - ret = filemap_write_and_wait_range(mapping,
> > start_off, end_off);
> > - if (ret)
> > - return ret;
> > -
> > - start_page = start_off >> PAGE_SHIFT;
> > - end_page = end_off >> PAGE_SHIFT;
> > -
> > - invalidate_inode_pages2_range(mapping, start_page,
> > end_page);
> > - }
> > -
> > - ret = blockdev_direct_IO(iocb, inode, iter,
> > hfsplus_get_block);
> > -
> > - /*
> > - * In case of error extending write may have instantiated
> > a few
> > - * blocks outside i_size. Trim these off again.
> > - */
> > - if (unlikely(iov_iter_rw(iter) == WRITE && ret < 0)) {
> > - isize = i_size_read(inode);
> > -
> > - if (end > isize)
> > - hfsplus_write_failed(mapping, end);
> > - }
> > -
> > - return ret;
> > + return block_read_full_folio(folio, hfsplus_get_block);
> > }
> >
> > -static int hfsplus_writepages(struct address_space *mapping,
> > - struct writeback_control *wbc)
> > +static int hfsplus_btree_writepages(struct address_space *mapping,
> > + struct writeback_control *wbc)
> > {
> > return mpage_writepages(mapping, wbc, hfsplus_get_block);
> > }
> > @@ -196,8 +139,8 @@ static int hfsplus_writepages(struct
> > address_space *mapping,
> > const struct address_space_operations hfsplus_btree_aops = {
> > .dirty_folio = block_dirty_folio,
> > .invalidate_folio = block_invalidate_folio,
> > - .read_folio = hfsplus_read_folio,
> > - .writepages = hfsplus_writepages,
> > + .read_folio = hfsplus_btree_read_folio,
> > + .writepages = hfsplus_btree_writepages,
> > .write_begin = hfsplus_write_begin,
> > .write_end = generic_write_end,
> > .migrate_folio = buffer_migrate_folio,
> > @@ -205,18 +148,70 @@ const struct address_space_operations
> > hfsplus_btree_aops = {
> > .release_folio = hfsplus_release_folio,
> > };
> >
> > -const struct address_space_operations hfsplus_aops = {
> > +static int hfsplus_symlink_read_folio(struct file *file, struct
> > folio *folio)
> > +{
> > + return block_read_full_folio(folio, hfsplus_get_block);
> > +}
> > +
> > +static int hfsplus_symlink_writepages(struct address_space
> > *mapping,
> > + struct writeback_control
> > *wbc)
> > +{
> > + return mpage_writepages(mapping, wbc, hfsplus_get_block);
> > +}
> > +
> > +const struct address_space_operations hfsplus_symlink_aops = {
> > .dirty_folio = block_dirty_folio,
> > .invalidate_folio = block_invalidate_folio,
> > - .read_folio = hfsplus_read_folio,
> > + .read_folio = hfsplus_symlink_read_folio,
> > .write_begin = hfsplus_write_begin,
> > .write_end = generic_write_end,
> > .bmap = hfsplus_bmap,
> > - .direct_IO = hfsplus_direct_IO,
> > - .writepages = hfsplus_writepages,
> > + .writepages = hfsplus_symlink_writepages,
> > .migrate_folio = buffer_migrate_folio,
> > };
> >
> > +static int hfsplus_read_folio(struct file *file, struct folio
> > *folio)
> > +{
> > + iomap_bio_read_folio(folio, &hfsplus_iomap_ops);
> > + return 0;
> > +}
> > +
> > +static void hfsplus_readahead(struct readahead_control *rac)
> > +{
> > + iomap_bio_readahead(rac, &hfsplus_iomap_ops);
> > +}
> > +
> > +static int hfsplus_writepages(struct address_space *mapping,
> > + struct writeback_control *wbc)
> > +{
> > + struct iomap_writepage_ctx wpc = {
> > + .inode = mapping->host,
> > + .wbc = wbc,
> > + .ops = &hfsplus_writeback_ops,
> > + };
> > +
> > + return iomap_writepages(&wpc);
> > +}
> > +
> > +static sector_t hfsplus_aop_bmap(struct address_space *mapping,
> > sector_t block)
> > +{
> > + return iomap_bmap(mapping, block, &hfsplus_iomap_ops);
> > +}
> > +
> > +const struct address_space_operations hfsplus_aops = {
> > + .read_folio = hfsplus_read_folio,
> > + .readahead = hfsplus_readahead,
> > + .writepages = hfsplus_writepages,
> > + .dirty_folio = iomap_dirty_folio,
> > + .bmap = hfsplus_aop_bmap,
> > + .migrate_folio = filemap_migrate_folio,
> > + .is_partially_uptodate = iomap_is_partially_uptodate,
> > + .error_remove_folio = generic_error_remove_folio,
> > + .release_folio = iomap_release_folio,
> > + .invalidate_folio = iomap_invalidate_folio,
> > + .swap_activate = hfsplus_iomap_swap_activate,
> > +};
> > +
> > const struct dentry_operations hfsplus_dentry_operations = {
> > .d_hash = hfsplus_hash_dentry,
> > .d_compare = hfsplus_compare_dentry,
> > @@ -290,13 +285,28 @@ static int hfsplus_setattr(struct mnt_idmap
> > *idmap,
> > attr->ia_size != i_size_read(inode)) {
> > inode_dio_wait(inode);
> > if (attr->ia_size > inode->i_size) {
> > - error = generic_cont_expand_simple(inode,
> > - attr-
> > >ia_size);
> > + loff_t old_size = inode->i_size;
> > +
> > + i_size_write(inode, attr->ia_size);
> > + error = hfsplus_iomap_cont_expand(inode,
> > old_size,
> > + attr-
> > >ia_size);
> > + if (error) {
> > + i_size_write(inode, old_size);
> > + return error;
> > + }
> > + truncate_setsize(inode, attr->ia_size);
> > + } else {
> > + bool did_zero = false;
> > +
> > + error = iomap_truncate_page(inode, attr-
> > >ia_size,
> > + &did_zero,
> > +
> > &hfsplus_write_iomap_ops,
> > + NULL, NULL);
> > if (error)
> > return error;
> > + truncate_setsize(inode, attr->ia_size);
> > + hfsplus_file_truncate(inode);
> > }
> > - truncate_setsize(inode, attr->ia_size);
> > - hfsplus_file_truncate(inode);
> > inode_set_mtime_to_ts(inode,
> > inode_set_ctime_current(inode));
> > }
> >
> > @@ -399,7 +409,7 @@ struct inode *hfsplus_new_inode(struct
> > super_block *sb, struct inode *dir,
> > sbi->file_count++;
> > inode->i_op = &hfsplus_symlink_inode_operations;
> > inode_nohighmem(inode);
> > - inode->i_mapping->a_ops = &hfsplus_aops;
> > + inode->i_mapping->a_ops = &hfsplus_symlink_aops;
> > hip->clump_blocks = 1;
> > } else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode-
> > >i_mode) ||
> > S_ISFIFO(inode->i_mode) || S_ISSOCK(inode-
> > >i_mode)) {
> > @@ -540,7 +550,7 @@ int hfsplus_cat_read_inode(struct inode *inode,
> > struct hfs_find_data *fd)
> > } else if (S_ISLNK(inode->i_mode)) {
> > inode->i_op =
> > &hfsplus_symlink_inode_operations;
> > inode_nohighmem(inode);
> > - inode->i_mapping->a_ops = &hfsplus_aops;
> > + inode->i_mapping->a_ops =
> > &hfsplus_symlink_aops;
> > } else {
> > inode->i_op =
> > &hfsplus_special_inode_operations;
> > init_special_inode(inode, inode->i_mode,
> > diff --git a/fs/hfsplus/iomap.c b/fs/hfsplus/iomap.c
> > index 5723e854e58e..0eb392789126 100644
> > --- a/fs/hfsplus/iomap.c
> > +++ b/fs/hfsplus/iomap.c
> > @@ -137,18 +137,16 @@ const struct iomap_ops
> > hfsplus_write_iomap_ops = {
> > /*
> > * hfsplus_iomap_cont_expand()
> > *
> > - * Zero-extend the backing store from the current phys_size up to
> > 'size'.
> > - * Used both by hfsplus_setattr() and by hfsplus_file_truncate().
> > + * Zero the byte range [from, to) of a file that is being
> > extended, where
> > + * 'from' is the old end-of-file and 'to' the new one. Used by the
> > extending
> > + * write path, hfsplus_setattr() (truncate up) and
> > hfsplus_file_truncate().
> > */
> > -int hfsplus_iomap_cont_expand(struct inode *inode, loff_t size)
> > +int hfsplus_iomap_cont_expand(struct inode *inode, loff_t from,
> > loff_t to)
> > {
> > - struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
> > - loff_t start = hip->phys_size;
> > -
> > - if (size <= start)
> > + if (to <= from)
> > return 0;
> >
> > - return iomap_zero_range(inode, start, size - start, NULL,
> > + return iomap_zero_range(inode, from, to - from, NULL,
> > &hfsplus_write_iomap_ops, NULL,
> > NULL);
> > }
> >
> > diff --git a/fs/hfsplus/iomap.h b/fs/hfsplus/iomap.h
> > index dac07a9d25f8..4dd4aca1804f 100644
> > --- a/fs/hfsplus/iomap.h
> > +++ b/fs/hfsplus/iomap.h
> > @@ -11,7 +11,7 @@ extern const struct iomap_ops
> > hfsplus_write_iomap_ops;
> > extern const struct iomap_writeback_ops hfsplus_writeback_ops;
> > extern const struct iomap_dio_ops hfsplus_write_dio_ops;
> >
> > -int hfsplus_iomap_cont_expand(struct inode *inode, loff_t size);
> > +int hfsplus_iomap_cont_expand(struct inode *inode, loff_t from,
> > loff_t to);
> > int hfsplus_iomap_swap_activate(struct swap_info_struct *sis,
> > struct file *file, sector_t
> > *span);
> >
> > --
> > 2.43.0
> >
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 7/7] hfsplus: switch address_space_operations on iomap-based support
2026-09-14 23:39 ` [PATCH v4 7/7] hfsplus: switch address_space_operations on iomap-based support Viacheslav Dubeyko
2026-09-15 2:21 ` Darrick J. Wong
@ 2026-09-18 13:57 ` Christoph Hellwig
1 sibling, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2026-09-18 13:57 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: glaubitz, frank.li, hch, linux-fsdevel, linux-kernel, vdubeyko,
willy, brauner, djwong
On Mon, Sep 14, 2026 at 04:39:41PM -0700, Viacheslav Dubeyko wrote:
> /* Compare two extents keys, returns 0 on same, pos/neg for difference */
> int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
> @@ -275,6 +276,8 @@ int hfsplus_map_extent(struct inode *inode, u32 ablock, int create,
> *max_blocks = hfsplus_ext_find_block(hip->first_extents,
> ablock,
> dblock);
> + if (!*max_blocks)
> + return -EIO;
What is this for? It feels like something that should go into a
well-documented prep patch.
> @@ -342,7 +348,7 @@ int hfsplus_get_block(struct inode *inode, sector_t iblock,
>
> if (create) {
> set_buffer_new(bh_result);
> - hip->phys_size += sb->s_blocksize;
> + hip->phys_size = (loff_t)(iblock + 1) << sb->s_blocksize_bits;
How is this change related to iomap?
> if (inode->i_size > hip->phys_size) {
> - struct address_space *mapping = inode->i_mapping;
> - struct folio *folio;
> - void *fsdata = NULL;
> - loff_t size = inode->i_size;
> + if (S_ISREG(inode->i_mode)) {
> + res = hfsplus_iomap_cont_expand(inode, hip->phys_size,
> + inode->i_size);
> + if (res)
> + return;
> +
> + mark_inode_dirty(inode);
> + } else {
> + struct address_space *mapping = inode->i_mapping;
> + struct folio *folio;
> + void *fsdata = NULL;
> +
> + res = hfsplus_write_begin(NULL, mapping,
> + inode->i_size, 0,
> + &folio, &fsdata);
> + if (res)
> + return;
> +
> + res = generic_write_end(NULL, mapping,
> + inode->i_size, 0, 0,
> + folio, fsdata);
> + if (res < 0)
> + return;
> +
> + mark_inode_dirty(inode);
> + }
Is this for symlinks? Any reason you can't pull in
iomap: add iomap_symlink_write
that has been posted on fsdevel and use iomap for symlinks as well?
^ permalink raw reply [flat|nested] 20+ messages in thread