[PATCH 0/2] Resource-managed blk_ksm_init()

[PATCH 0/2] Resource-managed blk_ksm_init()

[Eric Biggers]

This patchset adds a resource-managed variant of blk_ksm_init() so that
drivers don't have to worry about calling blk_ksm_destroy().

This was suggested during review of my patchset which adds eMMC inline
encryption support
(https://lkml.kernel.org/linux-mmc/20210104184542.4616-1-ebiggers@kernel.org/T/#u).
That patchset proposes a second caller of blk_ksm_init().  But it can
instead use the resource-managed variant, as can the UFS driver.

My preference is that patch #1 be taken through the MMC tree together
with my MMC patchset, so that we don't have to wait an extra cycle for
the MMC changes.  Patch #2 can then go in later.

Eric Biggers (2):
  block/keyslot-manager: introduce devm_blk_ksm_init()
  scsi: ufs: use devm_blk_ksm_init()

 Documentation/block/inline-encryption.rst | 12 +++++-----
 block/keyslot-manager.c                   | 29 +++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-crypto.c          |  9 ++-----
 drivers/scsi/ufs/ufshcd-crypto.h          |  5 ----
 drivers/scsi/ufs/ufshcd.c                 |  1 -
 include/linux/keyslot-manager.h           |  3 +++
 6 files changed, 40 insertions(+), 19 deletions(-)

-- 
2.30.0

[PATCH 1/2] block/keyslot-manager: introduce devm_blk_ksm_init()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Add a resource-managed variant of blk_ksm_init() so that drivers don't
have to worry about calling blk_ksm_destroy().

Note that the implementation uses a custom devres action to call
blk_ksm_destroy() rather than switching the two allocations to be
directly devres-managed, e.g. with devm_kmalloc().  This is because we
need to keep zeroing the memory containing the keyslots when it is
freed, and also because we want to continue using kvmalloc() (and there
is no devm_kvmalloc()).

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 Documentation/block/inline-encryption.rst | 12 +++++-----
 block/keyslot-manager.c                   | 29 +++++++++++++++++++++++
 include/linux/keyslot-manager.h           |  3 +++
 3 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/Documentation/block/inline-encryption.rst b/Documentation/block/inline-encryption.rst
index e75151e467d39..7f9b40d6b416b 100644
--- a/Documentation/block/inline-encryption.rst
+++ b/Documentation/block/inline-encryption.rst
@@ -182,8 +182,9 @@ API presented to device drivers
 
 A :c:type:``struct blk_keyslot_manager`` should be set up by device drivers in
 the ``request_queue`` of the device. The device driver needs to call
-``blk_ksm_init`` on the ``blk_keyslot_manager``, which specifying the number of
-keyslots supported by the hardware.
+``blk_ksm_init`` (or its resource-managed variant ``devm_blk_ksm_init``) on the
+``blk_keyslot_manager``, while specifying the number of keyslots supported by
+the hardware.
 
 The device driver also needs to tell the KSM how to actually manipulate the
 IE hardware in the device to do things like programming the crypto key into
@@ -202,10 +203,9 @@ needs each and every of its keyslots to be reprogrammed with the key it
 "should have" at the point in time when the function is called. This is useful
 e.g. if a device loses all its keys on runtime power down/up.
 
-``blk_ksm_destroy`` should be called to free up all resources used by a keyslot
-manager upon ``blk_ksm_init``, once the ``blk_keyslot_manager`` is no longer
-needed.
-
+If the driver used ``blk_ksm_init`` instead of ``devm_blk_ksm_init``, then
+``blk_ksm_destroy`` should be called to free up all resources used by a
+``blk_keyslot_manager`` once it is no longer needed.
 
 Layered Devices
 ===============
diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c
index 86f8195d8039e..324bf4244f5fb 100644
--- a/block/keyslot-manager.c
+++ b/block/keyslot-manager.c
@@ -29,6 +29,7 @@
 #define pr_fmt(fmt) "blk-crypto: " fmt
 
 #include <linux/keyslot-manager.h>
+#include <linux/device.h>
 #include <linux/atomic.h>
 #include <linux/mutex.h>
 #include <linux/pm_runtime.h>
@@ -127,6 +128,34 @@ int blk_ksm_init(struct blk_keyslot_manager *ksm, unsigned int num_slots)
 }
 EXPORT_SYMBOL_GPL(blk_ksm_init);
 
