| Message ID | 20190305135120.29284-8-igor.j.konopko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lightnvm: bugfixes and improvements | expand |
> On 5 Mar 2019, at 14.51, Igor Konopko <igor.j.konopko@intel.com> wrote: > > When we creating pblk instance with factory flag, there is a > possibility that some chunks are in open state, which does not allow > to issue erase request to them directly, based on OCSSD 2.0 spec. > Still most of the controllers allows for such a transition, but it is > not guaranteed that the particular drive will do so. This patch adds > the proper warning during pblk factory creation to let user know about > number of chunks in such a state, which can potentially be a reason > of erase failures. > > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > --- > drivers/lightnvm/pblk-core.c | 14 ++++++++++++++ > drivers/lightnvm/pblk-init.c | 14 +++++++++++--- > drivers/lightnvm/pblk.h | 1 + > 3 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > index 64280e6..4f16596 100644 > --- a/drivers/lightnvm/pblk-core.c > +++ b/drivers/lightnvm/pblk-core.c > @@ -161,6 +161,20 @@ struct nvm_chk_meta *pblk_chunk_get_off(struct pblk *pblk, > return meta + ch_off + lun_off + chk_off; > } > > +int pblk_count_opened_chunks(struct pblk *pblk, struct nvm_chk_meta *meta) > +{ > + struct nvm_tgt_dev *dev = pblk->dev; > + struct nvm_geo *geo = &dev->geo; > + int i, cnt = 0; > + > + for (i = 0; i < geo->all_luns; i++) { > + if (meta[i].state == NVM_CHK_ST_OPEN) > + cnt++; > + } > + > + return cnt; > +} > + > void __pblk_map_invalidate(struct pblk *pblk, struct pblk_line *line, > u64 paddr) > { > diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c > index 97b4c6e..f590f62 100644 > --- a/drivers/lightnvm/pblk-init.c > +++ b/drivers/lightnvm/pblk-init.c > @@ -1028,12 +1028,12 @@ static int pblk_line_meta_init(struct pblk *pblk) > return 0; > } > > -static int pblk_lines_init(struct pblk *pblk) > +static int pblk_lines_init(struct pblk *pblk, bool factory_init) > { > struct pblk_line_mgmt *l_mg = &pblk->l_mg; > struct pblk_line *line; > void *chunk_meta; > - int nr_free_chks = 0; > + int nr_free_chks = 0, nr_opened_chks; > int i, ret; > > ret = pblk_line_meta_init(pblk); > @@ -1054,6 +1054,14 @@ static int pblk_lines_init(struct pblk *pblk) > goto fail_free_luns; > } > > + if (factory_init) { > + nr_opened_chks = pblk_count_opened_chunks(pblk, chunk_meta); > + if (nr_opened_chks) { > + pblk_warn(pblk, "There are %d opened chunks\n", > + nr_opened_chks); > + } > + } > + > pblk->lines = kcalloc(l_mg->nr_lines, sizeof(struct pblk_line), > GFP_KERNEL); > if (!pblk->lines) { > @@ -1245,7 +1253,7 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, > goto fail; > } > > - ret = pblk_lines_init(pblk); > + ret = pblk_lines_init(pblk, flags & NVM_TARGET_FACTORY); > if (ret) { > pblk_err(pblk, "could not initialize lines\n"); > goto fail_free_core; > diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h > index 6c82776..c2f07ec 100644 > --- a/drivers/lightnvm/pblk.h > +++ b/drivers/lightnvm/pblk.h > @@ -795,6 +795,7 @@ struct nvm_chk_meta *pblk_get_chunk_meta(struct pblk *pblk); > struct nvm_chk_meta *pblk_chunk_get_off(struct pblk *pblk, > struct nvm_chk_meta *lp, > struct ppa_addr ppa); > +int pblk_count_opened_chunks(struct pblk *pblk, struct nvm_chk_meta *_meta); > void pblk_log_write_err(struct pblk *pblk, struct nvm_rq *rqd); > void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd); > int pblk_submit_io(struct pblk *pblk, struct nvm_rq *rqd); > -- > 2.9.5 I think the warning is a good tradeoff. If you want to avoid looping again across all chunks, note that we are already doing this in pblk_setup_line_meta_chk(). It should be easy to refactor it, check here for NVM_CHK_ST_OPEN and then return the counter.
On Tue, Mar 5, 2019 at 2:54 PM Igor Konopko <igor.j.konopko@intel.com> wrote: > > When we creating pblk instance with factory flag, there is a > possibility that some chunks are in open state, which does not allow > to issue erase request to them directly, based on OCSSD 2.0 spec. > Still most of the controllers allows for such a transition, but it is > not guaranteed that the particular drive will do so. This patch adds > the proper warning during pblk factory creation to let user know about > number of chunks in such a state, which can potentially be a reason > of erase failures. > > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > --- > drivers/lightnvm/pblk-core.c | 14 ++++++++++++++ > drivers/lightnvm/pblk-init.c | 14 +++++++++++--- > drivers/lightnvm/pblk.h | 1 + > 3 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > index 64280e6..4f16596 100644 > --- a/drivers/lightnvm/pblk-core.c > +++ b/drivers/lightnvm/pblk-core.c > @@ -161,6 +161,20 @@ struct nvm_chk_meta *pblk_chunk_get_off(struct pblk *pblk, > return meta + ch_off + lun_off + chk_off; > } > > +int pblk_count_opened_chunks(struct pblk *pblk, struct nvm_chk_meta *meta) > +{ > + struct nvm_tgt_dev *dev = pblk->dev; > + struct nvm_geo *geo = &dev->geo; > + int i, cnt = 0; > + > + for (i = 0; i < geo->all_luns; i++) { Shouldn't this be i < geo->all_chunks ? Otherwise the patch looks good. Thanks, Hans > + if (meta[i].state == NVM_CHK_ST_OPEN) > + cnt++; > + } > + > + return cnt; > +} > + > void __pblk_map_invalidate(struct pblk *pblk, struct pblk_line *line, > u64 paddr) > { > diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c > index 97b4c6e..f590f62 100644 > --- a/drivers/lightnvm/pblk-init.c > +++ b/drivers/lightnvm/pblk-init.c > @@ -1028,12 +1028,12 @@ static int pblk_line_meta_init(struct pblk *pblk) > return 0; > } > > -static int pblk_lines_init(struct pblk *pblk) > +static int pblk_lines_init(struct pblk *pblk, bool factory_init) > { > struct pblk_line_mgmt *l_mg = &pblk->l_mg; > struct pblk_line *line; > void *chunk_meta; > - int nr_free_chks = 0; > + int nr_free_chks = 0, nr_opened_chks; > int i, ret; > > ret = pblk_line_meta_init(pblk); > @@ -1054,6 +1054,14 @@ static int pblk_lines_init(struct pblk *pblk) > goto fail_free_luns; > } > > + if (factory_init) { > + nr_opened_chks = pblk_count_opened_chunks(pblk, chunk_meta); > + if (nr_opened_chks) { > + pblk_warn(pblk, "There are %d opened chunks\n", > + nr_opened_chks); > + } > + } > + > pblk->lines = kcalloc(l_mg->nr_lines, sizeof(struct pblk_line), > GFP_KERNEL); > if (!pblk->lines) { > @@ -1245,7 +1253,7 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, > goto fail; > } > > - ret = pblk_lines_init(pblk); > + ret = pblk_lines_init(pblk, flags & NVM_TARGET_FACTORY); > if (ret) { > pblk_err(pblk, "could not initialize lines\n"); > goto fail_free_core; > diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h > index 6c82776..c2f07ec 100644 > --- a/drivers/lightnvm/pblk.h > +++ b/drivers/lightnvm/pblk.h > @@ -795,6 +795,7 @@ struct nvm_chk_meta *pblk_get_chunk_meta(struct pblk *pblk); > struct nvm_chk_meta *pblk_chunk_get_off(struct pblk *pblk, > struct nvm_chk_meta *lp, > struct ppa_addr ppa); > +int pblk_count_opened_chunks(struct pblk *pblk, struct nvm_chk_meta *_meta); > void pblk_log_write_err(struct pblk *pblk, struct nvm_rq *rqd); > void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd); > int pblk_submit_io(struct pblk *pblk, struct nvm_rq *rqd); > -- > 2.9.5 >
On 06.03.2019 15:27, Hans Holmberg wrote: > On Tue, Mar 5, 2019 at 2:54 PM Igor Konopko <igor.j.konopko@intel.com> wrote: >> >> When we creating pblk instance with factory flag, there is a >> possibility that some chunks are in open state, which does not allow >> to issue erase request to them directly, based on OCSSD 2.0 spec. >> Still most of the controllers allows for such a transition, but it is >> not guaranteed that the particular drive will do so. This patch adds >> the proper warning during pblk factory creation to let user know about >> number of chunks in such a state, which can potentially be a reason >> of erase failures. >> >> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> >> --- >> drivers/lightnvm/pblk-core.c | 14 ++++++++++++++ >> drivers/lightnvm/pblk-init.c | 14 +++++++++++--- >> drivers/lightnvm/pblk.h | 1 + >> 3 files changed, 26 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c >> index 64280e6..4f16596 100644 >> --- a/drivers/lightnvm/pblk-core.c >> +++ b/drivers/lightnvm/pblk-core.c >> @@ -161,6 +161,20 @@ struct nvm_chk_meta *pblk_chunk_get_off(struct pblk *pblk, >> return meta + ch_off + lun_off + chk_off; >> } >> >> +int pblk_count_opened_chunks(struct pblk *pblk, struct nvm_chk_meta *meta) >> +{ >> + struct nvm_tgt_dev *dev = pblk->dev; >> + struct nvm_geo *geo = &dev->geo; >> + int i, cnt = 0; >> + >> + for (i = 0; i < geo->all_luns; i++) { > > Shouldn't this be i < geo->all_chunks ? > Yes, it should be. Anyway, Javier proposed to use existing pblk_setup_line_meta_chk() function for counting that, so I'm gonna to rewrite this patch. > Otherwise the patch looks good. > > Thanks, > Hans > >> + if (meta[i].state == NVM_CHK_ST_OPEN) >> + cnt++; >> + } >> + >> + return cnt; >> +} >> + >> void __pblk_map_invalidate(struct pblk *pblk, struct pblk_line *line, >> u64 paddr) >> { >> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c >> index 97b4c6e..f590f62 100644 >> --- a/drivers/lightnvm/pblk-init.c >> +++ b/drivers/lightnvm/pblk-init.c >> @@ -1028,12 +1028,12 @@ static int pblk_line_meta_init(struct pblk *pblk) >> return 0; >> } >> >> -static int pblk_lines_init(struct pblk *pblk) >> +static int pblk_lines_init(struct pblk *pblk, bool factory_init) >> { >> struct pblk_line_mgmt *l_mg = &pblk->l_mg; >> struct pblk_line *line; >> void *chunk_meta; >> - int nr_free_chks = 0; >> + int nr_free_chks = 0, nr_opened_chks; >> int i, ret; >> >> ret = pblk_line_meta_init(pblk); >> @@ -1054,6 +1054,14 @@ static int pblk_lines_init(struct pblk *pblk) >> goto fail_free_luns; >> } >> >> + if (factory_init) { >> + nr_opened_chks = pblk_count_opened_chunks(pblk, chunk_meta); >> + if (nr_opened_chks) { >> + pblk_warn(pblk, "There are %d opened chunks\n", >> + nr_opened_chks); >> + } >> + } >> + >> pblk->lines = kcalloc(l_mg->nr_lines, sizeof(struct pblk_line), >> GFP_KERNEL); >> if (!pblk->lines) { >> @@ -1245,7 +1253,7 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, >> goto fail; >> } >> >> - ret = pblk_lines_init(pblk); >> + ret = pblk_lines_init(pblk, flags & NVM_TARGET_FACTORY); >> if (ret) { >> pblk_err(pblk, "could not initialize lines\n"); >> goto fail_free_core; >> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h >> index 6c82776..c2f07ec 100644 >> --- a/drivers/lightnvm/pblk.h >> +++ b/drivers/lightnvm/pblk.h >> @@ -795,6 +795,7 @@ struct nvm_chk_meta *pblk_get_chunk_meta(struct pblk *pblk); >> struct nvm_chk_meta *pblk_chunk_get_off(struct pblk *pblk, >> struct nvm_chk_meta *lp, >> struct ppa_addr ppa); >> +int pblk_count_opened_chunks(struct pblk *pblk, struct nvm_chk_meta *_meta); >> void pblk_log_write_err(struct pblk *pblk, struct nvm_rq *rqd); >> void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd); >> int pblk_submit_io(struct pblk *pblk, struct nvm_rq *rqd); >> -- >> 2.9.5 >>
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c index 64280e6..4f16596 100644 --- a/drivers/lightnvm/pblk-core.c +++ b/drivers/lightnvm/pblk-core.c @@ -161,6 +161,20 @@ struct nvm_chk_meta *pblk_chunk_get_off(struct pblk *pblk, return meta + ch_off + lun_off + chk_off; } +int pblk_count_opened_chunks(struct pblk *pblk, struct nvm_chk_meta *meta) +{ + struct nvm_tgt_dev *dev = pblk->dev; + struct nvm_geo *geo = &dev->geo; + int i, cnt = 0; + + for (i = 0; i < geo->all_luns; i++) { + if (meta[i].state == NVM_CHK_ST_OPEN) + cnt++; + } + + return cnt; +} + void __pblk_map_invalidate(struct pblk *pblk, struct pblk_line *line, u64 paddr) { diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c index 97b4c6e..f590f62 100644 --- a/drivers/lightnvm/pblk-init.c +++ b/drivers/lightnvm/pblk-init.c @@ -1028,12 +1028,12 @@ static int pblk_line_meta_init(struct pblk *pblk) return 0; } -static int pblk_lines_init(struct pblk *pblk) +static int pblk_lines_init(struct pblk *pblk, bool factory_init) { struct pblk_line_mgmt *l_mg = &pblk->l_mg; struct pblk_line *line; void *chunk_meta; - int nr_free_chks = 0; + int nr_free_chks = 0, nr_opened_chks; int i, ret; ret = pblk_line_meta_init(pblk); @@ -1054,6 +1054,14 @@ static int pblk_lines_init(struct pblk *pblk) goto fail_free_luns; } + if (factory_init) { + nr_opened_chks = pblk_count_opened_chunks(pblk, chunk_meta); + if (nr_opened_chks) { + pblk_warn(pblk, "There are %d opened chunks\n", + nr_opened_chks); + } + } + pblk->lines = kcalloc(l_mg->nr_lines, sizeof(struct pblk_line), GFP_KERNEL); if (!pblk->lines) { @@ -1245,7 +1253,7 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, goto fail; } - ret = pblk_lines_init(pblk); + ret = pblk_lines_init(pblk, flags & NVM_TARGET_FACTORY); if (ret) { pblk_err(pblk, "could not initialize lines\n"); goto fail_free_core; diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h index 6c82776..c2f07ec 100644 --- a/drivers/lightnvm/pblk.h +++ b/drivers/lightnvm/pblk.h @@ -795,6 +795,7 @@ struct nvm_chk_meta *pblk_get_chunk_meta(struct pblk *pblk); struct nvm_chk_meta *pblk_chunk_get_off(struct pblk *pblk, struct nvm_chk_meta *lp, struct ppa_addr ppa); +int pblk_count_opened_chunks(struct pblk *pblk, struct nvm_chk_meta *_meta); void pblk_log_write_err(struct pblk *pblk, struct nvm_rq *rqd); void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd); int pblk_submit_io(struct pblk *pblk, struct nvm_rq *rqd);
When we creating pblk instance with factory flag, there is a possibility that some chunks are in open state, which does not allow to issue erase request to them directly, based on OCSSD 2.0 spec. Still most of the controllers allows for such a transition, but it is not guaranteed that the particular drive will do so. This patch adds the proper warning during pblk factory creation to let user know about number of chunks in such a state, which can potentially be a reason of erase failures. Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> --- drivers/lightnvm/pblk-core.c | 14 ++++++++++++++ drivers/lightnvm/pblk-init.c | 14 +++++++++++--- drivers/lightnvm/pblk.h | 1 + 3 files changed, 26 insertions(+), 3 deletions(-)