Re: [PATCH v10 08/10] erofs: support unencoded inodes for page cache share

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

On 2025/12/23 16:15, Gao Xiang wrote:
> 
> 
> On 2025/12/23 09:56, Hongbo Li wrote:
>> This patch adds inode page cache sharing functionality for unencoded
>> files.
>>
>> I conducted experiments in the container environment. Below is the
>> memory usage for reading all files in two different minor versions
>> of container images:
>>
>> +-------------------+------------------+-------------+---------------+
>> |       Image       | Page Cache Share | Memory (MB) |    Memory     |
>> |                   |                  |             | Reduction (%) |
>> +-------------------+------------------+-------------+---------------+
>> |                   |        No        |     241     |       -       |
>> |       redis       +------------------+-------------+---------------+
>> |   7.2.4 & 7.2.5   |        Yes       |     163     |      33%      |
>> +-------------------+------------------+-------------+---------------+
>> |                   |        No        |     872     |       -       |
>> |      postgres     +------------------+-------------+---------------+
>> |    16.1 & 16.2    |        Yes       |     630     |      28%      |
>> +-------------------+------------------+-------------+---------------+
>> |                   |        No        |     2771    |       -       |
>> |     tensorflow    +------------------+-------------+---------------+
>> |  2.11.0 & 2.11.1  |        Yes       |     2340    |      16%      |
>> +-------------------+------------------+-------------+---------------+
>> |                   |        No        |     926     |       -       |
>> |       mysql       +------------------+-------------+---------------+
>> |  8.0.11 & 8.0.12  |        Yes       |     735     |      21%      |
>> +-------------------+------------------+-------------+---------------+
>> |                   |        No        |     390     |       -       |
>> |       nginx       +------------------+-------------+---------------+
>> |   7.2.4 & 7.2.5   |        Yes       |     219     |      44%      |
>> +-------------------+------------------+-------------+---------------+
>> |       tomcat      |        No        |     924     |       -       |
>> | 10.1.25 & 10.1.26 +------------------+-------------+---------------+
>> |                   |        Yes       |     474     |      49%      |
>> +-------------------+------------------+-------------+---------------+
>>
>> Additionally, the table below shows the runtime memory usage of the
>> container:
>>
>> +-------------------+------------------+-------------+---------------+
>> |       Image       | Page Cache Share | Memory (MB) |    Memory     |
>> |                   |                  |             | Reduction (%) |
>> +-------------------+------------------+-------------+---------------+
>> |                   |        No        |      35     |       -       |
>> |       redis       +------------------+-------------+---------------+
>> |   7.2.4 & 7.2.5   |        Yes       |      28     |      20%      |
>> +-------------------+------------------+-------------+---------------+
>> |                   |        No        |     149     |       -       |
>> |      postgres     +------------------+-------------+---------------+
>> |    16.1 & 16.2    |        Yes       |      95     |      37%      |
>> +-------------------+------------------+-------------+---------------+
>> |                   |        No        |     1028    |       -       |
>> |     tensorflow    +------------------+-------------+---------------+
>> |  2.11.0 & 2.11.1  |        Yes       |     930     |      10%      |
>> +-------------------+------------------+-------------+---------------+
>> |                   |        No        |     155     |       -       |
>> |       mysql       +------------------+-------------+---------------+
>> |  8.0.11 & 8.0.12  |        Yes       |     132     |      15%      |
>> +-------------------+------------------+-------------+---------------+
>> |                   |        No        |      25     |       -       |
>> |       nginx       +------------------+-------------+---------------+
>> |   7.2.4 & 7.2.5   |        Yes       |      20     |      20%      |
>> +-------------------+------------------+-------------+---------------+
>> |       tomcat      |        No        |     186     |       -       |
>> | 10.1.25 & 10.1.26 +------------------+-------------+---------------+
>> |                   |        Yes       |      98     |      48%      |
>> +-------------------+------------------+-------------+---------------+
>>
>> Co-developed-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
>> Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
>> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
>> ---
> 
> ...
> 
>> index 4b46016bcd03..269b53b3ed79 100644
>> --- a/fs/erofs/ishare.c
>> +++ b/fs/erofs/ishare.c
>> @@ -197,6 +197,37 @@ const struct file_operations erofs_ishare_fops = {
>>       .splice_read    = filemap_splice_read,
>>   };
>> +/*
>> + * erofs_ishare_iget - find the backing inode.
>> + */
>> +struct inode *erofs_ishare_iget(struct inode *inode)
> 
> Just:
> 
> struct inode *erofs_get_real_inode(struct inode *inode)
> 
> `ishare_` prefix seems useless here.
> 
>> +{
>> +    struct erofs_inode *vi, *vi_dedup;
>> +    struct inode *realinode;
>> +
>> +    if (!erofs_is_ishare_inode(inode))
>> +        return igrab(inode);

Also please `return inode;` directly if `erofs_is_ishare_inode`
is off.

No need to bump the inode reference unnecessarily if ishare is off;

>> +
>> +    vi_dedup = EROFS_I(inode);
>> +    spin_lock(&vi_dedup->lock);
>> +    /* fall back to all backing inodes */
>> +    DBG_BUGON(list_empty(&vi_dedup->backing_head));
>> +    list_for_each_entry(vi, &vi_dedup->backing_head, backing_link) {
>> +        realinode = igrab(&vi->vfs_inode);
>> +        if (realinode)
>> +            break;
>> +    }
>> +    spin_unlock(&vi_dedup->lock);
>> +
>> +    DBG_BUGON(!realinode);
>> +    return realinode;
>> +}
>> +
>> +void erofs_ishare_iput(struct inode *realinode)
> 
> Just:
> 
> erofs_put_real_inode().
> 
> Thanks,
> Gao Xiang