Re: [PATCH v4 1/3] FUSE: Implement atomic lookup + create

From: Bernd Schubert
Date: Wed May 04 2022 - 11:46:45 EST




On 5/4/22 16:47, Vivek Goyal wrote:

Ok, naming is little confusing. I think we will have to put it in
commit message and where you define FUSE_ATOMIC_CREATE that what's
the difference between FUSE_CREATE and FUSE_ATOMIC_CREATE. This is
ATOMIC w.r.t what?

May be atomic here means that "lookup + create + open" is a single operation.
But then even FUSE_CREATE is atomic because "creat + open" is a single
operation.

In fact FUSE_CREATE does lookup anyway and returns all the information
in fuse_entry_out.

IIUC, only difference between FUSE_CREATE and FUSE_ATOMIC_CREATE is that
later also carries information in reply whether file was actually created
or not (FOPEN_FILE_CREATED). This will be set if file did not exist
already and it was created indeed. Is that right?

I see FOPEN_FILE_CREATED is being used to avoid calling
fuse_dir_changed(). That sounds like a separate optimization and probably
should be in a separate patch.

IOW, I think this patch should be broken in to multiple pieces. First
piece seems to be avoiding lookup() and given the way it is implemented,
looks like we can avoid lookup() even by using existing FUSE_CREATE
command. We don't necessarily need FUSE_ATOMIC_CREATE. Is that right?

The initial non-published patches had that, but I had actually asked not to go that route, because I'm scared that some user space file system implementations might get broken. Right now there is always a lookup before fuse_create_open() and when the resulting dentry is positive fuse_create_open/FUSE_CREATE is bypassed. I.e. user space implementations didn't need to handle existing files. Out of the sudden user space implementations might need to handle it and some of them might get broken with that kernel update. I guess even a single broken user space implementation would count as regression.
So I had asked to change the patch to require a user space flag.

-- Bernd