Re: [PATCH] nvmet: don't change model_number on live subsys

Chaitanya Kulkarni
2021-02-23T06:34:56Z

On 2/17/21 23:05, Christoph Hellwig wrote:
>> + mutex_lock(&subsys->lock);
>> + list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry) {
>> + connected = true;
>> + break;
>> + }
>> + mutex_unlock(&subsys->lock);
> This could just use list_empty. But we'd also need to hold the lock
> over the actual change of the model number to avoid a race window.
>
Since subsys model number actual change occurs later in the
nvmet_subsys_attr_model_store() are you saying something like this we need
instead of having a separate helper ?

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 635a7cb45d0b..c9fd98080b55 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -1145,6 +1145,7 @@ static ssize_t
nvmet_subsys_attr_model_store(struct config_item *item,
struct nvmet_subsys_model *new_model;
char *new_model_number;
int pos = 0, len;
+ int ret = 0;

len = strcspn(page, "\n");
if (!len)
@@ -1168,15 +1169,23 @@ static ssize_t
nvmet_subsys_attr_model_store(struct config_item *item,

down_write(&nvmet_config_sem);
mutex_lock(&subsys->lock);
+ /* make sure this subsys is not connected */
+ if (!list_empty(&subsys->ctrls)) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
new_model = rcu_replace_pointer(subsys->model, new_model,
mutex_is_locked(&subsys->lock));
+
+out_unlock:
mutex_unlock(&subsys->lock);
up_write(&nvmet_config_sem);

kfree_rcu(new_model, rcuhead);
kfree(new_model_number);

- return count;
+ return ret ? ret : count;
}
CONFIGFS_ATTR(nvmet_subsys_, attr_model);