On 5/11/22 11:40, Miklos Szeredi wrote:
On Thu, 5 May 2022 at 21:59, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
Oh, I have no issues with the intent. I will like to see cut in network
traffic too (if we can do this without introducing problems). My primary
interest is that this kind of change should benefit virtiofs as well.
One issue with that appears to be checking permissions. AFAIU this
patchset only enables the optimization if default_permissions is
turned off (i.e. all permission checking is done by the server). But
virtiofs uses the default_permissions model.
I'm not quite sure about this limitation, guessing that it's related
to the fact that the permissions may be stale at the time of checking?
Yes exactly, I had actually pointed this out in one of the mails.
<quote myself from 2022-04-05 >
Adding in a vfs ->open_revalidate might have the advantage that we could
also support 'default_permissions' - ->open_revalidate needs to
additionally check the retrieved file permissions and and needs to call
into generic_permissions for that. Right now that is not easily
feasible, without adding some code dup to convert flags in MAY_* flags -
a vfs change would be needed here to pass the right flags.
</quote>
Avoiding lookup for create should work without default_permissions, though.
With the current patches this comment should be added in
fuse_dentry_revalidate() to clarify things (somehow it got lost in the
(internal) review process).
/* Default permissions actually also would not need revalidate, if
atomic_open() could verify attributes after opening the file. But the
MAY_ flags are missing and the vfs build_open_flags() to recreate these
flags not exported. Thus, default_permissions() cannot be called from
here - vfs updates would be required */
Thanks,
Bernd
