On Wed, Jul 16, 2025 at 08:38:08AM -0400, Jeff Layton wrote:
> On Wed, 2025-07-16 at 14:19 +0200, Christian Brauner wrote:
> > On Wed, Jul 16, 2025 at 01:21:49PM +0200, Christoph Hellwig wrote:
> > > On Tue, Jul 15, 2025 at 04:35:24PM +0200, Christian Brauner wrote:
> > > > 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.
> > >
> > > I like getting rid of the fields. But adding all these indirect calls
> > > is a bit nasty.
> > >
> > > Given that all these fields should be in the file system specific inode
> > > that also embeddeds the vfs struct inode, what about just putting the
> > > relative offset into struct inode_operations.
> > >
> > > e.g. something like
> > >
> > > struct inode_operations {
> > > ...
> > > ptrdiff_t i_crypto_offset;
> > > }
> > >
> > > struct inode_operations foofs_iops {
> > > ...
> > >
> > > .i_crypto_offset = offsetoff(struct foofs_inode), vfs_inode) -
> > > offsetoff(struct foofs_inode, crypt_info);
> > > }
> > >
> > > static inline struct fscrypt_inode_info CRYPT_I(struct inode *inode)
> > > {
> > > return ((void *)inode) - inode->i_op->i_cryto_offset;
> > > }
> >
> > Sheesh, ugly in a different way imho. I could live with it. I'll let
> > @Jan be the tie-breaker.
> >
> > I've started working on this so best tell me soon... :)
>
> I agree with HCH. Both of these methods are equally ugly, but we
> eliminate extra function call by just storing the offset.
>
> It may also make things simpler for debugging with drgn and the like
> too. You can just do the pointer math with the info inside struct inode
> instead of having to track down an extra function call, etc.
struct inode {
...
};
struct filemap_inode {
struct inode inode;
struct address_space i_mapping;
struct fscrypt_struct i_fscrypt;
struct fsverity_struct i_fsverity;
struct quota_struct i_quota;
};
struct ext4_inode {
struct filemap_inode inode;
...
};
saves any messing with i_ops and offsets.
