| Message ID | 1542894410-14047-2-git-send-email-javier@cnexlabs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lightnvm: pblk: avoid ref warning on cache creation | expand |
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 >
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.
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; }
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(-)