Received-SPF: pass (google.com: domain of linux-nfs+bounces-3751-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <CAOQ4uxip8tD8H691qTcA8YFkcT78_iwQXy17=mJzyv6WWUaunQ@mail.gmail.com>
 <171815759406.14261.8092450471234028375@noble.neil.brown.name> <20240612071252.GA98398@pevik>
In-Reply-To: <20240612071252.GA98398@pevik>
From: Amir Goldstein <amir73il@gmail.com>
Date: Thu, 13 Jun 2024 13:26:38 +0300
Message-ID: <CAOQ4uxgyHrTR_-b5tKdtuCoyoKdrVWb=-h-CbiSP2pHVHM--CQ@mail.gmail.com>
Subject: Re: [LTP] [PATCH] NFS: add atomic_open for NFSv3 to handle O_TRUNC correctly.
To: Petr Vorel <pvorel@suse.cz>
Cc: NeilBrown <neilb@suse.de>, James Clark <james.clark@arm.com>, linux-nfs@vger.kernel.org, 
	broonie@kernel.org, Aishwarya.TCV@arm.com, ltp@lists.linux.it, 
	Jan Kara <jack@suse.cz>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jun 12, 2024 at 10:12=E2=80=AFAM Petr Vorel <pvorel@suse.cz> wrote:
>
> > On Tue, 11 Jun 2024, Amir Goldstein wrote:
> > > On Tue, Jun 11, 2024 at 5:30=E2=80=AFAM NeilBrown <neilb@suse.de> wro=
te:
>
> > > > On Fri, 07 Jun 2024, James Clark wrote:
>
> > > > > Hi Neil,
>
> > > > > Now that your fix is in linux-next the statvfs01 test is passing =
again.
> > > > > However inotify02 is still failing.
>
> > > > > This is because the test expects the IN_CREATE and IN_OPEN events=
 to
> > > > > come in that order after calling creat(), but now they are revers=
ed. To
> > > > > me it seems like it could be a test issue and the test should han=
dle
> > > > > them in either order? Or maybe there should be a single inotify e=
vent
> > > > > with both flags set for the atomic open?
>
> > > > Interesting....  I don't see how any filesystem that uses ->atomic_=
open
> > > > would get these in the "right" order - and I do think IN_CREATE sho=
uld
> > > > come before IN_OPEN.
>
> > > Correct.
>
>
> > > > Does NFSv4 pass this test?
>
> > > Probably not.
>
>
> > > > IN_OPEN is generated (by fsnotify_open()) when finish_open() is cal=
led,
> > > > which must be (and is) called by all atomic_open functions.
> > > > IN_CREATE is generated (by fsnotify_create()) when open_last_lookup=
s()
> > > > detects that FMODE_CREATE was set and that happens *after* lookup_o=
pen()
> > > > is called, which calls atomic_open().
>
> > > > For filesystems that don't use atomic_open, the IN_OPEN is generate=
d by
> > > > the call to do_open() which happens *after* open_last_lookups(), no=
t
> > > > inside it.
>
> > > Correct.
>
>
> > > > So the ltp test must already fail for NFSv4, 9p ceph fuse gfs2 ntfs=
3
> > > > overlayfs smb.
>
>
> > > inotify02 does not run on all_filesystems, only on the main test fs,
> > > which is not very often one of the above.
>
> > > This is how I averted the problem in fanotify16 (which does run on
> > > all_filesystems):
>
> > > commit 9062824a70b8da74aa5b1db08710d0018b48072e
> > > Author: Amir Goldstein <amir73il@gmail.com>
> > > Date:   Tue Nov 21 12:52:47 2023 +0200
>
> > >     fanotify16: Fix test failure on FUSE
>
> > >     Split SAFE_CREAT() into explicit SAFE_MKNOD() and SAFE_OPEN(),
> > >     because with atomic open (e.g. fuse), SAFE_CREAT() generates FAN_=
OPEN
> > >     before FAN_CREATE (tested with ntfs-3g), which is inconsistent wi=
th
> > >     the order of events expected from other filesystems.
>
> > > inotify02 should be fixed similarly.
>
> > Alternately - maybe the kernel should be fixed to always get the order
> > right.
> > I have a patch.  I'll post it separately.
>
> @Amir, if later Neil's branch get merged, maybe we should use on fanotify=
16
> creat() on the old kernels (as it was in before your fix 9062824a7 ("fano=
tify16:
> Fix test failure on FUSE")), based on kernel check.
>

I am hoping that the fix could be backported to v6.9.y and then we
won't need to worry about older LTS kernels for fanotify16, because
fuse only supports FAN_CREATE since v6.8.

Thanks,
Amir.