+static void blk_ksm_destroy_callback(void *ksm)
+{
+	blk_ksm_destroy(ksm);
+}
+
+/**
+ * devm_blk_ksm_init() - Resource-managed blk_ksm_init()
+ * @dev: The device which owns the blk_keyslot_manager.
+ * @ksm: The blk_keyslot_manager to initialize.
+ * @num_slots: The number of key slots to manage.
+ *
+ * Like blk_ksm_init(), but causes blk_ksm_destroy() to be called automatically
+ * on driver detach.
+ *
+ * Return: 0 on success, or else a negative error code.
+ */
+int devm_blk_ksm_init(struct device *dev, struct blk_keyslot_manager *ksm,
+		      unsigned int num_slots)
+{
+	int err = blk_ksm_init(ksm, num_slots);
+
+	if (err)
+		return err;
+
+	return devm_add_action_or_reset(dev, blk_ksm_destroy_callback, ksm);
+}
+EXPORT_SYMBOL_GPL(devm_blk_ksm_init);
+
 static inline struct hlist_head *
 blk_ksm_hash_bucket_for_key(struct blk_keyslot_manager *ksm,
 			    const struct blk_crypto_key *key)
diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h
index 18f3f5346843f..443ad817c6c57 100644
--- a/include/linux/keyslot-manager.h
+++ b/include/linux/keyslot-manager.h
@@ -85,6 +85,9 @@ struct blk_keyslot_manager {
 
 int blk_ksm_init(struct blk_keyslot_manager *ksm, unsigned int num_slots);
 
+int devm_blk_ksm_init(struct device *dev, struct blk_keyslot_manager *ksm,
+		      unsigned int num_slots);
+
 blk_status_t blk_ksm_get_slot_for_key(struct blk_keyslot_manager *ksm,
 				      const struct blk_crypto_key *key,
 				      struct blk_ksm_keyslot **slot_ptr);
-- 
2.30.0

Re: [PATCH 1/2] block/keyslot-manager: introduce devm_blk_ksm_init()

[Satya Tangirala]

On Thu, Jan 21, 2021 at 12:23 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> Add a resource-managed variant of blk_ksm_init() so that drivers don't
> have to worry about calling blk_ksm_destroy().
>
> Note that the implementation uses a custom devres action to call
> blk_ksm_destroy() rather than switching the two allocations to be
> directly devres-managed, e.g. with devm_kmalloc().  This is because we
> need to keep zeroing the memory containing the keyslots when it is
> freed, and also because we want to continue using kvmalloc() (and there
> is no devm_kvmalloc()).
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  Documentation/block/inline-encryption.rst | 12 +++++-----
>  block/keyslot-manager.c                   | 29 +++++++++++++++++++++++
>  include/linux/keyslot-manager.h           |  3 +++
>  3 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/block/inline-encryption.rst b/Documentation/block/inline-encryption.rst
> index e75151e467d39..7f9b40d6b416b 100644
> --- a/Documentation/block/inline-encryption.rst
> +++ b/Documentation/block/inline-encryption.rst
> @@ -182,8 +182,9 @@ API presented to device drivers
>
>  A :c:type:``struct blk_keyslot_manager`` should be set up by device drivers in
>  the ``request_queue`` of the device. The device driver needs to call
> -``blk_ksm_init`` on the ``blk_keyslot_manager``, which specifying the number of
> -keyslots supported by the hardware.
> +``blk_ksm_init`` (or its resource-managed variant ``devm_blk_ksm_init``) on the
> +``blk_keyslot_manager``, while specifying the number of keyslots supported by
> +the hardware.
>
>  The device driver also needs to tell the KSM how to actually manipulate the
>  IE hardware in the device to do things like programming the crypto key into
> @@ -202,10 +203,9 @@ needs each and every of its keyslots to be reprogrammed with the key it
>  "should have" at the point in time when the function is called. This is useful
>  e.g. if a device loses all its keys on runtime power down/up.
>
> -``blk_ksm_destroy`` should be called to free up all resources used by a keyslot
> -manager upon ``blk_ksm_init``, once the ``blk_keyslot_manager`` is no longer
> -needed.
> -
> +If the driver used ``blk_ksm_init`` instead of ``devm_blk_ksm_init``, then
> +``blk_ksm_destroy`` should be called to free up all resources used by a
> +``blk_keyslot_manager`` once it is no longer needed.
>
>  Layered Devices
>  ===============
> diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c
> index 86f8195d8039e..324bf4244f5fb 100644
> --- a/block/keyslot-manager.c
> +++ b/block/keyslot-manager.c
> @@ -29,6 +29,7 @@
>  #define pr_fmt(fmt) "blk-crypto: " fmt
>
>  #include <linux/keyslot-manager.h>
> +#include <linux/device.h>
>  #include <linux/atomic.h>
>  #include <linux/mutex.h>
>  #include <linux/pm_runtime.h>
> @@ -127,6 +128,34 @@ int blk_ksm_init(struct blk_keyslot_manager *ksm, unsigned int num_slots)
>  }
>  EXPORT_SYMBOL_GPL(blk_ksm_init);
>
> +static void blk_ksm_destroy_callback(void *ksm)
> +{
> +       blk_ksm_destroy(ksm);
> +}
> +
> +/**
> + * devm_blk_ksm_init() - Resource-managed blk_ksm_init()
> + * @dev: The device which owns the blk_keyslot_manager.
> + * @ksm: The blk_keyslot_manager to initialize.
> + * @num_slots: The number of key slots to manage.
> + *
> + * Like blk_ksm_init(), but causes blk_ksm_destroy() to be called automatically
> + * on driver detach.
> + *
> + * Return: 0 on success, or else a negative error code.
> + */
> +int devm_blk_ksm_init(struct device *dev, struct blk_keyslot_manager *ksm,
> +                     unsigned int num_slots)
> +{
> +       int err = blk_ksm_init(ksm, num_slots);
> +
> +       if (err)
> +               return err;
> +
> +       return devm_add_action_or_reset(dev, blk_ksm_destroy_callback, ksm);
> +}
> +EXPORT_SYMBOL_GPL(devm_blk_ksm_init);
> +
>  static inline struct hlist_head *
>  blk_ksm_hash_bucket_for_key(struct blk_keyslot_manager *ksm,
>                             const struct blk_crypto_key *key)
> diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h
> index 18f3f5346843f..443ad817c6c57 100644
> --- a/include/linux/keyslot-manager.h
> +++ b/include/linux/keyslot-manager.h
> @@ -85,6 +85,9 @@ struct blk_keyslot_manager {
>
>  int blk_ksm_init(struct blk_keyslot_manager *ksm, unsigned int num_slots);
>
> +int devm_blk_ksm_init(struct device *dev, struct blk_keyslot_manager *ksm,
> +                     unsigned int num_slots);
> +
>  blk_status_t blk_ksm_get_slot_for_key(struct blk_keyslot_manager *ksm,
>                                       const struct blk_crypto_key *key,
>                                       struct blk_ksm_keyslot **slot_ptr);
> --
> 2.30.0
Looks good to me. Please feel free to add
Reviewed-by: Satya Tangirala <satyat@google.com>

Re: [PATCH 1/2] block/keyslot-manager: introduce devm_blk_ksm_init()

[Eric Biggers]

On Mon, Jan 25, 2021 at 12:14:00PM -0800, Satya Tangirala wrote:
> On Thu, Jan 21, 2021 at 12:23 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > From: Eric Biggers <ebiggers@google.com>
> >
> > Add a resource-managed variant of blk_ksm_init() so that drivers don't
> > have to worry about calling blk_ksm_destroy().
> >
> > Note that the implementation uses a custom devres action to call
> > blk_ksm_destroy() rather than switching the two allocations to be
> > directly devres-managed, e.g. with devm_kmalloc().  This is because we
> > need to keep zeroing the memory containing the keyslots when it is
> > freed, and also because we want to continue using kvmalloc() (and there
> > is no devm_kvmalloc()).
> >
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
[..]
> > diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h
> > index 18f3f5346843f..443ad817c6c57 100644
> > --- a/include/linux/keyslot-manager.h
> > +++ b/include/linux/keyslot-manager.h
> > @@ -85,6 +85,9 @@ struct blk_keyslot_manager {
> >
> >  int blk_ksm_init(struct blk_keyslot_manager *ksm, unsigned int num_slots);
> >
> > +int devm_blk_ksm_init(struct device *dev, struct blk_keyslot_manager *ksm,
> > +                     unsigned int num_slots);
> > +
> >  blk_status_t blk_ksm_get_slot_for_key(struct blk_keyslot_manager *ksm,
> >                                       const struct blk_crypto_key *key,
> >                                       struct blk_ksm_keyslot **slot_ptr);
> > --
>
> Looks good to me. Please feel free to add
> Reviewed-by: Satya Tangirala <satyat@google.com>

Thanks Satya.  Jens, any objection to this patch going in through the MMC tree?

- Eric

Re: [PATCH 1/2] block/keyslot-manager: introduce devm_blk_ksm_init()

[Jens Axboe]

On 1/25/21 1:25 PM, Eric Biggers wrote:
> On Mon, Jan 25, 2021 at 12:14:00PM -0800, Satya Tangirala wrote:
>> On Thu, Jan 21, 2021 at 12:23 AM Eric Biggers <ebiggers@kernel.org> wrote:
>>>
>>> From: Eric Biggers <ebiggers@google.com>
>>>
>>> Add a resource-managed variant of blk_ksm_init() so that drivers don't
>>> have to worry about calling blk_ksm_destroy().
>>>
>>> Note that the implementation uses a custom devres action to call
>>> blk_ksm_destroy() rather than switching the two allocations to be
>>> directly devres-managed, e.g. with devm_kmalloc().  This is because we
>>> need to keep zeroing the memory containing the keyslots when it is
>>> freed, and also because we want to continue using kvmalloc() (and there
>>> is no devm_kvmalloc()).
>>>
>>> Signed-off-by: Eric Biggers <ebiggers@google.com>
> [..]
>>> diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h
>>> index 18f3f5346843f..443ad817c6c57 100644
>>> --- a/include/linux/keyslot-manager.h
>>> +++ b/include/linux/keyslot-manager.h
>>> @@ -85,6 +85,9 @@ struct blk_keyslot_manager {
>>>
>>>  int blk_ksm_init(struct blk_keyslot_manager *ksm, unsigned int num_slots);
>>>
>>> +int devm_blk_ksm_init(struct device *dev, struct blk_keyslot_manager *ksm,
>>> +                     unsigned int num_slots);
>>> +
>>>  blk_status_t blk_ksm_get_slot_for_key(struct blk_keyslot_manager *ksm,
>>>                                       const struct blk_crypto_key *key,
>>>                                       struct blk_ksm_keyslot **slot_ptr);
>>> --
>>
>> Looks good to me. Please feel free to add
>> Reviewed-by: Satya Tangirala <satyat@google.com>
> 
> Thanks Satya.  Jens, any objection to this patch going in through the MMC tree?

No objections from me, doesn't look like we have any real worries of
conflicts.

-- 
Jens Axboe

Re: [PATCH 1/2] block/keyslot-manager: introduce devm_blk_ksm_init()

[Ulf Hansson]

On Mon, 25 Jan 2021 at 22:16, Jens Axboe <axboe@kernel.dk> wrote:
>
> On 1/25/21 1:25 PM, Eric Biggers wrote:
> > On Mon, Jan 25, 2021 at 12:14:00PM -0800, Satya Tangirala wrote:
> >> On Thu, Jan 21, 2021 at 12:23 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >>>
> >>> From: Eric Biggers <ebiggers@google.com>
> >>>
> >>> Add a resource-managed variant of blk_ksm_init() so that drivers don't
> >>> have to worry about calling blk_ksm_destroy().
> >>>
> >>> Note that the implementation uses a custom devres action to call
> >>> blk_ksm_destroy() rather than switching the two allocations to be
> >>> directly devres-managed, e.g. with devm_kmalloc().  This is because we
> >>> need to keep zeroing the memory containing the keyslots when it is
> >>> freed, and also because we want to continue using kvmalloc() (and there
> >>> is no devm_kvmalloc()).
> >>>
> >>> Signed-off-by: Eric Biggers <ebiggers@google.com>
> > [..]
> >>> diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h
> >>> index 18f3f5346843f..443ad817c6c57 100644
> >>> --- a/include/linux/keyslot-manager.h
> >>> +++ b/include/linux/keyslot-manager.h
> >>> @@ -85,6 +85,9 @@ struct blk_keyslot_manager {
> >>>
> >>>  int blk_ksm_init(struct blk_keyslot_manager *ksm, unsigned int num_slots);
> >>>
> >>> +int devm_blk_ksm_init(struct device *dev, struct blk_keyslot_manager *ksm,
> >>> +                     unsigned int num_slots);
> >>> +
> >>>  blk_status_t blk_ksm_get_slot_for_key(struct blk_keyslot_manager *ksm,
> >>>                                       const struct blk_crypto_key *key,
> >>>                                       struct blk_ksm_keyslot **slot_ptr);
> >>> --
> >>
> >> Looks good to me. Please feel free to add
> >> Reviewed-by: Satya Tangirala <satyat@google.com>
> >
> > Thanks Satya.  Jens, any objection to this patch going in through the MMC tree?
>
> No objections from me, doesn't look like we have any real worries of
> conflicts.

Applied to my mmc for the next branch, by adding Jens' ack. Thanks everybody!

Kind regards
Uffe

[PATCH 2/2] scsi: ufs: use devm_blk_ksm_init()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Use the new resource-managed variant of blk_ksm_init() so that the UFS
driver doesn't have to manually call blk_ksm_destroy().

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 drivers/scsi/ufs/ufshcd-crypto.c | 9 ++-------
 drivers/scsi/ufs/ufshcd-crypto.h | 5 -----
 drivers/scsi/ufs/ufshcd.c        | 1 -
 3 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-crypto.c b/drivers/scsi/ufs/ufshcd-crypto.c
index 07310b12a5dc8..153dd5765d9ca 100644
--- a/drivers/scsi/ufs/ufshcd-crypto.c
+++ b/drivers/scsi/ufs/ufshcd-crypto.c
@@ -179,8 +179,8 @@ int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba)
 	}
 
 	/* The actual number of configurations supported is (CFGC+1) */
-	err = blk_ksm_init(&hba->ksm,
-			   hba->crypto_capabilities.config_count + 1);
+	err = devm_blk_ksm_init(hba->dev, &hba->ksm,
+				hba->crypto_capabilities.config_count + 1);
 	if (err)
 		goto out_free_caps;
 
@@ -238,8 +238,3 @@ void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
 	if (hba->caps & UFSHCD_CAP_CRYPTO)
 		blk_ksm_register(&hba->ksm, q);
 }
-
-void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba)
-{
-	blk_ksm_destroy(&hba->ksm);
-}
diff --git a/drivers/scsi/ufs/ufshcd-crypto.h b/drivers/scsi/ufs/ufshcd-crypto.h
index d53851be55416..78a58e788dff9 100644
--- a/drivers/scsi/ufs/ufshcd-crypto.h
+++ b/drivers/scsi/ufs/ufshcd-crypto.h
@@ -43,8 +43,6 @@ void ufshcd_init_crypto(struct ufs_hba *hba);
 void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
 					    struct request_queue *q);
 
