2023-11-21 18:37:41

by Damian Muszynski

[permalink] [raw]
Subject: [PATCH] crypto: qat - add sysfs_added flag for rate limiting

The qat_rl sysfs attribute group is registered within the adf_dev_start()
function, alongside other driver components.
If any of the functions preceding the group registration fails,
the adf_dev_start() function returns, and the caller, to undo the
operation, invokes adf_dev_stop() followed by adf_dev_shutdown().
However, the current flow lacks information about whether the
registration of the qat_rl attribute group was successful or not.

In cases where this condition is encountered, an error similar to
the following might be reported:

4xxx 0000:6b:00.0: Starting device qat_dev0
4xxx 0000:6b:00.0: qat_dev0 started 9 acceleration engines
4xxx 0000:6b:00.0: Failed to send init message
4xxx 0000:6b:00.0: Failed to start device qat_dev0
sysfs group 'qat_rl' not found for kobject '0000:6b:00.0'
...
sysfs_remove_groups+0x2d/0x50
adf_sysfs_rl_rm+0x44/0x70 [intel_qat]
adf_rl_stop+0x2d/0xb0 [intel_qat]
adf_dev_stop+0x33/0x1d0 [intel_qat]
adf_dev_down+0xf1/0x150 [intel_qat]
...
4xxx 0000:6b:00.0: qat_dev0 stopped 9 acceleration engines
4xxx 0000:6b:00.0: Resetting device qat_dev0

To prevent attempting to remove attributes from a group that has not
been added yet, a flag named 'sysfs_added' is introduced. This flag
is set to true upon the successful registration of the attribute group.

Fixes: d9fb8408376e ("crypto: qat - add rate limiting feature to qat_4xxx")
Signed-off-by: Damian Muszynski <[email protected]>
Reviewed-by: Giovanni Cabiddu <[email protected]>
Reviewed-by: Ahsan Atta <[email protected]>
---
drivers/crypto/intel/qat/qat_common/adf_rl.h | 1 +
drivers/crypto/intel/qat/qat_common/adf_sysfs_rl.c | 8 ++++++++
2 files changed, 9 insertions(+)

diff --git a/drivers/crypto/intel/qat/qat_common/adf_rl.h b/drivers/crypto/intel/qat/qat_common/adf_rl.h
index eb5a330f8543..269c6656fb90 100644
--- a/drivers/crypto/intel/qat/qat_common/adf_rl.h
+++ b/drivers/crypto/intel/qat/qat_common/adf_rl.h
@@ -79,6 +79,7 @@ struct adf_rl_interface_data {
struct adf_rl_sla_input_data input;
enum adf_base_services cap_rem_srv;
struct rw_semaphore lock;
+ bool sysfs_added;
};

struct adf_rl_hw_data {
diff --git a/drivers/crypto/intel/qat/qat_common/adf_sysfs_rl.c b/drivers/crypto/intel/qat/qat_common/adf_sysfs_rl.c
index abf9c52474ec..bedb514d4e30 100644
--- a/drivers/crypto/intel/qat/qat_common/adf_sysfs_rl.c
+++ b/drivers/crypto/intel/qat/qat_common/adf_sysfs_rl.c
@@ -441,11 +441,19 @@ int adf_sysfs_rl_add(struct adf_accel_dev *accel_dev)

data->cap_rem_srv = ADF_SVC_NONE;
data->input.srv = ADF_SVC_NONE;
+ data->sysfs_added = true;

return ret;
}

void adf_sysfs_rl_rm(struct adf_accel_dev *accel_dev)
{
+ struct adf_rl_interface_data *data;
+
+ data = &GET_RL_STRUCT(accel_dev);
+ if (!data->sysfs_added)
+ return;
+
device_remove_group(&GET_DEV(accel_dev), &qat_rl_group);
+ data->sysfs_added = false;
}

base-commit: f36285cc1e99472bb4c6741981594a5934ad4c4e
--
2.41.0



2023-12-01 10:38:39

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: qat - add sysfs_added flag for rate limiting

On Tue, Nov 21, 2023 at 06:02:23PM +0100, Damian Muszynski wrote:
> The qat_rl sysfs attribute group is registered within the adf_dev_start()
> function, alongside other driver components.
> If any of the functions preceding the group registration fails,
> the adf_dev_start() function returns, and the caller, to undo the
> operation, invokes adf_dev_stop() followed by adf_dev_shutdown().
> However, the current flow lacks information about whether the
> registration of the qat_rl attribute group was successful or not.
>
> In cases where this condition is encountered, an error similar to
> the following might be reported:
>
> 4xxx 0000:6b:00.0: Starting device qat_dev0
> 4xxx 0000:6b:00.0: qat_dev0 started 9 acceleration engines
> 4xxx 0000:6b:00.0: Failed to send init message
> 4xxx 0000:6b:00.0: Failed to start device qat_dev0
> sysfs group 'qat_rl' not found for kobject '0000:6b:00.0'
> ...
> sysfs_remove_groups+0x2d/0x50
> adf_sysfs_rl_rm+0x44/0x70 [intel_qat]
> adf_rl_stop+0x2d/0xb0 [intel_qat]
> adf_dev_stop+0x33/0x1d0 [intel_qat]
> adf_dev_down+0xf1/0x150 [intel_qat]
> ...
> 4xxx 0000:6b:00.0: qat_dev0 stopped 9 acceleration engines
> 4xxx 0000:6b:00.0: Resetting device qat_dev0
>
> To prevent attempting to remove attributes from a group that has not
> been added yet, a flag named 'sysfs_added' is introduced. This flag
> is set to true upon the successful registration of the attribute group.
>
> Fixes: d9fb8408376e ("crypto: qat - add rate limiting feature to qat_4xxx")
> Signed-off-by: Damian Muszynski <[email protected]>
> Reviewed-by: Giovanni Cabiddu <[email protected]>
> Reviewed-by: Ahsan Atta <[email protected]>
> ---
> drivers/crypto/intel/qat/qat_common/adf_rl.h | 1 +
> drivers/crypto/intel/qat/qat_common/adf_sysfs_rl.c | 8 ++++++++
> 2 files changed, 9 insertions(+)

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt