From: "Darrick J. Wong" <djwong@kernel.org>
To: Viacheslav Dubeyko <slava@dubeyko.com>
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 5/7] hfsplus: move file related operations to file.c
Date: Mon, 14 Sep 2026 19:22:05 -0700 [thread overview]
Message-ID: <20260915022205.GC6244@frogsfrogsfrogs> (raw)
In-Reply-To: <20260914233941.2966421-6-slava@dubeyko.com>
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
>
>
next prev parent reply other threads:[~2026-09-15 2:22 UTC|newest]
Thread overview: 11+ 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-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 [this message]
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
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=20260915022205.GC6244@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=brauner@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=slava@dubeyko.com \
--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®