-void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba);
-
 #else /* CONFIG_SCSI_UFS_CRYPTO */
 
 static inline void ufshcd_prepare_lrbp_crypto(struct request *rq,
@@ -69,9 +67,6 @@ static inline void ufshcd_init_crypto(struct ufs_hba *hba) { }
 static inline void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
 						struct request_queue *q) { }
 
-static inline void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba)
-{ }
-
 #endif /* CONFIG_SCSI_UFS_CRYPTO */
 
 #endif /* _UFSHCD_CRYPTO_H */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e31d2c5c7b23b..d905c84474c2c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -9139,7 +9139,6 @@ EXPORT_SYMBOL_GPL(ufshcd_remove);
  */
 void ufshcd_dealloc_host(struct ufs_hba *hba)
 {
-	ufshcd_crypto_destroy_keyslot_manager(hba);
 	scsi_host_put(hba->host);
 }
 EXPORT_SYMBOL_GPL(ufshcd_dealloc_host);
-- 
2.30.0

Re: [PATCH 2/2] scsi: ufs: use devm_blk_ksm_init()

[Satya Tangirala]

On Thu, Jan 21, 2021 at 12:23 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> Use the new resource-managed variant of blk_ksm_init() so that the UFS
> driver doesn't have to manually call blk_ksm_destroy().
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  drivers/scsi/ufs/ufshcd-crypto.c | 9 ++-------
>  drivers/scsi/ufs/ufshcd-crypto.h | 5 -----
>  drivers/scsi/ufs/ufshcd.c        | 1 -
>  3 files changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd-crypto.c b/drivers/scsi/ufs/ufshcd-crypto.c
> index 07310b12a5dc8..153dd5765d9ca 100644
> --- a/drivers/scsi/ufs/ufshcd-crypto.c
> +++ b/drivers/scsi/ufs/ufshcd-crypto.c
> @@ -179,8 +179,8 @@ int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba)
>         }
>
>         /* The actual number of configurations supported is (CFGC+1) */
> -       err = blk_ksm_init(&hba->ksm,
> -                          hba->crypto_capabilities.config_count + 1);
> +       err = devm_blk_ksm_init(hba->dev, &hba->ksm,
> +                               hba->crypto_capabilities.config_count + 1);
>         if (err)
>                 goto out_free_caps;
>
> @@ -238,8 +238,3 @@ void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
>         if (hba->caps & UFSHCD_CAP_CRYPTO)
>                 blk_ksm_register(&hba->ksm, q);
>  }
> -
> -void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba)
> -{
> -       blk_ksm_destroy(&hba->ksm);
> -}
> diff --git a/drivers/scsi/ufs/ufshcd-crypto.h b/drivers/scsi/ufs/ufshcd-crypto.h
> index d53851be55416..78a58e788dff9 100644
> --- a/drivers/scsi/ufs/ufshcd-crypto.h
> +++ b/drivers/scsi/ufs/ufshcd-crypto.h
> @@ -43,8 +43,6 @@ void ufshcd_init_crypto(struct ufs_hba *hba);
>  void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
>                                             struct request_queue *q);
>
> -void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba);
> -
>  #else /* CONFIG_SCSI_UFS_CRYPTO */
>
>  static inline void ufshcd_prepare_lrbp_crypto(struct request *rq,
> @@ -69,9 +67,6 @@ static inline void ufshcd_init_crypto(struct ufs_hba *hba) { }
>  static inline void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
>                                                 struct request_queue *q) { }
>
> -static inline void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba)
> -{ }
> -
>  #endif /* CONFIG_SCSI_UFS_CRYPTO */
>
>  #endif /* _UFSHCD_CRYPTO_H */
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index e31d2c5c7b23b..d905c84474c2c 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -9139,7 +9139,6 @@ EXPORT_SYMBOL_GPL(ufshcd_remove);
>   */
>  void ufshcd_dealloc_host(struct ufs_hba *hba)
>  {
> -       ufshcd_crypto_destroy_keyslot_manager(hba);
>         scsi_host_put(hba->host);
>  }
>  EXPORT_SYMBOL_GPL(ufshcd_dealloc_host);
> --
> 2.30.0
>
Looks good to me. Please feel free to add
Reviewed-by: Satya Tangirala <satyat@google.com>

