Re: [PATCH] erofs: allow sharing page cache with the same aops only

[Hongbo Li <lihongbo22@huawei.com>]

On 2026/2/14 7:27, Gao Xiang wrote:
> Hi Hongbo,
> 
> On Fri, Feb 13, 2026 at 07:33:45AM +0000, Hongbo Li wrote:
>> Inode with identical data but different @aops cannot be mixed
>> because the page cache is managed by different subsystems (e.g.,
>> @aops for compressed on-disk inodes cannot handle plain on-disk
>> inodes).
>>
...
>>
>> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
>> index 4f86169c23f1..5b05272fd9c4 100644
>> --- a/fs/erofs/inode.c
>> +++ b/fs/erofs/inode.c
>> @@ -254,7 +254,8 @@ static int erofs_fill_inode(struct inode *inode)
>>   	}
>>   
>>   	mapping_set_large_folios(inode->i_mapping);
>> -	return erofs_inode_set_aops(inode, inode, false);
>> +	inode->i_mapping->a_ops = erofs_get_aops(inode, false);
>> +	return IS_ERR(inode->i_mapping->a_ops) ? PTR_ERR(inode->i_mapping->a_ops) : 0;
> 
> I hope there is an aops variable instead of assigning
> inode->i_mapping->a_ops directly.
> 

Ok, thank you.
>>   }
>>   
>>   /*
>> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
>> index d1634455e389..764e81b3bc08 100644
>> --- a/fs/erofs/internal.h
>> +++ b/fs/erofs/internal.h
>> @@ -471,26 +471,24 @@ 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)
>> +static inline const struct address_space_operations *
>> +erofs_get_aops(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;
>> +			return ERR_PTR(-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;
>> +		return &z_erofs_aops;
>>   	}
>> -	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;
>> +		return &erofs_fileio_aops;
>> +	if (IS_ENABLED(CONFIG_EROFS_FS_ONDEMAND) && !no_fscache &&
>> +	    erofs_is_fscache_mode(realinode->i_sb))
>> +		return &erofs_fscache_access_aops;
> 
> Can you rearrange it as
> 	if (IS_ENABLED(CONFIG_EROFS_FS_ONDEMAND) && !no_fscache &&
> 	    erofs_is_fscache_mode(realinode->i_sb))
> 		return &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 &erofs_aops;
> 
Ok, they are equal. Since the EROFS in fscache mode can be fileio mode. 
And I will rearrange it in next version.

Thanks,
Hongbo

> ?
> 
> Thanks,
> Gao Xiang
>