Messages in this thread | | | From | Linus Torvalds <> | Date | Tue, 14 Nov 2017 17:11:47 -0800 | Subject | Re: [GIT PULL] ext4 updates for 4.15 |
| |
On Tue, Nov 14, 2017 at 4:56 PM, Theodore Ts'o <tytso@mit.edu> wrote: > > 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.
So I don't think you'd necessarily need to convert from one to the other, but wouldn't it be nice if you at least defined one in terms of the other, ie something like
#define EXT4_ENCRYPT_FL (1u << EXT4_INODE_ENCRYPT)
so that when you grep for one you see how they are directly related.
Now it was much less obvious, and I was nervous because that whole series did introduce _different_ bits that were not in the same space at all, and encoded the same thing (ie that S_ENCRYPTED bit).
Maybe this normally doesn't come up, but it was not all that obvious, particularly since there was a lot of indirection:
ext4_encrypted_inode() -> ext4_test_inode_flag(inode, EXT4_INODE_ENCRYPT) -> EXT4_INODE_BIT_FNS()
That EXT4_INODE_BIT_FNS thing was really fascinating to see.
So just confirming that yes,
ext4_encrypted_inode()
is the same thing as
EXT4_I(inode)->i_flags & EXT4_ENCRYPT_FL
was a real adventure.
Making it clear that EXT4_ENCRYPT_FL and EXT4_INODE_ENCRYPT are the same bit would maybe have lessened the confusion at least a tiny bit.
Of course, not having five different ways to test the same bit would have been even better.
Ok, I'm exaggerating.
But there really does seem to be a lot of different ways to check i_flags bits, with some uses checking it directly, the places _setting_ it using ext4_set_inode_flag(), and then other testers using bit-specific helper.
And that somewhat confusing model seems to be true of pretty much all the bits.
As long as you can keep track of it, I guess it's fine.
Linus
| |