Re: [PATCH 2/2] scsi: ufs: use devm_blk_ksm_init()

[Ulf Hansson]

+ Martin, James

On Thu, 21 Jan 2021 at 09:23, Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> Use the new resource-managed variant of blk_ksm_init() so that the UFS
> driver doesn't have to manually call blk_ksm_destroy().
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

I took the liberty of applying this one to my mmc tree along with
patch1, as it looks trivial to me.

Martin/James - if you have objections or want to ack it, please tell me.

Thanks and kind regards
Uffe





> ---
>  drivers/scsi/ufs/ufshcd-crypto.c | 9 ++-------
>  drivers/scsi/ufs/ufshcd-crypto.h | 5 -----
>  drivers/scsi/ufs/ufshcd.c        | 1 -
>  3 files changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd-crypto.c b/drivers/scsi/ufs/ufshcd-crypto.c
> index 07310b12a5dc8..153dd5765d9ca 100644
> --- a/drivers/scsi/ufs/ufshcd-crypto.c
> +++ b/drivers/scsi/ufs/ufshcd-crypto.c
> @@ -179,8 +179,8 @@ int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba)
>         }
>
>         /* The actual number of configurations supported is (CFGC+1) */
> -       err = blk_ksm_init(&hba->ksm,
> -                          hba->crypto_capabilities.config_count + 1);
> +       err = devm_blk_ksm_init(hba->dev, &hba->ksm,
> +                               hba->crypto_capabilities.config_count + 1);
>         if (err)
>                 goto out_free_caps;
>
> @@ -238,8 +238,3 @@ void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
>         if (hba->caps & UFSHCD_CAP_CRYPTO)
>                 blk_ksm_register(&hba->ksm, q);
>  }
> -
> -void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba)
> -{
> -       blk_ksm_destroy(&hba->ksm);
> -}
> diff --git a/drivers/scsi/ufs/ufshcd-crypto.h b/drivers/scsi/ufs/ufshcd-crypto.h
> index d53851be55416..78a58e788dff9 100644
> --- a/drivers/scsi/ufs/ufshcd-crypto.h
> +++ b/drivers/scsi/ufs/ufshcd-crypto.h
> @@ -43,8 +43,6 @@ void ufshcd_init_crypto(struct ufs_hba *hba);
>  void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
>                                             struct request_queue *q);
>
> -void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba);
> -
>  #else /* CONFIG_SCSI_UFS_CRYPTO */
>
>  static inline void ufshcd_prepare_lrbp_crypto(struct request *rq,
> @@ -69,9 +67,6 @@ static inline void ufshcd_init_crypto(struct ufs_hba *hba) { }
>  static inline void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
>                                                 struct request_queue *q) { }
>
> -static inline void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba)
> -{ }
> -
>  #endif /* CONFIG_SCSI_UFS_CRYPTO */
>
>  #endif /* _UFSHCD_CRYPTO_H */
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index e31d2c5c7b23b..d905c84474c2c 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -9139,7 +9139,6 @@ EXPORT_SYMBOL_GPL(ufshcd_remove);
>   */
>  void ufshcd_dealloc_host(struct ufs_hba *hba)
>  {
> -       ufshcd_crypto_destroy_keyslot_manager(hba);
>         scsi_host_put(hba->host);
>  }
>  EXPORT_SYMBOL_GPL(ufshcd_dealloc_host);
> --
> 2.30.0
>

