Re: [PATCH v8 6/9] erofs: introduce the page cache share feature

[Hongbo Li <lihongbo22@huawei.com>]

Hi Xiang

On 2025/11/17 11:06, Gao Xiang wrote:
> 
> 
> On 2025/11/14 17:55, Hongbo Li wrote:
>> From: Hongzhen Luo <hongzhen@linux.alibaba.com>
>>
>> Currently, reading files with different paths (or names) but the same
>> content will consume multiple copies of the page cache, even if the
>> content of these page caches is the same. For example, reading
>> identical files (e.g., *.so files) from two different minor versions of
>> container images will cost multiple copies of the same page cache,
>> since different containers have different mount points. Therefore,
>> sharing the page cache for files with the same content can save memory.
>>
>> This introduces the page cache share feature in erofs. It allocate a
>> deduplicated inode and use its page cache as shared. Reads for files
>> with identical content will ultimately be routed to the page cache of
>> the deduplicated inode. In this way, a single page cache satisfies
>> multiple read requests for different files with the same contents.
>>
>> Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
>> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
>> ---
> 
> ...
> 
> 
>> +
>> +static int erofs_ishare_file_open(struct inode *inode, struct file 
>> *file)
>> +{
>> +    struct file *realfile;
>> +    struct inode *dedup;
>> +
>> +    dedup = EROFS_I(inode)->ishare;
>> +    if (!dedup)
>> +        return -EINVAL;
>> +
>> +    realfile = alloc_file_pseudo(dedup, erofs_ishare_mnt, 
>> "erofs_ishare_file",
>> +                     O_RDONLY, &erofs_file_fops);
>> +    if (IS_ERR(realfile))
>> +        return PTR_ERR(realfile);
>> +
>> +    file_ra_state_init(&realfile->f_ra, file->f_mapping);
>> +    realfile->private_data = EROFS_I(inode);
>> +    file->private_data = realfile;
>> +    return 0;
> 

My apologies, I got it wrong. The latest code wasn't synced. The most 
current version should be this one.

static int erofs_ishare_file_open(struct inode *inode, struct file *file)
{
	struct file *realfile;
	struct inode *dedup;
	char *buf, *filepath;

	dedup = EROFS_I(inode)->ishare;
	if (!dedup)
		return -EINVAL;

	buf = kmalloc(PATH_MAX, GFP_KERNEL);
	if (!buf)
		return -ENOMEM;
	filepath = file_path(file, buf, PATH_MAX);
	if (IS_ERR(filepath)) {
		kfree(buf);
		return -PTR_ERR(filepath);
	}
	realfile = alloc_file_pseudo(dedup, erofs_ishare_mnt, filepath + 1,
				     O_RDONLY, &erofs_file_fops);
	kfree(buf);
	if (IS_ERR(realfile))
		return PTR_ERR(realfile);

	file_ra_state_init(&realfile->f_ra, file->f_mapping);
	ihold(dedup);
	realfile->private_data = EROFS_I(inode);
	file->private_data = realfile;
	return 0;
}

I changed the "erofs_ishare_file" with filepath + 1 to display the 
realpath of the original file.

Thanks,
Hongbo

> Again, as Amir mentioned before, it should be converted to use (at least)
> some of backing file interfaces, please see:
>    file_user_path() and file_user_inode() in include/linux/fs.h
> 
> Or are you sure /proc/<pid>/maps is shown as expected?
> 
> Thanks,
> Gao Xiang