From: Viacheslav Dubeyko <slava@dubeyko.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: glaubitz@physik.fu-berlin.de, frank.li@vivo.com, hch@lst.de,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
vdubeyko@coreweave.com, willy@infradead.org, brauner@kernel.org
Subject: Re: [PATCH v4 7/7] hfsplus: switch address_space_operations on iomap-based support
Date: Tue, 15 Sep 2026 11:37:20 -0700 [thread overview]
Message-ID: <e1a217fa86d2ea18dc817adf7d04bab4151fc117.camel@dubeyko.com> (raw)
In-Reply-To: <20260915022127.GB6244@frogsfrogsfrogs>
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
> >
> >
prev parent reply other threads:[~2026-09-15 18:37 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
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-15 2:34 ` Darrick J. Wong
2026-09-15 18:34 ` Viacheslav Dubeyko
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 ` [PATCH v4 4/7] hfsplus: add iomap operations for regular file data Viacheslav Dubeyko
2026-09-14 23:39 ` [PATCH v4 5/7] hfsplus: move file related operations to file.c Viacheslav Dubeyko
2026-09-15 2:22 ` Darrick J. Wong
2026-09-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
2026-09-15 2:21 ` Darrick J. Wong
2026-09-15 18:37 ` Viacheslav Dubeyko [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e1a217fa86d2ea18dc817adf7d04bab4151fc117.camel@dubeyko.com \
--to=slava@dubeyko.com \
--cc=brauner@kernel.org \
--cc=djwong@kernel.org \
--cc=frank.li@vivo.com \
--cc=glaubitz@physik.fu-berlin.de \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vdubeyko@coreweave.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®