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.
--
Jeff Layton <jlayton@xxxxxxxxxx>
