From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Chinner Subject: Re: Argument type for FS_IOC_GETFLAGS/FS_IOC_SETFLAGS ioctls Date: Fri, 29 Nov 2013 16:27:48 +1100 Message-ID: <20131129052748.GV10988@dastard> References: <20131126200559.GH20559@hall.aurel32.net> <20131127010141.GA10273@birch.djwong.org> <20131127040013.GA19941@thunk.org> <20131129045412.GA18142@thunk.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Andreas Dilger , "Darrick J. Wong" , Aurelien Jarno , Alexander Viro , linux-fsdevel , Robert Edmonds , Rob Browning To: Theodore Ts'o Return-path: Received: from ipmail04.adl6.internode.on.net ([150.101.137.141]:20252 "EHLO ipmail04.adl6.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752201Ab3K2F14 (ORCPT ); Fri, 29 Nov 2013 00:27:56 -0500 Content-Disposition: inline In-Reply-To: <20131129045412.GA18142@thunk.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Nov 28, 2013 at 11:54:12PM -0500, Theodore Ts'o wrote: > On Thu, Nov 28, 2013 at 05:53:10PM -0700, Andreas Dilger wrote: > > > > Note that there are already FS_IOC32_{GET,SET}{VERSION,FLAGS} ioctl > > definitions for that date back to 2.6.19 or so that correctly use "int" > > for the size argument. Those are unfortunately(?) under CONFIG_COMPAT > > instead of just being inline with the normal ioctl definitions, so I'm > > not sure if those are available on the systems in question. CONFIG_COMPAT > > was the default for RHEL5 and SLES10 kernels, but the compat ioctl code > > was only in ext4 and not ext3 in RHEL5 at least. > > This were introduced to support 32-bit userspace programs (where > sizeof(long) == sizeof(int) == 4) with a 64-bit kernel. The intent > was not to "fix" the ioctl, so much as it was to enable 32-bit > programs. The compat code is in ext3 as well, although it uses > EXT3_IOC32_[SG]ETFLAGS. > > > I suspect those have already been around long enough for chattr/lsattr to > > start trying to use them first, and fall back to using the "broken" IOC > > numbers if they fail. > > Nope, chattr/lsattr does not try using them first, because the intent > wasn't to "fix" the "broken" IOC numbers. If you look in the sources, > e2fsprogs is only using EXT2_IOC_[SG]ETFLAGS. (These ioctls were > originally only defined for ext2, and intended for use only for > ext[23]. They got adopted by other file systems, and then they get > moved into linux/fs..) > > > > P.S. If we were going to create a new ioctl, what I'd suggest is that > > > the new ioctl explicitly use a 64-bit type, instead of using "long" or > > > "int", to avoid the compat ioctl hair to allow 64-bit kernels to > > > support 32-bit userspace programs. > > > > Unfortunately, I don't think it would be possible to just use: > > > > #define FS_IOC_GETFLAGS_NEW _IOR('f', 1, __u64) > > > > since this would conflict with the existing "long" definition on 64-bit > > platforms that actually expect an "int" argument. It definitely would > > be desirable to get a 64-bit attributes API, since we are very close to > > running out of space in the 32-bit flags definitions. > > Sure, I was thinking about doing something like this instead: > > #define FS_IOC_GETFLAGS_WIDE _IOR('f', 32, __u64) > #define FS_IOC_SETFLAGS_WIDE _IOR('f', 32, __u64) > > And I agree that a good reason to do this is to get 64 bits worth of > attributes.... Why create a new ioctl for getting these generic attributes out of the kernel? Isn't that the problem xstat() is supposed to solve? And if it's truly generic stuff, then a syscall pair with enough bitmap space for the forseeable future is more appropriate than a new ioctl.... Cheers, Dave. -- Dave Chinner david@fromorbit.com