| Message ID | 20180528085841.26684-17-mb@lightnvm.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> On 28 May 2018, at 10.58, Matias Bjørling <mb@lightnvm.io> wrote: > > From: Igor Konopko <igor.j.konopko@intel.com> > > When all the blocks (chunks) in line are marked as bad (offline) > we shouldn't try to read smeta during init process. > > Currently we are trying to do so by passing -1 as PPA address, > what causes multiple warnings, that we issuing IOs to out-of-bound > PPAs. > > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > Signed-off-by: Marcin Dziegielewski <marcin.dziegielewski@intel.com> > Updated title. > Signed-off-by: Matias Bjørling <mb@lightnvm.io> > --- > drivers/lightnvm/pblk-core.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > index a20b41c355c5..e3e883547198 100644 > --- a/drivers/lightnvm/pblk-core.c > +++ b/drivers/lightnvm/pblk-core.c > @@ -868,6 +868,11 @@ int pblk_line_read_smeta(struct pblk *pblk, struct pblk_line *line) > { > u64 bpaddr = pblk_line_smeta_start(pblk, line); > > + if (bpaddr == -1) { > + /* Whole line is bad - do not try to read smeta. */ > + return 1; > + } This case cannot occur on the only user of the function (pblk_recov_l2p()). On the previous check (pblk_line_was_written()), we verify the state of the line and the position of the first good chunk. In the case of a bad line (less chunks than a given threshold to allow emeta), the recovery will not be carried out in the line. Javier
> From: Javier Gonzalez [mailto:javier@cnexlabs.com] > This case cannot occur on the only user of the function > (pblk_recov_l2p()). On the previous check (pblk_line_was_written()), we > verify the state of the line and the position of the first good chunk. In > the case of a bad line (less chunks than a given threshold to allow > emeta), the recovery will not be carried out in the line. You are right. It looks that during my testing there was some inconsistency between chunks state table which is verified inside pblk_line_was_written() and blk_bitmap which was read from emeta and is verified in pblk_line_smeta_start(). I'm living decision to maintainers whether we should keep this sanity check or not - it really just pass gracefully the result from pblk_line_smeta_start() where similar sanity check is present. Igor
> On 29 May 2018, at 15.15, Konopko, Igor J <igor.j.konopko@intel.com> wrote: > >> From: Javier Gonzalez [mailto:javier@cnexlabs.com] >> This case cannot occur on the only user of the function >> (pblk_recov_l2p()). On the previous check (pblk_line_was_written()), we >> verify the state of the line and the position of the first good chunk. In >> the case of a bad line (less chunks than a given threshold to allow >> emeta), the recovery will not be carried out in the line. > You are right. It looks that during my testing there was some > inconsistency between chunks state table which is verified inside > pblk_line_was_written() and blk_bitmap which was read from emeta and > is verified in pblk_line_smeta_start(). I'm living decision to > maintainers whether we should keep this sanity check or not - it > really just pass gracefully the result from pblk_line_smeta_start() > where similar sanity check is present. > Let's avoid an extra check now that there is no users for it in the current flow. If we have a new use for pblk_line_smeta_start() on a flow that does cannot offer the same guarantees, we can add it at that point. Javier
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c index a20b41c355c5..e3e883547198 100644 --- a/drivers/lightnvm/pblk-core.c +++ b/drivers/lightnvm/pblk-core.c @@ -868,6 +868,11 @@ int pblk_line_read_smeta(struct pblk *pblk, struct pblk_line *line) { u64 bpaddr = pblk_line_smeta_start(pblk, line); + if (bpaddr == -1) { + /* Whole line is bad - do not try to read smeta. */ + return 1; + } + return pblk_line_submit_smeta_io(pblk, line, bpaddr, PBLK_READ_RECOV); }