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
