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

[Gao Xiang <hsiangkao@linux.alibaba.com>]

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