diff mbox series

[v3,13/16] fscrypt: allow multiple extents to reference one info

Message ID 2fc070a3990716077dee122740f21abcea8121a8.1691505882.git.sweettea-kernel@dorminy.me (mailing list archive)
State Superseded
Headers show
Series fscrypt: add extent encryption | expand

Commit Message

Sweet Tea Dorminy Aug. 8, 2023, 5:08 p.m. UTC
btrfs occasionally splits in-memory extents while holding a mutex. This
means we can't just copy the info, since setting up a new inlinecrypt
key requires taking a semaphore. Thus adding a mechanism to split
extents and merely take a new reference on the info is necessary.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/fscrypt_private.h |  5 +++++
 fs/crypto/keysetup.c        | 18 +++++++++++++++++-
 include/linux/fscrypt.h     |  2 ++
 3 files changed, 24 insertions(+), 1 deletion(-)

Comments

Luis Henriques Aug. 10, 2023, 3:51 p.m. UTC | #1
Sweet Tea Dorminy <sweettea-kernel@dorminy.me> writes:

> btrfs occasionally splits in-memory extents while holding a mutex. This
> means we can't just copy the info, since setting up a new inlinecrypt
> key requires taking a semaphore. Thus adding a mechanism to split
> extents and merely take a new reference on the info is necessary.
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/fscrypt_private.h |  5 +++++
>  fs/crypto/keysetup.c        | 18 +++++++++++++++++-
>  include/linux/fscrypt.h     |  2 ++
>  3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index cd29c71b4349..03be2c136c0e 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -287,6 +287,11 @@ struct fscrypt_info {
>  
>  	/* Hashed inode number.  Only set for IV_INO_LBLK_32 */
>  	u32 ci_hashed_ino;
> +
> +	/* Reference count. Normally 1, unless a extent info is shared by
> +	 * several virtual extents.
> +	 */
> +	refcount_t refs;
>  };
>  
>  typedef enum {
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 8d50716bdf11..12c3851b7cd6 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -598,7 +598,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
>  {
>  	struct fscrypt_master_key *mk;
>  
> -	if (!ci)
> +	if (!ci || !refcount_dec_and_test(&ci->refs))
>  		return;
>  
>  	if (ci->ci_enc_key) {
> @@ -686,6 +686,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
>  	crypt_info->ci_inode = inode;
>  	crypt_info->ci_sb = inode->i_sb;
>  	crypt_info->ci_policy = *policy;
> +	refcount_set(&crypt_info->refs, 1);
>  	memcpy(crypt_info->ci_nonce, nonce, FSCRYPT_FILE_NONCE_SIZE);
>  
>  	mode = select_encryption_mode(&crypt_info->ci_policy, inode);
> @@ -1046,6 +1047,21 @@ int fscrypt_load_extent_info(struct inode *inode, void *buf, size_t len,
>  }
>  EXPORT_SYMBOL_GPL(fscrypt_load_extent_info);
>  
> +/**
> + * fscrypt_get_extent_info_ref() - mark a second extent using the same info
> + * @info: the info to be used by another extent
> + *
> + * Sometimes, an existing extent must be split into multiple extents in memory.
> + * In such a case, this function allows multiple extents to use the same extent
> + * info without allocating or taking any lock, which is necessary in certain IO
> + * paths.
> + */
> +void fscrypt_get_extent_info_ref(struct fscrypt_info *info)
> +{
> +	if (info)
> +		refcount_inc(&info->refs);
> +}
> +

There's an EXPORT_SYMBOL_GPL() missing here, right?

Cheers,
Sweet Tea Dorminy Aug. 11, 2023, 4:39 p.m. UTC | #2
On 8/10/23 11:51, Luís Henriques wrote:
> Sweet Tea Dorminy <sweettea-kernel@dorminy.me> writes:
> 
>> btrfs occasionally splits in-memory extents while holding a mutex. This
>> means we can't just copy the info, since setting up a new inlinecrypt
>> key requires taking a semaphore. Thus adding a mechanism to split
>> extents and merely take a new reference on the info is necessary.
>>
>> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
>> ---
>>   fs/crypto/fscrypt_private.h |  5 +++++
>>   fs/crypto/keysetup.c        | 18 +++++++++++++++++-
>>   include/linux/fscrypt.h     |  2 ++
>>   3 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
>> index cd29c71b4349..03be2c136c0e 100644
>> --- a/fs/crypto/fscrypt_private.h
>> +++ b/fs/crypto/fscrypt_private.h
>> @@ -287,6 +287,11 @@ struct fscrypt_info {
>>   
>>   	/* Hashed inode number.  Only set for IV_INO_LBLK_32 */
>>   	u32 ci_hashed_ino;
>> +
>> +	/* Reference count. Normally 1, unless a extent info is shared by
>> +	 * several virtual extents.
>> +	 */
>> +	refcount_t refs;
>>   };
>>   
>>   typedef enum {
>> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
>> index 8d50716bdf11..12c3851b7cd6 100644
>> --- a/fs/crypto/keysetup.c
>> +++ b/fs/crypto/keysetup.c
>> @@ -598,7 +598,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
>>   {
>>   	struct fscrypt_master_key *mk;
>>   
>> -	if (!ci)
>> +	if (!ci || !refcount_dec_and_test(&ci->refs))
>>   		return;
>>   
>>   	if (ci->ci_enc_key) {
>> @@ -686,6 +686,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
>>   	crypt_info->ci_inode = inode;
>>   	crypt_info->ci_sb = inode->i_sb;
>>   	crypt_info->ci_policy = *policy;
>> +	refcount_set(&crypt_info->refs, 1);
>>   	memcpy(crypt_info->ci_nonce, nonce, FSCRYPT_FILE_NONCE_SIZE);
>>   
>>   	mode = select_encryption_mode(&crypt_info->ci_policy, inode);
>> @@ -1046,6 +1047,21 @@ int fscrypt_load_extent_info(struct inode *inode, void *buf, size_t len,
>>   }
>>   EXPORT_SYMBOL_GPL(fscrypt_load_extent_info);
>>   
>> +/**
>> + * fscrypt_get_extent_info_ref() - mark a second extent using the same info
>> + * @info: the info to be used by another extent
>> + *
>> + * Sometimes, an existing extent must be split into multiple extents in memory.
>> + * In such a case, this function allows multiple extents to use the same extent
>> + * info without allocating or taking any lock, which is necessary in certain IO
>> + * paths.
>> + */
>> +void fscrypt_get_extent_info_ref(struct fscrypt_info *info)
>> +{
>> +	if (info)
>> +		refcount_inc(&info->refs);
>> +}
>> +
> 
> There's an EXPORT_SYMBOL_GPL() missing here, right?
> 
> Cheers,

Oof, guess I need to add 'build btrfs as a module' to my preflight 
checklist. Thank you.
diff mbox series

Patch

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index cd29c71b4349..03be2c136c0e 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -287,6 +287,11 @@  struct fscrypt_info {
 
 	/* Hashed inode number.  Only set for IV_INO_LBLK_32 */
 	u32 ci_hashed_ino;
+
+	/* Reference count. Normally 1, unless a extent info is shared by
+	 * several virtual extents.
+	 */
+	refcount_t refs;
 };
 
 typedef enum {
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 8d50716bdf11..12c3851b7cd6 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -598,7 +598,7 @@  static void put_crypt_info(struct fscrypt_info *ci)
 {
 	struct fscrypt_master_key *mk;
 
-	if (!ci)
+	if (!ci || !refcount_dec_and_test(&ci->refs))
 		return;
 
 	if (ci->ci_enc_key) {
@@ -686,6 +686,7 @@  fscrypt_setup_encryption_info(struct inode *inode,
 	crypt_info->ci_inode = inode;
 	crypt_info->ci_sb = inode->i_sb;
 	crypt_info->ci_policy = *policy;
+	refcount_set(&crypt_info->refs, 1);
 	memcpy(crypt_info->ci_nonce, nonce, FSCRYPT_FILE_NONCE_SIZE);
 
 	mode = select_encryption_mode(&crypt_info->ci_policy, inode);
@@ -1046,6 +1047,21 @@  int fscrypt_load_extent_info(struct inode *inode, void *buf, size_t len,
 }
 EXPORT_SYMBOL_GPL(fscrypt_load_extent_info);
 
+/**
+ * fscrypt_get_extent_info_ref() - mark a second extent using the same info
+ * @info: the info to be used by another extent
+ *
+ * Sometimes, an existing extent must be split into multiple extents in memory.
+ * In such a case, this function allows multiple extents to use the same extent
+ * info without allocating or taking any lock, which is necessary in certain IO
+ * paths.
+ */
+void fscrypt_get_extent_info_ref(struct fscrypt_info *info)
+{
+	if (info)
+		refcount_inc(&info->refs);
+}
+
 /**
  * fscrypt_put_encryption_info() - free most of an inode's fscrypt data
  * @inode: an inode being evicted
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 4ba624beea91..b67054a2c965 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -370,6 +370,8 @@  void fscrypt_free_extent_info(struct fscrypt_info **info_ptr);
 int fscrypt_load_extent_info(struct inode *inode, void *buf, size_t len,
 			     struct fscrypt_info **info_ptr);
 
+void fscrypt_get_extent_info_ref(struct fscrypt_info *info);
+
 /* fname.c */
 int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
 			  u8 *out, unsigned int olen);