* [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-14 23:39 ` [PATCH v4 2/7] hfsplus: rework hfsplus_get_block() logic Viacheslav Dubeyko
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ 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] 13+ 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-14 23:39 ` [PATCH v4 3/7] hfsplus: take the bitmap page lock for allocate/free Viacheslav Dubeyko
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ 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] 13+ 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
0 siblings, 1 reply; 13+ 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] 13+ 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; 13+ 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] 13+ 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-14 23:39 ` [PATCH v4 4/7] hfsplus: add iomap operations for regular file data Viacheslav Dubeyko
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ 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] 13+ 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-14 23:39 ` [PATCH v4 5/7] hfsplus: move file related operations to file.c Viacheslav Dubeyko
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ 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] 13+ 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-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, 1 reply; 13+ 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] 13+ 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
0 siblings, 0 replies; 13+ 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] 13+ 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-14 23:39 ` [PATCH v4 7/7] hfsplus: switch address_space_operations on iomap-based support Viacheslav Dubeyko
6 siblings, 0 replies; 13+ 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] 13+ 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
6 siblings, 1 reply; 13+ 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] 13+ 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
0 siblings, 1 reply; 13+ 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] 13+ 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; 13+ 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] 13+ messages in thread