Re: [PATCH v2 15/20] btrfs: store a fscrypt extent context per normal file extent

David Sterba
2022-09-09T10:10:17Z

On Wed, Sep 07, 2022 at 05:39:04PM -0400, Sweet Tea Dorminy wrote:
>
> > FSCRYPT_EXTENT_CONTEXT_MAX_SIZE is 33 and btrfs_fscrypt_extent_context
> > is part of extent_map, that's quite common object. Random sample from
> > my
> > desktop right now is around 800k objects so this is noticeable. Needs a
> > second look.
> >
> >> +
> >> extern const struct fscrypt_operations btrfs_fscrypt_ops;
> >> #endif /* BTRFS_FSCRYPT_H */
> >> --- a/fs/btrfs/ordered-data.h
> >> +++ b/fs/btrfs/ordered-data.h
> >> @@ -99,6 +99,7 @@ struct btrfs_ordered_extent {
> >> u64 disk_bytenr;
> >> u64 disk_num_bytes;
> >> u64 offset;
> >> + struct btrfs_fscrypt_extent_context fscrypt_context;
> >
> > And another embedded btrfs_fscrypt_extent_context, that can also get a
> > lot of slab objects.
>
> I could certainly define fscrypt_extent_context's as a separate btree
> object type, and/or have them be separately allocated and just have a
> pointer in the various structures to keep track of them. I didn't have a
> separate object for them since its only a 17 or 33 byte object (at
> present) on a per-btrfs_file_extent basis, but maybe that would be
> better?

btrfs_ordered_extent is in memory so I don't know what you mean by a
btree object here, that's used for on-disk structures. All the new
fscrypt code adds a lot of stack space and increases data structures but
that's something that needs to be effectively, so if the pointers can be
shared then please find a way. I still don't have the whole picture so
can't say what would be best, there are still other things to address so
once that's done we can focus on the memory consumption. You can keep it
as is for now but keep notes that it needs another look later.

> I could also #ifdef CONFIG_FS_ENCRYPTION the member in each structure,
> if that would help over and beyond either of the previous things.

I think all the fscrypt related code should be behind ifdef
CONFIG_FS_ENCRYPTION, so the struct members too.