Re: [PATCH v2 14/17] btrfs: create and free extent fscrypt_infos

[Josef Bacik <josef@toxicpanda.com>]

On Sun, Jul 16, 2023 at 11:52:45PM -0400, Sweet Tea Dorminy wrote:
> Each extent_map will end up with a pointer to its associated
> fscrypt_info if any, which should have the same lifetime as the
> extent_map. Add creation of fscrypt_infos for new extent_maps, and
> freeing as appropriate.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/btrfs/extent_map.c | 8 ++++++++
>  fs/btrfs/extent_map.h | 3 +++
>  fs/btrfs/inode.c      | 8 ++++++++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index 0cdb3e86f29b..4fa5f97559da 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -65,6 +65,8 @@ void free_extent_map(struct extent_map *em)
>  	if (!em)
>  		return;
>  	if (refcount_dec_and_test(&em->refs)) {
> +		if (em->fscrypt_info)
> +			fscrypt_free_extent_info(&em->fscrypt_info);
>  		WARN_ON(extent_map_in_tree(em));
>  		WARN_ON(!list_empty(&em->list));
>  		if (test_bit(EXTENT_FLAG_FS_MAPPING, &em->flags))
> @@ -207,6 +209,12 @@ static int mergable_maps(struct extent_map *prev, struct extent_map *next)
>  	if (!list_empty(&prev->list) || !list_empty(&next->list))
>  		return 0;
>  
> +	/*
> +	 * Don't merge adjacent encrypted maps.
> +	 */
> +	if (prev->fscrypt_info || next->fscrypt_info)
> +		return 0;
> +
>  	ASSERT(next->block_start != EXTENT_MAP_DELALLOC &&
>  	       prev->block_start != EXTENT_MAP_DELALLOC);
>  
> diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
> index 35d27c756e08..3a1b66b1cedf 100644
> --- a/fs/btrfs/extent_map.h
> +++ b/fs/btrfs/extent_map.h
> @@ -27,6 +27,8 @@ enum {
>  	EXTENT_FLAG_FS_MAPPING,
>  	/* This em is merged from two or more physically adjacent ems */
>  	EXTENT_FLAG_MERGED,
> +	/* This em has a fscrypt info */
> +	EXTENT_FLAG_ENCRYPTED,
>  };
>  
>  struct extent_map {
> @@ -50,6 +52,7 @@ struct extent_map {
>  	 */
>  	u64 generation;
>  	unsigned long flags;
> +	struct fscrypt_info *fscrypt_info;
>  	/* Used for chunk mappings, flag EXTENT_FLAG_FS_MAPPING must be set */
>  	struct map_lookup *map_lookup;
>  	refcount_t refs;
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 40c2ec328730..b43fc253ecd1 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7460,6 +7460,14 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start,
>  		em->compress_type = compress_type;
>  	}
>  
> +	ret = fscrypt_prepare_new_extent(&inode->vfs_inode, &em->fscrypt_info);
> +	if (ret < 0) {
> +		free_extent_map(em);
> +		return ERR_PTR(ret);
> +	}
> +	if (em->fscrypt_info)
> +		set_bit(EXTENT_FLAG_ENCRYPTED, &em->flags);

We don't check this flag anywhere, you can drop it.  Thanks,

Josef