From mboxrd@z Thu Jan 1 00:00:00 1970 From: Theodore Ts'o Subject: Re: Argument type for FS_IOC_GETFLAGS/FS_IOC_SETFLAGS ioctls Date: Thu, 28 Nov 2013 23:54:12 -0500 Message-ID: <20131129045412.GA18142@thunk.org> References: <20131126200559.GH20559@hall.aurel32.net> <20131127010141.GA10273@birch.djwong.org> <20131127040013.GA19941@thunk.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "Darrick J. Wong" , Aurelien Jarno , Alexander Viro , linux-fsdevel , Robert Edmonds , Rob Browning To: Andreas Dilger Return-path: Received: from imap.thunk.org ([74.207.234.97]:34595 "EHLO imap.thunk.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750972Ab3K2EyY (ORCPT ); Thu, 28 Nov 2013 23:54:24 -0500 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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.... Cheers, - Ted