On Wed, May 04, 2022 at 05:46:27PM +0200, Bernd Schubert wrote: > > > 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. Hmm..., So if dentry is positive, we will call FUSE_OPEN instead in current code. Now with this change, we will call FUSE_CREATE and file could still be present. If it is a shared filesystem, file could be created by another client anyway after lookup() completed and returned a non-existent file. So server can still get FUSE_CREATE and file could be there. But I understand that risk of regression is not zero. Given we are going to implement FUSE_CREATE_EXT in the same patch series, I guess we could fix it easily by switching to FUSE_CREATE_EXT. So that's my take. I will be willing to take this chance. Until and unless ofcourse Miklos disagrees. :-) Thanks Vivek > 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 >
