From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755982AbdKOA5I (ORCPT ); Tue, 14 Nov 2017 19:57:08 -0500 Received: from imap.thunk.org ([74.207.234.97]:45838 "EHLO imap.thunk.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753282AbdKOA5D (ORCPT ); Tue, 14 Nov 2017 19:57:03 -0500 Date: Tue, 14 Nov 2017 19:56:56 -0500 From: "Theodore Ts'o" To: Linus Torvalds Cc: Linux Kernel Mailing List , "linux-ext4@vger.kernel.org" Subject: Re: [GIT PULL] ext4 updates for 4.15 Message-ID: <20171115005656.6znjpa3ktqdre6dp@thunk.org> Mail-Followup-To: Theodore Ts'o , Linus Torvalds , Linux Kernel Mailing List , "linux-ext4@vger.kernel.org" References: <20171113031502.f6mctmlmgk5psh77@thunk.org> <20171113162534.5xta72w2boeaxk3s@thunk.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) X-SA-Exim-Connect-IP: X-SA-Exim-Mail-From: tytso@thunk.org X-SA-Exim-Scanned: No (on imap.thunk.org); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 14, 2017 at 12:59:17PM -0800, Linus Torvalds wrote: > Of course, > > (flags & EXT4_ENCRYPT_FL) > > _should_ be the same as > > ext4_test_inode_flag(inode, EXT4_INODE_ENCRYPT); And in the second is the preferred way to do things, actually. > I'll do that suggested resolution, but I have to say that the ext4 bit > testing is incredibly broken and non-obvious. Just as an example: > > fs/ext4/ext4.h:#define EXT4_ENCRYPT_FL 0x00000800 > /* encrypted file */ > fs/ext4/ext4.h: EXT4_INODE_ENCRYPT = 11, /* Encrypted file */ > > yeah, it's the same bit, but it sure as hell isn't obvious. Why the > two totally different ways to define that data? Yes, it's non-obvious and ugly. Sorry about that. We originally used EXT4_*_FL, and we needed to use the bit number encoding so we could use test_bit(). We just never converted all the way over. We do have a way to make sure the two ways of defining a bit position are in sync; see ext4_check_flag_values() and CHECK_FLAG_VALUE in ext4.h. It's a bit gross, and we probably should clean this up, at least in the kernel. The e2fsprogs user space libraries all use EXT4_*_FL, and we can't change that without breaking applications depending on userspace, but we can keep things consistent in the kernel, and that probably means completely converting away from EXT4_*_FL, if possible. - Ted