Re: [PATCH 2/2] scsi: ufs: use devm_blk_ksm_init()

[Martin K. Petersen]

Ulf,

> I took the liberty of applying this one to my mmc tree along with
> patch1, as it looks trivial to me.
>
> Martin/James - if you have objections or want to ack it, please tell
> me.

That's fine. Just a heads-up that there has been a lot of churn in UFS
in this cycle so there may be some merge fuzz.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/2] Resource-managed blk_ksm_init()

[Ulf Hansson]

+ Jens, Martin, James


On Thu, 21 Jan 2021 at 09:23, Eric Biggers <ebiggers@kernel.org> wrote:
>
> This patchset adds a resource-managed variant of blk_ksm_init() so that
> drivers don't have to worry about calling blk_ksm_destroy().
>
> This was suggested during review of my patchset which adds eMMC inline
> encryption support
> (https://lkml.kernel.org/linux-mmc/20210104184542.4616-1-ebiggers@kernel.org/T/#u).
> That patchset proposes a second caller of blk_ksm_init().  But it can
> instead use the resource-managed variant, as can the UFS driver.
>
> My preference is that patch #1 be taken through the MMC tree together
> with my MMC patchset, so that we don't have to wait an extra cycle for
> the MMC changes.  Patch #2 can then go in later.

Sure, I can pick patch #1 through my mmc tree, but need an ack from
Jens to do it. Or whatever he prefers.

Another solution is to host immutable branches (if needed), so they
can be shared between our tree's, that works for me as well.

>
> Eric Biggers (2):
>   block/keyslot-manager: introduce devm_blk_ksm_init()
>   scsi: ufs: use devm_blk_ksm_init()
>
>  Documentation/block/inline-encryption.rst | 12 +++++-----
>  block/keyslot-manager.c                   | 29 +++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd-crypto.c          |  9 ++-----
>  drivers/scsi/ufs/ufshcd-crypto.h          |  5 ----
>  drivers/scsi/ufs/ufshcd.c                 |  1 -
>  include/linux/keyslot-manager.h           |  3 +++
>  6 files changed, 40 insertions(+), 19 deletions(-)
>
> --
> 2.30.0
>

Kind regards
Uffe

Re: [PATCH 0/2] Resource-managed blk_ksm_init()

[Jens Axboe]

On 1/21/21 5:50 AM, Ulf Hansson wrote:
> + Jens, Martin, James
> 
> 
> On Thu, 21 Jan 2021 at 09:23, Eric Biggers <ebiggers@kernel.org> wrote:
>>
>> This patchset adds a resource-managed variant of blk_ksm_init() so that
>> drivers don't have to worry about calling blk_ksm_destroy().
>>
>> This was suggested during review of my patchset which adds eMMC inline
>> encryption support
>> (https://lkml.kernel.org/linux-mmc/20210104184542.4616-1-ebiggers@kernel.org/T/#u).
>> That patchset proposes a second caller of blk_ksm_init().  But it can
>> instead use the resource-managed variant, as can the UFS driver.
>>
>> My preference is that patch #1 be taken through the MMC tree together
>> with my MMC patchset, so that we don't have to wait an extra cycle for
>> the MMC changes.  Patch #2 can then go in later.
> 
> Sure, I can pick patch #1 through my mmc tree, but need an ack from
> Jens to do it. Or whatever he prefers.

Or we can take it through the block tree, usually the easiest as
it's the most likely source of potential conflicts. And that's true
for both of them, as long as the SCSI side signs off on patch 2/2.


-- 
Jens Axboe

Re: [PATCH 0/2] Resource-managed blk_ksm_init()

[Eric Biggers]

On Mon, Jan 25, 2021 at 02:14:32PM -0700, Jens Axboe wrote:
> On 1/21/21 5:50 AM, Ulf Hansson wrote:
> > + Jens, Martin, James
> > 
> > 
> > On Thu, 21 Jan 2021 at 09:23, Eric Biggers <ebiggers@kernel.org> wrote:
> >>
> >> This patchset adds a resource-managed variant of blk_ksm_init() so that
> >> drivers don't have to worry about calling blk_ksm_destroy().
> >>
> >> This was suggested during review of my patchset which adds eMMC inline
> >> encryption support
> >> (https://lkml.kernel.org/linux-mmc/20210104184542.4616-1-ebiggers@kernel.org/T/#u).
> >> That patchset proposes a second caller of blk_ksm_init().  But it can
> >> instead use the resource-managed variant, as can the UFS driver.
> >>
> >> My preference is that patch #1 be taken through the MMC tree together
> >> with my MMC patchset, so that we don't have to wait an extra cycle for
> >> the MMC changes.  Patch #2 can then go in later.
> > 
> > Sure, I can pick patch #1 through my mmc tree, but need an ack from
> > Jens to do it. Or whatever he prefers.
> 
> Or we can take it through the block tree, usually the easiest as
> it's the most likely source of potential conflicts. And that's true
> for both of them, as long as the SCSI side signs off on patch 2/2.
> 

As I mentioned, the issue is that my patchset to add eMMC inline encryption
support (https://lkml.kernel.org/r/20210121090140.326380-1-ebiggers@kernel.org)
depends on devm_blk_ksm_init(), as it was requested during review.

So if devm_blk_ksm_init() goes in through the block tree, Ulf won't be able to
take the eMMC patchset for 5.12.

- Eric

Re: [PATCH 0/2] Resource-managed blk_ksm_init()

[Jens Axboe]

On 1/25/21 2:21 PM, Eric Biggers wrote:
> On Mon, Jan 25, 2021 at 02:14:32PM -0700, Jens Axboe wrote:
>> On 1/21/21 5:50 AM, Ulf Hansson wrote:
>>> + Jens, Martin, James
>>>
>>>
>>> On Thu, 21 Jan 2021 at 09:23, Eric Biggers <ebiggers@kernel.org> wrote:
>>>>
>>>> This patchset adds a resource-managed variant of blk_ksm_init() so that
>>>> drivers don't have to worry about calling blk_ksm_destroy().
>>>>
>>>> This was suggested during review of my patchset which adds eMMC inline
>>>> encryption support
>>>> (https://lkml.kernel.org/linux-mmc/20210104184542.4616-1-ebiggers@kernel.org/T/#u).
>>>> That patchset proposes a second caller of blk_ksm_init().  But it can
>>>> instead use the resource-managed variant, as can the UFS driver.
>>>>
>>>> My preference is that patch #1 be taken through the MMC tree together
>>>> with my MMC patchset, so that we don't have to wait an extra cycle for
>>>> the MMC changes.  Patch #2 can then go in later.
>>>
>>> Sure, I can pick patch #1 through my mmc tree, but need an ack from
>>> Jens to do it. Or whatever he prefers.
>>
>> Or we can take it through the block tree, usually the easiest as
>> it's the most likely source of potential conflicts. And that's true
>> for both of them, as long as the SCSI side signs off on patch 2/2.
>>
> 
> As I mentioned, the issue is that my patchset to add eMMC inline encryption
> support (https://lkml.kernel.org/r/20210121090140.326380-1-ebiggers@kernel.org)
> depends on devm_blk_ksm_init(), as it was requested during review.
> 
> So if devm_blk_ksm_init() goes in through the block tree, Ulf won't be able to
> take the eMMC patchset for 5.12.

Well he could, such cross tree dependencies are not hard to solve. But
as mentioned in the other email, for this one, just queued it up
yourself. You can add my acked-by to it.

-- 
Jens Axboe