| Message ID | 20190227171442.11853-2-igor.j.konopko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lightnvm: bugfixes and improvements | expand |
On Wed, Feb 27, 2019 at 6:17 PM Igor Konopko <igor.j.konopko@intel.com> wrote: > > This patch fixes the error case in GC when > we both moves line back to closed state and > release additional reference, what cause illegal > transition from closed to free on pblk_line_put > when only gc to free line state transition is > allowed using that path. > > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > --- > drivers/lightnvm/pblk-gc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c > index 2fa118c8eb71..3feadfd9418d 100644 > --- a/drivers/lightnvm/pblk-gc.c > +++ b/drivers/lightnvm/pblk-gc.c > @@ -290,8 +290,11 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work) > fail_free_ws: > kfree(line_ws); > > + /* Line goes back to closed state, so we cannot release additional > + * reference for line, since we do that only when we want to do > + * gc to free line state transition. > + */ Good comment. It would be nice to document how the line recounting works in general, as it is, non-trivial to deduce from the code. I dug into this when supporting Heiner in his latest bugfix, and it's on my laundry list .. but if someone else feels inclined.. :) > pblk_put_line_back(pblk, line); > - kref_put(&line->ref, pblk_line_put); > atomic_dec(&gc->read_inflight_gc); > > pblk_err(pblk, "failed to GC line %d\n", line->id); > -- > 2.17.1 > Great catch! Reviewed-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> wrote: > > This patch fixes the error case in GC when > we both moves line back to closed state and > release additional reference, what cause illegal > transition from closed to free on pblk_line_put > when only gc to free line state transition is > allowed using that path. > > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > --- > drivers/lightnvm/pblk-gc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c > index 2fa118c8eb71..3feadfd9418d 100644 > --- a/drivers/lightnvm/pblk-gc.c > +++ b/drivers/lightnvm/pblk-gc.c > @@ -290,8 +290,11 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work) > fail_free_ws: > kfree(line_ws); > > + /* Line goes back to closed state, so we cannot release additional > + * reference for line, since we do that only when we want to do > + * gc to free line state transition. > + */ > pblk_put_line_back(pblk, line); > - kref_put(&line->ref, pblk_line_put); > atomic_dec(&gc->read_inflight_gc); > > pblk_err(pblk, "failed to GC line %d\n", line->id); > -- > 2.17.1 Looks good to me Reviewed-by: Javier González <javier@javigon.com>
On 2/27/19 6:14 PM, Igor Konopko wrote: > This patch fixes the error case in GC when > we both moves line back to closed state and > release additional reference, what cause illegal > transition from closed to free on pblk_line_put > when only gc to free line state transition is > allowed using that path. > > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > --- > drivers/lightnvm/pblk-gc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c > index 2fa118c8eb71..3feadfd9418d 100644 > --- a/drivers/lightnvm/pblk-gc.c > +++ b/drivers/lightnvm/pblk-gc.c > @@ -290,8 +290,11 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work) > fail_free_ws: > kfree(line_ws); > > + /* Line goes back to closed state, so we cannot release additional > + * reference for line, since we do that only when we want to do > + * gc to free line state transition. > + */ > pblk_put_line_back(pblk, line); > - kref_put(&line->ref, pblk_line_put); > atomic_dec(&gc->read_inflight_gc); > > pblk_err(pblk, "failed to GC line %d\n", line->id); > Thanks Igor. Applied for 5.2.
diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c index 2fa118c8eb71..3feadfd9418d 100644 --- a/drivers/lightnvm/pblk-gc.c +++ b/drivers/lightnvm/pblk-gc.c @@ -290,8 +290,11 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work) fail_free_ws: kfree(line_ws); + /* Line goes back to closed state, so we cannot release additional + * reference for line, since we do that only when we want to do + * gc to free line state transition. + */ pblk_put_line_back(pblk, line); - kref_put(&line->ref, pblk_line_put); atomic_dec(&gc->read_inflight_gc); pblk_err(pblk, "failed to GC line %d\n", line->id);
This patch fixes the error case in GC when we both moves line back to closed state and release additional reference, what cause illegal transition from closed to free on pblk_line_put when only gc to free line state transition is allowed using that path. Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> --- drivers/lightnvm/pblk-gc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)