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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 3 May 2022 22:48:00 +0200
From:   Bernd Schubert <bschubert@....com>
To:     Vivek Goyal <vgoyal@...hat.com>,
        Dharmendra Singh <dharamhans87@...il.com>
Cc:     miklos@...redi.hu, linux-fsdevel@...r.kernel.org,
        fuse-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
        Dharmendra Singh <dsingh@....com>
Subject: Re: [PATCH v4 1/3] FUSE: Implement atomic lookup + create

Hi Vivek,

On 5/3/22 21:53, Vivek Goyal wrote:
> Reading the existing code a little bit more and trying to understand
> existing semantics. And that will help me unerstand what new is being
> done.
> 
> So current fuse_atomic_open() does following.
> 
> A. Looks up dentry (if d_in_lookup() is set).
> B. If dentry is positive or O_CREAT is not set, return.
> C. If server supports atomic create + open, use that to create file and
>     open it as well.
> D. If server does not support atomic create + open, just create file
>     using "mknod" and return. VFS will take care of opening the file.
> 
> Now with this patch, new flow is.
> 
> A. Look up dentry if d_in_lookup() is set as well as either file is not
>     being created or fc->no_atomic_create is set. This basiclally means
>     skip lookup if atomic_create is supported and file is being created.
> 
> B. Remains same. if dentry is positive or O_CREATE is not set, return.
> 
> C. If server supports new atomic_create(), use that.
> 
> D. If not, if server supports atomic create + open, use that
> 
> E. If not, fall back to mknod and do not open file.
> 
> So to me this new functionality is basically atomic "lookup + create +
> open"?
> 
> Or may be not. I see we check "fc->no_create" and fallback to mknod.
> 
>          if (fc->no_create)
>                  goto mknod;
> 
> So fc->no_create is representing both old atomic "create + open" as well
> as new "lookup + create + open" ?
> 
> It might be obvious to you, but it is not to me. So will be great if
> you shed some light on this.

we are going to reply more in detail tomorrow, it gets rather late for 
me as well. Anyway yes, goal is basically to avoid lookups as much 
possible.
AFAIK, lookup-intents had been first introduced years ago by Lustre 
developers - I guess to reduce network and server load - same reason for 
us. Later Miklos had introduced atomic_open, which makes code 
using/avoiding lookup much easier to read.
I guess unoticed that time, fuse was not fully using all possibilities 
of atomic-open - we can see quite some lookup/revalidate traffic for our 
file system.


I guess the commit message and introduction letter should be updated 
with your A/B/C/D/E scheme. A) changes a bit in patch 2/3, which extents 
it to normal file open, and patch 3/3 adds in revalidate.


Hope it helps,
Bernd