Re: [PATCH v16 04/10] erofs: add erofs_inode_set_aops helper to set the aops.

[Hongbo Li <lihongbo22@huawei.com>]

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