On Wed 16-07-25 14:19:57, 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.
Heh, I feel biased because about 10 years ago I proposed something like
this and it got shot down as being too ugly to be worth the memory. But
times are changing :). I think hooks to return pointer are cleaner but this
is clearly going to be faster. I think since this would be fully
encapsulated in the few inline helpers the offset solution is (still) OK.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
