Add erofs_inode_set_aops helper to set the inode->i_mapping->a_ops,
and using IS_ENABLED to make it cleaner.
Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
fs/erofs/inode.c | 23 +----------------------
fs/erofs/internal.h | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index bce98c845a18..389632bb46c4 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -235,28 +235,7 @@ static int erofs_fill_inode(struct inode *inode)
}
mapping_set_large_folios(inode->i_mapping);
- if (erofs_inode_is_data_compressed(vi->datalayout)) {
-#ifdef CONFIG_EROFS_FS_ZIP
- DO_ONCE_LITE_IF(inode->i_blkbits != PAGE_SHIFT,
- erofs_info, inode->i_sb,
- "EXPERIMENTAL EROFS subpage compressed block support in use. Use at your own risk!");
- inode->i_mapping->a_ops = &z_erofs_aops;
-#else
- err = -EOPNOTSUPP;
-#endif
- } else {
- inode->i_mapping->a_ops = &erofs_aops;
-#ifdef CONFIG_EROFS_FS_ONDEMAND
- if (erofs_is_fscache_mode(inode->i_sb))
- inode->i_mapping->a_ops = &erofs_fscache_access_aops;
-#endif
-#ifdef CONFIG_EROFS_FS_BACKED_BY_FILE
- if (erofs_is_fileio_mode(EROFS_SB(inode->i_sb)))
- inode->i_mapping->a_ops = &erofs_fileio_aops;
-#endif
- }
-
- return err;
+ return erofs_inode_set_aops(inode, inode, false);
}
/*
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index ec79e8b44d3b..8e28c2fa8735 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -455,6 +455,29 @@ static inline void *erofs_vm_map_ram(struct page **pages, unsigned int count)
return NULL;
}
+static inline int erofs_inode_set_aops(struct inode *inode,
+ struct inode *realinode, bool no_fscache)
+{
+ if (erofs_inode_is_data_compressed(EROFS_I(realinode)->datalayout)) {
+ if (!IS_ENABLED(CONFIG_EROFS_FS_ZIP))
+ return -EOPNOTSUPP;
+ DO_ONCE_LITE_IF(realinode->i_blkbits != PAGE_SHIFT,
+ erofs_info, realinode->i_sb,
+ "EXPERIMENTAL EROFS subpage compressed block support in use. Use at your own risk!");
+ inode->i_mapping->a_ops = &z_erofs_aops;
+ return 0;
+ }
+ inode->i_mapping->a_ops = &erofs_aops;
+ if (IS_ENABLED(CONFIG_EROFS_FS_ONDEMAND)) {
+ if (!no_fscache && erofs_is_fscache_mode(realinode->i_sb))
+ inode->i_mapping->a_ops = &erofs_fscache_access_aops;
+ } else {
+ if (erofs_is_fileio_mode(EROFS_SB(realinode->i_sb)))
+ inode->i_mapping->a_ops = &erofs_fileio_aops;
+ }
+ return 0;
+}
+
int erofs_register_sysfs(struct super_block *sb);
void erofs_unregister_sysfs(struct super_block *sb);
int __init erofs_init_sysfs(void);
--
2.22.0
On 2026/1/22 21:37, Hongbo Li wrote:
> Add erofs_inode_set_aops helper to set the inode->i_mapping->a_ops,
> and using IS_ENABLED to make it cleaner.
>
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
> fs/erofs/inode.c | 23 +----------------------
> fs/erofs/internal.h | 23 +++++++++++++++++++++++
> 2 files changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index bce98c845a18..389632bb46c4 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -235,28 +235,7 @@ static int erofs_fill_inode(struct inode *inode)
> }
>
> mapping_set_large_folios(inode->i_mapping);
> - if (erofs_inode_is_data_compressed(vi->datalayout)) {
> -#ifdef CONFIG_EROFS_FS_ZIP
> - DO_ONCE_LITE_IF(inode->i_blkbits != PAGE_SHIFT,
> - erofs_info, inode->i_sb,
> - "EXPERIMENTAL EROFS subpage compressed block support in use. Use at your own risk!");
> - inode->i_mapping->a_ops = &z_erofs_aops;
> -#else
> - err = -EOPNOTSUPP;
> -#endif
> - } else {
> - inode->i_mapping->a_ops = &erofs_aops;
> -#ifdef CONFIG_EROFS_FS_ONDEMAND
> - if (erofs_is_fscache_mode(inode->i_sb))
> - inode->i_mapping->a_ops = &erofs_fscache_access_aops;
> -#endif
> -#ifdef CONFIG_EROFS_FS_BACKED_BY_FILE
> - if (erofs_is_fileio_mode(EROFS_SB(inode->i_sb)))
> - inode->i_mapping->a_ops = &erofs_fileio_aops;
> -#endif
> - }
> -
> - return err;
> + return erofs_inode_set_aops(inode, inode, false);
> }
>
> /*
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index ec79e8b44d3b..8e28c2fa8735 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -455,6 +455,29 @@ static inline void *erofs_vm_map_ram(struct page **pages, unsigned int count)
> return NULL;
> }
>
> +static inline int erofs_inode_set_aops(struct inode *inode,
> + struct inode *realinode, bool no_fscache)
> +{
> + if (erofs_inode_is_data_compressed(EROFS_I(realinode)->datalayout)) {
> + if (!IS_ENABLED(CONFIG_EROFS_FS_ZIP))
> + return -EOPNOTSUPP;
> + DO_ONCE_LITE_IF(realinode->i_blkbits != PAGE_SHIFT,
> + erofs_info, realinode->i_sb,
> + "EXPERIMENTAL EROFS subpage compressed block support in use. Use at your own risk!");
> + inode->i_mapping->a_ops = &z_erofs_aops;
Is that available if CONFIG_EROFS_FS_ZIP is undefined?
> + return 0;
> + }
> + inode->i_mapping->a_ops = &erofs_aops;
> + if (IS_ENABLED(CONFIG_EROFS_FS_ONDEMAND)) {
> + if (!no_fscache && erofs_is_fscache_mode(realinode->i_sb))
> + inode->i_mapping->a_ops = &erofs_fscache_access_aops;
> + } else {
I really don't think they are equal, could you just move
the code without any change?
Thanks,
Gao Xiang
> + if (erofs_is_fileio_mode(EROFS_SB(realinode->i_sb)))
> + inode->i_mapping->a_ops = &erofs_fileio_aops;
> + }
> + return 0;
> +}
> +
> int erofs_register_sysfs(struct super_block *sb);
> void erofs_unregister_sysfs(struct super_block *sb);
> int __init erofs_init_sysfs(void);
On Thu, Jan 22, 2026 at 09:54:15PM +0800, Gao Xiang wrote:
>> @@ -455,6 +455,29 @@ static inline void *erofs_vm_map_ram(struct page **pages, unsigned int count)
>> return NULL;
>> }
>> +static inline int erofs_inode_set_aops(struct inode *inode,
>> + struct inode *realinode, bool no_fscache)
>> +{
>> + if (erofs_inode_is_data_compressed(EROFS_I(realinode)->datalayout)) {
>> + if (!IS_ENABLED(CONFIG_EROFS_FS_ZIP))
>> + return -EOPNOTSUPP;
>> + DO_ONCE_LITE_IF(realinode->i_blkbits != PAGE_SHIFT,
>> + erofs_info, realinode->i_sb,
>> + "EXPERIMENTAL EROFS subpage compressed block support in use. Use at your own risk!");
>> + inode->i_mapping->a_ops = &z_erofs_aops;
>
> Is that available if CONFIG_EROFS_FS_ZIP is undefined?
z_erofs_aops is declared unconditionally, and the IS_ENABLED above
ensures the compiler will never generate a reference to it.
So this is fine, and a very usualy trick to make the code more
readable.
On 2026/1/23 14:18, Christoph Hellwig wrote:
> On Thu, Jan 22, 2026 at 09:54:15PM +0800, Gao Xiang wrote:
>>> @@ -455,6 +455,29 @@ static inline void *erofs_vm_map_ram(struct page **pages, unsigned int count)
>>> return NULL;
>>> }
>>> +static inline int erofs_inode_set_aops(struct inode *inode,
>>> + struct inode *realinode, bool no_fscache)
>>> +{
>>> + if (erofs_inode_is_data_compressed(EROFS_I(realinode)->datalayout)) {
>>> + if (!IS_ENABLED(CONFIG_EROFS_FS_ZIP))
>>> + return -EOPNOTSUPP;
>>> + DO_ONCE_LITE_IF(realinode->i_blkbits != PAGE_SHIFT,
>>> + erofs_info, realinode->i_sb,
>>> + "EXPERIMENTAL EROFS subpage compressed block support in use. Use at your own risk!");
>>> + inode->i_mapping->a_ops = &z_erofs_aops;
>>
>> Is that available if CONFIG_EROFS_FS_ZIP is undefined?
>
> z_erofs_aops is declared unconditionally, and the IS_ENABLED above
> ensures the compiler will never generate a reference to it.
>
> So this is fine, and a very usualy trick to make the code more
> readable.
Yeah, I get your point, that is really helpful and I haven't
used that trick.
The other problem was the else part is incorrect, Hongbo,
how about applying the following code and resend the next
version, I will apply all patches later:
static inline int erofs_inode_set_aops(struct inode *inode,
struct inode *realinode, bool no_fscache)
{
if (erofs_inode_is_data_compressed(EROFS_I(realinode)->datalayout)) {
if (!IS_ENABLED(CONFIG_EROFS_FS_ZIP))
return -EOPNOTSUPP;
DO_ONCE_LITE_IF(realinode->i_blkbits != PAGE_SHIFT,
erofs_info, realinode->i_sb,
"EXPERIMENTAL EROFS subpage compressed block support in use. Use at your own risk!");
inode->i_mapping->a_ops = &z_erofs_aops;
return 0;
}
inode->i_mapping->a_ops = &erofs_aops;
if (IS_ENABLED(CONFIG_EROFS_FS_ONDEMAND) && !no_fscache &&
erofs_is_fscache_mode(realinode->i_sb))
inode->i_mapping->a_ops = &erofs_fscache_access_aops;
if (IS_ENABLED(CONFIG_EROFS_FS_BACKED_BY_FILE) &&
erofs_is_fileio_mode(EROFS_SB(realinode->i_sb)))
inode->i_mapping->a_ops = &erofs_fileio_aops;
return 0;
}
Thanks,
Gao Xiang
Hi Xiang and Christoph,
On 2026/1/23 15:42, Gao Xiang wrote:
>
>
> On 2026/1/23 14:18, Christoph Hellwig wrote:
>> On Thu, Jan 22, 2026 at 09:54:15PM +0800, Gao Xiang wrote:
>>>> @@ -455,6 +455,29 @@ static inline void *erofs_vm_map_ram(struct
>>>> page **pages, unsigned int count)
>>>> return NULL;
>>>> }
>>>> +static inline int erofs_inode_set_aops(struct inode *inode,
>>>> + struct inode *realinode, bool no_fscache)
>>>> +{
>>>> + if
>>>> (erofs_inode_is_data_compressed(EROFS_I(realinode)->datalayout)) {
>>>> + if (!IS_ENABLED(CONFIG_EROFS_FS_ZIP))
>>>> + return -EOPNOTSUPP;
>>>> + DO_ONCE_LITE_IF(realinode->i_blkbits != PAGE_SHIFT,
>>>> + erofs_info, realinode->i_sb,
>>>> + "EXPERIMENTAL EROFS subpage compressed block support
>>>> in use. Use at your own risk!");
>>>> + inode->i_mapping->a_ops = &z_erofs_aops;
>>>
>>> Is that available if CONFIG_EROFS_FS_ZIP is undefined?
>>
>> z_erofs_aops is declared unconditionally, and the IS_ENABLED above
>> ensures the compiler will never generate a reference to it.
>>
>> So this is fine, and a very usualy trick to make the code more
>> readable.
>
> Yeah, I get your point, that is really helpful and I haven't
> used that trick.
>
> The other problem was the else part is incorrect, Hongbo,
> how about applying the following code and resend the next
> version, I will apply all patches later:
>
Thanks you very much for your careful review and help. It was indeed my
own mistake (I have been making errors too easily lately which taught me
a lot...).
I have updated the new version in:
https://lore.kernel.org/all/20260123075239.664330-1-lihongbo22@huawei.com/
Thanks,
Hongbo
> static inline int erofs_inode_set_aops(struct inode *inode,
> struct inode *realinode, bool
> no_fscache)
> {
> if
> (erofs_inode_is_data_compressed(EROFS_I(realinode)->datalayout)) {
> if (!IS_ENABLED(CONFIG_EROFS_FS_ZIP))
> return -EOPNOTSUPP;
> DO_ONCE_LITE_IF(realinode->i_blkbits != PAGE_SHIFT,
> erofs_info, realinode->i_sb,
> "EXPERIMENTAL EROFS subpage compressed block
> support in use. Use at your own risk!");
> inode->i_mapping->a_ops = &z_erofs_aops;
> return 0;
> }
> inode->i_mapping->a_ops = &erofs_aops;
> if (IS_ENABLED(CONFIG_EROFS_FS_ONDEMAND) && !no_fscache &&
> erofs_is_fscache_mode(realinode->i_sb))
> inode->i_mapping->a_ops = &erofs_fscache_access_aops;
> if (IS_ENABLED(CONFIG_EROFS_FS_BACKED_BY_FILE) &&
> erofs_is_fileio_mode(EROFS_SB(realinode->i_sb)))
> inode->i_mapping->a_ops = &erofs_fileio_aops;
> return 0;
> }
>
> Thanks,
> Gao Xiang
© 2016 - 2026 Red Hat, Inc.