* Re: [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Amir Goldstein @ 2025-03-23 8:56 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: Richard Henderson, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Günther Noack, Arnd Bergmann, Pali Rohár, Paul Moore,
James Morris, Serge E. Hallyn, linux-alpha, linux-kernel,
linux-arm-kernel, linux-m68k, linux-mips, linux-parisc,
linuxppc-dev, linux-s390, linux-sh, sparclinux, linux-fsdevel,
linux-security-module, linux-api, linux-arch, linux-xfs
In-Reply-To: <20250321-xattrat-syscall-v4-3-3e82e6fb3264@kernel.org>
On Fri, Mar 21, 2025 at 8:49 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
>
> From: Andrey Albershteyn <aalbersh@redhat.com>
>
> Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
> extended attributes/flags. The syscalls take parent directory fd and
> path to the child together with struct fsxattr.
>
> This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
> that file don't need to be open as we can reference it with a path
> instead of fd. By having this we can manipulated inode extended
> attributes not only on regular files but also on special ones. This
> is not possible with FS_IOC_FSSETXATTR ioctl as with special files
> we can not call ioctl() directly on the filesystem inode using fd.
>
> This patch adds two new syscalls which allows userspace to get/set
> extended inode attributes on special files by using parent directory
> and a path - *at() like syscall.
>
> CC: linux-api@vger.kernel.org
> CC: linux-fsdevel@vger.kernel.org
> CC: linux-xfs@vger.kernel.org
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
...
> +SYSCALL_DEFINE5(setfsxattrat, int, dfd, const char __user *, filename,
> + struct fsxattr __user *, ufsx, size_t, usize,
> + unsigned int, at_flags)
> +{
> + struct fileattr fa;
> + struct path filepath;
> + int error;
> + unsigned int lookup_flags = 0;
> + struct filename *name;
> + struct mnt_idmap *idmap;.
> + struct dentry *dentry;
> + struct vfsmount *mnt;
> + struct fsxattr fsx = {};
> +
> + BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> + BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> +
> + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> + return -EINVAL;
> +
> + if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> + lookup_flags |= LOOKUP_FOLLOW;
> +
> + if (at_flags & AT_EMPTY_PATH)
> + lookup_flags |= LOOKUP_EMPTY;
> +
> + if (usize > PAGE_SIZE)
> + return -E2BIG;
> +
> + if (usize < FSXATTR_SIZE_VER0)
> + return -EINVAL;
> +
> + error = copy_struct_from_user(&fsx, sizeof(struct fsxattr), ufsx, usize);
> + if (error)
> + return error;
> +
> + fsxattr_to_fileattr(&fsx, &fa);
> +
> + name = getname_maybe_null(filename, at_flags);
> + if (!name) {
> + CLASS(fd, f)(dfd);
> +
> + if (fd_empty(f))
> + return -EBADF;
> +
> + idmap = file_mnt_idmap(fd_file(f));
> + dentry = file_dentry(fd_file(f));
> + mnt = fd_file(f)->f_path.mnt;
> + } else {
> + error = filename_lookup(dfd, name, lookup_flags, &filepath,
> + NULL);
> + if (error)
> + return error;
> +
> + idmap = mnt_idmap(filepath.mnt);
> + dentry = filepath.dentry;
> + mnt = filepath.mnt;
> + }
> +
> + error = mnt_want_write(mnt);
> + if (!error) {
> + error = vfs_fileattr_set(idmap, dentry, &fa);
> + if (error == -ENOIOCTLCMD)
> + error = -EOPNOTSUPP;
This is awkward.
vfs_fileattr_set() should return -EOPNOTSUPP.
ioctl_setflags() could maybe convert it to -ENOIOCTLCMD,
but looking at similar cases ioctl_fiemap(), ioctl_fsfreeze() the
ioctl returns -EOPNOTSUPP.
I don't think it is necessarily a bad idea to start returning
-EOPNOTSUPP instead of -ENOIOCTLCMD for the ioctl
because that really reflects the fact that the ioctl is now implemented
in vfs and not in the specific fs.
and I think it would not be a bad idea at all to make that change
together with the merge of the syscalls as a sort of hint to userspace
that uses the ioctl, that the sycalls API exists.
Thanks,
Amir.
^ permalink raw reply
* Re: [PATCH v4 0/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Amir Goldstein @ 2025-03-23 8:45 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: Richard Henderson, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Günther Noack, Arnd Bergmann, Pali Rohár, Paul Moore,
James Morris, Serge E. Hallyn, linux-alpha, linux-kernel,
linux-arm-kernel, linux-m68k, linux-mips, linux-parisc,
linuxppc-dev, linux-s390, linux-sh, sparclinux, linux-fsdevel,
linux-security-module, linux-api, linux-arch, selinux,
Andrey Albershteyn, linux-xfs
In-Reply-To: <20250321-xattrat-syscall-v4-0-3e82e6fb3264@kernel.org>
On Fri, Mar 21, 2025 at 8:50 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
>
> This patchset introduced two new syscalls getfsxattrat() and
> setfsxattrat(). These syscalls are similar to FS_IOC_FSSETXATTR ioctl()
> except they use *at() semantics. Therefore, there's no need to open the
> file to get an fd.
>
> These syscalls allow userspace to set filesystem inode attributes on
> special files. One of the usage examples is XFS quota projects.
>
> XFS has project quotas which could be attached to a directory. All
> new inodes in these directories inherit project ID set on parent
> directory.
>
> The project is created from userspace by opening and calling
> FS_IOC_FSSETXATTR on each inode. This is not possible for special
> files such as FIFO, SOCK, BLK etc. Therefore, some inodes are left
> with empty project ID. Those inodes then are not shown in the quota
> accounting but still exist in the directory. This is not critical but in
> the case when special files are created in the directory with already
> existing project quota, these new inodes inherit extended attributes.
> This creates a mix of special files with and without attributes.
> Moreover, special files with attributes don't have a possibility to
> become clear or change the attributes. This, in turn, prevents userspace
> from re-creating quota project on these existing files.
>
> Christian, if this get in some mergeable state, please don't merge it
> yet. Amir suggested these syscalls better to use updated struct fsxattr
> with masking from Pali Rohár patchset, so, let's see how it goes.
Andrey,
To be honest I don't think it would be fair to delay your syscalls more
than needed.
If Pali can follow through and post patches on top of your syscalls for
next merge window that would be great, but otherwise, I think the
minimum requirement is that the syscalls return EINVAL if fsx_pad
is not zero. we can take it from there later.
We can always also increase the size of struct fsxattr, but let's first
use the padding space already available.
Thanks,
Amir.
>
> NAME
>
> getfsxattrat/setfsxattrat - get/set filesystem inode attributes
>
> SYNOPSIS
>
> #include <sys/syscall.h> /* Definition of SYS_* constants */
> #include <unistd.h>
>
> long syscall(SYS_getfsxattrat, int dirfd, const char *pathname,
> struct fsxattr *fsx, size_t size,
> unsigned int at_flags);
> long syscall(SYS_setfsxattrat, int dirfd, const char *pathname,
> struct fsxattr *fsx, size_t size,
> unsigned int at_flags);
>
> Note: glibc doesn't provide for getfsxattrat()/setfsxattrat(),
> use syscall(2) instead.
>
> DESCRIPTION
>
> The syscalls take fd and path to the child together with struct
> fsxattr. If path is absolute, fd is not used. If path is empty,
> inode under fd is used to get/set attributes on.
>
> This is an alternative to FS_IOC_FSGETXATTR/FS_IOC_FSSETXATTR
> ioctl with a difference that file don't need to be open as we
> can reference it with a path instead of fd. By having this we
> can manipulated filesystem inode attributes not only on regular
> files but also on special ones. This is not possible with
> FS_IOC_FSSETXATTR ioctl as with special files we can not call
> ioctl() directly on the filesystem inode using file descriptor.
>
> RETURN VALUE
>
> On success, 0 is returned. On error, -1 is returned, and errno
> is set to indicate the error.
>
> ERRORS
>
> EINVAL Invalid at_flag specified (only
> AT_SYMLINK_NOFOLLOW and AT_EMPTY_PATH is
> supported).
>
> EINVAL Size was smaller than any known version of
> struct fsxattr.
>
> EINVAL Invalid combination of parameters provided in
> fsxattr for this type of file.
>
> E2BIG Size of input argument **struct fsxattr** is too
> big.
>
> EBADF Invalid file descriptor was provided.
>
> EPERM No permission to change this file.
>
> EOPNOTSUPP Filesystem does not support setting attributes
> on this type of inode
>
> HISTORY
>
> Added in Linux 6.14.
>
> EXAMPLE
>
> Create directory and file "mkdir ./dir && touch ./dir/foo" and then
> execute the following program:
>
> #include <fcntl.h>
> #include <errno.h>
> #include <string.h>
> #include <linux/fs.h>
> #include <stdio.h>
> #include <sys/syscall.h>
> #include <unistd.h>
>
> int
> main(int argc, char **argv) {
> int dfd;
> int error;
> struct fsxattr fsx;
>
> dfd = open("./dir", O_RDONLY);
> if (dfd == -1) {
> printf("can not open ./dir");
> return dfd;
> }
>
> error = syscall(467, dfd, "./foo", &fsx, 0);
> if (error) {
> printf("can not call 467: %s", strerror(errno));
> return error;
> }
>
> printf("dir/foo flags: %d\n", fsx.fsx_xflags);
>
> fsx.fsx_xflags |= FS_XFLAG_NODUMP;
> error = syscall(468, dfd, "./foo", &fsx, 0);
> if (error) {
> printf("can not call 468: %s", strerror(errno));
> return error;
> }
>
> printf("dir/foo flags: %d\n", fsx.fsx_xflags);
>
> return error;
> }
>
> SEE ALSO
>
> ioctl(2), ioctl_iflags(2), ioctl_xfs_fsgetxattr(2)
>
> ---
> Changes in v4:
> - Use getname_maybe_null() for correct handling of dfd + path semantic
> - Remove restriction for special files on which flags are allowed
> - Utilize copy_struct_from_user() for better future compatibility
> - Add draft man page to cover letter
> - Convert -ENOIOCTLCMD to -EOPNOSUPP as more appropriate for syscall
> - Add missing __user to header declaration of syscalls
> - Link to v3: https://lore.kernel.org/r/20250211-xattrat-syscall-v3-1-a07d15f898b2@kernel.org
>
> Changes in v3:
> - Remove unnecessary "dfd is dir" check as it checked in user_path_at()
> - Remove unnecessary "same filesystem" check
> - Use CLASS() instead of directly calling fdget/fdput
> - Link to v2: https://lore.kernel.org/r/20250122-xattrat-syscall-v2-1-5b360d4fbcb2@kernel.org
>
> v1:
> https://lore.kernel.org/linuxppc-dev/20250109174540.893098-1-aalbersh@kernel.org/
>
> Previous discussion:
> https://lore.kernel.org/linux-xfs/20240520164624.665269-2-aalbersh@redhat.com/
>
> ---
> Andrey Albershteyn (3):
> lsm: introduce new hooks for setting/getting inode fsxattr
> fs: split fileattr/fsxattr converters into helpers
> fs: introduce getfsxattrat and setfsxattrat syscalls
>
> arch/alpha/kernel/syscalls/syscall.tbl | 2 +
> arch/arm/tools/syscall.tbl | 2 +
> arch/arm64/tools/syscall_32.tbl | 2 +
> arch/m68k/kernel/syscalls/syscall.tbl | 2 +
> arch/microblaze/kernel/syscalls/syscall.tbl | 2 +
> arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +
> arch/mips/kernel/syscalls/syscall_n64.tbl | 2 +
> arch/mips/kernel/syscalls/syscall_o32.tbl | 2 +
> arch/parisc/kernel/syscalls/syscall.tbl | 2 +
> arch/powerpc/kernel/syscalls/syscall.tbl | 2 +
> arch/s390/kernel/syscalls/syscall.tbl | 2 +
> arch/sh/kernel/syscalls/syscall.tbl | 2 +
> arch/sparc/kernel/syscalls/syscall.tbl | 2 +
> arch/x86/entry/syscalls/syscall_32.tbl | 2 +
> arch/x86/entry/syscalls/syscall_64.tbl | 2 +
> arch/xtensa/kernel/syscalls/syscall.tbl | 2 +
> fs/inode.c | 130 ++++++++++++++++++++++++++++
> fs/ioctl.c | 39 ++++++---
> include/linux/fileattr.h | 2 +
> include/linux/lsm_hook_defs.h | 4 +
> include/linux/security.h | 16 ++++
> include/linux/syscalls.h | 6 ++
> include/uapi/asm-generic/unistd.h | 8 +-
> include/uapi/linux/fs.h | 3 +
> security/security.c | 32 +++++++
> 25 files changed, 259 insertions(+), 13 deletions(-)
> ---
> base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
> change-id: 20250114-xattrat-syscall-6a1136d2db59
>
> Best regards,
> --
> Andrey Albershteyn <aalbersh@kernel.org>
>
>
^ permalink raw reply
* Re: [PATCH RFC] statx.2: Add stx_atomic_write_unit_max_opt
From: Christoph Hellwig @ 2025-03-23 6:40 UTC (permalink / raw)
To: John Garry
Cc: Christoph Hellwig, alx, brauner, djwong, dchinner, linux-man,
linux-fsdevel, linux-xfs, linux-kernel, ojaswin, ritesh.list,
martin.petersen, linux-api
In-Reply-To: <7311545c-e169-4875-bc6c-97446eea2c45@oracle.com>
On Fri, Mar 21, 2025 at 10:20:21AM +0000, John Garry wrote:
> Coming back to what was discussed about not adding a new flag to fetch this
> limit:
>
> > Does that actually work? Can userspace assume all unknown statx
> > fields are padded to zero?
>
> In cp_statx, we do pre-zero the statx structure. As such, the rule "if
> zero, just use hard limit unit max" seems to hold.
Ok, canwe document this somewhere?
^ permalink raw reply
* Re: [PATCH v4 1/3] lsm: introduce new hooks for setting/getting inode fsxattr
From: Paul Moore @ 2025-03-21 21:32 UTC (permalink / raw)
To: Andrey Albershteyn, Richard Henderson, Matt Turner, Russell King,
Catalin Marinas, Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Günther Noack, Arnd Bergmann, Pali Rohár, James Morris,
Serge E. Hallyn
Cc: linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, linux-fsdevel, linux-security-module, linux-api,
linux-arch, selinux, Andrey Albershteyn
In-Reply-To: <20250321-xattrat-syscall-v4-1-3e82e6fb3264@kernel.org>
On Mar 21, 2025 Andrey Albershteyn <aalbersh@redhat.com> wrote:
>
> Introduce new hooks for setting and getting filesystem extended
> attributes on inode (FS_IOC_FSGETXATTR).
>
> Cc: selinux@vger.kernel.org
> Cc: Paul Moore <paul@paul-moore.com>
>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
> fs/ioctl.c | 7 ++++++-
> include/linux/lsm_hook_defs.h | 4 ++++
> include/linux/security.h | 16 ++++++++++++++++
> security/security.c | 32 ++++++++++++++++++++++++++++++++
> 4 files changed, 58 insertions(+), 1 deletion(-)
Thanks Andrey, one small change below, but otherwise this looks pretty
good. If you feel like trying to work up the SELinux implementation but
need some assitance please let me know, I'll be happy to help :)
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 638a36be31c14afc66a7fd6eb237d9545e8ad997..4434c97bc5dff5a3e8635e28745cd99404ff353e 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -525,10 +525,15 @@ EXPORT_SYMBOL(fileattr_fill_flags);
> int vfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
> {
> struct inode *inode = d_inode(dentry);
> + int error;
>
> if (!inode->i_op->fileattr_get)
> return -ENOIOCTLCMD;
>
> + error = security_inode_getfsxattr(inode, fa);
> + if (error)
> + return error;
> +
> return inode->i_op->fileattr_get(dentry, fa);
> }
> EXPORT_SYMBOL(vfs_fileattr_get);
> @@ -692,7 +697,7 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
> fa->flags |= old_ma.flags & ~FS_COMMON_FL;
> }
> err = fileattr_set_prepare(inode, &old_ma, fa);
> - if (!err)
> + if (!err && !security_inode_setfsxattr(inode, fa))
> err = inode->i_op->fileattr_set(idmap, dentry, fa);
> }
> inode_unlock(inode);
I don't believe we want to hide or otherwise drop the LSM return code as
that could lead to odd behavior, e.g. returning 0/success despite not
having executed the fileattr_set operation.
--
paul-moore.com
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-21 21:28 UTC (permalink / raw)
To: Jens Axboe
Cc: Christoph Hellwig, netdev, linux-kernel, asml.silence,
linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
jolsa, linux-kselftest
In-Reply-To: <67a82595-0e2a-4218-92d4-a704ccb57125@kernel.dk>
On Fri, Mar 21, 2025 at 02:33:16PM -0600, Jens Axboe wrote:
> On 3/21/25 2:30 PM, Joe Damato wrote:
> > On Fri, Mar 21, 2025 at 09:36:34AM -0700, Joe Damato wrote:
> >> On Fri, Mar 21, 2025 at 05:14:59AM -0600, Jens Axboe wrote:
> >>> On 3/20/25 11:56 PM, Christoph Hellwig wrote:
> >>>>> I don't know the entire historical context, but I presume sendmsg
> >>>>> did that because there was no other mechanism at the time.
> >>>>
> >>>> At least aio had been around for about 15 years at the point, but
> >>>> networking folks tend to be pretty insular and reinvent things.
> >>>
> >>> Yep...
> >>>
> >>>>> It seems like Jens suggested that plumbing this through for splice
> >>>>> was a possibility, but sounds like you disagree.
> >>>>
> >>>> Yes, very strongly.
> >>>
> >>> And that is very much not what I suggested, fwiw.
> >>
> >> Your earlier message said:
> >>
> >> If the answer is "because splice", then it would seem saner to
> >> plumb up those bits only. Would be much simpler too...
> >>
> >> wherein I interpreted "plumb those bits" to mean plumbing the error
> >> queue notifications on TX completions.
> >>
> >> My sincere apologies that I misunderstood your prior message and/or
> >> misconstrued what you said -- it was not clear to me what you meant.
> >
> > I think what added to my confusion here was this bit, Jens:
> >
> > > > As far as the bit about plumbing only the splice bits, sorry if I'm
> > > > being dense here, do you mean plumbing the error queue through to
> > > > splice only and dropping sendfile2?
> > > >
> > > > That is an option. Then the apps currently using sendfile could use
> > > > splice instead and get completion notifications on the error queue.
> > > > That would probably work and be less work than rewriting to use
> > > > iouring, but probably a bit more work than using a new syscall.
> > >
> > > Yep
> >
> > I thought I was explicitly asking if adding SPLICE_F_ZC and plumbing
> > through the error queue notifications was OK and your response here
> > ("Yep") suggested to me that it would be a suitable path to
> > consider.
> >
> > I take it from your other responses, though, that I was mistaken.
>
> I guess I missed your error queue thing here, I was definitely pretty
> clear in other ones that I consider that part a hack and something that
> only exists because networking never looked into doing a proper async
> API for anything.
OK, so then I have no idea what you meant in your earlier response
with "Yep" :)
Combing everything said amongst a set of emails it sounds like the
summary is something like:
A safe, synchronous sendfile can be implemented in userland
using iouring, so there is no need to do anything in the kernel
at all. At most, some documentation or examples are needed
somewhere so people who want a safe version of sendfile know
what to do instead.
Is that right?
If so, then great, I guess there is nothing for me to do except
figure out what documentation or man pages to update.
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Pavel Begunkov @ 2025-03-21 20:51 UTC (permalink / raw)
To: Stefan Metzmacher, Jens Axboe, Joe Damato, Christoph Hellwig,
netdev, linux-kernel, linux-fsdevel, edumazet, pabeni, horms,
linux-api, linux-arch, viro, jack, kuba, shuah, sdf, mingo, arnd,
brauner, akpm, tglx, jolsa, linux-kselftest
Cc: David Wei
In-Reply-To: <0fc1032f-908c-4e59-8f64-f22b380ae639@samba.org>
On 3/21/25 07:55, Stefan Metzmacher wrote:
> Am 20.03.25 um 11:46 schrieb Pavel Begunkov:
>> On 3/19/25 19:15, Stefan Metzmacher wrote:
>>> Am 19.03.25 um 19:37 schrieb Jens Axboe:
>>>> On 3/19/25 11:45 AM, Joe Damato wrote:
>>>>> On Wed, Mar 19, 2025 at 11:20:50AM -0600, Jens Axboe wrote:
>> ...
>>>> My argument would be the same as for other features - if you can do it
>>>> simpler this other way, why not consider that? The end result would be
>>>> the same, you can do fast sendfile() with sane buffer reuse. But the
>>>> kernel side would be simpler, which is always a kernel main goal for
>>>> those of us that have to maintain it.
>>>>
>>>> Just adding sendfile2() works in the sense that it's an easier drop in
>>>> replacement for an app, though the error queue side does mean it needs
>>>> to change anyway - it's not just replacing one syscall with another. And
>>>> if we want to be lazy, sure that's fine. I just don't think it's the
>>>> best way to do it when we literally have a mechanism that's designed for
>>>> this and works with reuse already with normal send zc (and receive side
>>>> too, in the next kernel).
>>>
>>> A few month (or even years) back, Pavel came up with an idea
>>> to implement some kind of splice into a fixed buffer, if that
>>> would be implemented I guess it would help me in Samba too.
>>> My first usage was on the receive side (from the network).
>>
>> I did it as a testing ground for infra needed for ublk zerocopy,
>> but if that's of interest I can resurrect the patches and see
>> where it goes, especially since the aforementioned infra just got
>> queued.
>
> Would be great!
>
> Have you posted the work in progress somewhere?
Nope apart from a dirty hack I believe I posted back then.
--
Pavel Begunkov
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Jens Axboe @ 2025-03-21 20:35 UTC (permalink / raw)
To: Joe Damato, Christoph Hellwig, netdev, linux-kernel, asml.silence,
linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
jolsa, linux-kselftest
In-Reply-To: <Z92VkgwS1SAaad2Q@LQ3V64L9R2>
On 3/21/25 10:36 AM, Joe Damato wrote:
> On Fri, Mar 21, 2025 at 05:14:59AM -0600, Jens Axboe wrote:
>> On 3/20/25 11:56 PM, Christoph Hellwig wrote:
>>>> I don't know the entire historical context, but I presume sendmsg
>>>> did that because there was no other mechanism at the time.
>>>
>>> At least aio had been around for about 15 years at the point, but
>>> networking folks tend to be pretty insular and reinvent things.
>>
>> Yep...
>>
>>>> It seems like Jens suggested that plumbing this through for splice
>>>> was a possibility, but sounds like you disagree.
>>>
>>> Yes, very strongly.
>>
>> And that is very much not what I suggested, fwiw.
>
> Your earlier message said:
>
> If the answer is "because splice", then it would seem saner to
> plumb up those bits only. Would be much simpler too...
>
> wherein I interpreted "plumb those bits" to mean plumbing the error
> queue notifications on TX completions.
>
> My sincere apologies that I misunderstood your prior message and/or
> misconstrued what you said -- it was not clear to me what you meant.
>
> It is clear to me now, though, that adding a flag to splice as
> previously proposed and extending sendfile based on the SO_ZEROCOPY
> sock flag being set are both unacceptable solutions.
>
> If you happen to have a suggestion of some piece of code that I
> should read (other than the iouring implementation) to inform how I
> might build an RFCv2, I would appreciate the pointer.
I don't know what to point you at - you need an API that can deliver
notifications, and I'm obviously going to say that io_uring would be one
way to do that. Nothing else exists on the networking side, as far as
I'm aware.
Like Christoph said, struct kiocb is generally how the kernel passes
around async or sync IO, which comes with a completion callback for IO
that initially returns -EIOCBQUEUED, meaning the operation is started
but not yet complete. Outside of that, yeah need some delivery
mechanism, as you're generating two events per zero copy send here. You
could obviously roll your own, good luck with that, or use the existing
one.
--
Jens Axboe
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Jens Axboe @ 2025-03-21 20:33 UTC (permalink / raw)
To: Joe Damato, Christoph Hellwig, netdev, linux-kernel, asml.silence,
linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
jolsa, linux-kselftest
In-Reply-To: <Z93Mc27xaz5sAo5m@LQ3V64L9R2>
On 3/21/25 2:30 PM, Joe Damato wrote:
> On Fri, Mar 21, 2025 at 09:36:34AM -0700, Joe Damato wrote:
>> On Fri, Mar 21, 2025 at 05:14:59AM -0600, Jens Axboe wrote:
>>> On 3/20/25 11:56 PM, Christoph Hellwig wrote:
>>>>> I don't know the entire historical context, but I presume sendmsg
>>>>> did that because there was no other mechanism at the time.
>>>>
>>>> At least aio had been around for about 15 years at the point, but
>>>> networking folks tend to be pretty insular and reinvent things.
>>>
>>> Yep...
>>>
>>>>> It seems like Jens suggested that plumbing this through for splice
>>>>> was a possibility, but sounds like you disagree.
>>>>
>>>> Yes, very strongly.
>>>
>>> And that is very much not what I suggested, fwiw.
>>
>> Your earlier message said:
>>
>> If the answer is "because splice", then it would seem saner to
>> plumb up those bits only. Would be much simpler too...
>>
>> wherein I interpreted "plumb those bits" to mean plumbing the error
>> queue notifications on TX completions.
>>
>> My sincere apologies that I misunderstood your prior message and/or
>> misconstrued what you said -- it was not clear to me what you meant.
>
> I think what added to my confusion here was this bit, Jens:
>
> > > As far as the bit about plumbing only the splice bits, sorry if I'm
> > > being dense here, do you mean plumbing the error queue through to
> > > splice only and dropping sendfile2?
> > >
> > > That is an option. Then the apps currently using sendfile could use
> > > splice instead and get completion notifications on the error queue.
> > > That would probably work and be less work than rewriting to use
> > > iouring, but probably a bit more work than using a new syscall.
> >
> > Yep
>
> I thought I was explicitly asking if adding SPLICE_F_ZC and plumbing
> through the error queue notifications was OK and your response here
> ("Yep") suggested to me that it would be a suitable path to
> consider.
>
> I take it from your other responses, though, that I was mistaken.
I guess I missed your error queue thing here, I was definitely pretty
clear in other ones that I consider that part a hack and something that
only exists because networking never looked into doing a proper async
API for anything.
--
Jens Axboe
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-21 20:30 UTC (permalink / raw)
To: Jens Axboe, Christoph Hellwig, netdev, linux-kernel, asml.silence,
linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
jolsa, linux-kselftest
In-Reply-To: <Z92VkgwS1SAaad2Q@LQ3V64L9R2>
On Fri, Mar 21, 2025 at 09:36:34AM -0700, Joe Damato wrote:
> On Fri, Mar 21, 2025 at 05:14:59AM -0600, Jens Axboe wrote:
> > On 3/20/25 11:56 PM, Christoph Hellwig wrote:
> > >> I don't know the entire historical context, but I presume sendmsg
> > >> did that because there was no other mechanism at the time.
> > >
> > > At least aio had been around for about 15 years at the point, but
> > > networking folks tend to be pretty insular and reinvent things.
> >
> > Yep...
> >
> > >> It seems like Jens suggested that plumbing this through for splice
> > >> was a possibility, but sounds like you disagree.
> > >
> > > Yes, very strongly.
> >
> > And that is very much not what I suggested, fwiw.
>
> Your earlier message said:
>
> If the answer is "because splice", then it would seem saner to
> plumb up those bits only. Would be much simpler too...
>
> wherein I interpreted "plumb those bits" to mean plumbing the error
> queue notifications on TX completions.
>
> My sincere apologies that I misunderstood your prior message and/or
> misconstrued what you said -- it was not clear to me what you meant.
I think what added to my confusion here was this bit, Jens:
> > As far as the bit about plumbing only the splice bits, sorry if I'm
> > being dense here, do you mean plumbing the error queue through to
> > splice only and dropping sendfile2?
> >
> > That is an option. Then the apps currently using sendfile could use
> > splice instead and get completion notifications on the error queue.
> > That would probably work and be less work than rewriting to use
> > iouring, but probably a bit more work than using a new syscall.
>
> Yep
I thought I was explicitly asking if adding SPLICE_F_ZC and plumbing
through the error queue notifications was OK and your response here
("Yep") suggested to me that it would be a suitable path to
consider.
I take it from your other responses, though, that I was mistaken.
^ permalink raw reply
* [PATCH v4 3/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Andrey Albershteyn @ 2025-03-21 19:48 UTC (permalink / raw)
To: Richard Henderson, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Günther Noack, Arnd Bergmann, Pali Rohár, Paul Moore,
James Morris, Serge E. Hallyn
Cc: linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, linux-fsdevel, linux-security-module, linux-api,
linux-arch, Andrey Albershteyn, linux-xfs
In-Reply-To: <20250321-xattrat-syscall-v4-0-3e82e6fb3264@kernel.org>
From: Andrey Albershteyn <aalbersh@redhat.com>
Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
extended attributes/flags. The syscalls take parent directory fd and
path to the child together with struct fsxattr.
This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
that file don't need to be open as we can reference it with a path
instead of fd. By having this we can manipulated inode extended
attributes not only on regular files but also on special ones. This
is not possible with FS_IOC_FSSETXATTR ioctl as with special files
we can not call ioctl() directly on the filesystem inode using fd.
This patch adds two new syscalls which allows userspace to get/set
extended inode attributes on special files by using parent directory
and a path - *at() like syscall.
CC: linux-api@vger.kernel.org
CC: linux-fsdevel@vger.kernel.org
CC: linux-xfs@vger.kernel.org
Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/alpha/kernel/syscalls/syscall.tbl | 2 +
arch/arm/tools/syscall.tbl | 2 +
arch/arm64/tools/syscall_32.tbl | 2 +
arch/m68k/kernel/syscalls/syscall.tbl | 2 +
arch/microblaze/kernel/syscalls/syscall.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 2 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 2 +
arch/parisc/kernel/syscalls/syscall.tbl | 2 +
arch/powerpc/kernel/syscalls/syscall.tbl | 2 +
arch/s390/kernel/syscalls/syscall.tbl | 2 +
arch/sh/kernel/syscalls/syscall.tbl | 2 +
arch/sparc/kernel/syscalls/syscall.tbl | 2 +
arch/x86/entry/syscalls/syscall_32.tbl | 2 +
arch/x86/entry/syscalls/syscall_64.tbl | 2 +
arch/xtensa/kernel/syscalls/syscall.tbl | 2 +
fs/inode.c | 130 ++++++++++++++++++++++++++++
include/linux/syscalls.h | 6 ++
include/uapi/asm-generic/unistd.h | 8 +-
include/uapi/linux/fs.h | 3 +
20 files changed, 178 insertions(+), 1 deletion(-)
diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index c59d53d6d3f3490f976ca179ddfe02e69265ae4d..4b9e687494c16b60c6fd6ca1dc4d6564706a7e25 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -506,3 +506,5 @@
574 common getxattrat sys_getxattrat
575 common listxattrat sys_listxattrat
576 common removexattrat sys_removexattrat
+577 common getfsxattrat sys_getfsxattrat
+578 common setfsxattrat sys_setfsxattrat
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 49eeb2ad8dbd8e074c6240417693f23fb328afa8..66466257f3c2debb3e2299f0b608c6740c98cab2 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -481,3 +481,5 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat
diff --git a/arch/arm64/tools/syscall_32.tbl b/arch/arm64/tools/syscall_32.tbl
index 69a829912a05eb8a3e21ed701d1030e31c0148bc..9c516118b154811d8d11d5696f32817430320dbf 100644
--- a/arch/arm64/tools/syscall_32.tbl
+++ b/arch/arm64/tools/syscall_32.tbl
@@ -478,3 +478,5 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index f5ed71f1910d09769c845c2d062d99ee0449437c..159476387f394a92ee5e29db89b118c630372db2 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -466,3 +466,5 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 680f568b77f2cbefc3eacb2517f276041f229b1e..a6d59ee740b58cacf823702003cf9bad17c0d3b7 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -472,3 +472,5 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 0b9b7e25b69ad592642f8533bee9ccfe95ce9626..cfe38fcebe1a0279e11751378d3e71c5ec6b6569 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -405,3 +405,5 @@
464 n32 getxattrat sys_getxattrat
465 n32 listxattrat sys_listxattrat
466 n32 removexattrat sys_removexattrat
+467 n32 getfsxattrat sys_getfsxattrat
+468 n32 setfsxattrat sys_setfsxattrat
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index c844cd5cda620b2809a397cdd6f4315ab6a1bfe2..29a0c5974d1aa2f01e33edc0252d75fb97abe230 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -381,3 +381,5 @@
464 n64 getxattrat sys_getxattrat
465 n64 listxattrat sys_listxattrat
466 n64 removexattrat sys_removexattrat
+467 n64 getfsxattrat sys_getfsxattrat
+468 n64 setfsxattrat sys_setfsxattrat
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index 349b8aad1159f404103bd2057a1e64e9bf309f18..6c00436807c57c492ba957fcd59af1202231cf80 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -454,3 +454,5 @@
464 o32 getxattrat sys_getxattrat
465 o32 listxattrat sys_listxattrat
466 o32 removexattrat sys_removexattrat
+467 o32 getfsxattrat sys_getfsxattrat
+468 o32 setfsxattrat sys_setfsxattrat
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index d9fc94c869657fcfbd7aca1d5f5abc9fae2fb9d8..b3578fac43d6b65167787fcc97d2d09f5a9828e7 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -465,3 +465,5 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index d8b4ab78bef076bd50d49b87dea5060fd8c1686a..808045d82c9465c3bfa96b15947546efe5851e9a 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -557,3 +557,5 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index e9115b4d8b635b846e5c9ad6ce229605323723a5..78dfc2c184d4815baf8a9e61c546c9936d58a47c 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -469,3 +469,5 @@
464 common getxattrat sys_getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat sys_setfsxattrat
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index c8cad33bf250ea110de37bd1407f5a43ec5e38f2..d5a5c8339f0ed25ea07c4aba90351d352033c8a0 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -470,3 +470,5 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index 727f99d333b304b3db0711953a3d91ece18a28eb..817dcd8603bcbffc47f3f59aa3b74b16486453d0 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -512,3 +512,5 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 4d0fb2fba7e208ae9455459afe11e277321d9f74..b4842c027c5d00c0236b2ba89387c5e2267447bd 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -472,3 +472,5 @@
464 i386 getxattrat sys_getxattrat
465 i386 listxattrat sys_listxattrat
466 i386 removexattrat sys_removexattrat
+467 i386 getfsxattrat sys_getfsxattrat
+468 i386 setfsxattrat sys_setfsxattrat
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 5eb708bff1c791debd6cfc5322583b2ae53f6437..b6f0a7236aaee624cf9b484239a1068085a8ffe1 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -390,6 +390,8 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat
#
# Due to a historical design error, certain syscalls are numbered differently
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 37effc1b134eea061f2c350c1d68b4436b65a4dd..425d56be337d1de22f205ac503df61ff86224fee 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -437,3 +437,5 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common getfsxattrat sys_getfsxattrat
+468 common setfsxattrat sys_setfsxattrat
diff --git a/fs/inode.c b/fs/inode.c
index 6b4c77268fc0ecace4ac78a9ca777fbffc277f4a..811debf379ab299f287ed90863277cfda27db30c 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -23,6 +23,9 @@
#include <linux/rw_hint.h>
#include <linux/seq_file.h>
#include <linux/debugfs.h>
+#include <linux/syscalls.h>
+#include <linux/fileattr.h>
+#include <linux/namei.h>
#include <trace/events/writeback.h>
#define CREATE_TRACE_POINTS
#include <trace/events/timestamp.h>
@@ -2953,3 +2956,130 @@ umode_t mode_strip_sgid(struct mnt_idmap *idmap,
return mode & ~S_ISGID;
}
EXPORT_SYMBOL(mode_strip_sgid);
+
+SYSCALL_DEFINE5(getfsxattrat, int, dfd, const char __user *, filename,
+ struct fsxattr __user *, ufsx, size_t, usize,
+ unsigned int, at_flags)
+{
+ struct fileattr fa = {};
+ struct path filepath;
+ int error;
+ unsigned int lookup_flags = 0;
+ struct filename *name;
+ struct fsxattr fsx = {};
+
+ BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
+ BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
+
+ if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ return -EINVAL;
+
+ if (!(at_flags & AT_SYMLINK_NOFOLLOW))
+ lookup_flags |= LOOKUP_FOLLOW;
+
+ if (at_flags & AT_EMPTY_PATH)
+ lookup_flags |= LOOKUP_EMPTY;
+
+ if (usize > PAGE_SIZE)
+ return -E2BIG;
+
+ if (usize < FSXATTR_SIZE_VER0)
+ return -EINVAL;
+
+ name = getname_maybe_null(filename, at_flags);
+ if (!name) {
+ CLASS(fd, f)(dfd);
+
+ if (fd_empty(f))
+ return -EBADF;
+ error = vfs_fileattr_get(file_dentry(fd_file(f)), &fa);
+ } else {
+ error = filename_lookup(dfd, name, lookup_flags, &filepath,
+ NULL);
+ if (error)
+ goto out;
+ error = vfs_fileattr_get(filepath.dentry, &fa);
+ path_put(&filepath);
+ }
+ if (error == -ENOIOCTLCMD)
+ error = -EOPNOTSUPP;
+ if (!error) {
+ fileattr_to_fsxattr(&fa, &fsx);
+ error = copy_struct_to_user(ufsx, usize, &fsx,
+ sizeof(struct fsxattr), NULL);
+ }
+out:
+ putname(name);
+ return error;
+}
+
+SYSCALL_DEFINE5(setfsxattrat, int, dfd, const char __user *, filename,
+ struct fsxattr __user *, ufsx, size_t, usize,
+ unsigned int, at_flags)
+{
+ struct fileattr fa;
+ struct path filepath;
+ int error;
+ unsigned int lookup_flags = 0;
+ struct filename *name;
+ struct mnt_idmap *idmap;
+ struct dentry *dentry;
+ struct vfsmount *mnt;
+ struct fsxattr fsx = {};
+
+ BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
+ BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
+
+ if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ return -EINVAL;
+
+ if (!(at_flags & AT_SYMLINK_NOFOLLOW))
+ lookup_flags |= LOOKUP_FOLLOW;
+
+ if (at_flags & AT_EMPTY_PATH)
+ lookup_flags |= LOOKUP_EMPTY;
+
+ if (usize > PAGE_SIZE)
+ return -E2BIG;
+
+ if (usize < FSXATTR_SIZE_VER0)
+ return -EINVAL;
+
+ error = copy_struct_from_user(&fsx, sizeof(struct fsxattr), ufsx, usize);
+ if (error)
+ return error;
+
+ fsxattr_to_fileattr(&fsx, &fa);
+
+ name = getname_maybe_null(filename, at_flags);
+ if (!name) {
+ CLASS(fd, f)(dfd);
+
+ if (fd_empty(f))
+ return -EBADF;
+
+ idmap = file_mnt_idmap(fd_file(f));
+ dentry = file_dentry(fd_file(f));
+ mnt = fd_file(f)->f_path.mnt;
+ } else {
+ error = filename_lookup(dfd, name, lookup_flags, &filepath,
+ NULL);
+ if (error)
+ return error;
+
+ idmap = mnt_idmap(filepath.mnt);
+ dentry = filepath.dentry;
+ mnt = filepath.mnt;
+ }
+
+ error = mnt_want_write(mnt);
+ if (!error) {
+ error = vfs_fileattr_set(idmap, dentry, &fa);
+ if (error == -ENOIOCTLCMD)
+ error = -EOPNOTSUPP;
+ mnt_drop_write(mnt);
+ }
+
+ path_put(&filepath);
+ return error;
+}
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index c6333204d45130eb022f6db460eea34a1f6e91db..e242ea39b3e63a8008bc777764b616fd63bd40c4 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -371,6 +371,12 @@ asmlinkage long sys_removexattrat(int dfd, const char __user *path,
asmlinkage long sys_lremovexattr(const char __user *path,
const char __user *name);
asmlinkage long sys_fremovexattr(int fd, const char __user *name);
+asmlinkage long sys_getfsxattrat(int dfd, const char __user *filename,
+ struct fsxattr __user *ufsx, size_t usize,
+ unsigned int at_flags);
+asmlinkage long sys_setfsxattrat(int dfd, const char __user *filename,
+ struct fsxattr __user *ufsx, size_t usize,
+ unsigned int at_flags);
asmlinkage long sys_getcwd(char __user *buf, unsigned long size);
asmlinkage long sys_eventfd2(unsigned int count, int flags);
asmlinkage long sys_epoll_create1(int flags);
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 88dc393c2bca38c0fa1b3fae579f7cfe4931223c..50be2e1007bc2779120d05c6e9512a689f86779c 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -850,8 +850,14 @@ __SYSCALL(__NR_listxattrat, sys_listxattrat)
#define __NR_removexattrat 466
__SYSCALL(__NR_removexattrat, sys_removexattrat)
+/* fs/inode.c */
+#define __NR_getfsxattrat 467
+__SYSCALL(__NR_getfsxattrat, sys_getfsxattrat)
+#define __NR_setfsxattrat 468
+__SYSCALL(__NR_setfsxattrat, sys_setfsxattrat)
+
#undef __NR_syscalls
-#define __NR_syscalls 467
+#define __NR_syscalls 469
/*
* 32 bit systems traditionally used different
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 7539717707337a8cb22396a869baba3bafa08371..aed753e5d50c97da9b895a187fdaecf0477db74b 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -139,6 +139,9 @@ struct fsxattr {
unsigned char fsx_pad[8];
};
+#define FSXATTR_SIZE_VER0 28
+#define FSXATTR_SIZE_LATEST FSXATTR_SIZE_VER0
+
/*
* Flags for the fsx_xflags field
*/
--
2.47.2
^ permalink raw reply related
* [PATCH v4 0/3] fs: introduce getfsxattrat and setfsxattrat syscalls
From: Andrey Albershteyn @ 2025-03-21 19:48 UTC (permalink / raw)
To: Richard Henderson, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Günther Noack, Arnd Bergmann, Pali Rohár, Paul Moore,
James Morris, Serge E. Hallyn
Cc: linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, linux-fsdevel, linux-security-module, linux-api,
linux-arch, selinux, Andrey Albershteyn, Andrey Albershteyn,
linux-xfs
This patchset introduced two new syscalls getfsxattrat() and
setfsxattrat(). These syscalls are similar to FS_IOC_FSSETXATTR ioctl()
except they use *at() semantics. Therefore, there's no need to open the
file to get an fd.
These syscalls allow userspace to set filesystem inode attributes on
special files. One of the usage examples is XFS quota projects.
XFS has project quotas which could be attached to a directory. All
new inodes in these directories inherit project ID set on parent
directory.
The project is created from userspace by opening and calling
FS_IOC_FSSETXATTR on each inode. This is not possible for special
files such as FIFO, SOCK, BLK etc. Therefore, some inodes are left
with empty project ID. Those inodes then are not shown in the quota
accounting but still exist in the directory. This is not critical but in
the case when special files are created in the directory with already
existing project quota, these new inodes inherit extended attributes.
This creates a mix of special files with and without attributes.
Moreover, special files with attributes don't have a possibility to
become clear or change the attributes. This, in turn, prevents userspace
from re-creating quota project on these existing files.
Christian, if this get in some mergeable state, please don't merge it
yet. Amir suggested these syscalls better to use updated struct fsxattr
with masking from Pali Rohár patchset, so, let's see how it goes.
NAME
getfsxattrat/setfsxattrat - get/set filesystem inode attributes
SYNOPSIS
#include <sys/syscall.h> /* Definition of SYS_* constants */
#include <unistd.h>
long syscall(SYS_getfsxattrat, int dirfd, const char *pathname,
struct fsxattr *fsx, size_t size,
unsigned int at_flags);
long syscall(SYS_setfsxattrat, int dirfd, const char *pathname,
struct fsxattr *fsx, size_t size,
unsigned int at_flags);
Note: glibc doesn't provide for getfsxattrat()/setfsxattrat(),
use syscall(2) instead.
DESCRIPTION
The syscalls take fd and path to the child together with struct
fsxattr. If path is absolute, fd is not used. If path is empty,
inode under fd is used to get/set attributes on.
This is an alternative to FS_IOC_FSGETXATTR/FS_IOC_FSSETXATTR
ioctl with a difference that file don't need to be open as we
can reference it with a path instead of fd. By having this we
can manipulated filesystem inode attributes not only on regular
files but also on special ones. This is not possible with
FS_IOC_FSSETXATTR ioctl as with special files we can not call
ioctl() directly on the filesystem inode using file descriptor.
RETURN VALUE
On success, 0 is returned. On error, -1 is returned, and errno
is set to indicate the error.
ERRORS
EINVAL Invalid at_flag specified (only
AT_SYMLINK_NOFOLLOW and AT_EMPTY_PATH is
supported).
EINVAL Size was smaller than any known version of
struct fsxattr.
EINVAL Invalid combination of parameters provided in
fsxattr for this type of file.
E2BIG Size of input argument **struct fsxattr** is too
big.
EBADF Invalid file descriptor was provided.
EPERM No permission to change this file.
EOPNOTSUPP Filesystem does not support setting attributes
on this type of inode
HISTORY
Added in Linux 6.14.
EXAMPLE
Create directory and file "mkdir ./dir && touch ./dir/foo" and then
execute the following program:
#include <fcntl.h>
#include <errno.h>
#include <string.h>
#include <linux/fs.h>
#include <stdio.h>
#include <sys/syscall.h>
#include <unistd.h>
int
main(int argc, char **argv) {
int dfd;
int error;
struct fsxattr fsx;
dfd = open("./dir", O_RDONLY);
if (dfd == -1) {
printf("can not open ./dir");
return dfd;
}
error = syscall(467, dfd, "./foo", &fsx, 0);
if (error) {
printf("can not call 467: %s", strerror(errno));
return error;
}
printf("dir/foo flags: %d\n", fsx.fsx_xflags);
fsx.fsx_xflags |= FS_XFLAG_NODUMP;
error = syscall(468, dfd, "./foo", &fsx, 0);
if (error) {
printf("can not call 468: %s", strerror(errno));
return error;
}
printf("dir/foo flags: %d\n", fsx.fsx_xflags);
return error;
}
SEE ALSO
ioctl(2), ioctl_iflags(2), ioctl_xfs_fsgetxattr(2)
---
Changes in v4:
- Use getname_maybe_null() for correct handling of dfd + path semantic
- Remove restriction for special files on which flags are allowed
- Utilize copy_struct_from_user() for better future compatibility
- Add draft man page to cover letter
- Convert -ENOIOCTLCMD to -EOPNOSUPP as more appropriate for syscall
- Add missing __user to header declaration of syscalls
- Link to v3: https://lore.kernel.org/r/20250211-xattrat-syscall-v3-1-a07d15f898b2@kernel.org
Changes in v3:
- Remove unnecessary "dfd is dir" check as it checked in user_path_at()
- Remove unnecessary "same filesystem" check
- Use CLASS() instead of directly calling fdget/fdput
- Link to v2: https://lore.kernel.org/r/20250122-xattrat-syscall-v2-1-5b360d4fbcb2@kernel.org
v1:
https://lore.kernel.org/linuxppc-dev/20250109174540.893098-1-aalbersh@kernel.org/
Previous discussion:
https://lore.kernel.org/linux-xfs/20240520164624.665269-2-aalbersh@redhat.com/
---
Andrey Albershteyn (3):
lsm: introduce new hooks for setting/getting inode fsxattr
fs: split fileattr/fsxattr converters into helpers
fs: introduce getfsxattrat and setfsxattrat syscalls
arch/alpha/kernel/syscalls/syscall.tbl | 2 +
arch/arm/tools/syscall.tbl | 2 +
arch/arm64/tools/syscall_32.tbl | 2 +
arch/m68k/kernel/syscalls/syscall.tbl | 2 +
arch/microblaze/kernel/syscalls/syscall.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 2 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 2 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 2 +
arch/parisc/kernel/syscalls/syscall.tbl | 2 +
arch/powerpc/kernel/syscalls/syscall.tbl | 2 +
arch/s390/kernel/syscalls/syscall.tbl | 2 +
arch/sh/kernel/syscalls/syscall.tbl | 2 +
arch/sparc/kernel/syscalls/syscall.tbl | 2 +
arch/x86/entry/syscalls/syscall_32.tbl | 2 +
arch/x86/entry/syscalls/syscall_64.tbl | 2 +
arch/xtensa/kernel/syscalls/syscall.tbl | 2 +
fs/inode.c | 130 ++++++++++++++++++++++++++++
fs/ioctl.c | 39 ++++++---
include/linux/fileattr.h | 2 +
include/linux/lsm_hook_defs.h | 4 +
include/linux/security.h | 16 ++++
include/linux/syscalls.h | 6 ++
include/uapi/asm-generic/unistd.h | 8 +-
include/uapi/linux/fs.h | 3 +
security/security.c | 32 +++++++
25 files changed, 259 insertions(+), 13 deletions(-)
---
base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
change-id: 20250114-xattrat-syscall-6a1136d2db59
Best regards,
--
Andrey Albershteyn <aalbersh@kernel.org>
^ permalink raw reply
* [PATCH v4 2/3] fs: split fileattr/fsxattr converters into helpers
From: Andrey Albershteyn @ 2025-03-21 19:48 UTC (permalink / raw)
To: Richard Henderson, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Günther Noack, Arnd Bergmann, Pali Rohár, Paul Moore,
James Morris, Serge E. Hallyn
Cc: linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, linux-fsdevel, linux-security-module, linux-api,
linux-arch, Andrey Albershteyn
In-Reply-To: <20250321-xattrat-syscall-v4-0-3e82e6fb3264@kernel.org>
This will be helpful for get/setfsxattrat syscalls to convert
between fileattr and fsxattr.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/ioctl.c | 32 +++++++++++++++++++++-----------
include/linux/fileattr.h | 2 ++
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 4434c97bc5dff5a3e8635e28745cd99404ff353e..840283d8c406623d8d26790f89b62ebcbd39e2de 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -538,6 +538,16 @@ int vfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
}
EXPORT_SYMBOL(vfs_fileattr_get);
+void fileattr_to_fsxattr(const struct fileattr *fa, struct fsxattr *fsx)
+{
+ memset(fsx, 0, sizeof(struct fsxattr));
+ fsx->fsx_xflags = fa->fsx_xflags;
+ fsx->fsx_extsize = fa->fsx_extsize;
+ fsx->fsx_nextents = fa->fsx_nextents;
+ fsx->fsx_projid = fa->fsx_projid;
+ fsx->fsx_cowextsize = fa->fsx_cowextsize;
+}
+
/**
* copy_fsxattr_to_user - copy fsxattr to userspace.
* @fa: fileattr pointer
@@ -549,12 +559,7 @@ int copy_fsxattr_to_user(const struct fileattr *fa, struct fsxattr __user *ufa)
{
struct fsxattr xfa;
- memset(&xfa, 0, sizeof(xfa));
- xfa.fsx_xflags = fa->fsx_xflags;
- xfa.fsx_extsize = fa->fsx_extsize;
- xfa.fsx_nextents = fa->fsx_nextents;
- xfa.fsx_projid = fa->fsx_projid;
- xfa.fsx_cowextsize = fa->fsx_cowextsize;
+ fileattr_to_fsxattr(fa, &xfa);
if (copy_to_user(ufa, &xfa, sizeof(xfa)))
return -EFAULT;
@@ -563,6 +568,15 @@ int copy_fsxattr_to_user(const struct fileattr *fa, struct fsxattr __user *ufa)
}
EXPORT_SYMBOL(copy_fsxattr_to_user);
+void fsxattr_to_fileattr(const struct fsxattr *fsx, struct fileattr *fa)
+{
+ fileattr_fill_xflags(fa, fsx->fsx_xflags);
+ fa->fsx_extsize = fsx->fsx_extsize;
+ fa->fsx_nextents = fsx->fsx_nextents;
+ fa->fsx_projid = fsx->fsx_projid;
+ fa->fsx_cowextsize = fsx->fsx_cowextsize;
+}
+
static int copy_fsxattr_from_user(struct fileattr *fa,
struct fsxattr __user *ufa)
{
@@ -571,11 +585,7 @@ static int copy_fsxattr_from_user(struct fileattr *fa,
if (copy_from_user(&xfa, ufa, sizeof(xfa)))
return -EFAULT;
- fileattr_fill_xflags(fa, xfa.fsx_xflags);
- fa->fsx_extsize = xfa.fsx_extsize;
- fa->fsx_nextents = xfa.fsx_nextents;
- fa->fsx_projid = xfa.fsx_projid;
- fa->fsx_cowextsize = xfa.fsx_cowextsize;
+ fsxattr_to_fileattr(&xfa, fa);
return 0;
}
diff --git a/include/linux/fileattr.h b/include/linux/fileattr.h
index 47c05a9851d0600964b644c9c7218faacfd865f8..31888fa2edf10050be134f587299256088344365 100644
--- a/include/linux/fileattr.h
+++ b/include/linux/fileattr.h
@@ -33,7 +33,9 @@ struct fileattr {
bool fsx_valid:1;
};
+void fileattr_to_fsxattr(const struct fileattr *fa, struct fsxattr *fsx);
int copy_fsxattr_to_user(const struct fileattr *fa, struct fsxattr __user *ufa);
+void fsxattr_to_fileattr(const struct fsxattr *fsx, struct fileattr *fa);
void fileattr_fill_xflags(struct fileattr *fa, u32 xflags);
void fileattr_fill_flags(struct fileattr *fa, u32 flags);
--
2.47.2
^ permalink raw reply related
* [PATCH v4 1/3] lsm: introduce new hooks for setting/getting inode fsxattr
From: Andrey Albershteyn @ 2025-03-21 19:48 UTC (permalink / raw)
To: Richard Henderson, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Günther Noack, Arnd Bergmann, Pali Rohár, Paul Moore,
James Morris, Serge E. Hallyn
Cc: linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, linux-fsdevel, linux-security-module, linux-api,
linux-arch, selinux, Andrey Albershteyn
In-Reply-To: <20250321-xattrat-syscall-v4-0-3e82e6fb3264@kernel.org>
Introduce new hooks for setting and getting filesystem extended
attributes on inode (FS_IOC_FSGETXATTR).
Cc: selinux@vger.kernel.org
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/ioctl.c | 7 ++++++-
include/linux/lsm_hook_defs.h | 4 ++++
include/linux/security.h | 16 ++++++++++++++++
security/security.c | 32 ++++++++++++++++++++++++++++++++
4 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 638a36be31c14afc66a7fd6eb237d9545e8ad997..4434c97bc5dff5a3e8635e28745cd99404ff353e 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -525,10 +525,15 @@ EXPORT_SYMBOL(fileattr_fill_flags);
int vfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
{
struct inode *inode = d_inode(dentry);
+ int error;
if (!inode->i_op->fileattr_get)
return -ENOIOCTLCMD;
+ error = security_inode_getfsxattr(inode, fa);
+ if (error)
+ return error;
+
return inode->i_op->fileattr_get(dentry, fa);
}
EXPORT_SYMBOL(vfs_fileattr_get);
@@ -692,7 +697,7 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
fa->flags |= old_ma.flags & ~FS_COMMON_FL;
}
err = fileattr_set_prepare(inode, &old_ma, fa);
- if (!err)
+ if (!err && !security_inode_setfsxattr(inode, fa))
err = inode->i_op->fileattr_set(idmap, dentry, fa);
}
inode_unlock(inode);
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index eb2937599cb029004f491012b3bf5a3d6d2731df..49e64d23e9049568af133bf3f30ca719c9ec5f25 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -157,6 +157,10 @@ LSM_HOOK(int, 0, inode_removexattr, struct mnt_idmap *idmap,
struct dentry *dentry, const char *name)
LSM_HOOK(void, LSM_RET_VOID, inode_post_removexattr, struct dentry *dentry,
const char *name)
+LSM_HOOK(int, 0, inode_setfsxattr, const struct inode *inode,
+ const struct fileattr *fa)
+LSM_HOOK(int, 0, inode_getfsxattr, const struct inode *inode,
+ const struct fileattr *fa)
LSM_HOOK(int, 0, inode_set_acl, struct mnt_idmap *idmap,
struct dentry *dentry, const char *acl_name, struct posix_acl *kacl)
LSM_HOOK(void, LSM_RET_VOID, inode_post_set_acl, struct dentry *dentry,
diff --git a/include/linux/security.h b/include/linux/security.h
index cbdba435b798660130779d6919388779edd41d54..dd58ace29c6e325ee49470596d0abb6ecc38ba07 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -439,6 +439,10 @@ int security_inode_listxattr(struct dentry *dentry);
int security_inode_removexattr(struct mnt_idmap *idmap,
struct dentry *dentry, const char *name);
void security_inode_post_removexattr(struct dentry *dentry, const char *name);
+int security_inode_setfsxattr(const struct inode *inode,
+ const struct fileattr *fa);
+int security_inode_getfsxattr(const struct inode *inode,
+ const struct fileattr *fa);
int security_inode_need_killpriv(struct dentry *dentry);
int security_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry);
int security_inode_getsecurity(struct mnt_idmap *idmap,
@@ -1042,6 +1046,18 @@ static inline void security_inode_post_removexattr(struct dentry *dentry,
const char *name)
{ }
+static inline int security_inode_setfsxattr(const struct inode *inode,
+ const struct fileattr *fa)
+{
+ return 0;
+}
+
+static inline int security_inode_getfsxattr(const struct inode *inode,
+ const struct fileattr *fa)
+{
+ return 0;
+}
+
static inline int security_inode_need_killpriv(struct dentry *dentry)
{
return cap_inode_need_killpriv(dentry);
diff --git a/security/security.c b/security/security.c
index 09664e09fec9a1d502a23847aa2e87a6d19837db..d3b527f55ed52209d8e22c05adf278b164374d35 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2617,6 +2617,38 @@ void security_inode_post_removexattr(struct dentry *dentry, const char *name)
call_void_hook(inode_post_removexattr, dentry, name);
}
+/**
+ * security_inode_setfsxattr() - check if setting fsxattr is allowed
+ * @inode: inode to set filesystem extended attributes on
+ * @fa: extended attributes to set on the inode
+ *
+ * Called when setfsxattrat() syscall or FS_IOC_FSSETXATTR ioctl() is called on
+ * inode
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_inode_setfsxattr(const struct inode *inode,
+ const struct fileattr *fa)
+{
+ return call_int_hook(inode_setfsxattr, inode, fa);
+}
+
+/**
+ * security_inode_getfsxattr() - check if retrieving fsxattr is allowed
+ * @inode: inode to retrieve filesystem extended attributes from
+ * @fa: extended attributes to get
+ *
+ * Called when getfsxattrat() syscall or FS_IOC_FSGETXATTR ioctl() is called on
+ * inode
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_inode_getfsxattr(const struct inode *inode,
+ const struct fileattr *fa)
+{
+ return call_int_hook(inode_getfsxattr, inode, fa);
+}
+
/**
* security_inode_need_killpriv() - Check if security_inode_killpriv() required
* @dentry: associated dentry
--
2.47.2
^ permalink raw reply related
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-21 16:44 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, netdev, linux-kernel, asml.silence, linux-fsdevel,
edumazet, pabeni, horms, linux-api, linux-arch, viro, jack, kuba,
shuah, sdf, mingo, arnd, brauner, akpm, tglx, jolsa,
linux-kselftest
In-Reply-To: <Z9z_f-kR0lBx8P_9@infradead.org>
On Thu, Mar 20, 2025 at 10:56:15PM -0700, Christoph Hellwig wrote:
> On Thu, Mar 20, 2025 at 11:23:57AM -0700, Joe Damato wrote:
> > In my other message to Jens I proposed:
> > - SPLICE_F_ZC for splice to generate zc completion notifications
> > to the error queue
> > - Modifying sendfile so that if SO_ZEROCOPY (which already exists)
> > is set on a network socket, zc completion notifications are
> > generated.
> >
> > In both cases no new system call is needed and both splice and
> > sendfile become safer to use.
> >
> > At some point in the future a mechanism built on top of iouring
> > introduced as new system calls (sendmsg2, sendfile2, splice2, etc)
> > can be built.
>
> I strongly disagree with this. This is spreading the broken
> SO_ZEROCOPY to futher places outside the pure networking realm. Don't
> do that.
OK. I won't proceed down that path. Thank you for the feedback.
> > > Because sendmsg should never have done that it certainly should not
> > > spread beyond purely socket specific syscalls.
> >
> > I don't know the entire historical context, but I presume sendmsg
> > did that because there was no other mechanism at the time.
>
> At least aio had been around for about 15 years at the point, but
> networking folks tend to be pretty insular and reinvent things.
Sorry, but whatever issue there is between networking and other
folks is well beyond my understanding and historical context. I'm
not a reviewer or maintainer or anything like that; I'm just a
developer who saw a problem and wanted a solution.
I've read your message loud and clear, though, and I won't proceed
down the path I've proposed.
I appreciate your feedback; this is precisely why I sent the RFC -
to get comments - so thank you for taking a look and letting me
know.
> > As mentioned above and in other messages, it seems like it is
> > possible to improve the networking parts of splice (and therefore
> > sendfile) to make them safer to use without introducing a new system
> > call.
> >
> > Are you saying that you are against doing that, even if the code is
> > network specific (but lives in fs/)?
>
> Yes.
>
> Please take the work and integrate it with the kiocb-based system
> we use for all other in-kernel I/O that needs completion notifications
> and which makes it trivial to integate with io_uring instead of
> spreading an imcompatible and inferior event system.
If you have any suggestions or pointers to code I should look at for
inspiration I would very much appreciate the guidance.
Thanks for your time and energy in reviewing my RFC and responding.
- Joe
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-21 16:36 UTC (permalink / raw)
To: Jens Axboe
Cc: Christoph Hellwig, netdev, linux-kernel, asml.silence,
linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
jolsa, linux-kselftest
In-Reply-To: <ca1fbeba-b749-4c34-b4be-c80056eccc3a@kernel.dk>
On Fri, Mar 21, 2025 at 05:14:59AM -0600, Jens Axboe wrote:
> On 3/20/25 11:56 PM, Christoph Hellwig wrote:
> >> I don't know the entire historical context, but I presume sendmsg
> >> did that because there was no other mechanism at the time.
> >
> > At least aio had been around for about 15 years at the point, but
> > networking folks tend to be pretty insular and reinvent things.
>
> Yep...
>
> >> It seems like Jens suggested that plumbing this through for splice
> >> was a possibility, but sounds like you disagree.
> >
> > Yes, very strongly.
>
> And that is very much not what I suggested, fwiw.
Your earlier message said:
If the answer is "because splice", then it would seem saner to
plumb up those bits only. Would be much simpler too...
wherein I interpreted "plumb those bits" to mean plumbing the error
queue notifications on TX completions.
My sincere apologies that I misunderstood your prior message and/or
misconstrued what you said -- it was not clear to me what you meant.
It is clear to me now, though, that adding a flag to splice as
previously proposed and extending sendfile based on the SO_ZEROCOPY
sock flag being set are both unacceptable solutions.
If you happen to have a suggestion of some piece of code that I
should read (other than the iouring implementation) to inform how I
might build an RFCv2, I would appreciate the pointer.
Thanks for your time and energy,
Joe
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Jens Axboe @ 2025-03-21 11:14 UTC (permalink / raw)
To: Christoph Hellwig, Joe Damato, netdev, linux-kernel, asml.silence,
linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
jolsa, linux-kselftest
In-Reply-To: <Z9z_f-kR0lBx8P_9@infradead.org>
On 3/20/25 11:56 PM, Christoph Hellwig wrote:
>> I don't know the entire historical context, but I presume sendmsg
>> did that because there was no other mechanism at the time.
>
> At least aio had been around for about 15 years at the point, but
> networking folks tend to be pretty insular and reinvent things.
Yep...
>> It seems like Jens suggested that plumbing this through for splice
>> was a possibility, but sounds like you disagree.
>
> Yes, very strongly.
And that is very much not what I suggested, fwiw.
>> As mentioned above and in other messages, it seems like it is
>> possible to improve the networking parts of splice (and therefore
>> sendfile) to make them safer to use without introducing a new system
>> call.
>>
>> Are you saying that you are against doing that, even if the code is
>> network specific (but lives in fs/)?
>
> Yes.
>
> Please take the work and integrate it with the kiocb-based system
> we use for all other in-kernel I/O that needs completion notifications
> and which makes it trivial to integate with io_uring instead of
> spreading an imcompatible and inferior event system.
Exactly, this is how we do async IO elsewhere, not sure why networking
needs to be special here, and definitely not special in a good way.
--
Jens Axboe
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Jens Axboe @ 2025-03-21 11:13 UTC (permalink / raw)
To: Joe Damato, Christoph Hellwig, netdev, linux-kernel, asml.silence,
linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
jolsa, linux-kselftest
In-Reply-To: <Z9tRyeJE5uKDJAdo@LQ3V64L9R2>
On 3/19/25 5:22 PM, Joe Damato wrote:
> Would you be open to the idea that sendfile could be extended to
> generate error queue completions if the network socket has
> SO_ZEROCOPY set?
I thought I was quite clear on my view of SO_ZEROCOPY and its error
queue usage, I guess I was not. No I don't think this is a good path at
all, when the whole issue is that pretending to handle two different
types of completions via two different interfaces is pretty dumb and
inefficient to begin with, particularly when we have a method of doing
exactly that where the reuse notifications arrive in the normal
completion stream.
--
Jens Axboe
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Jens Axboe @ 2025-03-21 11:11 UTC (permalink / raw)
To: Joe Damato, Christoph Hellwig, netdev, linux-kernel, asml.silence,
linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
jolsa, linux-kselftest
In-Reply-To: <Z9sX98Y0Xy9-Vzqf@LQ3V64L9R2>
On 3/19/25 1:16 PM, Joe Damato wrote:
>>> In general: it does seem a bit odd to me that there isn't a safe
>>> sendfile syscall in Linux that uses existing completion notification
>>> mechanisms.
>>
>> Pretty natural, I think. sendfile(2) predates that by quite a bit, and
>> the last real change to sendfile was using splice underneath. Which I
>> did, and that was probably almost 20 years ago at this point...
>>
>> I do think it makes sense to have a sendfile that's both fast and
>> efficient, and can be used sanely with buffer reuse without relying on
>> odd heuristics.
>
> Just trying to tie this together in my head -- are you saying that
> you think the kernel internals of sendfile could be changed in a
> different way or that this a userland problem (and they should use
> the io_uring wrapper you suggested above) ?
I'm saying that it of course makes sense to have a way to do sendfile
where you know when reuse is safe, and that we have an API that provides
that very nicely already without needing to add syscalls. If you used
io_uring for this, then the "tx is done, reuse is fine" notification is
just another notification, not anything special that needs new plumbing.
>>>>> I would also argue that there are likely user apps out there that
>>>>> use both sendmsg MSG_ZEROCOPY for certain writes (for data in
>>>>> memory) and also use sendfile (for data on disk). One example would
>>>>> be a reverse proxy that might write HTTP headers to clients via
>>>>> sendmsg but transmit the response body with sendfile.
>>>>>
>>>>> For those apps, the code to check the error queue already exists for
>>>>> sendmsg + MSG_ZEROCOPY, so swapping in sendfile2 seems like an easy
>>>>> way to ensure safe sendfile usage.
>>>>
>>>> Sure that is certainly possible. I didn't say that wasn't the case,
>>>> rather that the error queue approach is a work-around in the first place
>>>> for not having some kind of async notification mechanism for when it's
>>>> free to reuse.
>>>
>>> Of course, I certainly agree that the error queue is a work around.
>>> But it works, app use it, and its fairly well known. I don't see any
>>> reason, other than historical context, why sendmsg can use this
>>> mechanism, splice can, but sendfile shouldn't?
>>
>> My argument would be the same as for other features - if you can do it
>> simpler this other way, why not consider that? The end result would be
>> the same, you can do fast sendfile() with sane buffer reuse. But the
>> kernel side would be simpler, which is always a kernel main goal for
>> those of us that have to maintain it.
>>
>> Just adding sendfile2() works in the sense that it's an easier drop in
>> replacement for an app, though the error queue side does mean it needs
>> to change anyway - it's not just replacing one syscall with another. And
>> if we want to be lazy, sure that's fine. I just don't think it's the
>> best way to do it when we literally have a mechanism that's designed for
>> this and works with reuse already with normal send zc (and receive side
>> too, in the next kernel).
>
> It seems like you've answered the question I asked above and that
> you are suggesting there might be a better and simpler sendfile2
> kernel-side implementation that doesn't rely on splice internals at
> all.
>
> Am I following you? If so, I'll drop the sendfile2 stuff from this
> series and stick with the splice changes only, if you are (at a high
> level) OK with the idea of adding a flag for this to splice.
>
> In the meantime, I'll take a few more reads through the iouring code
> to see if I can work out how sendfile2 might be built on top of that
> instead of splice in the kernel.
Heh I don't know how you jumped to that conclusion based on my feedback,
and seems like it's solidified through other replies. No I'm not saying
that the approach makes sense for the kernel, it makes some vague amount
of sense only on the premise of "oh but this is easy for applications as
they already know how to use sendfile(2)".
--
Jens Axboe
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Stefan Metzmacher @ 2025-03-21 7:55 UTC (permalink / raw)
To: Pavel Begunkov, Jens Axboe, Joe Damato, Christoph Hellwig, netdev,
linux-kernel, linux-fsdevel, edumazet, pabeni, horms, linux-api,
linux-arch, viro, jack, kuba, shuah, sdf, mingo, arnd, brauner,
akpm, tglx, jolsa, linux-kselftest
Cc: David Wei
In-Reply-To: <fbcd759e-2453-4570-a2a0-c9ad67ae9277@gmail.com>
Am 20.03.25 um 11:46 schrieb Pavel Begunkov:
> On 3/19/25 19:15, Stefan Metzmacher wrote:
>> Am 19.03.25 um 19:37 schrieb Jens Axboe:
>>> On 3/19/25 11:45 AM, Joe Damato wrote:
>>>> On Wed, Mar 19, 2025 at 11:20:50AM -0600, Jens Axboe wrote:
> ...
>>> My argument would be the same as for other features - if you can do it
>>> simpler this other way, why not consider that? The end result would be
>>> the same, you can do fast sendfile() with sane buffer reuse. But the
>>> kernel side would be simpler, which is always a kernel main goal for
>>> those of us that have to maintain it.
>>>
>>> Just adding sendfile2() works in the sense that it's an easier drop in
>>> replacement for an app, though the error queue side does mean it needs
>>> to change anyway - it's not just replacing one syscall with another. And
>>> if we want to be lazy, sure that's fine. I just don't think it's the
>>> best way to do it when we literally have a mechanism that's designed for
>>> this and works with reuse already with normal send zc (and receive side
>>> too, in the next kernel).
>>
>> A few month (or even years) back, Pavel came up with an idea
>> to implement some kind of splice into a fixed buffer, if that
>> would be implemented I guess it would help me in Samba too.
>> My first usage was on the receive side (from the network).
>
> I did it as a testing ground for infra needed for ublk zerocopy,
> but if that's of interest I can resurrect the patches and see
> where it goes, especially since the aforementioned infra just got
> queued.
Would be great!
Have you posted the work in progress somewhere?
Thanks!
metze
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Christoph Hellwig @ 2025-03-21 5:56 UTC (permalink / raw)
To: Joe Damato, Christoph Hellwig, Jens Axboe, netdev, linux-kernel,
asml.silence, linux-fsdevel, edumazet, pabeni, horms, linux-api,
linux-arch, viro, jack, kuba, shuah, sdf, mingo, arnd, brauner,
akpm, tglx, jolsa, linux-kselftest
In-Reply-To: <Z9xdPVQeLBrB-Anu@LQ3V64L9R2>
On Thu, Mar 20, 2025 at 11:23:57AM -0700, Joe Damato wrote:
> In my other message to Jens I proposed:
> - SPLICE_F_ZC for splice to generate zc completion notifications
> to the error queue
> - Modifying sendfile so that if SO_ZEROCOPY (which already exists)
> is set on a network socket, zc completion notifications are
> generated.
>
> In both cases no new system call is needed and both splice and
> sendfile become safer to use.
>
> At some point in the future a mechanism built on top of iouring
> introduced as new system calls (sendmsg2, sendfile2, splice2, etc)
> can be built.
I strongly disagree with this. This is spreading the broken
SO_ZEROCOPY to futher places outside the pure networking realm. Don't
do that.
It also doesn't help that more than 7 years after adding it,
SO_ZEROCOPY is still completely undocumented.
> > Because sendmsg should never have done that it certainly should not
> > spread beyond purely socket specific syscalls.
>
> I don't know the entire historical context, but I presume sendmsg
> did that because there was no other mechanism at the time.
At least aio had been around for about 15 years at the point, but
networking folks tend to be pretty insular and reinvent things.
> It seems like Jens suggested that plumbing this through for splice
> was a possibility, but sounds like you disagree.
Yes, very strongly.
> As mentioned above and in other messages, it seems like it is
> possible to improve the networking parts of splice (and therefore
> sendfile) to make them safer to use without introducing a new system
> call.
>
> Are you saying that you are against doing that, even if the code is
> network specific (but lives in fs/)?
Yes.
Please take the work and integrate it with the kiocb-based system
we use for all other in-kernel I/O that needs completion notifications
and which makes it trivial to integate with io_uring instead of
spreading an imcompatible and inferior event system.
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-20 18:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, netdev, linux-kernel, asml.silence, linux-fsdevel,
edumazet, pabeni, horms, linux-api, linux-arch, viro, jack, kuba,
shuah, sdf, mingo, arnd, brauner, akpm, tglx, jolsa,
linux-kselftest
In-Reply-To: <Z9uuSQ7SrigAsLmt@infradead.org>
On Wed, Mar 19, 2025 at 10:57:29PM -0700, Christoph Hellwig wrote:
> On Wed, Mar 19, 2025 at 10:45:22AM -0700, Joe Damato wrote:
> > I don't disagree; I just don't know if app developers:
> > a.) know that this is possible to do, and
> > b.) know how to do it
>
> So if you don't know that why do you even do the work?
I am doing the work because I use splice and sendfile and it seems
relatively straightforward to make them safer using an existing
mechanism, at least for network sockets.
After dropping the sendfile2 patches completely, it looks like in my
new set all of the code is within CONFIG_NET defines in fs/splice.c.
> > In general: it does seem a bit odd to me that there isn't a safe
> > sendfile syscall in Linux that uses existing completion notification
> > mechanisms.
>
> Agreed. Where the existing notification mechanism is called io_uring.
Sure. As I mentioned to Jens: I agree that any new system call
should be built differently.
But does that mean we should leave splice and sendfile as-is when
there is a way to potentially make them safer?
In my other message to Jens I proposed:
- SPLICE_F_ZC for splice to generate zc completion notifications
to the error queue
- Modifying sendfile so that if SO_ZEROCOPY (which already exists)
is set on a network socket, zc completion notifications are
generated.
In both cases no new system call is needed and both splice and
sendfile become safer to use.
At some point in the future a mechanism built on top of iouring
introduced as new system calls (sendmsg2, sendfile2, splice2, etc)
can be built.
> > Of course, I certainly agree that the error queue is a work around.
> > But it works, app use it, and its fairly well known. I don't see any
> > reason, other than historical context, why sendmsg can use this
> > mechanism, splice can, but sendfile shouldn't?
>
> Because sendmsg should never have done that it certainly should not
> spread beyond purely socket specific syscalls.
I don't know the entire historical context, but I presume sendmsg
did that because there was no other mechanism at the time.
I will explain it more clearly in the next cover letter, but the way
I see the situation is:
- There are existing system calls which operate on network sockets
(splice and sendfile) that avoid copies
- There is a mechanism already in the kernel in the networking
stack for generating completion notifications
- Both splice and sendfile could be extended to support this for
network sockets so they can be used more safely, without
introducing a new system call
> > If you feel very strongly that this cannot be merged without
> > dropping sendfile2 and only plumbing this through for splice, then
> > I'll drop the sendfile2 syscall when I submit officially (probably
> > next week?).
>
> Splice should also not do "error queue notifications". Nothing
> new and certainly nothing outside of net/ should.
It seems like Jens suggested that plumbing this through for splice
was a possibility, but sounds like you disagree.
Not really sure how to proceed here?
If code I am modifying is within CONFIG_NET defines, but lives in
fs/splice.c ... is that within the realm of net or fs ?
I am asking because I genuinely don't know.
As mentioned above and in other messages, it seems like it is
possible to improve the networking parts of splice (and therefore
sendfile) to make them safer to use without introducing a new system
call.
Are you saying that you are against doing that, even if the code is
network specific (but lives in fs/)?
> > I do feel pretty strongly that it's more likely apps would use
> > sendfile2 and we'd have safer apps out in the wild. But, I could be
> > wrong.
>
> A purely synchronous sendfile that is safe is a good thing. Spreading
> non-standard out of band notifications is not. How to build that
> safe sendmsg is a good question, and a sendmsg2 might be a sane
> option for that. The important thing is that the underlying code
> should use iocbs and ki_complete to notify I/O completion so that
> all the existing infrastucture like io_uring and in-kernel callers
> can reuse this.
I'm not currently planning to build sendmsg2 (and I've already
mentioned to Jens and above I will drop sendfile2), but if I have the time
it sounds like an interesting project.
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Joe Damato @ 2025-03-20 18:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: netdev, linux-kernel, asml.silence, linux-fsdevel, edumazet,
pabeni, horms, linux-api, linux-arch, viro, jack, kuba, shuah,
sdf, mingo, arnd, brauner, akpm, tglx, jolsa, linux-kselftest
In-Reply-To: <Z9usmpFw7y75eOhk@infradead.org>
On Wed, Mar 19, 2025 at 10:50:18PM -0700, Christoph Hellwig wrote:
> On Wed, Mar 19, 2025 at 08:32:19AM -0700, Joe Damato wrote:
> > See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
> > sendmsg and passes MSG_ZEROCOPY a completion notification is added
> > to the error queue. The user app can poll for these to find out when
> > the TX has completed and the buffer it passed to the kernel can be
> > overwritten.
>
> Yikes. That's not just an ugly interface, but something entirely
> specific to sockets and incompatible with all other asynchronous I/O
> interfaces.
I don't really know but I would assume it was introduced, as Jens
said, as a work-around long before other completion mechanisms
existed.
> > > and why aren't you simply plugging this into io_uring and generate
> > > a CQE so that it works like all other asynchronous operations?
> >
> > I linked to the iouring work that Pavel did in the cover letter.
> > Please take a look.
>
> Please write down what matters in the cover letter, including all the
> important tradeoffs.
OK, I will enhance the cover letter for the next submission. I had
originally thought I'd submit something officially, but I think I'll
probably submit another RFC with some of the changes I've made based
on the discussion with Jens.
Namely: dropping sendfile2 completely and plumbing the bits through
for splice. I'll wait a bit to hear what Jens thinks about the
SO_ZEROCOPY thing (basically: if a network socket has that option
set, maybe the existing sendfile can generate error queue
completions without needing a separate system call?).
I agree overall that sendfile2 or sendmsg2 or whatever else could
likely be built differently now that better interfaces and
mechanisms exist in the kernel - but I still think there's room to
improve existing system calls so they can be used safely.
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Pavel Begunkov @ 2025-03-20 10:46 UTC (permalink / raw)
To: Stefan Metzmacher, Jens Axboe, Joe Damato, Christoph Hellwig,
netdev, linux-kernel, linux-fsdevel, edumazet, pabeni, horms,
linux-api, linux-arch, viro, jack, kuba, shuah, sdf, mingo, arnd,
brauner, akpm, tglx, jolsa, linux-kselftest
Cc: David Wei
In-Reply-To: <356ce660-fc2e-4016-a0d9-6896936669c2@samba.org>
On 3/19/25 19:15, Stefan Metzmacher wrote:
> Am 19.03.25 um 19:37 schrieb Jens Axboe:
>> On 3/19/25 11:45 AM, Joe Damato wrote:
>>> On Wed, Mar 19, 2025 at 11:20:50AM -0600, Jens Axboe wrote:
...
>> My argument would be the same as for other features - if you can do it
>> simpler this other way, why not consider that? The end result would be
>> the same, you can do fast sendfile() with sane buffer reuse. But the
>> kernel side would be simpler, which is always a kernel main goal for
>> those of us that have to maintain it.
>>
>> Just adding sendfile2() works in the sense that it's an easier drop in
>> replacement for an app, though the error queue side does mean it needs
>> to change anyway - it's not just replacing one syscall with another. And
>> if we want to be lazy, sure that's fine. I just don't think it's the
>> best way to do it when we literally have a mechanism that's designed for
>> this and works with reuse already with normal send zc (and receive side
>> too, in the next kernel).
>
> A few month (or even years) back, Pavel came up with an idea
> to implement some kind of splice into a fixed buffer, if that
> would be implemented I guess it would help me in Samba too.
> My first usage was on the receive side (from the network).
I did it as a testing ground for infra needed for ublk zerocopy,
but if that's of interest I can resurrect the patches and see
where it goes, especially since the aforementioned infra just got
queued.
> But the other side might also be possible now we have RWF_DONTCACHE.
> Instead of dropping the pages from the page cache, it might
> be possible move them to fixed buffer instead.
> It would mean the pages would be 'stable' when they are
> no longer part of the pagecache.
> But maybe my assumption for that is too naive...
That's an interesting idea
> Anyway that splice into a fixed buffer would great to have,
> as the new IORING_OP_RECV_ZC, requires control over the
> hardware queues of the nic and only allows a single process
Right, it basically borrows a hardware rx queue and that
needs CAP_NET_ADMIN, and the user also has to set up steering
rules.
> to provide buffers for that receive queue (at least that's how
> I understand it). And that's not possible for multiple process
> (maybe not belonging to the same high level application and likely
It's up to the user to decide who returns buffers back (and how to
sychronise that) as the api is just a user mapped ring. Regardless,
it's not a finished project, David and I looked at features we want
to add to make life easier for multithreaded apps that can't throw
that many queues. I see your point though.
> non-root applications). So it would be great have splice into
> fixed buffer as alternative to IORING_OP_SPLICE/IORING_OP_TEE,
> as it would be more flexible to use in combination with
> IORING_OP_SENDMSG_ZC as well as IORING_OP_WRITE[V]_FIXED with RWF_DONTCACHE.
>
> I guess such a splice into fixed buffer linked to IORING_OP_SENDMSG_ZC
> would be the way to simulate the sendfile2() in userspace?
Right, and that approach allows to handle intermediate errors,
which is why it doesn't need to put restrictions on the input
file.
--
Pavel Begunkov
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Christoph Hellwig @ 2025-03-20 5:57 UTC (permalink / raw)
To: Joe Damato, Jens Axboe, Christoph Hellwig, netdev, linux-kernel,
asml.silence, linux-fsdevel, edumazet, pabeni, horms, linux-api,
linux-arch, viro, jack, kuba, shuah, sdf, mingo, arnd, brauner,
akpm, tglx, jolsa, linux-kselftest
In-Reply-To: <Z9sCsooW7OSTgyAk@LQ3V64L9R2>
On Wed, Mar 19, 2025 at 10:45:22AM -0700, Joe Damato wrote:
> I don't disagree; I just don't know if app developers:
> a.) know that this is possible to do, and
> b.) know how to do it
So if you don't know that why do you even do the work?
> In general: it does seem a bit odd to me that there isn't a safe
> sendfile syscall in Linux that uses existing completion notification
> mechanisms.
Agreed. Where the existing notification mechanism is called io_uring.
> Of course, I certainly agree that the error queue is a work around.
> But it works, app use it, and its fairly well known. I don't see any
> reason, other than historical context, why sendmsg can use this
> mechanism, splice can, but sendfile shouldn't?
Because sendmsg should never have done that it certainly should not
spread beyond purely socket specific syscalls.
> If you feel very strongly that this cannot be merged without
> dropping sendfile2 and only plumbing this through for splice, then
> I'll drop the sendfile2 syscall when I submit officially (probably
> next week?).
Splice should also not do "error queue notifications". Nothing
new and certainly nothing outside of net/ should.
> I do feel pretty strongly that it's more likely apps would use
> sendfile2 and we'd have safer apps out in the wild. But, I could be
> wrong.
A purely synchronous sendfile that is safe is a good thing. Spreading
non-standard out of band notifications is not. How to build that
safe sendmsg is a good question, and a sendmsg2 might be a sane
option for that. The important thing is that the underlying code
should use iocbs and ki_complete to notify I/O completion so that
all the existing infrastucture like io_uring and in-kernel callers
can reuse this.
Note that this also ties into the currently broken memory mamangement
in the networking code that directly messeѕ with page references
rather than notifying the caller about I/O completion.
^ permalink raw reply
* Re: [RFC -next 00/10] Add ZC notifications to splice and sendfile
From: Christoph Hellwig @ 2025-03-20 5:50 UTC (permalink / raw)
To: Joe Damato, Christoph Hellwig, netdev, linux-kernel, asml.silence,
linux-fsdevel, edumazet, pabeni, horms, linux-api, linux-arch,
viro, jack, kuba, shuah, sdf, mingo, arnd, brauner, akpm, tglx,
jolsa, linux-kselftest
In-Reply-To: <Z9rjgyl7_61Ddzrq@LQ3V64L9R2>
On Wed, Mar 19, 2025 at 08:32:19AM -0700, Joe Damato wrote:
> See the docs on MSG_ZEROCOPY [1], but in short when a user app calls
> sendmsg and passes MSG_ZEROCOPY a completion notification is added
> to the error queue. The user app can poll for these to find out when
> the TX has completed and the buffer it passed to the kernel can be
> overwritten.
Yikes. That's not just an ugly interface, but something entirely
specific to sockets and incompatible with all other asynchronous I/O
interfaces.
> > and why aren't you simply plugging this into io_uring and generate
> > a CQE so that it works like all other asynchronous operations?
>
> I linked to the iouring work that Pavel did in the cover letter.
> Please take a look.
Please write down what matters in the cover letter, including all the
important tradeoffs.
^ 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