From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9D2A4275103 for ; Mon, 19 May 2025 11:37:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747654633; cv=none; b=Y3CFiiG+JEJOovHZ+X+0ghRDBZoe61owHySPnO5kfpqRVc18w6P2q9o2ESSxrFt0FzmziPKvfKJBmsFKgVcd8FAPq7RZDNjNGtED0Fj3lZLnAFh3vUIRLUSocl/bv5xmpf4v7Xwua9kyWev9BI2Xke4KvFDvWneCoBZpUpriRWQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747654633; c=relaxed/simple; bh=GFhTh8qt5cp6q80eo9lSCWZXWbRWYcGWMwJc66usfSM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lFG+S3/aykM6ODEhrZKmWdUpf6Np4YMbfjxUAA8wnRR+ucZq6RHq7fD/10Wt1dxqt/gtQceDJKK+92Jp50WzAuhbxkYUiBJuKr3lkQfRAFj/BLpE1oJvXJ0G7ENYel2Xe2VK1cjFkTNJGpP/Z7ncD5y1faTzHCSDc1b4RxvLbJA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=fromorbit.com; spf=pass smtp.mailfrom=fromorbit.com; dkim=pass (2048-bit key) header.d=fromorbit-com.20230601.gappssmtp.com header.i=@fromorbit-com.20230601.gappssmtp.com header.b=3TzfpJjN; arc=none smtp.client-ip=209.85.210.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=fromorbit.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=fromorbit.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=fromorbit-com.20230601.gappssmtp.com header.i=@fromorbit-com.20230601.gappssmtp.com header.b="3TzfpJjN" Received: by mail-pf1-f172.google.com with SMTP id d2e1a72fcca58-7390d21bb1cso3713513b3a.2 for ; Mon, 19 May 2025 04:37:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20230601.gappssmtp.com; s=20230601; t=1747654629; x=1748259429; darn=lists.linux-m68k.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=DkYbc28ZqmxwTCsqpy3spmOraOHqionupf/htIjrysQ=; b=3TzfpJjNg5QqBPkDa1sETHxhGlgNfz+TG6EUA2Bsc6nhrPkaLVkuXE50b5W2cf5SoY 8Ts1XoRXZbOCOh/JeguUiOiYsWWFjFUQo3XxHtk/qMSdQYAWRq264H+BLZqW8JOuMR54 KEIWyPYSYimU5fHB/3fKbi0NgLlcvWgDrmCZCq9HfnQreHmtAzSDhiWiI6qDq9aNBf88 VX9NfsEWa/bdkwYIoeKqxBnub7Um5xpyVE0F8ThuOyMz2USt/K5bf8MNPktBRwSV0evB MzxMpaGEVCSgQKyYqMvhVUxnOtmyOvebf1/7zkRdoCzbNdBDx7CC4meULpt/fDV0Gxj8 klBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747654629; x=1748259429; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=DkYbc28ZqmxwTCsqpy3spmOraOHqionupf/htIjrysQ=; b=rANZsPV3UKPVJV+QqTXC5vvWKBHbhau2JiHvafUIxqHR+gTDfxzmvxi0acrLExWuSv KkrGgFRCeBecY80hHRPJvZPKJCYxPEOa5FeJ7oyL0VNGneD61iFAsgnCCx4CrRB0kJt1 q1gneZ0L8zROVia2rpMR1EKcTlxIoYwL/z0lmA8maVViPD9ivz/ovaq5zRBMAfv/N4kw E5fnTSVECIWGM4lDy3Ra61JGAyMIe9/K/LWHYSPAznXltOiI/SyIG7RxfmoHtj2rgJPN Zu96CYDffZKGYvjWdl+utHE5kXikD07r6DC46o+q8GvG04N/4ELpJKgt/NNJMMXfgA6L gjlg== X-Forwarded-Encrypted: i=1; AJvYcCU7jC1lXLwBaOyBoIxKFuMk75bWo64qvs3Uu7l80gY4EAygUmw54ldskPXBHOwEl9fvYC1NrtEgKrVJ@lists.linux-m68k.org X-Gm-Message-State: AOJu0YyzYeM6TO3S3G8XQDaTE8SaZjrtq6y16tIR30gKQmeFWW3/YiqD WnLMZGwNItcQpPh5yC+tqQ0PnrD251O1N8wa/cwjEubR7naXkd+oktjuaI6QKMTjDjk= X-Gm-Gg: ASbGncsMyk8nKxE8BMC0DCyE0mGLLYqMxZWzlk3rBO+1fukxzg4yQ05juvhps4gHZfr MzTIQpERUW6nvXArkOf7BuI/4GeY3SoxrheSNI2jZjpRmW2ZzYMBv7Kv7f5vedC1SAOFG75KnVN 01zeqlWmXGdC1bVjkTnlnWzpklbnL9ioH8+bem9etCtpDcjX9bSvUMwHYVwtRQeGXNrBU4JVBhD wnIJ0NAy3PsXrskwV1yhqRh/xzgO4yk3wBC9XYSTqllUibIhKiUXo52lmNf7eU8d96lGzqmba73 N26lXujkRVW1QuYd/iHl2w3+3yY80IZD6/mHVssUox2g8qFaGX277pWxZazNxs8oAeUni1pJgWL usRUSsZeFGrUsAcW+U5ufjzXmak0= X-Google-Smtp-Source: AGHT+IEUy25uZy6yhylI082QEDzxGQ+0mJz25NQX6XuPMH4jpAhK+MU3tNtraGB6TKng6SdbDnt0Eg== X-Received: by 2002:a05:6a00:3a20:b0:736:a6e0:e66d with SMTP id d2e1a72fcca58-742a97a6df2mr15123631b3a.6.1747654628667; Mon, 19 May 2025 04:37:08 -0700 (PDT) Received: from dread.disaster.area (pa49-180-184-88.pa.nsw.optusnet.com.au. [49.180.184.88]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-742a973a2f8sm5957134b3a.81.2025.05.19.04.37.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 May 2025 04:37:08 -0700 (PDT) Received: from dave by dread.disaster.area with local (Exim 4.98.2) (envelope-from ) id 1uGyno-00000005Si8-44qA; Mon, 19 May 2025 21:37:04 +1000 Date: Mon, 19 May 2025 21:37:04 +1000 From: Dave Chinner To: Amir Goldstein Cc: Christian Brauner , Arnd Bergmann , 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@kernel.org, "H. Peter Anvin" , Chris Zankel , Max Filippov , Alexander Viro , Jan Kara , =?iso-8859-1?Q?Micka=EBl_Sala=FCn?= , =?iso-8859-1?Q?G=FCnther?= Noack , Pali =?iso-8859-1?Q?Roh=E1r?= , Paul Moore , James Morris , "Serge E. Hallyn" , Stephen Smalley , Ondrej Mosnacek , Tyler Hicks , Miklos Szeredi , linux-alpha@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, sparclinux@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-api@vger.kernel.org, Linux-Arch , selinux@vger.kernel.org, ecryptfs@vger.kernel.org, linux-unionfs@vger.kernel.org, linux-xfs@vger.kernel.org, Andrey Albershteyn Subject: Re: [PATCH v5 0/7] fs: introduce file_getattr and file_setattr syscalls Message-ID: References: <20250513-xattrat-syscall-v5-0-22bb9c6c767f@kernel.org> <399fdabb-74d3-4dd6-9eee-7884a986dab1@app.fastmail.com> <20250515-bedarf-absagen-464773be3e72@brauner> Precedence: bulk X-Mailing-List: linux-m68k@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Thu, May 15, 2025 at 12:33:31PM +0200, Amir Goldstein wrote: > On Thu, May 15, 2025 at 11:02 AM Christian Brauner wrote: > > > > On Tue, May 13, 2025 at 11:53:23AM +0200, Arnd Bergmann wrote: > > > On Tue, May 13, 2025, at 11:17, Andrey Albershteyn wrote: > > > > > > > > > > > long syscall(SYS_file_getattr, int dirfd, const char *pathname, > > > > struct fsxattr *fsx, size_t size, unsigned int at_flags); > > > > long syscall(SYS_file_setattr, int dirfd, const char *pathname, > > > > struct fsxattr *fsx, size_t size, unsigned int at_flags); > > > > > > I don't think we can have both the "struct fsxattr" from the uapi > > > headers, and a variable size as an additional argument. I would > > > still prefer not having the extensible structure at all and just > > > > We're not going to add new interfaces that are fixed size unless for the > > very basic cases. I don't care if we're doing that somewhere else in the > > kernel but we're not doing that for vfs apis. > > > > > use fsxattr, but if you want to make it extensible in this way, > > > it should use a different structure (name). Otherwise adding > > > fields after fsx_pad[] would break the ioctl interface. > > > > Would that really be a problem? Just along the syscall simply add > > something like: > > > > diff --git a/fs/ioctl.c b/fs/ioctl.c > > index c91fd2b46a77..d3943805c4be 100644 > > --- a/fs/ioctl.c > > +++ b/fs/ioctl.c > > @@ -868,12 +868,6 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd, > > case FS_IOC_SETFLAGS: > > return ioctl_setflags(filp, argp); > > > > - case FS_IOC_FSGETXATTR: > > - return ioctl_fsgetxattr(filp, argp); > > - > > - case FS_IOC_FSSETXATTR: > > - return ioctl_fssetxattr(filp, argp); > > - > > case FS_IOC_GETFSUUID: > > return ioctl_getfsuuid(filp, argp); > > > > @@ -886,6 +880,20 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd, > > break; > > } > > > > + switch (_IOC_NR(cmd)) { > > + case _IOC_NR(FS_IOC_FSGETXATTR): > > + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FS_IOC_FSGETXATTR))) > > + return SOMETHING_SOMETHING; > > + /* Only handle original size. */ > > + return ioctl_fsgetxattr(filp, argp); > > + > > + case _IOC_NR(FFS_IOC_FSSETXATTR): > > + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FFS_IOC_FSSETXATTR))) > > + return SOMETHING_SOMETHING; > > + /* Only handle original size. */ > > + return ioctl_fssetxattr(filp, argp); > > + } > > + > > I think what Arnd means is that we will not be able to change struct > sfxattr in uapi > going forward, because we are not going to deprecate the ioctls and There's no need to deprecate anything to rev an ioctl API. We have had to solve this "changing struct size" problem previously in XFS ioctls. See XFS_IOC_FSGEOMETRY and the older XFS_IOC_FSGEOMETRY_V4 and XFS_IOC_FSGEOMETRY_V1 versions of the API/ABI. If we need to increase the structure size, we can rename the existing ioctl and struct to fix the version in the API, then use the original name for the new ioctl and structure definition. The only thing we have to make sure of is that the old and new structures have exactly the same overlapping structure. i.e. extension must always be done by appending new varibles, they can't be put in the middle of the structure. This way applications being rebuild will pick up the new definition automatically when the system asserts that it is suppored, whilst existing binaries will always still be supported by the kernel. If the application wants/needs to support all possible kernels, then if XFS_IOC_FSGEOMETRY is not supported, call XFS_IOC_FSGEOMETRY_V4, and if that fails (only on really old irix!) or you only need something in that original subset, call XFS_IOC_FSGEOMETRY_V1 which will always succeed.... > Should we will need to depart from this struct definition and we might > as well do it for the initial release of the syscall rather than later on, e.g.: > > --- a/include/uapi/linux/fs.h > +++ b/include/uapi/linux/fs.h > @@ -148,6 +148,17 @@ struct fsxattr { > unsigned char fsx_pad[8]; > }; > > +/* > + * Variable size structure for file_[sg]et_attr(). > + */ > +struct fsx_fileattr { > + __u32 fsx_xflags; /* xflags field value (get/set) */ > + __u32 fsx_extsize; /* extsize field value (get/set)*/ > + __u32 fsx_nextents; /* nextents field value (get) */ > + __u32 fsx_projid; /* project identifier (get/set) */ > + __u32 fsx_cowextsize; /* CoW extsize field value (get/set)*/ > +}; > + > +#define FSXATTR_SIZE_VER0 20 > +#define FSXATTR_SIZE_LATEST FSXATTR_SIZE_VER0 If all the structures overlap the same, all that is needed in the code is to define the structure size that should be copied in and parsed. i.e: case FSXATTR..._V1: return ioctl_fsxattr...(args, sizeof(fsx_fileattr_v1)); case FSXATTR..._V2: return ioctl_fsxattr...(args, sizeof(fsx_fileattr_v2)); case FSXATTR...: return ioctl_fsxattr...(args, sizeof(fsx_fileattr)); -Dave. -- Dave Chinner david@fromorbit.com