Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1403455 > unrolled thread

[PATCH 1/2] lightnvm: break the loop when rqd is not null

Started byWenwei Tao <wwtao0320@163.com>
First post2016-05-19 08:40 +0200
Last post2016-05-19 17:20 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/2] lightnvm: break the loop when rqd is not null Wenwei Tao <wwtao0320@163.com> - 2016-05-19 08:40 +0200
    [PATCH 2/2] lightnvm: hold nvm_lock until finish the target creation Wenwei Tao <wwtao0320@163.com> - 2016-05-19 08:40 +0200
      Re: [PATCH 2/2] lightnvm: hold nvm_lock until finish the target  creation Matias Bjørling <mb@lightnvm.io> - 2016-05-19 14:50 +0200
        Re: [PATCH 2/2] lightnvm: hold nvm_lock until finish the target creation Wenwei Tao <ww.tao0320@gmail.com> - 2016-05-19 17:20 +0200
    Re: [PATCH 1/2] lightnvm: break the loop when rqd is not null Matias Bjørling <mb@lightnvm.io> - 2016-05-19 14:50 +0200
      Re: [PATCH 1/2] lightnvm: break the loop when rqd is not null Wenwei Tao <ww.tao0320@gmail.com> - 2016-05-19 17:20 +0200

#1403455 — [PATCH 1/2] lightnvm: break the loop when rqd is not null

FromWenwei Tao <wwtao0320@163.com>
Date2016-05-19 08:40 +0200
Subject[PATCH 1/2] lightnvm: break the loop when rqd is not null
Message-ID<rApMK-4Bp-3@gated-at.bofh.it>
From: Wenwei Tao <ww.tao0320@gmail.com>

Break the loop when rqd is not null to reduce
unnecessary schedule.

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

diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
index 2103e97..2915e39 100644
--- a/drivers/lightnvm/rrpc.c
+++ b/drivers/lightnvm/rrpc.c
@@ -98,6 +98,9 @@ static void rrpc_discard(struct rrpc *rrpc, struct bio *bio)
 
 	do {
 		rqd = rrpc_inflight_laddr_acquire(rrpc, slba, len);
+		if (rqd)
+			break;
+
 		schedule();
 	} while (!rqd);
 
-- 
1.8.3.1

[toc] | [next] | [standalone]


#1403456 — [PATCH 2/2] lightnvm: hold nvm_lock until finish the target creation

FromWenwei Tao <wwtao0320@163.com>
Date2016-05-19 08:40 +0200
Subject[PATCH 2/2] lightnvm: hold nvm_lock until finish the target creation
Message-ID<rApMK-4Bp-13@gated-at.bofh.it>
In reply to#1403455
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)
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]


#1403715 — Re: [PATCH 2/2] lightnvm: hold nvm_lock until finish the target creation

FromMatias Bjørling <mb@lightnvm.io>
Date2016-05-19 14:50 +0200
SubjectRe: [PATCH 2/2] lightnvm: hold nvm_lock until finish the target creation
Message-ID<rAvyN-8iS-11@gated-at.bofh.it>
In reply to#1403456
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?

[toc] | [prev] | [next] | [standalone]


#1403809 — Re: [PATCH 2/2] lightnvm: hold nvm_lock until finish the target creation

FromWenwei Tao <ww.tao0320@gmail.com>
Date2016-05-19 17:20 +0200
SubjectRe: [PATCH 2/2] lightnvm: hold nvm_lock until finish the target creation
Message-ID<rAxTY-1sm-15@gated-at.bofh.it>
In reply to#1403715
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?

[toc] | [prev] | [next] | [standalone]


#1403717

FromMatias Bjørling <mb@lightnvm.io>
Date2016-05-19 14:50 +0200
Message-ID<rAvyO-8iS-21@gated-at.bofh.it>
In reply to#1403455
On 05/19/2016 08:38 AM, Wenwei Tao wrote:
> From: Wenwei Tao <ww.tao0320@gmail.com>
>
> Break the loop when rqd is not null to reduce
> unnecessary schedule.
>
> Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
> ---
>   drivers/lightnvm/rrpc.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
> index 2103e97..2915e39 100644
> --- a/drivers/lightnvm/rrpc.c
> +++ b/drivers/lightnvm/rrpc.c
> @@ -98,6 +98,9 @@ static void rrpc_discard(struct rrpc *rrpc, struct bio *bio)
>
>   	do {
>   		rqd = rrpc_inflight_laddr_acquire(rrpc, slba, len);
> +		if (rqd)
> +			break;
> +
>   		schedule();
>   	} while (!rqd);
>
>

If that case, mind if I convert the do..while to while?

while(1) {
	rqd = rrpc_inflight_laddr_acquire(rrpc, slba, len);
	if (rqd)
		break;
	schedule();
}

[toc] | [prev] | [next] | [standalone]


#1403811

FromWenwei Tao <ww.tao0320@gmail.com>
Date2016-05-19 17:20 +0200
Message-ID<rAxTY-1sm-25@gated-at.bofh.it>
In reply to#1403717
That is okay with me.

2016-05-19 20:40 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>
>>
>> Break the loop when rqd is not null to reduce
>> unnecessary schedule.
>>
>> Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
>> ---
>>   drivers/lightnvm/rrpc.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
>> index 2103e97..2915e39 100644
>> --- a/drivers/lightnvm/rrpc.c
>> +++ b/drivers/lightnvm/rrpc.c
>> @@ -98,6 +98,9 @@ static void rrpc_discard(struct rrpc *rrpc, struct bio
>> *bio)
>>
>>         do {
>>                 rqd = rrpc_inflight_laddr_acquire(rrpc, slba, len);
>> +               if (rqd)
>> +                       break;
>> +
>>                 schedule();
>>         } while (!rqd);
>>
>>
>
> If that case, mind if I convert the do..while to while?
>
> while(1) {
>         rqd = rrpc_inflight_laddr_acquire(rrpc, slba, len);
>         if (rqd)
>                 break;
>         schedule();
> }

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web