* lightnvm: pblk: avoid ref warning on cache creation
@ 2018-11-22 13:46 Javier González
2018-11-22 13:46 ` [PATCH] " Javier González
2018-11-23 8:14 ` Matias Bjørling
0 siblings, 2 replies; 9+ messages in thread
From: Javier González @ 2018-11-22 13:46 UTC (permalink / raw)
To: mb; +Cc: hans.holmberg, linux-block, linux-kernel, Javier González
Matias,
Can you pick this up for 4.20? Even though it is not an error per se, it
does trigger an ugly false positive warning when CONFIG_REFCOUNT_FULL is
set.
Thanks,
Javier
Javier González (1):
lightnvm: pblk: avoid ref warning on cache creation
drivers/lightnvm/pblk-init.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] lightnvm: pblk: avoid ref warning on cache creation
2018-11-22 13:46 lightnvm: pblk: avoid ref warning on cache creation Javier González
@ 2018-11-22 13:46 ` Javier González
2018-11-23 10:04 ` Hans Holmberg
2018-11-23 8:14 ` Matias Bjørling
1 sibling, 1 reply; 9+ messages in thread
From: Javier González @ 2018-11-22 13:46 UTC (permalink / raw)
To: mb; +Cc: hans.holmberg, linux-block, linux-kernel, Javier González
The current kref implementation around pblk global caches triggers a
false positive on refcount_inc_checked() (when called) as the kref is
initialized to 0. Instead of usint kref_inc() on a 0 reference, which is
in principle correct, use kref_init() to avoid the check. This is also
more explicit about what actually happens on cache creation.
In the process, do a small refactoring to use kref helpers.
Signed-off-by: Javier González <javier@cnexlabs.com>
---
drivers/lightnvm/pblk-init.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index 13822594647c..e225bd60cbb4 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -350,23 +350,19 @@ static int pblk_create_global_caches(void)
static int pblk_get_global_caches(void)
{
- int ret;
+ int ret = 0;
mutex_lock(&pblk_caches.mutex);
- if (kref_read(&pblk_caches.kref) > 0) {
- kref_get(&pblk_caches.kref);
- mutex_unlock(&pblk_caches.mutex);
- return 0;
- }
+ if (kref_get_unless_zero(&pblk_caches.kref))
+ goto out;
ret = pblk_create_global_caches();
-
if (!ret)
- kref_get(&pblk_caches.kref);
+ kref_init(&pblk_caches.kref);
+out:
mutex_unlock(&pblk_caches.mutex);
-
return ret;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: lightnvm: pblk: avoid ref warning on cache creation
2018-11-22 13:46 lightnvm: pblk: avoid ref warning on cache creation Javier González
2018-11-22 13:46 ` [PATCH] " Javier González
@ 2018-11-23 8:14 ` Matias Bjørling
2018-11-23 8:16 ` Javier Gonzalez
1 sibling, 1 reply; 9+ messages in thread
From: Matias Bjørling @ 2018-11-23 8:14 UTC (permalink / raw)
To: javier; +Cc: hans.holmberg, linux-block, linux-kernel, javier
On 11/22/2018 02:46 PM, Javier González wrote:
> Matias,
>
> Can you pick this up for 4.20? Even though it is not an error per se, it
> does trigger an ugly false positive warning when CONFIG_REFCOUNT_FULL is
> set.
>
> Thanks,
> Javier
>
> Javier González (1):
> lightnvm: pblk: avoid ref warning on cache creation
>
> drivers/lightnvm/pblk-init.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
It is too late to get it for 4.20. I'll pull it in for 4.21.
We can put on a Fixes: 1864de94ec9d6 "lightnvm: pblk: stop recreating
global caches" if you want Greg and friends to pick picked it up for
stable kernels?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: lightnvm: pblk: avoid ref warning on cache creation
2018-11-23 8:14 ` Matias Bjørling
@ 2018-11-23 8:16 ` Javier Gonzalez
2018-11-23 8:23 ` Matias Bjørling
0 siblings, 1 reply; 9+ messages in thread
From: Javier Gonzalez @ 2018-11-23 8:16 UTC (permalink / raw)
To: Matias Bjørling; +Cc: javier, Hans Holmberg, linux-block, linux-kernel
> On 23 Nov 2018, at 09.14, Matias Bjørling <mb@lightnvm.io> wrote:
>
>> On 11/22/2018 02:46 PM, Javier González wrote:
>> Matias,
>> Can you pick this up for 4.20? Even though it is not an error per se, it
>> does trigger an ugly false positive warning when CONFIG_REFCOUNT_FULL is
>> set.
>> Thanks,
>> Javier
>> Javier González (1):
>> lightnvm: pblk: avoid ref warning on cache creation
>> drivers/lightnvm/pblk-init.c | 14 +++++---------
>> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> It is too late to get it for 4.20. I'll pull it in for 4.21.
Well, if it is a fix it’s not too late AFAIK. It also applies to a patch picked up in this series. We should have catches this before.
But I guess it’s your call.
>
> We can put on a Fixes: 1864de94ec9d6 "lightnvm: pblk: stop recreating global caches" if you want Greg and friends to pick picked it up for stable kernels?
I didn’t add it as it is not a bug in itself. But it is good to add it. Can you do it.
Javier.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: lightnvm: pblk: avoid ref warning on cache creation
2018-11-23 8:16 ` Javier Gonzalez
@ 2018-11-23 8:23 ` Matias Bjørling
2018-11-23 8:31 ` Javier Gonzalez
0 siblings, 1 reply; 9+ messages in thread
From: Matias Bjørling @ 2018-11-23 8:23 UTC (permalink / raw)
To: javier; +Cc: javier, hans.holmberg, linux-block, linux-kernel
On 11/23/2018 09:16 AM, Javier Gonzalez wrote:
>
>
>> On 23 Nov 2018, at 09.14, Matias Bjørling <mb@lightnvm.io> wrote:
>>
>>> On 11/22/2018 02:46 PM, Javier González wrote:
>>> Matias,
>>> Can you pick this up for 4.20? Even though it is not an error per se, it
>>> does trigger an ugly false positive warning when CONFIG_REFCOUNT_FULL is
>>> set.
>>> Thanks,
>>> Javier
>>> Javier González (1):
>>> lightnvm: pblk: avoid ref warning on cache creation
>>> drivers/lightnvm/pblk-init.c | 14 +++++---------
>>> 1 file changed, 5 insertions(+), 9 deletions(-)
>>
>> It is too late to get it for 4.20. I'll pull it in for 4.21.
>
> Well, if it is a fix it’s not too late AFAIK. It also applies to a patch picked up in this series. We should have catches this before.
>
> But I guess it’s your call.
There is a likelihood that Linus would catch it, with the result of fire
& fury. I don't feel like using one of my cat life's for this one.
>
>>
>> We can put on a Fixes: 1864de94ec9d6 "lightnvm: pblk: stop recreating global caches" if you want Greg and friends to pick picked it up for stable kernels?
>
> I didn’t add it as it is not a bug in itself. But it is good to add it. Can you do it.
>
I'll do it.
> Javier.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: lightnvm: pblk: avoid ref warning on cache creation
2018-11-23 8:23 ` Matias Bjørling
@ 2018-11-23 8:31 ` Javier Gonzalez
2018-11-23 8:41 ` Matias Bjørling
0 siblings, 1 reply; 9+ messages in thread
From: Javier Gonzalez @ 2018-11-23 8:31 UTC (permalink / raw)
To: Matias Bjørling; +Cc: javier, Hans Holmberg, linux-block, linux-kernel
> On 23 Nov 2018, at 09.24, Matias Bjørling <mb@lightnvm.io> wrote:
>
> On 11/23/2018 09:16 AM, Javier Gonzalez wrote:
>>> On 23 Nov 2018, at 09.14, Matias Bjørling <mb@lightnvm.io> wrote:
>>>
>>>> On 11/22/2018 02:46 PM, Javier González wrote:
>>>> Matias,
>>>> Can you pick this up for 4.20? Even though it is not an error per se, it
>>>> does trigger an ugly false positive warning when CONFIG_REFCOUNT_FULL is
>>>> set.
>>>> Thanks,
>>>> Javier
>>>> Javier González (1):
>>>> lightnvm: pblk: avoid ref warning on cache creation
>>>> drivers/lightnvm/pblk-init.c | 14 +++++---------
>>>> 1 file changed, 5 insertions(+), 9 deletions(-)
>>>
>>> It is too late to get it for 4.20. I'll pull it in for 4.21.
>> Well, if it is a fix it’s not too late AFAIK. It also applies to a patch picked up in this series. We should have catches this before.
>> But I guess it’s your call.
>
> There is a likelihood that Linus would catch it, with the result of fire & fury. I don't feel like using one of my cat life's for this one.
I don’t get how a fix in rc3 will result in Jens or Linus complaining. But Ok, let’s release 4.20 with a false positive warning for use-after-free on pblk creation.
>>> We can put on a Fixes: 1864de94ec9d6 "lightnvm: pblk: stop recreating global caches" if you want Greg and friends to pick picked it up for stable kernels?
>> I didn’t add it as it is not a bug in itself. But it is good to add it. Can you do it.
>
> I'll do it.
Thanks.
Javier.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: lightnvm: pblk: avoid ref warning on cache creation
2018-11-23 8:31 ` Javier Gonzalez
@ 2018-11-23 8:41 ` Matias Bjørling
0 siblings, 0 replies; 9+ messages in thread
From: Matias Bjørling @ 2018-11-23 8:41 UTC (permalink / raw)
To: javier; +Cc: javier, hans.holmberg, linux-block, linux-kernel
On 11/23/2018 09:31 AM, Javier Gonzalez wrote:
>
>
>> On 23 Nov 2018, at 09.24, Matias Bjørling <mb@lightnvm.io> wrote:
>>
>> On 11/23/2018 09:16 AM, Javier Gonzalez wrote:
>>>> On 23 Nov 2018, at 09.14, Matias Bjørling <mb@lightnvm.io> wrote:
>>>>
>>>>> On 11/22/2018 02:46 PM, Javier González wrote:
>>>>> Matias,
>>>>> Can you pick this up for 4.20? Even though it is not an error per se, it
>>>>> does trigger an ugly false positive warning when CONFIG_REFCOUNT_FULL is
>>>>> set.
>>>>> Thanks,
>>>>> Javier
>>>>> Javier González (1):
>>>>> lightnvm: pblk: avoid ref warning on cache creation
>>>>> drivers/lightnvm/pblk-init.c | 14 +++++---------
>>>>> 1 file changed, 5 insertions(+), 9 deletions(-)
>>>>
>>>> It is too late to get it for 4.20. I'll pull it in for 4.21.
>>> Well, if it is a fix it’s not too late AFAIK. It also applies to a patch picked up in this series. We should have catches this before.
>>> But I guess it’s your call.
>>
>> There is a likelihood that Linus would catch it, with the result of fire & fury. I don't feel like using one of my cat life's for this one.
>
> I don’t get how a fix in rc3 will result in Jens or Linus complaining. But Ok, let’s release 4.20 with a false positive warning for use-after-free on pblk creation.
>
It is a warning that would trigger only for kernel developers. The
REFCOUNT_FULL is used when debugging code, and is usually not on in
production kernels.
To my understanding, the rc's, are meant for errors that would affect
end-users.
>
>>>> We can put on a Fixes: 1864de94ec9d6 "lightnvm: pblk: stop recreating global caches" if you want Greg and friends to pick picked it up for stable kernels?
>>> I didn’t add it as it is not a bug in itself. But it is good to add it. Can you do it.
>>
>> I'll do it.
>
> Thanks.
>
> Javier.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] lightnvm: pblk: avoid ref warning on cache creation
2018-11-22 13:46 ` [PATCH] " Javier González
@ 2018-11-23 10:04 ` Hans Holmberg
2018-11-30 8:36 ` Matias Bjørling
0 siblings, 1 reply; 9+ messages in thread
From: Hans Holmberg @ 2018-11-23 10:04 UTC (permalink / raw)
To: Javier González
Cc: Matias Bjorling, Hans Holmberg, linux-block,
Linux Kernel Mailing List, Javier Gonzalez
Great catch Javier! Nice refactoring work too.
Reviewed-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
On Thu, Nov 22, 2018 at 2:47 PM Javier González <javier@javigon.com> wrote:
>
> The current kref implementation around pblk global caches triggers a
> false positive on refcount_inc_checked() (when called) as the kref is
> initialized to 0. Instead of usint kref_inc() on a 0 reference, which is
> in principle correct, use kref_init() to avoid the check. This is also
> more explicit about what actually happens on cache creation.
>
> In the process, do a small refactoring to use kref helpers.
>
> Signed-off-by: Javier González <javier@cnexlabs.com>
> ---
> drivers/lightnvm/pblk-init.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> index 13822594647c..e225bd60cbb4 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -350,23 +350,19 @@ static int pblk_create_global_caches(void)
>
> static int pblk_get_global_caches(void)
> {
> - int ret;
> + int ret = 0;
>
> mutex_lock(&pblk_caches.mutex);
>
> - if (kref_read(&pblk_caches.kref) > 0) {
> - kref_get(&pblk_caches.kref);
> - mutex_unlock(&pblk_caches.mutex);
> - return 0;
> - }
> + if (kref_get_unless_zero(&pblk_caches.kref))
> + goto out;
>
> ret = pblk_create_global_caches();
> -
> if (!ret)
> - kref_get(&pblk_caches.kref);
> + kref_init(&pblk_caches.kref);
>
> +out:
> mutex_unlock(&pblk_caches.mutex);
> -
> return ret;
> }
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] lightnvm: pblk: avoid ref warning on cache creation
2018-11-23 10:04 ` Hans Holmberg
@ 2018-11-30 8:36 ` Matias Bjørling
0 siblings, 0 replies; 9+ messages in thread
From: Matias Bjørling @ 2018-11-30 8:36 UTC (permalink / raw)
To: Hans Holmberg, Javier González
Cc: Hans Holmberg, linux-block, Linux Kernel Mailing List, Javier Gonzalez
On 11/23/2018 11:04 AM, Hans Holmberg wrote:
> Great catch Javier! Nice refactoring work too.
>
> Reviewed-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> On Thu, Nov 22, 2018 at 2:47 PM Javier González <javier@javigon.com> wrote:
>>
>> The current kref implementation around pblk global caches triggers a
>> false positive on refcount_inc_checked() (when called) as the kref is
>> initialized to 0. Instead of usint kref_inc() on a 0 reference, which is
>> in principle correct, use kref_init() to avoid the check. This is also
>> more explicit about what actually happens on cache creation.
>>
>> In the process, do a small refactoring to use kref helpers.
>>
>> Signed-off-by: Javier González <javier@cnexlabs.com>
>> ---
>> drivers/lightnvm/pblk-init.c | 14 +++++---------
>> 1 file changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
>> index 13822594647c..e225bd60cbb4 100644
>> --- a/drivers/lightnvm/pblk-init.c
>> +++ b/drivers/lightnvm/pblk-init.c
>> @@ -350,23 +350,19 @@ static int pblk_create_global_caches(void)
>>
>> static int pblk_get_global_caches(void)
>> {
>> - int ret;
>> + int ret = 0;
>>
>> mutex_lock(&pblk_caches.mutex);
>>
>> - if (kref_read(&pblk_caches.kref) > 0) {
>> - kref_get(&pblk_caches.kref);
>> - mutex_unlock(&pblk_caches.mutex);
>> - return 0;
>> - }
>> + if (kref_get_unless_zero(&pblk_caches.kref))
>> + goto out;
>>
>> ret = pblk_create_global_caches();
>> -
>> if (!ret)
>> - kref_get(&pblk_caches.kref);
>> + kref_init(&pblk_caches.kref);
>>
>> +out:
>> mutex_unlock(&pblk_caches.mutex);
>> -
>> return ret;
>> }
>>
>> --
>> 2.7.4
>>
Picked up for 4.21.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-11-30 8:36 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-22 13:46 lightnvm: pblk: avoid ref warning on cache creation Javier González
2018-11-22 13:46 ` [PATCH] " Javier González
2018-11-23 10:04 ` Hans Holmberg
2018-11-30 8:36 ` Matias Bjørling
2018-11-23 8:14 ` Matias Bjørling
2018-11-23 8:16 ` Javier Gonzalez
2018-11-23 8:23 ` Matias Bjørling
2018-11-23 8:31 ` Javier Gonzalez
2018-11-23 8:41 ` Matias Bjørling
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).