Re: [PATCH RFC DRAFT DOESNOTBUILD] inode: free up more space

[Eric Biggers <ebiggers@kernel.org>]

On Tue, Jul 15, 2025 at 04:35:24PM +0200, Christian Brauner wrote:
> struct inode is bloated as everyone is aware and we should try and
> shrink it as that's potentially a lot of memory savings. I've already
> freed up around 8 bytes but we can probably do better.
> 
> There's a bunch of stuff that got shoved into struct inode that I don't
> think deserves a spot in there. There are two members I'm currently
> particularly interested in:
> 
> (1) #ifdef CONFIG_FS_ENCRYPTION
>             struct fscrypt_inode_info *i_crypt_info;
>     #endif
> 
>     ceph, ext4, f2fs, ubifs
> 
> (2) #ifdef CONFIG_FS_VERITY
>             struct fsverity_info *i_verity_info;
>     #endif
> 
>     btrfs, ext4, f2fs
> 
> So we have 4 users for fscrypt and 3 users for fsverity with both
> features having been around for a decent amount of time.
> 
> For all other filesystems the 16 bytes are just wasted bloating inodes
> for every pseudo filesystem and most other regular filesystems.
> 
> We should be able to move both of these out of struct inode by adding
> inode operations and making it the filesystem's responsibility to
> accommodate the information in their respective inodes.
> 
> Unless there are severe performance penalties for the extra pointer
> dereferences getting our hands on 16 bytes is a good reason to at least
> consider doing this.
> 
> I've drafted one way of doing this using ext4 as my victim^wexample. I'd
> like to hear some early feedback whether this is something we would want
> to pursue.
> 
> Build failures very much expected!
> 
> Not-Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/crypto/bio.c             |  2 +-
>  fs/crypto/crypto.c          |  8 ++++----
>  fs/crypto/fname.c           |  8 ++++----
>  fs/crypto/fscrypt_private.h |  3 +--
>  fs/crypto/hooks.c           |  2 +-
>  fs/crypto/inline_crypt.c    | 10 +++++-----
>  fs/crypto/keysetup.c        | 21 ++++----------------
>  fs/crypto/policy.c          |  8 ++++----
>  fs/ext4/ext4.h              |  9 +++++++++
>  fs/ext4/file.c              |  4 ++++
>  fs/ext4/namei.c             | 22 +++++++++++++++++++++
>  fs/ext4/super.c             |  6 +++++-
>  fs/ext4/symlink.c           | 12 ++++++++++++
>  include/linux/fs.h          |  9 +++++----
>  include/linux/fscrypt.h     | 47 ++++++++++++++++++++++++++++++---------------
>  15 files changed, 112 insertions(+), 59 deletions(-)

This should have been Cc'ed to linux-fscrypt@vger.kernel.org and
fsverity@lists.linux.dev.  I almost missed this.

If done properly, fixing this would be great.  I've tried to minimize
the overhead of CONFIG_FS_ENCRYPTION and CONFIG_FS_VERITY when those
features are not actually being used at runtime.  The struct inode
fields are the main case where we still don't do a good job at that.

However, as was mentioned elsewhere in the thread, unfortunately
indirect calls are really expensive and would not be a great solution.
I've been cleaning up indirect calls in other parts of the kernel, such
as the crypto and crc subsystems, and getting some significant
performance improvements from that.  It would be unfortunate to add a
large number of indirect calls for basically all operations done on
encrypted and/or verity files.

Doing the dereferences to get an offset stored in the inode_operations
(or fscrypt_operations or fsverity_operations, or maybe even super_block
which would just need one dereference rather than two?), and then doing
the pointer arithmetic, would be faster than an indirect call.  It won't
be all that great either, but it would do.

There are some cases where we could instead modify the functions in
fs/crypto/ to have the filesystem pass in a pointer to the
fscrypt_inode_info.  But that won't work in all cases.

Just throwing another idea out there: there could also be an
optimization for the case where only a single *filesystem type* is using
fscrypt (or fsverity) at runtime, which is the usual case.  This would
look something like:

    if (static_branch_likely(&fscrypt_single_fs_type))
            ci = *(struct fscrypt_inode_info **)((void *)inode + fscrypt_crypt_info_offset);
    else
            ci = // get it in slower way.  Maybe even just the indirect call.

In theory, the first case could even use a "runtime_const" for
fscrypt_crypt_info_offset.  In that case, the only difference between
the fast code path (where fscrypt_single_fs_type==true) and the current
inode->i_crypt_info would be the nop associated with the static branch.

But, that might be too fancy.  For now we probably should go with a
single code path that loads the offset from a per-filesystem-instance
struct, like inode_operations or super_block.

- Eric