Hi Amir, On 2025/7/5 16:25, Amir Goldstein wrote:
...
2. When is it ok to use the backing_file helpers? The current patch allocates a struct file with alloc_file_pseudo() and not a struct backing_file, so using the backing_file helpers is going to be awkward and misleading and I think in this case it will we wize to refrain from using a local var name backing_file. The thing that you need to ask yourself is do you want backing_file_set_user_path() for the erofs shared inode.
Yes, agreed, that should be improved as you said.
That means, what do you want users to see when they look at /proc/self/map_files symlinks. With the current patch set I believe they will see a symlink to something like "[erofs_pcs_f]" for any mapped file which is somewhat odd.
Agreed.
I think it would have been nice if users saw something like "[erofs_pcs_md5digest:XXXXXXX]"
But if we use `overlay.metacopy` for example, it's not a string anymore. IOWs, I'd like to support binary footprint too. And I tend to avoid md5 calcuation or something in the kernel. The kernel just uses footprint xattrs instead.
IMO, making the page cache dedupe visible in map_files is a very nice feature. > Overlayfs took the approach that users are expecting to see the actual path of the (non-anonymous) file that they mapped when looking at map_files symlinks. If you do not display the path to erofs mount in map_files, then lsof will not be able to blame a process with mapped files as the reason for keeping erofs mount busy.
I think users should see `the actual path` rather than "[erofs_pcs_xxxxx]" too, but I don't have any chance to check this part yet. If it's possible for overlayfs, I guess it's possible for erofs page cache sharing, maybe? Thanks, Gao Xiang
Thanks, Amir.