* Re: [PATCH v5] epoll.7: clarify the event distribution under edge-triggered mode
From: Alejandro Colomar @ 2024-08-02 6:38 UTC (permalink / raw)
To: i; +Cc: linux-man, linux-api, mtk.manpages, Andy Pan
In-Reply-To: <20240801-epoll-et-desc-v5-1-7fcb9260a3b2@andypan.me>
[-- Attachment #1: Type: text/plain, Size: 4594 bytes --]
Hi Andy,
On Thu, Aug 01, 2024 at 11:38:38AM GMT, Andy Pan via B4 Relay wrote:
> From: Andy Pan <i@andypan.me>
>
> For the moment, the edge-triggered epoll generates an event for each
> receipt of a chunk of data, that is to say, epoll_wait() will return
> and tell us a monitored file descriptor is ready whenever there is a
> new activity on that FD since we were last informed about that FD.
> This is not a real _edge_ implementation for epoll, but it's been
> working this way for years and plenty of projects are relying on it
> to eliminate the overhead of one system call of read(2) per wakeup event.
>
> There are several renowned open-source projects relying on this feature
> for notification function (with eventfd): register eventfd with EPOLLET
> and avoid calling read(2) on the eventfd when there is wakeup event (eventfd being written).
> Examples: nginx [1], netty [2], tokio [3], libevent [4], ect. [5]
> These projects are widely used in today's Internet infrastructures.
> Thus, changing this behavior of epoll ET will fundamentally break them
> and cause a significant negative impact.
> Linux has changed it for pipe before [6], breaking some Android libraries,
> which had got "reverted" somehow. [7] [8]
>
> Nevertheless, the paragraph in the manual pages describing this
> characteristic of epoll ET seems ambiguous, I think a more explict
> sentence should be used to clarify it. We're improving the notification
> mechanism for libuv recently by exploiting this feature with eventfd,
> which brings us a significant performance boost. [9]
>
> Therefore, we (as well as the maintainers of nginx, netty, tokio, etc.)
> would have a sense of security to build an enhanced notification function
> based on this feature if there is a guarantee of retaining this implementation
> of epoll ET for the backward compatibility in the man pages.
>
> [1]: https://github.com/nginx/nginx/blob/efc6a217b92985a1ee211b6bb7337cd2f62deb90/src/event/modules/ngx_epoll_module.c#L386-L457
> [2]: https://github.com/netty/netty/pull/9192
> [3]: https://github.com/tokio-rs/mio/blob/309daae21ecb1d46203a7dbc0cf4c80310240cba/src/sys/unix/waker.rs#L111-L143
> [4]: https://github.com/libevent/libevent/blob/525f5d0a14c9c103be750f2ca175328c25505ea4/event.c#L2597-L2614
> [5]: https://github.com/libuv/libuv/pull/4400#issuecomment-2123798748
> [6]: https://lkml.iu.edu/hypermail/linux/kernel/2010.1/04363.html
> [7]: https://github.com/torvalds/linux/commit/3a34b13a88caeb2800ab44a4918f230041b37dd9
> [8]: https://github.com/torvalds/linux/commit/3b844826b6c6affa80755254da322b017358a2f4
> [9]: https://github.com/libuv/libuv/pull/4400#issuecomment-2103232402
>
> Signed-off-by: Andy Pan <i@andypan.me>
> ---
> Changes in v5:
> - Update the sentence of clarifying the epoll ET
> - Link to v4: https://lore.kernel.org/r/20240731-epoll-et-desc-v4-1-7eb819bdde0d@andypan.me
>
> Changes in v4:
> - Move the added sentence elsewhere to make more sense
> - Link to v3: https://lore.kernel.org/r/20240730-epoll-et-desc-v3-1-6aa81b1c400d@andypan.me
>
> Changes in v3:
> - Updated the git commit description
> - Link to v2: https://lore.kernel.org/r/20240727-epoll-et-desc-v2-1-c99b2ac66775@andypan.me
>
> Changes in v2:
> - Added the git commit description based on feedback
> - Link to v1: https://lore.kernel.org/r/20240727-epoll-et-desc-v1-1-390bafc678b9@andypan.me
> ---
> man/man7/epoll.7 | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/man/man7/epoll.7 b/man/man7/epoll.7
> index 951500131..86e5f8363 100644
> --- a/man/man7/epoll.7
> +++ b/man/man7/epoll.7
> @@ -121,7 +121,8 @@ .SS Level-triggered and edge-triggered
> meanwhile the remote peer might be expecting a response based on the
> data it already sent.
> The reason for this is that edge-triggered mode
> -delivers events only when changes occur on the monitored file descriptor.
> +delivers events only when changes occur on the monitored file descriptor,
> +that is, an event will be generated upon each receipt of a chunk of data.
LGTM. Thanks for the patch, and the detailed commit message!
I'll probably merge it later today (I'll let you know).
Have a lovely day!
Alex
> So, in step
> .B 5
> the caller might end up waiting for some data that is already present inside
>
> ---
> base-commit: cbc0a111e4dceea2037c51098de33e6bc8c16a5c
> change-id: 20240727-epoll-et-desc-04ea9a590f3b
>
> Best regards,
> --
> Andy Pan <i@andypan.me>
>
>
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v3 0/2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Aleksa Sarai @ 2024-08-02 1:43 UTC (permalink / raw)
To: Josef Bacik
Cc: Alexander Viro, Christian Brauner, Jan Kara, Chuck Lever,
Jeff Layton, Amir Goldstein, Alexander Aring, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
linux-fsdevel, linux-nfs, linux-kernel, linux-api,
linux-perf-users
In-Reply-To: <20240801142812.GA4187848@perftesting>
[-- Attachment #1: Type: text/plain, Size: 3873 bytes --]
On 2024-08-01, Josef Bacik <josef@toxicpanda.com> wrote:
> On Thu, Aug 01, 2024 at 01:52:39PM +1000, Aleksa Sarai wrote:
> > Now that we provide a unique 64-bit mount ID interface in statx(2), 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 worry about
> > racing with /proc/mountinfo parsing or having to open a file just to do
> > statx(2).
> >
> > While this is not necessary if you are using AT_EMPTY_PATH and don't
> > care about an extra statx(2) call, users that pass full paths into
> > name_to_handle_at(2) need to know which mount the file handle comes from
> > (to make sure they don't try to open_by_handle_at a file handle from a
> > different filesystem) and switching to AT_EMPTY_PATH would require
> > allocating a file for every name_to_handle_at(2) call, turning
> >
> > err = name_to_handle_at(-EBADF, "/foo/bar/baz", &handle, &mntid,
> > AT_HANDLE_MNT_ID_UNIQUE);
> >
> > into
> >
> > int fd = openat(-EBADF, "/foo/bar/baz", O_PATH | O_CLOEXEC);
> > err1 = name_to_handle_at(fd, "", &handle, &unused_mntid, AT_EMPTY_PATH);
> > err2 = statx(fd, "", AT_EMPTY_PATH, STATX_MNT_ID_UNIQUE, &statxbuf);
> > mntid = statxbuf.stx_mnt_id;
> > close(fd);
> >
> > Also, this series adds a patch to clarify how AT_* flag allocation
> > should work going forwards.
> >
> > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > ---
> > Changes in v3:
> > - Added a patch describing how AT_* flags should be allocated in the
> > future, based on Amir's suggestions.
> > - Included AT_* aliases for RENAME_* flags to further indicate that
> > renameat2(2) is an *at(2) syscall and to indicate that those flags
> > have been allocated already in the per-syscall range.
> > - Switched AT_HANDLE_MNT_ID_UNIQUE to use 0x01 (to reuse
> > (AT_)RENAME_NOREPLACE).
> > - v2: <https://lore.kernel.org/r/20240523-exportfs-u64-mount-id-v2-1-f9f959f17eb1@cyphar.com>
> > Changes in v2:
> > - Fixed a few minor compiler warnings and a buggy copy_to_user() check.
> > - Rename AT_HANDLE_UNIQUE_MOUNT_ID -> AT_HANDLE_MNT_ID_UNIQUE to match statx.
> > - Switched to using an AT_* bit from 0xFF and defining that range as
> > being "per-syscall" for future usage.
> > - Sync tools/ copy of <linux/fcntl.h> to include changes.
> > - v1: <https://lore.kernel.org/r/20240520-exportfs-u64-mount-id-v1-1-f55fd9215b8e@cyphar.com>
> >
> > ---
> > Aleksa Sarai (2):
> > uapi: explain how per-syscall AT_* flags should be allocated
> > fhandle: expose u64 mount id to name_to_handle_at(2)
> >
>
> Wasn't the conclusion from this discussion last time that we needed to revisit
> this API completely? Christoph had some pretty adamant objections.
There was a discussion about reworking the API and I agree with most of
the issues raised about file handles (I personally don't really like
this interface and it's a bit of a shame that it seems this is going to
be the interface that replaces inode numbers) so I'm not at all opposed
to reworking it.
However, I agree with Christian[1] that we can fix this existing issue
in the existing API fairly easily and then work on a new API separately.
The existing usage of name_to_handle_at() is fundamentally unsafe (as
outlined in the man page) and we can fix that fairly easily.
[1]: https://lore.kernel.org/all/20240527-hagel-thunfisch-75781b0cf75d@brauner/
> That being said the uapi comments patch looks good to me, you can add
>
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
>
> to that one. The other one I'm going to let others who have stronger opinions
> than me argue about. Thanks,
>
> Josef
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re:[PATCH v5] epoll.7: clarify the event distribution under edge-triggered mode
From: 潘少 @ 2024-08-02 0:59 UTC (permalink / raw)
To: i, Alejandro Colomar; +Cc: linux-man, linux-api, Michael Kerrisk, panjf2000
In-Reply-To: <20240801-epoll-et-desc-v5-1-7fcb9260a3b2@andypan.me>
Hi folks,
I'm not trying to rush you, but please do consider reviewing this patch when you have a moment, thanks!
-----------
Best regards,
Andy Pan
^ permalink raw reply
* Re: [PATCH RFC v3 2/2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Jeff Layton @ 2024-08-01 19:46 UTC (permalink / raw)
To: Aleksa Sarai, Alexander Viro, Christian Brauner, Jan Kara,
Chuck Lever, Amir Goldstein, Alexander Aring, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter
Cc: linux-fsdevel, linux-nfs, linux-kernel, linux-api,
linux-perf-users
In-Reply-To: <20240801-exportfs-u64-mount-id-v3-2-be5d6283144a@cyphar.com>
On Thu, 2024-08-01 at 13:52 +1000, Aleksa Sarai wrote:
> Now that we provide a unique 64-bit mount ID interface in statx(2), 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 worry about
> racing with /proc/mountinfo parsing or having to open a file just to do
> statx(2).
>
> While this is not necessary if you are using AT_EMPTY_PATH and don't
> care about an extra statx(2) call, users that pass full paths into
> name_to_handle_at(2) need to know which mount the file handle comes from
> (to make sure they don't try to open_by_handle_at a file handle from a
> different filesystem) and switching to AT_EMPTY_PATH would require
> allocating a file for every name_to_handle_at(2) call, turning
>
> err = name_to_handle_at(-EBADF, "/foo/bar/baz", &handle, &mntid,
> AT_HANDLE_MNT_ID_UNIQUE);
>
> into
>
> int fd = openat(-EBADF, "/foo/bar/baz", O_PATH | O_CLOEXEC);
> err1 = name_to_handle_at(fd, "", &handle, &unused_mntid, AT_EMPTY_PATH);
> err2 = statx(fd, "", AT_EMPTY_PATH, STATX_MNT_ID_UNIQUE, &statxbuf);
> mntid = statxbuf.stx_mnt_id;
> close(fd);
>
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> ---
> fs/fhandle.c | 29 ++++++++++++++++------
> include/linux/syscalls.h | 2 +-
> include/uapi/linux/fcntl.h | 1 +
> tools/perf/trace/beauty/include/uapi/linux/fcntl.h | 1 +
> 4 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index 6e8cea16790e..8cb665629f4a 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -16,7 +16,8 @@
>
> static long do_sys_name_to_handle(const struct path *path,
> struct file_handle __user *ufh,
> - int __user *mnt_id, int fh_flags)
> + void __user *mnt_id, bool unique_mntid,
> + int fh_flags)
> {
> long retval;
> struct file_handle f_handle;
> @@ -69,9 +70,19 @@ static long do_sys_name_to_handle(const struct path *path,
> } else
> retval = 0;
> /* copy the mount id */
> - if (put_user(real_mount(path->mnt)->mnt_id, mnt_id) ||
> - copy_to_user(ufh, handle,
> - struct_size(handle, f_handle, handle_bytes)))
> + if (unique_mntid) {
> + if (put_user(real_mount(path->mnt)->mnt_id_unique,
> + (u64 __user *) mnt_id))
> + retval = -EFAULT;
> + } else {
> + if (put_user(real_mount(path->mnt)->mnt_id,
> + (int __user *) mnt_id))
> + retval = -EFAULT;
> + }
> + /* copy the handle */
> + if (retval != -EFAULT &&
> + copy_to_user(ufh, handle,
> + struct_size(handle, f_handle, handle_bytes)))
> retval = -EFAULT;
> kfree(handle);
> return retval;
> @@ -83,6 +94,7 @@ static long do_sys_name_to_handle(const struct path *path,
> * @name: name that should be converted to handle.
> * @handle: resulting file handle
> * @mnt_id: mount id of the file system containing the file
> + * (u64 if AT_HANDLE_MNT_ID_UNIQUE, otherwise int)
> * @flag: flag value to indicate whether to follow symlink or not
> * and whether a decodable file handle is required.
> *
> @@ -92,7 +104,7 @@ static long do_sys_name_to_handle(const struct path *path,
> * value required.
> */
> SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
> - struct file_handle __user *, handle, int __user *, mnt_id,
> + struct file_handle __user *, handle, void __user *, mnt_id,
> int, flag)
> {
> struct path path;
> @@ -100,7 +112,8 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
> int fh_flags;
> int err;
>
> - if (flag & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH | AT_HANDLE_FID))
> + if (flag & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH | AT_HANDLE_FID |
> + AT_HANDLE_MNT_ID_UNIQUE))
> return -EINVAL;
>
> lookup_flags = (flag & AT_SYMLINK_FOLLOW) ? LOOKUP_FOLLOW : 0;
> @@ -109,7 +122,9 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
> lookup_flags |= LOOKUP_EMPTY;
> err = user_path_at(dfd, name, lookup_flags, &path);
> if (!err) {
> - err = do_sys_name_to_handle(&path, handle, mnt_id, fh_flags);
> + err = do_sys_name_to_handle(&path, handle, mnt_id,
> + flag & AT_HANDLE_MNT_ID_UNIQUE,
> + fh_flags);
> path_put(&path);
> }
> return err;
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 4bcf6754738d..5758104921e6 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -870,7 +870,7 @@ asmlinkage long sys_fanotify_mark(int fanotify_fd, unsigned int flags,
> #endif
> asmlinkage long sys_name_to_handle_at(int dfd, const char __user *name,
> struct file_handle __user *handle,
> - int __user *mnt_id, int flag);
> + void __user *mnt_id, int flag);
> asmlinkage long sys_open_by_handle_at(int mountdirfd,
> struct file_handle __user *handle,
> int flags);
> diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> index 38a6d66d9e88..87e2dec79fea 100644
> --- a/include/uapi/linux/fcntl.h
> +++ b/include/uapi/linux/fcntl.h
> @@ -152,6 +152,7 @@
> #define AT_HANDLE_FID 0x200 /* File handle is needed to compare
> object identity and may not be
> usable with open_by_handle_at(2). */
> +#define AT_HANDLE_MNT_ID_UNIQUE 0x001 /* Return the u64 unique mount ID. */
>
> #if defined(__KERNEL__)
> #define AT_GETATTR_NOSEC 0x80000000
> diff --git a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> index 38a6d66d9e88..87e2dec79fea 100644
> --- a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> +++ b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> @@ -152,6 +152,7 @@
> #define AT_HANDLE_FID 0x200 /* File handle is needed to compare
> object identity and may not be
> usable with open_by_handle_at(2). */
> +#define AT_HANDLE_MNT_ID_UNIQUE 0x001 /* Return the u64 unique mount ID. */
>
> #if defined(__KERNEL__)
> #define AT_GETATTR_NOSEC 0x80000000
>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH RFC v3 1/2] uapi: explain how per-syscall AT_* flags should be allocated
From: Jeff Layton @ 2024-08-01 19:21 UTC (permalink / raw)
To: Aleksa Sarai, Alexander Viro, Christian Brauner, Jan Kara,
Chuck Lever, Amir Goldstein, Alexander Aring, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter
Cc: linux-fsdevel, linux-nfs, linux-kernel, linux-api,
linux-perf-users
In-Reply-To: <20240801-exportfs-u64-mount-id-v3-1-be5d6283144a@cyphar.com>
On Thu, 2024-08-01 at 13:52 +1000, Aleksa Sarai wrote:
> Unfortunately, the way we have gone about adding new AT_* flags has
> been a little messy. In the beginning, all of the AT_* flags had generic
> meanings and so it made sense to share the flag bits indiscriminately.
> However, we inevitably ran into syscalls that needed their own
> syscall-specific flags. Due to the lack of a planned out policy, we
> ended up with the following situations:
>
> * Existing syscalls adding new features tended to use new AT_* bits,
> with some effort taken to try to re-use bits for flags that were so
> obviously syscall specific that they only make sense for a single
> syscall (such as the AT_EACCESS/AT_REMOVEDIR/AT_HANDLE_FID triplet).
>
> Given the constraints of bitflags, this works well in practice, but
> ideally (to avoid future confusion) we would plan ahead and define a
> set of "per-syscall bits" ahead of time so that when allocating new
> bits we don't end up with a complete mish-mash of which bits are
> supposed to be per-syscall and which aren't.
>
> * New syscalls dealt with this in several ways:
>
> - Some syscalls (like renameat2(2), move_mount(2), fsopen(2), and
> fspick(2)) created their separate own flag spaces that have no
> overlap with the AT_* flags. Most of these ended up allocating
> their bits sequentually.
>
> In the case of move_mount(2) and fspick(2), several flags have
> identical meanings to AT_* flags but were allocated in their own
> flag space.
>
> This makes sense for syscalls that will never share AT_* flags, but
> for some syscalls this leads to duplication with AT_* flags in a
> way that could cause confusion (if renameat2(2) grew a
> RENAME_EMPTY_PATH it seems likely that users could mistake it for
> AT_EMPTY_PATH since it is an *at(2) syscall).
>
> - Some syscalls unfortunately ended up both creating their own flag
> space while also using bits from other flag spaces. The most
> obvious example is open_tree(2), where the standard usage ends up
> using flags from *THREE* separate flag spaces:
>
> open_tree(AT_FDCWD, "/foo", OPEN_TREE_CLONE|O_CLOEXEC|AT_RECURSIVE);
>
> (Note that O_CLOEXEC is also platform-specific, so several future
> OPEN_TREE_* bits are also made unusable in one fell swoop.)
>
> It's not entirely clear to me what the "right" choice is for new
> syscalls. Just saying that all future VFS syscalls should use AT_* flags
> doesn't seem practical. openat2(2) has RESOLVE_* flags (many of which
> don't make much sense to burn generic AT_* flags for) and move_mount(2)
> has separate AT_*-like flags for both the source and target so separate
> flags are needed anyway (though it seems possible that renameat2(2)
> could grow *_EMPTY_PATH flags at some point, and it's a bit of a shame
> they can't be reused).
>
> But at least for syscalls that _do_ choose to use AT_* flags, we should
> explicitly state the policy that 0x2ff is currently intended for
> per-syscall flags and that new flags should err on the side of
> overlapping with existing flag bits (so we can extend the scope of
> generic flags in the future if necessary).
>
> And add AT_* aliases for the RENAME_* flags to further cement that
> renameat2(2) is an *at(2) flag, just with its own per-syscall flags.
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> ---
> include/uapi/linux/fcntl.h | 80 ++++++++++++++-------
> tools/perf/trace/beauty/include/uapi/linux/fcntl.h | 83 +++++++++++++++-------
> 2 files changed, 115 insertions(+), 48 deletions(-)
>
This seems like a reasonable proposal. We haven't been as careful about
how these flags get allocated as we probably should have in the past.
Nothing we can do about the existing ones, but some guidelines going
forward are welcome.
> diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> index e55a3314bcb0..38a6d66d9e88 100644
> --- a/include/uapi/linux/fcntl.h
> +++ b/include/uapi/linux/fcntl.h
> @@ -90,37 +90,69 @@
> #define DN_ATTRIB 0x00000020 /* File changed attibutes */
> #define DN_MULTISHOT 0x80000000 /* Don't remove notifier */
>
> +#define AT_FDCWD -100 /* Special value for dirfd used to
> + indicate openat should use the
> + current working directory. */
> +
> +
> +/* Generic flags for the *at(2) family of syscalls. */
> +
> +/* Reserved for per-syscall flags 0xff. */
> +#define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic
> + links. */
> +/* Reserved for per-syscall flags 0x200 */
> +#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 to operate on dirfd
> + directly. */
> /*
> - * The constants AT_REMOVEDIR and AT_EACCESS have the same value. AT_EACCESS is
> - * meaningful only to faccessat, while AT_REMOVEDIR is meaningful only to
> - * unlinkat. The two functions do completely different things and therefore,
> - * the flags can be allowed to overlap. For example, passing AT_REMOVEDIR to
> - * faccessat would be undefined behavior and thus treating it equivalent to
> - * AT_EACCESS is valid undefined behavior.
> + * These flags are currently statx(2)-specific, but they could be made generic
> + * in the future and so they should not be used for other per-syscall flags.
> */
> -#define AT_FDCWD -100 /* Special value used to indicate
> - openat should use the current
> - working directory. */
> -#define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic links. */
> +#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_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the server */
> +
> +#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
> +
> +/*
> + * Per-syscall flags for the *at(2) family of syscalls.
> + *
> + * These are flags that are so syscall-specific that a user passing these flags
> + * to the wrong syscall is so "clearly wrong" that we can safely call such
> + * usage "undefined behaviour".
> + *
> + * For example, the constants AT_REMOVEDIR and AT_EACCESS have the same value.
> + * AT_EACCESS is meaningful only to faccessat, while AT_REMOVEDIR is meaningful
> + * only to unlinkat. The two functions do completely different things and
> + * therefore, the flags can be allowed to overlap. For example, passing
> + * AT_REMOVEDIR to faccessat would be undefined behavior and thus treating it
> + * equivalent to AT_EACCESS is valid undefined behavior.
> + *
> + * Note for implementers: When picking a new per-syscall AT_* flag, try to
> + * reuse already existing flags first. This leaves us with as many unused bits
> + * as possible, so we can use them for generic bits in the future if necessary.
> + */
> +
> +/* Flags for renameat2(2) (must match legacy RENAME_* flags). */
> +#define AT_RENAME_NOREPLACE 0x0001
> +#define AT_RENAME_EXCHANGE 0x0002
> +#define AT_RENAME_WHITEOUT 0x0004
> +
> +/* Flag for faccessat(2). */
> #define AT_EACCESS 0x200 /* Test access permitted for
> effective IDs, not real IDs. */
> +/* Flag for unlinkat(2). */
> #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 */
> -
> -#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_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the server */
> -
> -#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
> +/* 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 with open_by_handle_at(2). */
>
> -/* 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
> - compare object identity and may not
> - be usable to open_by_handle_at(2) */
> #if defined(__KERNEL__)
> #define AT_GETATTR_NOSEC 0x80000000
> #endif
> diff --git a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> index c0bcc185fa48..38a6d66d9e88 100644
> --- a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> +++ b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> @@ -16,6 +16,9 @@
>
> #define F_DUPFD_QUERY (F_LINUX_SPECIFIC_BASE + 3)
>
> +/* Was the file just created? */
> +#define F_CREATED_QUERY (F_LINUX_SPECIFIC_BASE + 4)
> +
> /*
> * Cancel a blocking posix lock; internal use only until we expose an
> * asynchronous lock api to userspace:
> @@ -87,37 +90,69 @@
> #define DN_ATTRIB 0x00000020 /* File changed attibutes */
> #define DN_MULTISHOT 0x80000000 /* Don't remove notifier */
>
> +#define AT_FDCWD -100 /* Special value for dirfd used to
> + indicate openat should use the
> + current working directory. */
> +
> +
> +/* Generic flags for the *at(2) family of syscalls. */
> +
> +/* Reserved for per-syscall flags 0xff. */
> +#define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic
> + links. */
> +/* Reserved for per-syscall flags 0x200 */
> +#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 to operate on dirfd
> + directly. */
> +/*
> + * These flags are currently statx(2)-specific, but they could be made generic
> + * in the future and so they should not be used for other per-syscall flags.
> + */
> +#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_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the server */
> +
> +#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
> +
> /*
> - * The constants AT_REMOVEDIR and AT_EACCESS have the same value. AT_EACCESS is
> - * meaningful only to faccessat, while AT_REMOVEDIR is meaningful only to
> - * unlinkat. The two functions do completely different things and therefore,
> - * the flags can be allowed to overlap. For example, passing AT_REMOVEDIR to
> - * faccessat would be undefined behavior and thus treating it equivalent to
> - * AT_EACCESS is valid undefined behavior.
> + * Per-syscall flags for the *at(2) family of syscalls.
> + *
> + * These are flags that are so syscall-specific that a user passing these flags
> + * to the wrong syscall is so "clearly wrong" that we can safely call such
> + * usage "undefined behaviour".
> + *
> + * For example, the constants AT_REMOVEDIR and AT_EACCESS have the same value.
> + * AT_EACCESS is meaningful only to faccessat, while AT_REMOVEDIR is meaningful
> + * only to unlinkat. The two functions do completely different things and
> + * therefore, the flags can be allowed to overlap. For example, passing
> + * AT_REMOVEDIR to faccessat would be undefined behavior and thus treating it
> + * equivalent to AT_EACCESS is valid undefined behavior.
> + *
> + * Note for implementers: When picking a new per-syscall AT_* flag, try to
> + * reuse already existing flags first. This leaves us with as many unused bits
> + * as possible, so we can use them for generic bits in the future if necessary.
> */
> -#define AT_FDCWD -100 /* Special value used to indicate
> - openat should use the current
> - working directory. */
> -#define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic links. */
> +
> +/* Flags for renameat2(2) (must match legacy RENAME_* flags). */
> +#define AT_RENAME_NOREPLACE 0x0001
> +#define AT_RENAME_EXCHANGE 0x0002
> +#define AT_RENAME_WHITEOUT 0x0004
> +
> +/* Flag for faccessat(2). */
> #define AT_EACCESS 0x200 /* Test access permitted for
> effective IDs, not real IDs. */
> +/* Flag for unlinkat(2). */
> #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 */
> -
> -#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_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the server */
> -
> -#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
> +/* 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 with open_by_handle_at(2). */
>
> -/* 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
> - compare object identity and may not
> - be usable to open_by_handle_at(2) */
> #if defined(__KERNEL__)
> #define AT_GETATTR_NOSEC 0x80000000
> #endif
>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next v8 3/3] selftests: add MSG_ZEROCOPY msg_control notification test
From: Zijian Zhang @ 2024-08-01 18:15 UTC (permalink / raw)
To: Willem de Bruijn
Cc: netdev, linux-api, almasrymina, edumazet, davem, kuba, pabeni,
dsahern, axboe, shuah, linux-kselftest, cong.wang, xiaochun.lu
In-Reply-To: <CAF=yD-Jt6XWSCLfZE1C+9=vcXyG-XcC2q-7Ai-HHSUt=1OrWsg@mail.gmail.com>
On 8/1/24 10:36 AM, Willem de Bruijn wrote:
> On Thu, Aug 1, 2024 at 1:30 PM Zijian Zhang <zijianzhang@bytedance.com> wrote:
>>>
>>>> -static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
>>>> +static void add_zcopy_info(struct msghdr *msg)
>>>> +{
>>>> + struct zc_info *zc_info;
>>>> + struct cmsghdr *cm;
>>>> +
>>>> + if (!msg->msg_control)
>>>> + error(1, errno, "NULL user arg");
>>>
>>> Don't add precondition checks for code entirely under your control.
>>> This is not a user API.
>>>
>>
>> Ack.
>>
>>>> + cm = (struct cmsghdr *)msg->msg_control;
>>>> + cm->cmsg_len = CMSG_LEN(ZC_INFO_SIZE);
>>>> + cm->cmsg_level = SOL_SOCKET;
>>>> + cm->cmsg_type = SCM_ZC_NOTIFICATION;
>>>> +
>>>> + zc_info = (struct zc_info *)CMSG_DATA(cm);
>>>> + zc_info->size = ZC_NOTIFICATION_MAX;
>>>> +
>>>> + added_zcopy_info = true;
>>>
>>> Just initialize every time? Is this here to reuse the same msg_control
>>> as long as metadata is returned?
>>>
>>
>> Yes, the same msg_control will be reused.
>>
>> The overall paradiagm is,
>> start:
>> sendmsg(..)
>> sendmsg(..)
>> ... sends_since_notify sendmsgs in total
>>
>> add_zcopy_info(..)
>> sendmsg(.., msg_control)
>> do_recv_completions_sendmsg(..)
>> goto start;
>>
>> if (sends_since_notify + 1 >= cfg_notification_limit), add_zcopy_info
>> will be invoked, and the right next sendmsg will have the msg_control
>> passed in.
>>
>> If (added_zcopy_info), do_recv_completions_sendmsg will be invoked,
>> and added_zcopy_info will be set to false in it.
>
> This does not seem like it would need a global variable?
>
Agreed, maybe I can use sends_since_notify to check whether we
need to do_recv_completions_sendmsg, then we get rid of
added_zcopy_info.
>> Btw, before I put some efforts to solve the current issues, I think
>> I should wait for comments about api change from linux-api@vger.kernel.org?
>
> I'm not sure whether anyone on that list will give feedback.
>
> I would continue with revisions at a normal schedule, as long as that
> stays in the Cc.
Got it, thanks
^ permalink raw reply
* Re: [PATCH net-next v8 2/3] sock: add MSG_ZEROCOPY notification mechanism based on msg_control
From: Willem de Bruijn @ 2024-08-01 17:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: zijianzhang, netdev, linux-api, almasrymina, edumazet, davem,
pabeni, dsahern, axboe, shuah, linux-kselftest, cong.wang,
xiaochun.lu
In-Reply-To: <20240731182908.4584a3e1@kernel.org>
On Wed, Jul 31, 2024 at 9:29 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 31 Jul 2024 18:20:35 -0400 Willem de Bruijn wrote:
> > Btw patchwork shows red for patch 1/3 due to a new error or warning.
> > Not sure if it's a false positive, but take a look.
>
> Patchwork is not for contributors, I keep repeating this :|
> Were you not in the room at netdev when I was talking about NIPA
> or am I this shit at communicating?
>
> Next person pointing someone to patchwork will get a task to fix
> something in NIPA.
:-)
It's a super informative tool. I did miss the point about the intended
audience, use cases and known limitations (such as false positives).
Got it now!
Looking forward to the netdev talks and slides online soon.
^ permalink raw reply
* Re: [PATCH net-next v8 3/3] selftests: add MSG_ZEROCOPY msg_control notification test
From: Willem de Bruijn @ 2024-08-01 17:36 UTC (permalink / raw)
To: Zijian Zhang
Cc: netdev, linux-api, almasrymina, edumazet, davem, kuba, pabeni,
dsahern, axboe, shuah, linux-kselftest, cong.wang, xiaochun.lu
In-Reply-To: <570fe8a0-4b93-4f3d-a4d7-34a3a61167e4@bytedance.com>
On Thu, Aug 1, 2024 at 1:30 PM Zijian Zhang <zijianzhang@bytedance.com> wrote:
>
> On 7/31/24 3:32 PM, Willem de Bruijn wrote:
> > zijianzhang@ wrote:
> >> From: Zijian Zhang <zijianzhang@bytedance.com>
> >>
> >> We update selftests/net/msg_zerocopy.c to accommodate the new mechanism,
>
> First of all, thanks for the detailed suggestions!
>
> >
> > Please make commit messages stand on their own. If someone does a git
> > blame, make the message self explanatory. Replace "the new mechanism"
> > with sendmsg SCM_ZC_NOTIFICATION.
> >
> > In patch 2 or as a separate patch 4, also add a new short section on
> > this API in Documentation/networking/msg_zerocopy.rst. Probably with
> > the same contents as a good explanation of the feature in the commit
> > message of patch 2.
> >
>
> Agreed.
>
> >> cfg_notification_limit has the same semantics for both methods. Test
> >> results are as follows, we update skb_orphan_frags_rx to the same as
> >> skb_orphan_frags to support zerocopy in the localhost test.
> >>
> >> cfg_notification_limit = 1, both method get notifications after 1 calling
> >> of sendmsg. In this case, the new method has around 17% cpu savings in TCP
> >> and 23% cpu savings in UDP.
> >> +---------------------+---------+---------+---------+---------+
> >> | Test Type / Protocol| TCP v4 | TCP v6 | UDP v4 | UDP v6 |
> >> +---------------------+---------+---------+---------+---------+
> >> | ZCopy (MB) | 7523 | 7706 | 7489 | 7304 |
> >> +---------------------+---------+---------+---------+---------+
> >> | New ZCopy (MB) | 8834 | 8993 | 9053 | 9228 |
> >> +---------------------+---------+---------+---------+---------+
> >> | New ZCopy / ZCopy | 117.42% | 116.70% | 120.88% | 126.34% |
> >> +---------------------+---------+---------+---------+---------+
> >>
> >> cfg_notification_limit = 32, both get notifications after 32 calling of
> >> sendmsg, which means more chances to coalesce notifications, and less
> >> overhead of poll + recvmsg for the original method. In this case, the new
> >> method has around 7% cpu savings in TCP and slightly better cpu usage in
> >> UDP. In the env of selftest, notifications of TCP are more likely to be
> >> out of order than UDP, it's easier to coalesce more notifications in UDP.
> >> The original method can get one notification with range of 32 in a recvmsg
> >> most of the time. In TCP, most notifications' range is around 2, so the
> >> original method needs around 16 recvmsgs to get notified in one round.
> >> That's the reason for the "New ZCopy / ZCopy" diff in TCP and UDP here.
> >> +---------------------+---------+---------+---------+---------+
> >> | Test Type / Protocol| TCP v4 | TCP v6 | UDP v4 | UDP v6 |
> >> +---------------------+---------+---------+---------+---------+
> >> | ZCopy (MB) | 8842 | 8735 | 10072 | 9380 |
> >> +---------------------+---------+---------+---------+---------+
> >> | New ZCopy (MB) | 9366 | 9477 | 10108 | 9385 |
> >> +---------------------+---------+---------+---------+---------+
> >> | New ZCopy / ZCopy | 106.00% | 108.28% | 100.31% | 100.01% |
> >> +---------------------+---------+---------+---------+---------+
> >>
> >> In conclusion, when notification interval is small or notifications are
> >> hard to be coalesced, the new mechanism is highly recommended. Otherwise,
> >> the performance gain from the new mechanism is very limited.
> >>
> >> Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
> >> Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>
> >
> >> -static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
> >> +static void add_zcopy_info(struct msghdr *msg)
> >> +{
> >> + struct zc_info *zc_info;
> >> + struct cmsghdr *cm;
> >> +
> >> + if (!msg->msg_control)
> >> + error(1, errno, "NULL user arg");
> >
> > Don't add precondition checks for code entirely under your control.
> > This is not a user API.
> >
>
> Ack.
>
> >> + cm = (struct cmsghdr *)msg->msg_control;
> >> + cm->cmsg_len = CMSG_LEN(ZC_INFO_SIZE);
> >> + cm->cmsg_level = SOL_SOCKET;
> >> + cm->cmsg_type = SCM_ZC_NOTIFICATION;
> >> +
> >> + zc_info = (struct zc_info *)CMSG_DATA(cm);
> >> + zc_info->size = ZC_NOTIFICATION_MAX;
> >> +
> >> + added_zcopy_info = true;
> >
> > Just initialize every time? Is this here to reuse the same msg_control
> > as long as metadata is returned?
> >
>
> Yes, the same msg_control will be reused.
>
> The overall paradiagm is,
> start:
> sendmsg(..)
> sendmsg(..)
> ... sends_since_notify sendmsgs in total
>
> add_zcopy_info(..)
> sendmsg(.., msg_control)
> do_recv_completions_sendmsg(..)
> goto start;
>
> if (sends_since_notify + 1 >= cfg_notification_limit), add_zcopy_info
> will be invoked, and the right next sendmsg will have the msg_control
> passed in.
>
> If (added_zcopy_info), do_recv_completions_sendmsg will be invoked,
> and added_zcopy_info will be set to false in it.
This does not seem like it would need a global variable?
> >> +}
> >> +
> >> +static bool do_sendmsg(int fd, struct msghdr *msg,
> >> + enum notification_type do_zerocopy, int domain)
> >> {
> >> int ret, len, i, flags;
> >> static uint32_t cookie;
> >> @@ -200,6 +233,12 @@ static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
> >> msg->msg_controllen = CMSG_SPACE(sizeof(cookie));
> >> msg->msg_control = (struct cmsghdr *)ckbuf;
> >> add_zcopy_cookie(msg, ++cookie);
> >> + } else if (do_zerocopy == MSG_ZEROCOPY_NOTIFY_SENDMSG &&
> >> + sends_since_notify + 1 >= cfg_notification_limit) {
> >> + memset(&msg->msg_control, 0, sizeof(msg->msg_control));
> >> + msg->msg_controllen = CMSG_SPACE(ZC_INFO_SIZE);
> >> + msg->msg_control = (struct cmsghdr *)zc_ckbuf;
> >> + add_zcopy_info(msg);
> >> }
> >> }
> >>
> >> @@ -218,7 +257,7 @@ static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
> >> if (do_zerocopy && ret)
> >> expected_completions++;
> >> }
> >> - if (do_zerocopy && domain == PF_RDS) {
> >> + if (msg->msg_control) {
> >> msg->msg_control = NULL;
> >> msg->msg_controllen = 0;
> >> }
> >> @@ -466,6 +505,44 @@ static void do_recv_completions(int fd, int domain)
> >> sends_since_notify = 0;
> >> }
> >>
> >> +static void do_recv_completions2(void)
> >
> > functionname2 is very uninformative.
> >
> > do_recv_completions_sendmsg or so.
> >
>
> Ack.
>
> >> +{
> >> + struct cmsghdr *cm = (struct cmsghdr *)zc_ckbuf;
> >> + struct zc_info *zc_info;
> >> + __u32 hi, lo, range;
> >> + __u8 zerocopy;
> >> + int i;
> >> +
> >> + zc_info = (struct zc_info *)CMSG_DATA(cm);
> >> + for (i = 0; i < zc_info->size; i++) {
> >> + hi = zc_info->arr[i].hi;
> >> + lo = zc_info->arr[i].lo;
> >> + zerocopy = zc_info->arr[i].zerocopy;
> >> + range = hi - lo + 1;
> >> +
> >> + if (cfg_verbose && lo != next_completion)
> >> + fprintf(stderr, "gap: %u..%u does not append to %u\n",
> >> + lo, hi, next_completion);
> >> + next_completion = hi + 1;
> >> +
> >> + if (zerocopied == -1) {
> >> + zerocopied = zerocopy;
> >> + } else if (zerocopied != zerocopy) {
> >> + fprintf(stderr, "serr: inconsistent\n");
> >> + zerocopied = zerocopy;
> >> + }
> >> +
> >> + completions += range;
> >> + sends_since_notify -= range;
> >> +
> >> + if (cfg_verbose >= 2)
> >> + fprintf(stderr, "completed: %u (h=%u l=%u)\n",
> >> + range, hi, lo);
> >> + }
> >> +
> >> + added_zcopy_info = false;
> >> +}
> >> +
> >> /* Wait for all remaining completions on the errqueue */
> >> static void do_recv_remaining_completions(int fd, int domain)
> >> {
> >> @@ -553,11 +630,16 @@ static void do_tx(int domain, int type, int protocol)
> >> else
> >> do_sendmsg(fd, &msg, cfg_zerocopy, domain);
> >>
> >> - if (cfg_zerocopy && sends_since_notify >= cfg_notification_limit)
> >> + if (cfg_zerocopy == MSG_ZEROCOPY_NOTIFY_ERRQUEUE &&
> >> + sends_since_notify >= cfg_notification_limit)
> >> do_recv_completions(fd, domain);
> >>
> >> + if (cfg_zerocopy == MSG_ZEROCOPY_NOTIFY_SENDMSG &&
> >> + added_zcopy_info)
> >> + do_recv_completions2();
> >> +
> >> while (!do_poll(fd, POLLOUT)) {
> >> - if (cfg_zerocopy)
> >> + if (cfg_zerocopy == MSG_ZEROCOPY_NOTIFY_ERRQUEUE)
> >> do_recv_completions(fd, domain);
> >> }
> >>
> >> @@ -715,7 +797,7 @@ static void parse_opts(int argc, char **argv)
> >>
> >> cfg_payload_len = max_payload_len;
> >>
> >> - while ((c = getopt(argc, argv, "46c:C:D:i:l:mp:rs:S:t:vz")) != -1) {
> >> + while ((c = getopt(argc, argv, "46c:C:D:i:l:mnp:rs:S:t:vz")) != -1) {
> >> switch (c) {
> >> case '4':
> >> if (cfg_family != PF_UNSPEC)
> >> @@ -749,6 +831,9 @@ static void parse_opts(int argc, char **argv)
> >> case 'm':
> >> cfg_cork_mixed = true;
> >> break;
> >> + case 'n':
> >> + cfg_zerocopy = MSG_ZEROCOPY_NOTIFY_SENDMSG;
> >> + break;
> >
> > How about -Z to make clear that this is still MSG_ZEROCOPY, just with
> > a different notification mechanism.
> >
> > And perhaps add a testcase that exercises both this mechanism and
> > existing recvmsg MSG_ERRQUEUE. As they should work in parallel and
> > concurrently in a multithreaded environment.
> >
>
> -Z is more clear, and the hybrid testcase will be helpful.
>
> Btw, before I put some efforts to solve the current issues, I think
> I should wait for comments about api change from linux-api@vger.kernel.org?
I'm not sure whether anyone on that list will give feedback.
I would continue with revisions at a normal schedule, as long as that
stays in the Cc.
^ permalink raw reply
* Re: [PATCH net-next v8 3/3] selftests: add MSG_ZEROCOPY msg_control notification test
From: Zijian Zhang @ 2024-08-01 17:30 UTC (permalink / raw)
To: Willem de Bruijn, netdev
Cc: linux-api, almasrymina, edumazet, davem, kuba, pabeni, dsahern,
axboe, shuah, linux-kselftest, cong.wang, xiaochun.lu
In-Reply-To: <66aabb616714_21c08c29432@willemb.c.googlers.com.notmuch>
On 7/31/24 3:32 PM, Willem de Bruijn wrote:
> zijianzhang@ wrote:
>> From: Zijian Zhang <zijianzhang@bytedance.com>
>>
>> We update selftests/net/msg_zerocopy.c to accommodate the new mechanism,
First of all, thanks for the detailed suggestions!
>
> Please make commit messages stand on their own. If someone does a git
> blame, make the message self explanatory. Replace "the new mechanism"
> with sendmsg SCM_ZC_NOTIFICATION.
>
> In patch 2 or as a separate patch 4, also add a new short section on
> this API in Documentation/networking/msg_zerocopy.rst. Probably with
> the same contents as a good explanation of the feature in the commit
> message of patch 2.
>
Agreed.
>> cfg_notification_limit has the same semantics for both methods. Test
>> results are as follows, we update skb_orphan_frags_rx to the same as
>> skb_orphan_frags to support zerocopy in the localhost test.
>>
>> cfg_notification_limit = 1, both method get notifications after 1 calling
>> of sendmsg. In this case, the new method has around 17% cpu savings in TCP
>> and 23% cpu savings in UDP.
>> +---------------------+---------+---------+---------+---------+
>> | Test Type / Protocol| TCP v4 | TCP v6 | UDP v4 | UDP v6 |
>> +---------------------+---------+---------+---------+---------+
>> | ZCopy (MB) | 7523 | 7706 | 7489 | 7304 |
>> +---------------------+---------+---------+---------+---------+
>> | New ZCopy (MB) | 8834 | 8993 | 9053 | 9228 |
>> +---------------------+---------+---------+---------+---------+
>> | New ZCopy / ZCopy | 117.42% | 116.70% | 120.88% | 126.34% |
>> +---------------------+---------+---------+---------+---------+
>>
>> cfg_notification_limit = 32, both get notifications after 32 calling of
>> sendmsg, which means more chances to coalesce notifications, and less
>> overhead of poll + recvmsg for the original method. In this case, the new
>> method has around 7% cpu savings in TCP and slightly better cpu usage in
>> UDP. In the env of selftest, notifications of TCP are more likely to be
>> out of order than UDP, it's easier to coalesce more notifications in UDP.
>> The original method can get one notification with range of 32 in a recvmsg
>> most of the time. In TCP, most notifications' range is around 2, so the
>> original method needs around 16 recvmsgs to get notified in one round.
>> That's the reason for the "New ZCopy / ZCopy" diff in TCP and UDP here.
>> +---------------------+---------+---------+---------+---------+
>> | Test Type / Protocol| TCP v4 | TCP v6 | UDP v4 | UDP v6 |
>> +---------------------+---------+---------+---------+---------+
>> | ZCopy (MB) | 8842 | 8735 | 10072 | 9380 |
>> +---------------------+---------+---------+---------+---------+
>> | New ZCopy (MB) | 9366 | 9477 | 10108 | 9385 |
>> +---------------------+---------+---------+---------+---------+
>> | New ZCopy / ZCopy | 106.00% | 108.28% | 100.31% | 100.01% |
>> +---------------------+---------+---------+---------+---------+
>>
>> In conclusion, when notification interval is small or notifications are
>> hard to be coalesced, the new mechanism is highly recommended. Otherwise,
>> the performance gain from the new mechanism is very limited.
>>
>> Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
>> Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>
>
>> -static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
>> +static void add_zcopy_info(struct msghdr *msg)
>> +{
>> + struct zc_info *zc_info;
>> + struct cmsghdr *cm;
>> +
>> + if (!msg->msg_control)
>> + error(1, errno, "NULL user arg");
>
> Don't add precondition checks for code entirely under your control.
> This is not a user API.
>
Ack.
>> + cm = (struct cmsghdr *)msg->msg_control;
>> + cm->cmsg_len = CMSG_LEN(ZC_INFO_SIZE);
>> + cm->cmsg_level = SOL_SOCKET;
>> + cm->cmsg_type = SCM_ZC_NOTIFICATION;
>> +
>> + zc_info = (struct zc_info *)CMSG_DATA(cm);
>> + zc_info->size = ZC_NOTIFICATION_MAX;
>> +
>> + added_zcopy_info = true;
>
> Just initialize every time? Is this here to reuse the same msg_control
> as long as metadata is returned?
>
Yes, the same msg_control will be reused.
The overall paradiagm is,
start:
sendmsg(..)
sendmsg(..)
... sends_since_notify sendmsgs in total
add_zcopy_info(..)
sendmsg(.., msg_control)
do_recv_completions_sendmsg(..)
goto start;
if (sends_since_notify + 1 >= cfg_notification_limit), add_zcopy_info
will be invoked, and the right next sendmsg will have the msg_control
passed in.
If (added_zcopy_info), do_recv_completions_sendmsg will be invoked,
and added_zcopy_info will be set to false in it.
>> +}
>> +
>> +static bool do_sendmsg(int fd, struct msghdr *msg,
>> + enum notification_type do_zerocopy, int domain)
>> {
>> int ret, len, i, flags;
>> static uint32_t cookie;
>> @@ -200,6 +233,12 @@ static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
>> msg->msg_controllen = CMSG_SPACE(sizeof(cookie));
>> msg->msg_control = (struct cmsghdr *)ckbuf;
>> add_zcopy_cookie(msg, ++cookie);
>> + } else if (do_zerocopy == MSG_ZEROCOPY_NOTIFY_SENDMSG &&
>> + sends_since_notify + 1 >= cfg_notification_limit) {
>> + memset(&msg->msg_control, 0, sizeof(msg->msg_control));
>> + msg->msg_controllen = CMSG_SPACE(ZC_INFO_SIZE);
>> + msg->msg_control = (struct cmsghdr *)zc_ckbuf;
>> + add_zcopy_info(msg);
>> }
>> }
>>
>> @@ -218,7 +257,7 @@ static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
>> if (do_zerocopy && ret)
>> expected_completions++;
>> }
>> - if (do_zerocopy && domain == PF_RDS) {
>> + if (msg->msg_control) {
>> msg->msg_control = NULL;
>> msg->msg_controllen = 0;
>> }
>> @@ -466,6 +505,44 @@ static void do_recv_completions(int fd, int domain)
>> sends_since_notify = 0;
>> }
>>
>> +static void do_recv_completions2(void)
>
> functionname2 is very uninformative.
>
> do_recv_completions_sendmsg or so.
>
Ack.
>> +{
>> + struct cmsghdr *cm = (struct cmsghdr *)zc_ckbuf;
>> + struct zc_info *zc_info;
>> + __u32 hi, lo, range;
>> + __u8 zerocopy;
>> + int i;
>> +
>> + zc_info = (struct zc_info *)CMSG_DATA(cm);
>> + for (i = 0; i < zc_info->size; i++) {
>> + hi = zc_info->arr[i].hi;
>> + lo = zc_info->arr[i].lo;
>> + zerocopy = zc_info->arr[i].zerocopy;
>> + range = hi - lo + 1;
>> +
>> + if (cfg_verbose && lo != next_completion)
>> + fprintf(stderr, "gap: %u..%u does not append to %u\n",
>> + lo, hi, next_completion);
>> + next_completion = hi + 1;
>> +
>> + if (zerocopied == -1) {
>> + zerocopied = zerocopy;
>> + } else if (zerocopied != zerocopy) {
>> + fprintf(stderr, "serr: inconsistent\n");
>> + zerocopied = zerocopy;
>> + }
>> +
>> + completions += range;
>> + sends_since_notify -= range;
>> +
>> + if (cfg_verbose >= 2)
>> + fprintf(stderr, "completed: %u (h=%u l=%u)\n",
>> + range, hi, lo);
>> + }
>> +
>> + added_zcopy_info = false;
>> +}
>> +
>> /* Wait for all remaining completions on the errqueue */
>> static void do_recv_remaining_completions(int fd, int domain)
>> {
>> @@ -553,11 +630,16 @@ static void do_tx(int domain, int type, int protocol)
>> else
>> do_sendmsg(fd, &msg, cfg_zerocopy, domain);
>>
>> - if (cfg_zerocopy && sends_since_notify >= cfg_notification_limit)
>> + if (cfg_zerocopy == MSG_ZEROCOPY_NOTIFY_ERRQUEUE &&
>> + sends_since_notify >= cfg_notification_limit)
>> do_recv_completions(fd, domain);
>>
>> + if (cfg_zerocopy == MSG_ZEROCOPY_NOTIFY_SENDMSG &&
>> + added_zcopy_info)
>> + do_recv_completions2();
>> +
>> while (!do_poll(fd, POLLOUT)) {
>> - if (cfg_zerocopy)
>> + if (cfg_zerocopy == MSG_ZEROCOPY_NOTIFY_ERRQUEUE)
>> do_recv_completions(fd, domain);
>> }
>>
>> @@ -715,7 +797,7 @@ static void parse_opts(int argc, char **argv)
>>
>> cfg_payload_len = max_payload_len;
>>
>> - while ((c = getopt(argc, argv, "46c:C:D:i:l:mp:rs:S:t:vz")) != -1) {
>> + while ((c = getopt(argc, argv, "46c:C:D:i:l:mnp:rs:S:t:vz")) != -1) {
>> switch (c) {
>> case '4':
>> if (cfg_family != PF_UNSPEC)
>> @@ -749,6 +831,9 @@ static void parse_opts(int argc, char **argv)
>> case 'm':
>> cfg_cork_mixed = true;
>> break;
>> + case 'n':
>> + cfg_zerocopy = MSG_ZEROCOPY_NOTIFY_SENDMSG;
>> + break;
>
> How about -Z to make clear that this is still MSG_ZEROCOPY, just with
> a different notification mechanism.
>
> And perhaps add a testcase that exercises both this mechanism and
> existing recvmsg MSG_ERRQUEUE. As they should work in parallel and
> concurrently in a multithreaded environment.
>
-Z is more clear, and the hybrid testcase will be helpful.
Btw, before I put some efforts to solve the current issues, I think
I should wait for comments about api change from linux-api@vger.kernel.org?
^ permalink raw reply
* Re: [PATCH RFC v3 0/2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Josef Bacik @ 2024-08-01 14:28 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Alexander Viro, Christian Brauner, Jan Kara, Chuck Lever,
Jeff Layton, Amir Goldstein, Alexander Aring, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
linux-fsdevel, linux-nfs, linux-kernel, linux-api,
linux-perf-users
In-Reply-To: <20240801-exportfs-u64-mount-id-v3-0-be5d6283144a@cyphar.com>
On Thu, Aug 01, 2024 at 01:52:39PM +1000, Aleksa Sarai wrote:
> Now that we provide a unique 64-bit mount ID interface in statx(2), 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 worry about
> racing with /proc/mountinfo parsing or having to open a file just to do
> statx(2).
>
> While this is not necessary if you are using AT_EMPTY_PATH and don't
> care about an extra statx(2) call, users that pass full paths into
> name_to_handle_at(2) need to know which mount the file handle comes from
> (to make sure they don't try to open_by_handle_at a file handle from a
> different filesystem) and switching to AT_EMPTY_PATH would require
> allocating a file for every name_to_handle_at(2) call, turning
>
> err = name_to_handle_at(-EBADF, "/foo/bar/baz", &handle, &mntid,
> AT_HANDLE_MNT_ID_UNIQUE);
>
> into
>
> int fd = openat(-EBADF, "/foo/bar/baz", O_PATH | O_CLOEXEC);
> err1 = name_to_handle_at(fd, "", &handle, &unused_mntid, AT_EMPTY_PATH);
> err2 = statx(fd, "", AT_EMPTY_PATH, STATX_MNT_ID_UNIQUE, &statxbuf);
> mntid = statxbuf.stx_mnt_id;
> close(fd);
>
> Also, this series adds a patch to clarify how AT_* flag allocation
> should work going forwards.
>
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> ---
> Changes in v3:
> - Added a patch describing how AT_* flags should be allocated in the
> future, based on Amir's suggestions.
> - Included AT_* aliases for RENAME_* flags to further indicate that
> renameat2(2) is an *at(2) syscall and to indicate that those flags
> have been allocated already in the per-syscall range.
> - Switched AT_HANDLE_MNT_ID_UNIQUE to use 0x01 (to reuse
> (AT_)RENAME_NOREPLACE).
> - v2: <https://lore.kernel.org/r/20240523-exportfs-u64-mount-id-v2-1-f9f959f17eb1@cyphar.com>
> Changes in v2:
> - Fixed a few minor compiler warnings and a buggy copy_to_user() check.
> - Rename AT_HANDLE_UNIQUE_MOUNT_ID -> AT_HANDLE_MNT_ID_UNIQUE to match statx.
> - Switched to using an AT_* bit from 0xFF and defining that range as
> being "per-syscall" for future usage.
> - Sync tools/ copy of <linux/fcntl.h> to include changes.
> - v1: <https://lore.kernel.org/r/20240520-exportfs-u64-mount-id-v1-1-f55fd9215b8e@cyphar.com>
>
> ---
> Aleksa Sarai (2):
> uapi: explain how per-syscall AT_* flags should be allocated
> fhandle: expose u64 mount id to name_to_handle_at(2)
>
Wasn't the conclusion from this discussion last time that we needed to revisit
this API completely? Christoph had some pretty adamant objections.
That being said the uapi comments patch looks good to me, you can add
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
to that one. The other one I'm going to let others who have stronger opinions
than me argue about. Thanks,
Josef
^ permalink raw reply
* [PATCH] syscalls: fix syscall macros for newfstat/newfstatat
From: Arnd Bergmann @ 2024-08-01 12:32 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Huacai Chen, Paul Walmsley,
Palmer Dabbelt, Albert Ou
Cc: Andreas Schwab, Florian Weimer, linux-riscv, linux-arm-kernel,
linux-api, Arnd Bergmann, WANG Xuerui, Masami Hiramatsu (Google),
linux-kernel, loongarch
From: Arnd Bergmann <arnd@arndb.de>
The __NR_newfstat and __NR_newfstatat macros accidentally got renamed
in the conversion to the syscall.tbl format, dropping the 'new' portion
of the name.
In an unrelated change, the two syscalls are no longer architecture
specific but are once more defined on all 64-bit architectures, so the
'newstat' ABI keyword can be dropped from the table as a simplification.
Fixes: Fixes: 4fe53bf2ba0a ("syscalls: add generic scripts/syscall.tbl")
Closes: https://lore.kernel.org/lkml/838053e0-b186-4e9f-9668-9a3384a71f23@app.fastmail.com/T/#t
Reported-by: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm64/kernel/Makefile.syscalls | 2 +-
arch/loongarch/kernel/Makefile.syscalls | 3 ++-
arch/riscv/kernel/Makefile.syscalls | 2 +-
scripts/syscall.tbl | 4 ++--
4 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/Makefile.syscalls b/arch/arm64/kernel/Makefile.syscalls
index 3cfafd003b2d..0542a718871a 100644
--- a/arch/arm64/kernel/Makefile.syscalls
+++ b/arch/arm64/kernel/Makefile.syscalls
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
syscall_abis_32 +=
-syscall_abis_64 += renameat newstat rlimit memfd_secret
+syscall_abis_64 += renameat rlimit memfd_secret
syscalltbl = arch/arm64/tools/syscall_%.tbl
diff --git a/arch/loongarch/kernel/Makefile.syscalls b/arch/loongarch/kernel/Makefile.syscalls
index 523bb411a3bc..ab7d9baa2915 100644
--- a/arch/loongarch/kernel/Makefile.syscalls
+++ b/arch/loongarch/kernel/Makefile.syscalls
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
-syscall_abis_64 += newstat
+# No special ABIs on loongarch so far
+syscall_abis_64 +=
diff --git a/arch/riscv/kernel/Makefile.syscalls b/arch/riscv/kernel/Makefile.syscalls
index 52087a023b3d..9668fd1faf60 100644
--- a/arch/riscv/kernel/Makefile.syscalls
+++ b/arch/riscv/kernel/Makefile.syscalls
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
syscall_abis_32 += riscv memfd_secret
-syscall_abis_64 += riscv newstat rlimit memfd_secret
+syscall_abis_64 += riscv rlimit memfd_secret
diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl
index 797e20ea99a2..4586a18dfe9b 100644
--- a/scripts/syscall.tbl
+++ b/scripts/syscall.tbl
@@ -98,9 +98,9 @@
77 common tee sys_tee
78 common readlinkat sys_readlinkat
79 stat64 fstatat64 sys_fstatat64
-79 newstat fstatat sys_newfstatat
+79 64 newfstatat sys_newfstatat
80 stat64 fstat64 sys_fstat64
-80 newstat fstat sys_newfstat
+80 64 newfstat sys_newfstat
81 common sync sys_sync
82 common fsync sys_fsync
83 common fdatasync sys_fdatasync
--
2.39.2
^ permalink raw reply related
* Re: syscall.tbl refactoring seems to have dropped definition of __NR_newfstatat on arm64 and riscv (64-bit) at least
From: Arnd Bergmann @ 2024-08-01 12:20 UTC (permalink / raw)
To: Andreas Schwab, Florian Weimer
Cc: linux-riscv, linux-arm-kernel, linux-api, linux-kernel,
Catalin Marinas
In-Reply-To: <mvmh6c4fors.fsf@suse.de>
On Thu, Aug 1, 2024, at 13:47, Andreas Schwab wrote:
> On Aug 01 2024, Florian Weimer wrote:
>
>> It's been repported that __NR_newfstatat has gone missing from the UAPI
>> headers.
>
> It has actually been wrongly renamed to __NR_fstatat.
Ah, I see what happened now, this was part of a semi-automated
conversion from the old format and I incorrectly split up the
entries between 32-bit and 64-bit.
There is another bug in the same entry, since in the meantime,
we have also decided to make newfstat/newfstatat unconditional
again for all 64-bit architectures, and this hasn't made it into
the new syscall.tbl format. This one is only important for
loongarch64.
I'll fix up both ASAP, thanks a lot for the report!
Arnd
^ permalink raw reply
* Re: syscall.tbl refactoring seems to have dropped definition of __NR_newfstatat on arm64 and riscv (64-bit) at least
From: Andreas Schwab @ 2024-08-01 11:47 UTC (permalink / raw)
To: Florian Weimer
Cc: linux-riscv, linux-arm-kernel, linux-api, linux-kernel,
Arnd Bergmann, Catalin Marinas
In-Reply-To: <87sevoqy7v.fsf@oldenburg.str.redhat.com>
On Aug 01 2024, Florian Weimer wrote:
> It's been repported that __NR_newfstatat has gone missing from the UAPI
> headers.
It has actually been wrongly renamed to __NR_fstatat.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH v3] epoll.7: clarify the event distribution under edge-triggered mode
From: 潘少 @ 2024-08-01 11:40 UTC (permalink / raw)
To: Alejandro Colomar; +Cc: Michael Kerrisk, linux-man, linux-api
In-Reply-To: <tencent_23435D9572735FC704D2B4B1@qq.com>
Updated to [PATCH v5].
-----------
Best regards,
Andy Pan
------------------ Original ------------------
From: "潘少"<i@andypan.me>;
Date: Wed, Jul 31, 2024 11:22 PMTo: "Alejandro Colomar"<alx@kernel.org>;
Cc: "Michael Kerrisk"<mtk.manpages@gmail.com>; "linux-man"<linux-man@vger.kernel.org>; "linux-api"<linux-api@vger.kernel.org>;
Subject: Re: [PATCH v3] epoll.7: clarify the event distribution under edge-triggered mode
Hi Alejandro,
I believe I've found a more suitable place to put this added clarification sentence. Please check out the [PATCH v4], thanks!
-----------
Best regards,Andy Pan
^ permalink raw reply
* [PATCH v5] epoll.7: clarify the event distribution under edge-triggered mode
From: Andy Pan via B4 Relay @ 2024-08-01 11:38 UTC (permalink / raw)
To: Alejandro Colomar; +Cc: linux-man, linux-api, mtk.manpages, Andy Pan, Andy Pan
From: Andy Pan <i@andypan.me>
For the moment, the edge-triggered epoll generates an event for each
receipt of a chunk of data, that is to say, epoll_wait() will return
and tell us a monitored file descriptor is ready whenever there is a
new activity on that FD since we were last informed about that FD.
This is not a real _edge_ implementation for epoll, but it's been
working this way for years and plenty of projects are relying on it
to eliminate the overhead of one system call of read(2) per wakeup event.
There are several renowned open-source projects relying on this feature
for notification function (with eventfd): register eventfd with EPOLLET
and avoid calling read(2) on the eventfd when there is wakeup event (eventfd being written).
Examples: nginx [1], netty [2], tokio [3], libevent [4], ect. [5]
These projects are widely used in today's Internet infrastructures.
Thus, changing this behavior of epoll ET will fundamentally break them
and cause a significant negative impact.
Linux has changed it for pipe before [6], breaking some Android libraries,
which had got "reverted" somehow. [7] [8]
Nevertheless, the paragraph in the manual pages describing this
characteristic of epoll ET seems ambiguous, I think a more explict
sentence should be used to clarify it. We're improving the notification
mechanism for libuv recently by exploiting this feature with eventfd,
which brings us a significant performance boost. [9]
Therefore, we (as well as the maintainers of nginx, netty, tokio, etc.)
would have a sense of security to build an enhanced notification function
based on this feature if there is a guarantee of retaining this implementation
of epoll ET for the backward compatibility in the man pages.
[1]: https://github.com/nginx/nginx/blob/efc6a217b92985a1ee211b6bb7337cd2f62deb90/src/event/modules/ngx_epoll_module.c#L386-L457
[2]: https://github.com/netty/netty/pull/9192
[3]: https://github.com/tokio-rs/mio/blob/309daae21ecb1d46203a7dbc0cf4c80310240cba/src/sys/unix/waker.rs#L111-L143
[4]: https://github.com/libevent/libevent/blob/525f5d0a14c9c103be750f2ca175328c25505ea4/event.c#L2597-L2614
[5]: https://github.com/libuv/libuv/pull/4400#issuecomment-2123798748
[6]: https://lkml.iu.edu/hypermail/linux/kernel/2010.1/04363.html
[7]: https://github.com/torvalds/linux/commit/3a34b13a88caeb2800ab44a4918f230041b37dd9
[8]: https://github.com/torvalds/linux/commit/3b844826b6c6affa80755254da322b017358a2f4
[9]: https://github.com/libuv/libuv/pull/4400#issuecomment-2103232402
Signed-off-by: Andy Pan <i@andypan.me>
---
Changes in v5:
- Update the sentence of clarifying the epoll ET
- Link to v4: https://lore.kernel.org/r/20240731-epoll-et-desc-v4-1-7eb819bdde0d@andypan.me
Changes in v4:
- Move the added sentence elsewhere to make more sense
- Link to v3: https://lore.kernel.org/r/20240730-epoll-et-desc-v3-1-6aa81b1c400d@andypan.me
Changes in v3:
- Updated the git commit description
- Link to v2: https://lore.kernel.org/r/20240727-epoll-et-desc-v2-1-c99b2ac66775@andypan.me
Changes in v2:
- Added the git commit description based on feedback
- Link to v1: https://lore.kernel.org/r/20240727-epoll-et-desc-v1-1-390bafc678b9@andypan.me
---
man/man7/epoll.7 | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/man/man7/epoll.7 b/man/man7/epoll.7
index 951500131..86e5f8363 100644
--- a/man/man7/epoll.7
+++ b/man/man7/epoll.7
@@ -121,7 +121,8 @@ .SS Level-triggered and edge-triggered
meanwhile the remote peer might be expecting a response based on the
data it already sent.
The reason for this is that edge-triggered mode
-delivers events only when changes occur on the monitored file descriptor.
+delivers events only when changes occur on the monitored file descriptor,
+that is, an event will be generated upon each receipt of a chunk of data.
So, in step
.B 5
the caller might end up waiting for some data that is already present inside
---
base-commit: cbc0a111e4dceea2037c51098de33e6bc8c16a5c
change-id: 20240727-epoll-et-desc-04ea9a590f3b
Best regards,
--
Andy Pan <i@andypan.me>
^ permalink raw reply related
* syscall.tbl refactoring seems to have dropped definition of __NR_newfstatat on arm64 and riscv (64-bit) at least
From: Florian Weimer @ 2024-08-01 11:28 UTC (permalink / raw)
To: linux-riscv, linux-arm-kernel, linux-api, linux-kernel
Cc: Arnd Bergmann, Catalin Marinas
It's been repported that __NR_newfstatat has gone missing from the UAPI
headers.
[sanitizer] __NR_newfstatat not defined in linux kernel headers in
aarch64
<https://github.com/llvm/llvm-project/issues/100098>
__NR_newfstatat no longer defined on aarch64
<https://bugzilla.redhat.com/show_bug.cgi?id=2301919>
I suspect that's related to the syscall.tbl refactoring around these
commits:
commit e632bca07c8eef1de9dc50f4e4066c56e9d68b07
Author: Arnd Bergmann <arnd@arndb.de>
Date: Thu Jul 4 14:33:34 2024 +0200
arm64: generate 64-bit syscall.tbl
commit 3db80c999debbadd5d627fb30f8b06fee331ffb6
Author: Arnd Bergmann <arnd@arndb.de>
Date: Wed Apr 24 09:14:39 2024 +0200
riscv: convert to generic syscall table
Thanks,
Florian
^ permalink raw reply
* [PATCH RFC v3 2/2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Aleksa Sarai @ 2024-08-01 3:52 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Chuck Lever,
Jeff Layton, Amir Goldstein, Alexander Aring, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter
Cc: linux-fsdevel, linux-nfs, linux-kernel, linux-api,
linux-perf-users, Aleksa Sarai
In-Reply-To: <20240801-exportfs-u64-mount-id-v3-0-be5d6283144a@cyphar.com>
Now that we provide a unique 64-bit mount ID interface in statx(2), 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 worry about
racing with /proc/mountinfo parsing or having to open a file just to do
statx(2).
While this is not necessary if you are using AT_EMPTY_PATH and don't
care about an extra statx(2) call, users that pass full paths into
name_to_handle_at(2) need to know which mount the file handle comes from
(to make sure they don't try to open_by_handle_at a file handle from a
different filesystem) and switching to AT_EMPTY_PATH would require
allocating a file for every name_to_handle_at(2) call, turning
err = name_to_handle_at(-EBADF, "/foo/bar/baz", &handle, &mntid,
AT_HANDLE_MNT_ID_UNIQUE);
into
int fd = openat(-EBADF, "/foo/bar/baz", O_PATH | O_CLOEXEC);
err1 = name_to_handle_at(fd, "", &handle, &unused_mntid, AT_EMPTY_PATH);
err2 = statx(fd, "", AT_EMPTY_PATH, STATX_MNT_ID_UNIQUE, &statxbuf);
mntid = statxbuf.stx_mnt_id;
close(fd);
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
fs/fhandle.c | 29 ++++++++++++++++------
include/linux/syscalls.h | 2 +-
include/uapi/linux/fcntl.h | 1 +
tools/perf/trace/beauty/include/uapi/linux/fcntl.h | 1 +
4 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/fs/fhandle.c b/fs/fhandle.c
index 6e8cea16790e..8cb665629f4a 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -16,7 +16,8 @@
static long do_sys_name_to_handle(const struct path *path,
struct file_handle __user *ufh,
- int __user *mnt_id, int fh_flags)
+ void __user *mnt_id, bool unique_mntid,
+ int fh_flags)
{
long retval;
struct file_handle f_handle;
@@ -69,9 +70,19 @@ static long do_sys_name_to_handle(const struct path *path,
} else
retval = 0;
/* copy the mount id */
- if (put_user(real_mount(path->mnt)->mnt_id, mnt_id) ||
- copy_to_user(ufh, handle,
- struct_size(handle, f_handle, handle_bytes)))
+ if (unique_mntid) {
+ if (put_user(real_mount(path->mnt)->mnt_id_unique,
+ (u64 __user *) mnt_id))
+ retval = -EFAULT;
+ } else {
+ if (put_user(real_mount(path->mnt)->mnt_id,
+ (int __user *) mnt_id))
+ retval = -EFAULT;
+ }
+ /* copy the handle */
+ if (retval != -EFAULT &&
+ copy_to_user(ufh, handle,
+ struct_size(handle, f_handle, handle_bytes)))
retval = -EFAULT;
kfree(handle);
return retval;
@@ -83,6 +94,7 @@ static long do_sys_name_to_handle(const struct path *path,
* @name: name that should be converted to handle.
* @handle: resulting file handle
* @mnt_id: mount id of the file system containing the file
+ * (u64 if AT_HANDLE_MNT_ID_UNIQUE, otherwise int)
* @flag: flag value to indicate whether to follow symlink or not
* and whether a decodable file handle is required.
*
@@ -92,7 +104,7 @@ static long do_sys_name_to_handle(const struct path *path,
* value required.
*/
SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
- struct file_handle __user *, handle, int __user *, mnt_id,
+ struct file_handle __user *, handle, void __user *, mnt_id,
int, flag)
{
struct path path;
@@ -100,7 +112,8 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
int fh_flags;
int err;
- if (flag & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH | AT_HANDLE_FID))
+ if (flag & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH | AT_HANDLE_FID |
+ AT_HANDLE_MNT_ID_UNIQUE))
return -EINVAL;
lookup_flags = (flag & AT_SYMLINK_FOLLOW) ? LOOKUP_FOLLOW : 0;
@@ -109,7 +122,9 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
lookup_flags |= LOOKUP_EMPTY;
err = user_path_at(dfd, name, lookup_flags, &path);
if (!err) {
- err = do_sys_name_to_handle(&path, handle, mnt_id, fh_flags);
+ err = do_sys_name_to_handle(&path, handle, mnt_id,
+ flag & AT_HANDLE_MNT_ID_UNIQUE,
+ fh_flags);
path_put(&path);
}
return err;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 4bcf6754738d..5758104921e6 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -870,7 +870,7 @@ asmlinkage long sys_fanotify_mark(int fanotify_fd, unsigned int flags,
#endif
asmlinkage long sys_name_to_handle_at(int dfd, const char __user *name,
struct file_handle __user *handle,
- int __user *mnt_id, int flag);
+ void __user *mnt_id, int flag);
asmlinkage long sys_open_by_handle_at(int mountdirfd,
struct file_handle __user *handle,
int flags);
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index 38a6d66d9e88..87e2dec79fea 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -152,6 +152,7 @@
#define AT_HANDLE_FID 0x200 /* File handle is needed to compare
object identity and may not be
usable with open_by_handle_at(2). */
+#define AT_HANDLE_MNT_ID_UNIQUE 0x001 /* Return the u64 unique mount ID. */
#if defined(__KERNEL__)
#define AT_GETATTR_NOSEC 0x80000000
diff --git a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
index 38a6d66d9e88..87e2dec79fea 100644
--- a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
+++ b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
@@ -152,6 +152,7 @@
#define AT_HANDLE_FID 0x200 /* File handle is needed to compare
object identity and may not be
usable with open_by_handle_at(2). */
+#define AT_HANDLE_MNT_ID_UNIQUE 0x001 /* Return the u64 unique mount ID. */
#if defined(__KERNEL__)
#define AT_GETATTR_NOSEC 0x80000000
--
2.45.2
^ permalink raw reply related
* [PATCH RFC v3 1/2] uapi: explain how per-syscall AT_* flags should be allocated
From: Aleksa Sarai @ 2024-08-01 3:52 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Chuck Lever,
Jeff Layton, Amir Goldstein, Alexander Aring, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter
Cc: linux-fsdevel, linux-nfs, linux-kernel, linux-api,
linux-perf-users, Aleksa Sarai
In-Reply-To: <20240801-exportfs-u64-mount-id-v3-0-be5d6283144a@cyphar.com>
Unfortunately, the way we have gone about adding new AT_* flags has
been a little messy. In the beginning, all of the AT_* flags had generic
meanings and so it made sense to share the flag bits indiscriminately.
However, we inevitably ran into syscalls that needed their own
syscall-specific flags. Due to the lack of a planned out policy, we
ended up with the following situations:
* Existing syscalls adding new features tended to use new AT_* bits,
with some effort taken to try to re-use bits for flags that were so
obviously syscall specific that they only make sense for a single
syscall (such as the AT_EACCESS/AT_REMOVEDIR/AT_HANDLE_FID triplet).
Given the constraints of bitflags, this works well in practice, but
ideally (to avoid future confusion) we would plan ahead and define a
set of "per-syscall bits" ahead of time so that when allocating new
bits we don't end up with a complete mish-mash of which bits are
supposed to be per-syscall and which aren't.
* New syscalls dealt with this in several ways:
- Some syscalls (like renameat2(2), move_mount(2), fsopen(2), and
fspick(2)) created their separate own flag spaces that have no
overlap with the AT_* flags. Most of these ended up allocating
their bits sequentually.
In the case of move_mount(2) and fspick(2), several flags have
identical meanings to AT_* flags but were allocated in their own
flag space.
This makes sense for syscalls that will never share AT_* flags, but
for some syscalls this leads to duplication with AT_* flags in a
way that could cause confusion (if renameat2(2) grew a
RENAME_EMPTY_PATH it seems likely that users could mistake it for
AT_EMPTY_PATH since it is an *at(2) syscall).
- Some syscalls unfortunately ended up both creating their own flag
space while also using bits from other flag spaces. The most
obvious example is open_tree(2), where the standard usage ends up
using flags from *THREE* separate flag spaces:
open_tree(AT_FDCWD, "/foo", OPEN_TREE_CLONE|O_CLOEXEC|AT_RECURSIVE);
(Note that O_CLOEXEC is also platform-specific, so several future
OPEN_TREE_* bits are also made unusable in one fell swoop.)
It's not entirely clear to me what the "right" choice is for new
syscalls. Just saying that all future VFS syscalls should use AT_* flags
doesn't seem practical. openat2(2) has RESOLVE_* flags (many of which
don't make much sense to burn generic AT_* flags for) and move_mount(2)
has separate AT_*-like flags for both the source and target so separate
flags are needed anyway (though it seems possible that renameat2(2)
could grow *_EMPTY_PATH flags at some point, and it's a bit of a shame
they can't be reused).
But at least for syscalls that _do_ choose to use AT_* flags, we should
explicitly state the policy that 0x2ff is currently intended for
per-syscall flags and that new flags should err on the side of
overlapping with existing flag bits (so we can extend the scope of
generic flags in the future if necessary).
And add AT_* aliases for the RENAME_* flags to further cement that
renameat2(2) is an *at(2) flag, just with its own per-syscall flags.
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
include/uapi/linux/fcntl.h | 80 ++++++++++++++-------
tools/perf/trace/beauty/include/uapi/linux/fcntl.h | 83 +++++++++++++++-------
2 files changed, 115 insertions(+), 48 deletions(-)
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index e55a3314bcb0..38a6d66d9e88 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -90,37 +90,69 @@
#define DN_ATTRIB 0x00000020 /* File changed attibutes */
#define DN_MULTISHOT 0x80000000 /* Don't remove notifier */
+#define AT_FDCWD -100 /* Special value for dirfd used to
+ indicate openat should use the
+ current working directory. */
+
+
+/* Generic flags for the *at(2) family of syscalls. */
+
+/* Reserved for per-syscall flags 0xff. */
+#define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic
+ links. */
+/* Reserved for per-syscall flags 0x200 */
+#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 to operate on dirfd
+ directly. */
/*
- * The constants AT_REMOVEDIR and AT_EACCESS have the same value. AT_EACCESS is
- * meaningful only to faccessat, while AT_REMOVEDIR is meaningful only to
- * unlinkat. The two functions do completely different things and therefore,
- * the flags can be allowed to overlap. For example, passing AT_REMOVEDIR to
- * faccessat would be undefined behavior and thus treating it equivalent to
- * AT_EACCESS is valid undefined behavior.
+ * These flags are currently statx(2)-specific, but they could be made generic
+ * in the future and so they should not be used for other per-syscall flags.
*/
-#define AT_FDCWD -100 /* Special value used to indicate
- openat should use the current
- working directory. */
-#define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic links. */
+#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_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the server */
+
+#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
+
+/*
+ * Per-syscall flags for the *at(2) family of syscalls.
+ *
+ * These are flags that are so syscall-specific that a user passing these flags
+ * to the wrong syscall is so "clearly wrong" that we can safely call such
+ * usage "undefined behaviour".
+ *
+ * For example, the constants AT_REMOVEDIR and AT_EACCESS have the same value.
+ * AT_EACCESS is meaningful only to faccessat, while AT_REMOVEDIR is meaningful
+ * only to unlinkat. The two functions do completely different things and
+ * therefore, the flags can be allowed to overlap. For example, passing
+ * AT_REMOVEDIR to faccessat would be undefined behavior and thus treating it
+ * equivalent to AT_EACCESS is valid undefined behavior.
+ *
+ * Note for implementers: When picking a new per-syscall AT_* flag, try to
+ * reuse already existing flags first. This leaves us with as many unused bits
+ * as possible, so we can use them for generic bits in the future if necessary.
+ */
+
+/* Flags for renameat2(2) (must match legacy RENAME_* flags). */
+#define AT_RENAME_NOREPLACE 0x0001
+#define AT_RENAME_EXCHANGE 0x0002
+#define AT_RENAME_WHITEOUT 0x0004
+
+/* Flag for faccessat(2). */
#define AT_EACCESS 0x200 /* Test access permitted for
effective IDs, not real IDs. */
+/* Flag for unlinkat(2). */
#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 */
-
-#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_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the server */
-
-#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
+/* 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 with open_by_handle_at(2). */
-/* 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
- compare object identity and may not
- be usable to open_by_handle_at(2) */
#if defined(__KERNEL__)
#define AT_GETATTR_NOSEC 0x80000000
#endif
diff --git a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
index c0bcc185fa48..38a6d66d9e88 100644
--- a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
+++ b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
@@ -16,6 +16,9 @@
#define F_DUPFD_QUERY (F_LINUX_SPECIFIC_BASE + 3)
+/* Was the file just created? */
+#define F_CREATED_QUERY (F_LINUX_SPECIFIC_BASE + 4)
+
/*
* Cancel a blocking posix lock; internal use only until we expose an
* asynchronous lock api to userspace:
@@ -87,37 +90,69 @@
#define DN_ATTRIB 0x00000020 /* File changed attibutes */
#define DN_MULTISHOT 0x80000000 /* Don't remove notifier */
+#define AT_FDCWD -100 /* Special value for dirfd used to
+ indicate openat should use the
+ current working directory. */
+
+
+/* Generic flags for the *at(2) family of syscalls. */
+
+/* Reserved for per-syscall flags 0xff. */
+#define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic
+ links. */
+/* Reserved for per-syscall flags 0x200 */
+#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 to operate on dirfd
+ directly. */
+/*
+ * These flags are currently statx(2)-specific, but they could be made generic
+ * in the future and so they should not be used for other per-syscall flags.
+ */
+#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_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the server */
+
+#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
+
/*
- * The constants AT_REMOVEDIR and AT_EACCESS have the same value. AT_EACCESS is
- * meaningful only to faccessat, while AT_REMOVEDIR is meaningful only to
- * unlinkat. The two functions do completely different things and therefore,
- * the flags can be allowed to overlap. For example, passing AT_REMOVEDIR to
- * faccessat would be undefined behavior and thus treating it equivalent to
- * AT_EACCESS is valid undefined behavior.
+ * Per-syscall flags for the *at(2) family of syscalls.
+ *
+ * These are flags that are so syscall-specific that a user passing these flags
+ * to the wrong syscall is so "clearly wrong" that we can safely call such
+ * usage "undefined behaviour".
+ *
+ * For example, the constants AT_REMOVEDIR and AT_EACCESS have the same value.
+ * AT_EACCESS is meaningful only to faccessat, while AT_REMOVEDIR is meaningful
+ * only to unlinkat. The two functions do completely different things and
+ * therefore, the flags can be allowed to overlap. For example, passing
+ * AT_REMOVEDIR to faccessat would be undefined behavior and thus treating it
+ * equivalent to AT_EACCESS is valid undefined behavior.
+ *
+ * Note for implementers: When picking a new per-syscall AT_* flag, try to
+ * reuse already existing flags first. This leaves us with as many unused bits
+ * as possible, so we can use them for generic bits in the future if necessary.
*/
-#define AT_FDCWD -100 /* Special value used to indicate
- openat should use the current
- working directory. */
-#define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic links. */
+
+/* Flags for renameat2(2) (must match legacy RENAME_* flags). */
+#define AT_RENAME_NOREPLACE 0x0001
+#define AT_RENAME_EXCHANGE 0x0002
+#define AT_RENAME_WHITEOUT 0x0004
+
+/* Flag for faccessat(2). */
#define AT_EACCESS 0x200 /* Test access permitted for
effective IDs, not real IDs. */
+/* Flag for unlinkat(2). */
#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 */
-
-#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_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the server */
-
-#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
+/* 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 with open_by_handle_at(2). */
-/* 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
- compare object identity and may not
- be usable to open_by_handle_at(2) */
#if defined(__KERNEL__)
#define AT_GETATTR_NOSEC 0x80000000
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH RFC v3 0/2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Aleksa Sarai @ 2024-08-01 3:52 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Chuck Lever,
Jeff Layton, Amir Goldstein, Alexander Aring, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter
Cc: linux-fsdevel, linux-nfs, linux-kernel, linux-api,
linux-perf-users, Aleksa Sarai
Now that we provide a unique 64-bit mount ID interface in statx(2), 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 worry about
racing with /proc/mountinfo parsing or having to open a file just to do
statx(2).
While this is not necessary if you are using AT_EMPTY_PATH and don't
care about an extra statx(2) call, users that pass full paths into
name_to_handle_at(2) need to know which mount the file handle comes from
(to make sure they don't try to open_by_handle_at a file handle from a
different filesystem) and switching to AT_EMPTY_PATH would require
allocating a file for every name_to_handle_at(2) call, turning
err = name_to_handle_at(-EBADF, "/foo/bar/baz", &handle, &mntid,
AT_HANDLE_MNT_ID_UNIQUE);
into
int fd = openat(-EBADF, "/foo/bar/baz", O_PATH | O_CLOEXEC);
err1 = name_to_handle_at(fd, "", &handle, &unused_mntid, AT_EMPTY_PATH);
err2 = statx(fd, "", AT_EMPTY_PATH, STATX_MNT_ID_UNIQUE, &statxbuf);
mntid = statxbuf.stx_mnt_id;
close(fd);
Also, this series adds a patch to clarify how AT_* flag allocation
should work going forwards.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Changes in v3:
- Added a patch describing how AT_* flags should be allocated in the
future, based on Amir's suggestions.
- Included AT_* aliases for RENAME_* flags to further indicate that
renameat2(2) is an *at(2) syscall and to indicate that those flags
have been allocated already in the per-syscall range.
- Switched AT_HANDLE_MNT_ID_UNIQUE to use 0x01 (to reuse
(AT_)RENAME_NOREPLACE).
- v2: <https://lore.kernel.org/r/20240523-exportfs-u64-mount-id-v2-1-f9f959f17eb1@cyphar.com>
Changes in v2:
- Fixed a few minor compiler warnings and a buggy copy_to_user() check.
- Rename AT_HANDLE_UNIQUE_MOUNT_ID -> AT_HANDLE_MNT_ID_UNIQUE to match statx.
- Switched to using an AT_* bit from 0xFF and defining that range as
being "per-syscall" for future usage.
- Sync tools/ copy of <linux/fcntl.h> to include changes.
- v1: <https://lore.kernel.org/r/20240520-exportfs-u64-mount-id-v1-1-f55fd9215b8e@cyphar.com>
---
Aleksa Sarai (2):
uapi: explain how per-syscall AT_* flags should be allocated
fhandle: expose u64 mount id to name_to_handle_at(2)
fs/fhandle.c | 29 ++++++--
include/linux/syscalls.h | 2 +-
include/uapi/linux/fcntl.h | 81 ++++++++++++++-------
tools/perf/trace/beauty/include/uapi/linux/fcntl.h | 84 +++++++++++++++-------
4 files changed, 140 insertions(+), 56 deletions(-)
---
base-commit: c7b9563b58a77423d4c6e026ff831a69612b02fc
change-id: 20240515-exportfs-u64-mount-id-9ebb5c58b53c
Best regards,
--
Aleksa Sarai <cyphar@cyphar.com>
^ permalink raw reply
* Re: [PATCH net-next v8 2/3] sock: add MSG_ZEROCOPY notification mechanism based on msg_control
From: Jakub Kicinski @ 2024-08-01 1:29 UTC (permalink / raw)
To: Willem de Bruijn
Cc: zijianzhang, netdev, linux-api, almasrymina, edumazet, davem,
pabeni, dsahern, axboe, shuah, linux-kselftest, cong.wang,
xiaochun.lu
In-Reply-To: <66aab8b37157d_21c08c2941@willemb.c.googlers.com.notmuch>
On Wed, 31 Jul 2024 18:20:35 -0400 Willem de Bruijn wrote:
> Btw patchwork shows red for patch 1/3 due to a new error or warning.
> Not sure if it's a false positive, but take a look.
Patchwork is not for contributors, I keep repeating this :|
Were you not in the room at netdev when I was talking about NIPA
or am I this shit at communicating?
Next person pointing someone to patchwork will get a task to fix
something in NIPA.
^ permalink raw reply
* Re: [PATCH v3] usb: gadget: f_fs: add capability for dfu run-time descriptor
From: Chris Wulff @ 2024-08-01 0:09 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Chris Wulff, linux-usb@vger.kernel.org, Christian Brauner,
Paul Cercueil, Jan Kara, Jeff Layton, Dmitry Antipov, David Sands,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org
In-Reply-To: <2024073122-shakable-photo-67d1@gregkh>
On Wed, Jul 31, 2024 at 4:28 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Apr 24, 2024 at 10:14:58PM +0000, Chris Wulff wrote:
> > From: David Sands <david.sands@biamp.com>
> >
> > Add the ability for FunctionFS driver to be able to create DFU Run-Time
> > descriptors.
>
> Don't you need some userspace documentation for this as well?
Yes, I will add some.
>
> > --- a/include/uapi/linux/usb/ch9.h
> > +++ b/include/uapi/linux/usb/ch9.h
> > @@ -254,6 +254,9 @@ struct usb_ctrlrequest {
> > #define USB_DT_DEVICE_CAPABILITY 0x10
> > #define USB_DT_WIRELESS_ENDPOINT_COMP 0x11
> > #define USB_DT_WIRE_ADAPTER 0x21
> > +/* From USB Device Firmware Upgrade Specification, Revision 1.1 */
> > +#define USB_DT_DFU_FUNCTIONAL 0x21
>
> So USB_DT_WIRE_ADAPTER and USB_DT_DFU_FUNCTIONAL are the same? That
> seems wrong.
>
> > +/* these are from the Wireless USB spec */
>
> What spec? What "these"?
I inserted the DFU constant in numerical order, splitting the original
section, so I
duplicated the comment. (Partly to make it obvious that there were two with the
same number.) It looks like possibly wireless usb is a dead spec.
You can find wireless usb v1.0 on usb.org in the wayback machine, but it looks
like most of the references have been purged from the current site, and have
been gone for a few years. There was apparently a v1.1 too, but I never found
a copy of that anywhere.
https://web.archive.org/web/20090325042850/http://www.usb.org/developers/wusb/docs/
A few references remain but say the specification is no longer available. Eg.
https://www.usb.org/bos-descriptor-types
The original section of code that I inserted the new constant into was:
/* these are from the Wireless USB spec */
#define USB_DT_SECURITY 0x0c
#define USB_DT_KEY 0x0d
#define USB_DT_ENCRYPTION_TYPE 0x0e
#define USB_DT_BOS 0x0f
#define USB_DT_DEVICE_CAPABILITY 0x10
#define USB_DT_WIRELESS_ENDPOINT_COMP 0x11
#define USB_DT_WIRE_ADAPTER 0x21
#define USB_DT_RPIPE 0x22
#define USB_DT_CS_RADIO_CONTROL 0x23
/* From the T10 UAS specification */
>
> > #define USB_DT_RPIPE 0x22
> > #define USB_DT_CS_RADIO_CONTROL 0x23
> > /* From the T10 UAS specification */
> > @@ -263,6 +266,7 @@ struct usb_ctrlrequest {
> > /* From the USB 3.1 spec */
> > #define USB_DT_SSP_ISOC_ENDPOINT_COMP 0x31
> >
> > +
> > /* Conventional codes for class-specific descriptors. The convention is
> > * defined in the USB "Common Class" Spec (3.11). Individual class specs
> > * are authoritative for their usage, not the "common class" writeup.
>
> Unneeded change?
Yeah, I'll clean that up.
>
> > @@ -329,9 +333,10 @@ struct usb_device_descriptor {
> > #define USB_CLASS_USB_TYPE_C_BRIDGE 0x12
> > #define USB_CLASS_MISC 0xef
> > #define USB_CLASS_APP_SPEC 0xfe
> > -#define USB_CLASS_VENDOR_SPEC 0xff
> > +#define USB_SUBCLASS_DFU 0x01
> >
> > -#define USB_SUBCLASS_VENDOR_SPEC 0xff
> > +#define USB_CLASS_VENDOR_SPEC 0xff
> > +#define USB_SUBCLASS_VENDOR_SPEC 0xff
>
> Why reorder these?
The subclasses are specific to the class they are under.
USB_SUBCLASS_DFU is part of USB_CLASS_APP_SPEC
USB_SUBCLASS_VENDOR_SPEC is part of USB_CLASS_VENDOR_SPEC
>
> >
> > /*-------------------------------------------------------------------------*/
> >
> > diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
> > index 9f88de9c3d66..6d2061500184 100644
> > --- a/include/uapi/linux/usb/functionfs.h
> > +++ b/include/uapi/linux/usb/functionfs.h
> > @@ -37,6 +37,31 @@ struct usb_endpoint_descriptor_no_audio {
> > __u8 bInterval;
> > } __attribute__((packed));
> >
> > +/**
> > + * struct usb_dfu_functional_descriptor - DFU Functional descriptor
> > + * @bLength: Size of the descriptor (bytes)
> > + * @bDescriptorType: USB_DT_DFU_FUNCTIONAL
> > + * @bmAttributes: DFU attributes
> > + * @wDetachTimeOut: Maximum time to wait after DFU_DETACH (ms, le16)
> > + * @wTransferSize: Maximum number of bytes per control-write (le16)
> > + * @bcdDFUVersion: DFU Spec version (BCD, le16)
> > + */
> > +struct usb_dfu_functional_descriptor {
> > + __u8 bLength;
> > + __u8 bDescriptorType;
> > + __u8 bmAttributes;
> > + __le16 wDetachTimeOut;
> > + __le16 wTransferSize;
> > + __le16 bcdDFUVersion;
> > +} __attribute__ ((packed));
> > +
> > +/* from DFU functional descriptor bmAttributes */
> > +#define DFU_FUNC_ATT_WILL_DETACH (1 << 3)
> > +#define DFU_FUNC_ATT_MANIFEST_TOLERANT (1 << 2)
> > +#define DFU_FUNC_ATT_CAN_UPLOAD (1 << 1)
> > +#define DFU_FUNC_ATT_CAN_DOWNLOAD (1 << 0)
>
> Please use proper BIT macros here to make this more obvious. And in
> sorted order?
Ok. I'll fix this up
>
> thanks,
>
> greg k-h
>
^ permalink raw reply
* Re: [PATCH net-next v8 3/3] selftests: add MSG_ZEROCOPY msg_control notification test
From: Willem de Bruijn @ 2024-07-31 22:32 UTC (permalink / raw)
To: zijianzhang, netdev
Cc: linux-api, willemdebruijn.kernel, almasrymina, edumazet, davem,
kuba, pabeni, dsahern, axboe, shuah, linux-kselftest, cong.wang,
xiaochun.lu, Zijian Zhang
In-Reply-To: <20240730184120.4089835-4-zijianzhang@bytedance.com>
zijianzhang@ wrote:
> From: Zijian Zhang <zijianzhang@bytedance.com>
>
> We update selftests/net/msg_zerocopy.c to accommodate the new mechanism,
Please make commit messages stand on their own. If someone does a git
blame, make the message self explanatory. Replace "the new mechanism"
with sendmsg SCM_ZC_NOTIFICATION.
In patch 2 or as a separate patch 4, also add a new short section on
this API in Documentation/networking/msg_zerocopy.rst. Probably with
the same contents as a good explanation of the feature in the commit
message of patch 2.
> cfg_notification_limit has the same semantics for both methods. Test
> results are as follows, we update skb_orphan_frags_rx to the same as
> skb_orphan_frags to support zerocopy in the localhost test.
>
> cfg_notification_limit = 1, both method get notifications after 1 calling
> of sendmsg. In this case, the new method has around 17% cpu savings in TCP
> and 23% cpu savings in UDP.
> +---------------------+---------+---------+---------+---------+
> | Test Type / Protocol| TCP v4 | TCP v6 | UDP v4 | UDP v6 |
> +---------------------+---------+---------+---------+---------+
> | ZCopy (MB) | 7523 | 7706 | 7489 | 7304 |
> +---------------------+---------+---------+---------+---------+
> | New ZCopy (MB) | 8834 | 8993 | 9053 | 9228 |
> +---------------------+---------+---------+---------+---------+
> | New ZCopy / ZCopy | 117.42% | 116.70% | 120.88% | 126.34% |
> +---------------------+---------+---------+---------+---------+
>
> cfg_notification_limit = 32, both get notifications after 32 calling of
> sendmsg, which means more chances to coalesce notifications, and less
> overhead of poll + recvmsg for the original method. In this case, the new
> method has around 7% cpu savings in TCP and slightly better cpu usage in
> UDP. In the env of selftest, notifications of TCP are more likely to be
> out of order than UDP, it's easier to coalesce more notifications in UDP.
> The original method can get one notification with range of 32 in a recvmsg
> most of the time. In TCP, most notifications' range is around 2, so the
> original method needs around 16 recvmsgs to get notified in one round.
> That's the reason for the "New ZCopy / ZCopy" diff in TCP and UDP here.
> +---------------------+---------+---------+---------+---------+
> | Test Type / Protocol| TCP v4 | TCP v6 | UDP v4 | UDP v6 |
> +---------------------+---------+---------+---------+---------+
> | ZCopy (MB) | 8842 | 8735 | 10072 | 9380 |
> +---------------------+---------+---------+---------+---------+
> | New ZCopy (MB) | 9366 | 9477 | 10108 | 9385 |
> +---------------------+---------+---------+---------+---------+
> | New ZCopy / ZCopy | 106.00% | 108.28% | 100.31% | 100.01% |
> +---------------------+---------+---------+---------+---------+
>
> In conclusion, when notification interval is small or notifications are
> hard to be coalesced, the new mechanism is highly recommended. Otherwise,
> the performance gain from the new mechanism is very limited.
>
> Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
> Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>
> -static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
> +static void add_zcopy_info(struct msghdr *msg)
> +{
> + struct zc_info *zc_info;
> + struct cmsghdr *cm;
> +
> + if (!msg->msg_control)
> + error(1, errno, "NULL user arg");
Don't add precondition checks for code entirely under your control.
This is not a user API.
> + cm = (struct cmsghdr *)msg->msg_control;
> + cm->cmsg_len = CMSG_LEN(ZC_INFO_SIZE);
> + cm->cmsg_level = SOL_SOCKET;
> + cm->cmsg_type = SCM_ZC_NOTIFICATION;
> +
> + zc_info = (struct zc_info *)CMSG_DATA(cm);
> + zc_info->size = ZC_NOTIFICATION_MAX;
> +
> + added_zcopy_info = true;
Just initialize every time? Is this here to reuse the same msg_control
as long as metadata is returned?
> +}
> +
> +static bool do_sendmsg(int fd, struct msghdr *msg,
> + enum notification_type do_zerocopy, int domain)
> {
> int ret, len, i, flags;
> static uint32_t cookie;
> @@ -200,6 +233,12 @@ static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
> msg->msg_controllen = CMSG_SPACE(sizeof(cookie));
> msg->msg_control = (struct cmsghdr *)ckbuf;
> add_zcopy_cookie(msg, ++cookie);
> + } else if (do_zerocopy == MSG_ZEROCOPY_NOTIFY_SENDMSG &&
> + sends_since_notify + 1 >= cfg_notification_limit) {
> + memset(&msg->msg_control, 0, sizeof(msg->msg_control));
> + msg->msg_controllen = CMSG_SPACE(ZC_INFO_SIZE);
> + msg->msg_control = (struct cmsghdr *)zc_ckbuf;
> + add_zcopy_info(msg);
> }
> }
>
> @@ -218,7 +257,7 @@ static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
> if (do_zerocopy && ret)
> expected_completions++;
> }
> - if (do_zerocopy && domain == PF_RDS) {
> + if (msg->msg_control) {
> msg->msg_control = NULL;
> msg->msg_controllen = 0;
> }
> @@ -466,6 +505,44 @@ static void do_recv_completions(int fd, int domain)
> sends_since_notify = 0;
> }
>
> +static void do_recv_completions2(void)
functionname2 is very uninformative.
do_recv_completions_sendmsg or so.
> +{
> + struct cmsghdr *cm = (struct cmsghdr *)zc_ckbuf;
> + struct zc_info *zc_info;
> + __u32 hi, lo, range;
> + __u8 zerocopy;
> + int i;
> +
> + zc_info = (struct zc_info *)CMSG_DATA(cm);
> + for (i = 0; i < zc_info->size; i++) {
> + hi = zc_info->arr[i].hi;
> + lo = zc_info->arr[i].lo;
> + zerocopy = zc_info->arr[i].zerocopy;
> + range = hi - lo + 1;
> +
> + if (cfg_verbose && lo != next_completion)
> + fprintf(stderr, "gap: %u..%u does not append to %u\n",
> + lo, hi, next_completion);
> + next_completion = hi + 1;
> +
> + if (zerocopied == -1) {
> + zerocopied = zerocopy;
> + } else if (zerocopied != zerocopy) {
> + fprintf(stderr, "serr: inconsistent\n");
> + zerocopied = zerocopy;
> + }
> +
> + completions += range;
> + sends_since_notify -= range;
> +
> + if (cfg_verbose >= 2)
> + fprintf(stderr, "completed: %u (h=%u l=%u)\n",
> + range, hi, lo);
> + }
> +
> + added_zcopy_info = false;
> +}
> +
> /* Wait for all remaining completions on the errqueue */
> static void do_recv_remaining_completions(int fd, int domain)
> {
> @@ -553,11 +630,16 @@ static void do_tx(int domain, int type, int protocol)
> else
> do_sendmsg(fd, &msg, cfg_zerocopy, domain);
>
> - if (cfg_zerocopy && sends_since_notify >= cfg_notification_limit)
> + if (cfg_zerocopy == MSG_ZEROCOPY_NOTIFY_ERRQUEUE &&
> + sends_since_notify >= cfg_notification_limit)
> do_recv_completions(fd, domain);
>
> + if (cfg_zerocopy == MSG_ZEROCOPY_NOTIFY_SENDMSG &&
> + added_zcopy_info)
> + do_recv_completions2();
> +
> while (!do_poll(fd, POLLOUT)) {
> - if (cfg_zerocopy)
> + if (cfg_zerocopy == MSG_ZEROCOPY_NOTIFY_ERRQUEUE)
> do_recv_completions(fd, domain);
> }
>
> @@ -715,7 +797,7 @@ static void parse_opts(int argc, char **argv)
>
> cfg_payload_len = max_payload_len;
>
> - while ((c = getopt(argc, argv, "46c:C:D:i:l:mp:rs:S:t:vz")) != -1) {
> + while ((c = getopt(argc, argv, "46c:C:D:i:l:mnp:rs:S:t:vz")) != -1) {
> switch (c) {
> case '4':
> if (cfg_family != PF_UNSPEC)
> @@ -749,6 +831,9 @@ static void parse_opts(int argc, char **argv)
> case 'm':
> cfg_cork_mixed = true;
> break;
> + case 'n':
> + cfg_zerocopy = MSG_ZEROCOPY_NOTIFY_SENDMSG;
> + break;
How about -Z to make clear that this is still MSG_ZEROCOPY, just with
a different notification mechanism.
And perhaps add a testcase that exercises both this mechanism and
existing recvmsg MSG_ERRQUEUE. As they should work in parallel and
concurrently in a multithreaded environment.
^ permalink raw reply
* Re: [PATCH net-next v8 2/3] sock: add MSG_ZEROCOPY notification mechanism based on msg_control
From: Willem de Bruijn @ 2024-07-31 22:20 UTC (permalink / raw)
To: zijianzhang, netdev
Cc: linux-api, willemdebruijn.kernel, almasrymina, edumazet, davem,
kuba, pabeni, dsahern, axboe, shuah, linux-kselftest, cong.wang,
xiaochun.lu, Zijian Zhang
In-Reply-To: <20240730184120.4089835-3-zijianzhang@bytedance.com>
zijianzhang@ wrote:
> From: Zijian Zhang <zijianzhang@bytedance.com>
>
> The MSG_ZEROCOPY flag enables copy avoidance for socket send calls.
> However, zerocopy is not a free lunch. Apart from the management of user
> pages, the combination of poll + recvmsg to receive notifications incurs
> unignorable overhead in the applications. We try to mitigate this overhead
> with a new notification mechanism based on msg_control. Leveraging the
> general framework to copy cmsgs to the user space, we copy zerocopy
> notifications to the user upon returning of sendmsgs.
May want to
- Explicitly state that receiving notifications on sendmsg is
optional and existing recvmsg MSG_ERRQUEUE continues to work
- Include a very brief example of how this interface is used.
Probably pseudo-code, as msghdr setup and CMSG processing are
verbose operations
Btw patchwork shows red for patch 1/3 due to a new error or warning.
Not sure if it's a false positive, but take a look.
> Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
> Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>
> +/*
> + * zc_info is the struct used for the SCM_ZC_NOTIFICATION control message.
> + */
> +struct zc_info {
> + __u32 size; /* size of the zc_info_elem arr */
Size is ambiguous, could mean byte size. Perhaps length, or number of
elements in arr[].
> + struct zc_info_elem arr[];
> +};
^ permalink raw reply
* Re: Testing if two open descriptors refer to the same inode
From: David Sterba @ 2024-07-31 18:07 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Mateusz Guzik, Florian Weimer, linux-fsdevel, linux-kernel,
linux-api, Dave Chinner, Christian Brauner
In-Reply-To: <20240729.113049-lax.waffle.foxy.nit-U1v9CY38xge@cyphar.com>
On Mon, Jul 29, 2024 at 09:40:57PM +1000, Aleksa Sarai wrote:
> On 2024-07-29, Mateusz Guzik <mjguzik@gmail.com> wrote:
> > On Mon, Jul 29, 2024 at 12:57 PM Florian Weimer <fweimer@redhat.com> wrote:
> > > > On Mon, Jul 29, 2024 at 12:40:35PM +0200, Florian Weimer wrote:
> > > >> > On Mon, Jul 29, 2024 at 08:55:46AM +0200, Florian Weimer wrote:
> > > >> >> It was pointed out to me that inode numbers on Linux are no longer
> > > >> >> expected to be unique per file system, even for local file systems.
> > > >> >
> > > >> > I don't know if I'm parsing this correctly.
> > > >> >
> > > >> > Are you claiming on-disk inode numbers are not guaranteed unique per
> > > >> > filesystem? It sounds like utter breakage, with capital 'f'.
> > > >>
> > > >> Yes, POSIX semantics and traditional Linux semantics for POSIX-like
> > > >> local file systems are different.
> > > >
> > > > Can you link me some threads about this?
> > >
> > > Sorry, it was an internal thread. It's supposed to be common knowledge
> > > among Linux file system developers. Aleksa referenced LSF/MM
> > > discussions.
> >
> > So much for open development :-P
>
> To be clear, this wasn't _decided_ at LSF/MM, it was brought up as a
> topic. There is an LWN article about the session that mentions the
> issue[1].
A discussion about inode numbers or subvolumes comes up every year with
better of worse suggestions what to do about it.
> My understanding is that the btrfs and bcachefs folks independently
> determined they cannot provide this guarantee. As far as I understand,
> the reason why is that inode number allocation on btree filesystems
> stores information about location and some other bits (maybe subvolumes)
> in the bits, making it harder to guarantee there will be no collisions.
No, on btrfs the inode numbers don't encode anything about location,
it's a simple number. The inode numbers remain the same when a snapshot
is taken as it's a 1:1 clone of the file hierarchy, the directory
representing a subvolume/snapshot has fixed inode number 256. The only
difference is the internal subvolume id.
^ permalink raw reply
* Re: [PATCH v3] epoll.7: clarify the event distribution under edge-triggered mode
From: 潘少 @ 2024-07-31 15:22 UTC (permalink / raw)
To: Alejandro Colomar; +Cc: Michael Kerrisk, linux-man, linux-api
In-Reply-To: <l5ezb77kqvbmypsvv4nek7ce46sghrm6ox6zbeq7hzd6j2ouv2@mhaihw3rkwjl>
Hi Alejandro,
I believe I've found a more suitable place to put this added clarification sentence. Please check out the [PATCH v4], thanks!
-----------
Best regards,
Andy Pan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox