diff mbox

[2/2] lightnvm: hold nvm_lock until finish the target creation

Message ID 1463639921-1824-2-git-send-email-wwtao0320@163.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wenwei Tao May 19, 2016, 6:38 a.m. UTC
From: Wenwei Tao <ww.tao0320@gmail.com>

When create a target, we check whether the target is
already exist first. If the answer is no, we release
the lock and continue the creation. This cannot prevent
concurrent creation of the same target, so hold the lock
until finish the target creation.

Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
---
 drivers/lightnvm/core.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Matias Bjorling May 19, 2016, 12:42 p.m. UTC | #1
On 05/19/2016 08:38 AM, Wenwei Tao wrote:
> From: Wenwei Tao <ww.tao0320@gmail.com>
>
> When create a target, we check whether the target is
> already exist first. If the answer is no, we release
> the lock and continue the creation. This cannot prevent
> concurrent creation of the same target, so hold the lock
> until finish the target creation.
>
> Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
> ---
>   drivers/lightnvm/core.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 160c1a6..a622081 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -791,6 +791,7 @@ static int nvm_create_target(struct nvm_dev *dev,
>   	struct nvm_tgt_type *tt;
>   	struct nvm_target *t;
>   	void *targetdata;
> +	int ret = -ENOMEM;
>
>   	if (!dev->mt) {
>   		pr_info("nvm: device has no media manager registered.\n");
> @@ -801,21 +802,20 @@ static int nvm_create_target(struct nvm_dev *dev,
>   	tt = nvm_find_target_type(create->tgttype);
>   	if (!tt) {
>   		pr_err("nvm: target type %s not found\n", create->tgttype);
> -		up_write(&nvm_lock);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto err_unlock;
>   	}
>
>   	t = nvm_find_target(create->tgtname);
>   	if (t) {
>   		pr_err("nvm: target name already exists.\n");
> -		up_write(&nvm_lock);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto err_unlock;
>   	}
> -	up_write(&nvm_lock);
>
>   	t = kmalloc(sizeof(struct nvm_target), GFP_KERNEL);
>   	if (!t)
> -		return -ENOMEM;
> +		goto err_unlock;
>
>   	tqueue = blk_alloc_queue_node(GFP_KERNEL, dev->q->node);
>   	if (!tqueue)
> @@ -848,8 +848,6 @@ static int nvm_create_target(struct nvm_dev *dev,
>   	t->type = tt;
>   	t->disk = tdisk;
>   	t->dev = dev;
> -
> -	down_write(&nvm_lock);
>   	list_add_tail(&t->list, &nvm_targets);
>   	up_write(&nvm_lock);
>
> @@ -860,7 +858,9 @@ err_queue:
>   	blk_cleanup_queue(tqueue);
>   err_t:
>   	kfree(t);
> -	return -ENOMEM;
> +err_unlock:
> +	up_write(&nvm_lock);
> +	return ret;
>   }
>
>   static int __nvm_configure_create(struct nvm_ioctl_create *create)
>

Thanks. There is a couple of patches in the for-4.8/core that moves this 
logic to the gennvm media manager. Could you rebase this on top?
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wenwei Tao May 19, 2016, 3:19 p.m. UTC | #2
Okay, I will rebase it on for-4.8/core.

2016-05-19 20:42 GMT+08:00 Matias Bjørling <mb@lightnvm.io>:
> On 05/19/2016 08:38 AM, Wenwei Tao wrote:
>>
>> From: Wenwei Tao <ww.tao0320@gmail.com>
>>
>> When create a target, we check whether the target is
>> already exist first. If the answer is no, we release
>> the lock and continue the creation. This cannot prevent
>> concurrent creation of the same target, so hold the lock
>> until finish the target creation.
>>
>> Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
>> ---
>>   drivers/lightnvm/core.c | 18 +++++++++---------
>>   1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
>> index 160c1a6..a622081 100644
>> --- a/drivers/lightnvm/core.c
>> +++ b/drivers/lightnvm/core.c
>> @@ -791,6 +791,7 @@ static int nvm_create_target(struct nvm_dev *dev,
>>         struct nvm_tgt_type *tt;
>>         struct nvm_target *t;
>>         void *targetdata;
>> +       int ret = -ENOMEM;
>>
>>         if (!dev->mt) {
>>                 pr_info("nvm: device has no media manager registered.\n");
>> @@ -801,21 +802,20 @@ static int nvm_create_target(struct nvm_dev *dev,
>>         tt = nvm_find_target_type(create->tgttype);
>>         if (!tt) {
>>                 pr_err("nvm: target type %s not found\n",
>> create->tgttype);
>> -               up_write(&nvm_lock);
>> -               return -EINVAL;
>> +               ret = -EINVAL;
>> +               goto err_unlock;
>>         }
>>
>>         t = nvm_find_target(create->tgtname);
>>         if (t) {
>>                 pr_err("nvm: target name already exists.\n");
>> -               up_write(&nvm_lock);
>> -               return -EINVAL;
>> +               ret = -EINVAL;
>> +               goto err_unlock;
>>         }
>> -       up_write(&nvm_lock);
>>
>>         t = kmalloc(sizeof(struct nvm_target), GFP_KERNEL);
>>         if (!t)
>> -               return -ENOMEM;
>> +               goto err_unlock;
>>
>>         tqueue = blk_alloc_queue_node(GFP_KERNEL, dev->q->node);
>>         if (!tqueue)
>> @@ -848,8 +848,6 @@ static int nvm_create_target(struct nvm_dev *dev,
>>         t->type = tt;
>>         t->disk = tdisk;
>>         t->dev = dev;
>> -
>> -       down_write(&nvm_lock);
>>         list_add_tail(&t->list, &nvm_targets);
>>         up_write(&nvm_lock);
>>
>> @@ -860,7 +858,9 @@ err_queue:
>>         blk_cleanup_queue(tqueue);
>>   err_t:
>>         kfree(t);
>> -       return -ENOMEM;
>> +err_unlock:
>> +       up_write(&nvm_lock);
>> +       return ret;
>>   }
>>
>>   static int __nvm_configure_create(struct nvm_ioctl_create *create)
>>
>
> Thanks. There is a couple of patches in the for-4.8/core that moves this
> logic to the gennvm media manager. Could you rebase this on top?
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 160c1a6..a622081 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -791,6 +791,7 @@  static int nvm_create_target(struct nvm_dev *dev,
 	struct nvm_tgt_type *tt;
 	struct nvm_target *t;
 	void *targetdata;
+	int ret = -ENOMEM;
 
 	if (!dev->mt) {
 		pr_info("nvm: device has no media manager registered.\n");
@@ -801,21 +802,20 @@  static int nvm_create_target(struct nvm_dev *dev,
 	tt = nvm_find_target_type(create->tgttype);
 	if (!tt) {
 		pr_err("nvm: target type %s not found\n", create->tgttype);
-		up_write(&nvm_lock);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_unlock;
 	}
 
 	t = nvm_find_target(create->tgtname);
 	if (t) {
 		pr_err("nvm: target name already exists.\n");
-		up_write(&nvm_lock);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_unlock;
 	}
-	up_write(&nvm_lock);
 
 	t = kmalloc(sizeof(struct nvm_target), GFP_KERNEL);
 	if (!t)
-		return -ENOMEM;
+		goto err_unlock;
 
 	tqueue = blk_alloc_queue_node(GFP_KERNEL, dev->q->node);
 	if (!tqueue)
@@ -848,8 +848,6 @@  static int nvm_create_target(struct nvm_dev *dev,
 	t->type = tt;
 	t->disk = tdisk;
 	t->dev = dev;
-
-	down_write(&nvm_lock);
 	list_add_tail(&t->list, &nvm_targets);
 	up_write(&nvm_lock);
 
@@ -860,7 +858,9 @@  err_queue:
 	blk_cleanup_queue(tqueue);
 err_t:
 	kfree(t);
-	return -ENOMEM;
+err_unlock:
+	up_write(&nvm_lock);
+	return ret;
 }
 
 static int __nvm_configure_create(struct nvm_ioctl_create *create)