Received-SPF: pass (google.com: domain of linux-nfs+bounces-3297-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240520-exportfs-u64-mount-id-v1-1-f55fd9215b8e@cyphar.com>
 <f51a4bf68289268206475e3af226994607222be4.camel@kernel.org> <20240520.221843-swanky.buyers.maroon.prison-MAgYEXR0vg7P@cyphar.com>
In-Reply-To: <20240520.221843-swanky.buyers.maroon.prison-MAgYEXR0vg7P@cyphar.com>
From: Amir Goldstein <amir73il@gmail.com>
Date: Tue, 21 May 2024 08:04:09 +0300
Message-ID: <CAOQ4uxiaRGypAB0v49FW8Se+=4e4to1FAg77sxLPCkO55KcuHQ@mail.gmail.com>
Subject: Re: [PATCH RFC] fhandle: expose u64 mount id to name_to_handle_at(2)
To: Aleksa Sarai <cyphar@cyphar.com>
Cc: Jeff Layton <jlayton@kernel.org>, Alexander Viro <viro@zeniv.linux.org.uk>, 
	Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>, Chuck Lever <chuck.lever@oracle.com>, 
	Alexander Aring <alex.aring@gmail.com>, linux-fsdevel@vger.kernel.org, 
	linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, 
	=?UTF-8?Q?Christian_G=C3=B6ttsche?= <cgoettsche@seltendoof.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, May 21, 2024 at 1:28=E2=80=AFAM Aleksa Sarai <cyphar@cyphar.com> wr=
ote:
>
> On 2024-05-20, Jeff Layton <jlayton@kernel.org> wrote:
> > On Mon, 2024-05-20 at 17:35 -0400, Aleksa Sarai wrote:
> > > Now that we have stabilised the unique 64-bit mount ID interface in
> > > statx, we can now provide a race-free way for name_to_handle_at(2) to
> > > provide a file handle and corresponding mount without needing to worr=
y
> > > about racing with /proc/mountinfo parsing.

Both statx() and name_to_handle_at() support AT_EMPTY_PATH, so
there is a race-free way to get a file handle and unique mount id
for statmount().

Why do you mean /proc/mountinfo parsing?

> > >
> > > As with AT_HANDLE_FID, AT_HANDLE_UNIQUE_MNT_ID reuses a statx AT_* bi=
t
> > > that doesn't make sense for name_to_handle_at(2).

Christian is probably regretting merging AT_HANDLE_FID now ;-)

Seriously, I would rearrange the AT_* flags namespace this way to
explicitly declare the overloaded per-syscall AT_* flags and possibly
prepare for the upcoming setxattrat(2) syscall [1].

[1] https://lore.kernel.org/linux-fsdevel/20240426162042.191916-1-cgoettsch=
e@seltendoof.de/

The reason I would avoid overloading the AT_STATX_* flags is that
they describe a generic behavior that could potentially be relevant to
other syscalls in the future, e.g.:
renameat2(..., AT_RENAME_TEMPFILE | AT_FORCE_SYNC);

But then again, I don't understand why you need to extend name_to_handle_at=
()
at all for your purpose...

Thanks,
Amir.

--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
[...]
+
+#define AT_PRIVATE_FLAGS       0x2ff   /* Per-syscall flags mask.  */
+
+/* Common flags for *at() syscalls */
 #define AT_SYMLINK_NOFOLLOW    0x100   /* Do not follow symbolic links.  *=
/
-#define AT_EACCESS             0x200   /* Test access permitted for
-                                           effective IDs, not real IDs.  *=
/
-#define AT_REMOVEDIR           0x200   /* Remove directory instead of
-                                           unlinking file.  */
 #define AT_SYMLINK_FOLLOW      0x400   /* Follow symbolic links.  */
 #define AT_NO_AUTOMOUNT                0x800   /* Suppress terminal
automount traversal */
 #define AT_EMPTY_PATH          0x1000  /* Allow empty relative pathname */

+/* Flags for statx(2) */
 #define AT_STATX_SYNC_TYPE     0x6000  /* Type of synchronisation
required from statx() */
 #define AT_STATX_SYNC_AS_STAT  0x0000  /* - Do whatever stat() does */
 #define AT_STATX_FORCE_SYNC    0x2000  /* - Force the attributes to
be sync'd with the server */
[...]

 #define AT_RECURSIVE           0x8000  /* Apply to the entire subtree */

-/* Flags for name_to_handle_at(2). We reuse AT_ flag space to save bits...=
 */
-#define AT_HANDLE_FID          AT_REMOVEDIR    /* file handle is needed to
+/* Flags for name_to_handle_at(2) */
+#define AT_HANDLE_FID          0x200   /* file handle is needed to
                                        compare object identity and may not
                                        be usable to open_by_handle_at(2) *=
/
+/* Flags for faccessat(2) */
+#define AT_EACCESS             0x200   /* Test access permitted for
+                                           effective IDs, not real IDs.  *=
/
+/* Flags for unlinkat(2) */
+#define AT_REMOVEDIR           0x200   /* Remove directory instead of
+                                           unlinking file.  */
+
+/* Flags for renameat2(2) (should match legacy RENAME_* flags) */
+#define AT_RENAME_NOREPLACE    0x001   /* Don't overwrite target */
+#define AT_RENAME_EXCHANGE     0x002   /* Exchange source and dest */
+#define AT_RENAME_WHITEOUT     0x004   /* Whiteout source */
+#define AT_RENAME_TEMPFILE     0x008   /* Source file is O_TMPFILE */
+
+/* Flags for setxattrat(2) (should match legacy XATTR_* flags) */
+#define AT_XATTR_CREATE                0x001   /* set value, fail if
attr already exists */
+#define AT_XATTR_REPLACE       0x002   /* set value, fail if attr
does not exist */
+