| Message ID | 20190305135120.29284-4-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: > > Currently when unknown error occurs on read path there is only dmesg > information about it, but it is not counted in sysfs statistics. > > We would also like to track the number of such a requests, so new type > of counter 'read_ctrl_errors" is added in that patch. > > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > --- > drivers/lightnvm/pblk-core.c | 1 + > drivers/lightnvm/pblk-init.c | 1 + > drivers/lightnvm/pblk-sysfs.c | 3 ++- > drivers/lightnvm/pblk.h | 1 + > 4 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > index 39280c1..64280e6 100644 > --- a/drivers/lightnvm/pblk-core.c > +++ b/drivers/lightnvm/pblk-core.c > @@ -493,6 +493,7 @@ void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd) > atomic_long_inc(&pblk->read_failed); > break; > default: > + atomic_long_inc(&pblk->read_ctrl_errors); > pblk_err(pblk, "unknown read error:%d\n", rqd->error); > } > #ifdef CONFIG_NVM_PBLK_DEBUG > diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c > index 81e8ed4..f4b6d8f2 100644 > --- a/drivers/lightnvm/pblk-init.c > +++ b/drivers/lightnvm/pblk-init.c > @@ -1230,6 +1230,7 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, > atomic_long_set(&pblk->read_empty, 0); > atomic_long_set(&pblk->read_high_ecc, 0); > atomic_long_set(&pblk->read_failed_gc, 0); > + atomic_long_set(&pblk->read_ctrl_errors, 0); > atomic_long_set(&pblk->write_failed, 0); > atomic_long_set(&pblk->erase_failed, 0); > > diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c > index 7d8958d..922273c 100644 > --- a/drivers/lightnvm/pblk-sysfs.c > +++ b/drivers/lightnvm/pblk-sysfs.c > @@ -94,11 +94,12 @@ static ssize_t pblk_sysfs_stats(struct pblk *pblk, char *page) > ssize_t sz; > > sz = snprintf(page, PAGE_SIZE, > - "read_failed=%lu, read_high_ecc=%lu, read_empty=%lu, read_failed_gc=%lu, write_failed=%lu, erase_failed=%lu\n", > + "read_failed=%lu, read_high_ecc=%lu, read_empty=%lu, read_failed_gc=%lu, read_ctrl_errors=%lu, write_failed=%lu, erase_failed=%lu\n", > atomic_long_read(&pblk->read_failed), > atomic_long_read(&pblk->read_high_ecc), > atomic_long_read(&pblk->read_empty), > atomic_long_read(&pblk->read_failed_gc), > + atomic_long_read(&pblk->read_ctrl_errors), > atomic_long_read(&pblk->write_failed), > atomic_long_read(&pblk->erase_failed)); > > diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h > index 381f074..6c82776 100644 > --- a/drivers/lightnvm/pblk.h > +++ b/drivers/lightnvm/pblk.h > @@ -684,6 +684,7 @@ struct pblk { > atomic_long_t read_empty; > atomic_long_t read_high_ecc; > atomic_long_t read_failed_gc; > + atomic_long_t read_ctrl_errors; > atomic_long_t write_failed; > atomic_long_t erase_failed; > > -- > 2.9.5 The only nitpick would be that if any user space script is relying on the syses output for the errors, this could break them. Maybe we could add a sysfs version file to mange this? Otherwise, it looks good. Reviewed-by: Javier González <javier@javigon.com>
On Wed, Mar 6, 2019 at 8:28 AM Javier González <javier@javigon.com> wrote: > > > On 5 Mar 2019, at 14.51, Igor Konopko <igor.j.konopko@intel.com> wrote: > > > > Currently when unknown error occurs on read path there is only dmesg > > information about it, but it is not counted in sysfs statistics. > > > > We would also like to track the number of such a requests, so new type > > of counter 'read_ctrl_errors" is added in that patch. > > > > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > > --- > > drivers/lightnvm/pblk-core.c | 1 + > > drivers/lightnvm/pblk-init.c | 1 + > > drivers/lightnvm/pblk-sysfs.c | 3 ++- > > drivers/lightnvm/pblk.h | 1 + > > 4 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > > index 39280c1..64280e6 100644 > > --- a/drivers/lightnvm/pblk-core.c > > +++ b/drivers/lightnvm/pblk-core.c > > @@ -493,6 +493,7 @@ void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd) > > atomic_long_inc(&pblk->read_failed); > > break; > > default: > > + atomic_long_inc(&pblk->read_ctrl_errors); > > pblk_err(pblk, "unknown read error:%d\n", rqd->error); > > } > > #ifdef CONFIG_NVM_PBLK_DEBUG > > diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c > > index 81e8ed4..f4b6d8f2 100644 > > --- a/drivers/lightnvm/pblk-init.c > > +++ b/drivers/lightnvm/pblk-init.c > > @@ -1230,6 +1230,7 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, > > atomic_long_set(&pblk->read_empty, 0); > > atomic_long_set(&pblk->read_high_ecc, 0); > > atomic_long_set(&pblk->read_failed_gc, 0); > > + atomic_long_set(&pblk->read_ctrl_errors, 0); > > atomic_long_set(&pblk->write_failed, 0); > > atomic_long_set(&pblk->erase_failed, 0); > > > > diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c > > index 7d8958d..922273c 100644 > > --- a/drivers/lightnvm/pblk-sysfs.c > > +++ b/drivers/lightnvm/pblk-sysfs.c > > @@ -94,11 +94,12 @@ static ssize_t pblk_sysfs_stats(struct pblk *pblk, char *page) > > ssize_t sz; > > > > sz = snprintf(page, PAGE_SIZE, > > - "read_failed=%lu, read_high_ecc=%lu, read_empty=%lu, read_failed_gc=%lu, write_failed=%lu, erase_failed=%lu\n", > > + "read_failed=%lu, read_high_ecc=%lu, read_empty=%lu, read_failed_gc=%lu, read_ctrl_errors=%lu, write_failed=%lu, erase_failed=%lu\n", > > atomic_long_read(&pblk->read_failed), > > atomic_long_read(&pblk->read_high_ecc), > > atomic_long_read(&pblk->read_empty), > > atomic_long_read(&pblk->read_failed_gc), > > + atomic_long_read(&pblk->read_ctrl_errors), > > atomic_long_read(&pblk->write_failed), > > atomic_long_read(&pblk->erase_failed)); > > > > diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h > > index 381f074..6c82776 100644 > > --- a/drivers/lightnvm/pblk.h > > +++ b/drivers/lightnvm/pblk.h > > @@ -684,6 +684,7 @@ struct pblk { > > atomic_long_t read_empty; > > atomic_long_t read_high_ecc; > > atomic_long_t read_failed_gc; > > + atomic_long_t read_ctrl_errors; > > atomic_long_t write_failed; > > atomic_long_t erase_failed; > > > > -- > > 2.9.5 > > The only nitpick would be that if any user space script is relying on > the syses output for the errors, this could break them. Maybe we could > add a sysfs version file to mange this? Otherwise, it looks good. > > Reviewed-by: Javier González <javier@javigon.com> Agree with Javier here, as sysfs interfaces are supposed to be stable (in the absence of a version file), so adding one would be a good idea.
On 06.03.2019 15:19, Hans Holmberg wrote: > On Wed, Mar 6, 2019 at 8:28 AM Javier González <javier@javigon.com> wrote: >> >>> On 5 Mar 2019, at 14.51, Igor Konopko <igor.j.konopko@intel.com> wrote: >>> >>> Currently when unknown error occurs on read path there is only dmesg >>> information about it, but it is not counted in sysfs statistics. >>> >>> We would also like to track the number of such a requests, so new type >>> of counter 'read_ctrl_errors" is added in that patch. >>> >>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> >>> --- >>> drivers/lightnvm/pblk-core.c | 1 + >>> drivers/lightnvm/pblk-init.c | 1 + >>> drivers/lightnvm/pblk-sysfs.c | 3 ++- >>> drivers/lightnvm/pblk.h | 1 + >>> 4 files changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c >>> index 39280c1..64280e6 100644 >>> --- a/drivers/lightnvm/pblk-core.c >>> +++ b/drivers/lightnvm/pblk-core.c >>> @@ -493,6 +493,7 @@ void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd) >>> atomic_long_inc(&pblk->read_failed); >>> break; >>> default: >>> + atomic_long_inc(&pblk->read_ctrl_errors); >>> pblk_err(pblk, "unknown read error:%d\n", rqd->error); >>> } >>> #ifdef CONFIG_NVM_PBLK_DEBUG >>> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c >>> index 81e8ed4..f4b6d8f2 100644 >>> --- a/drivers/lightnvm/pblk-init.c >>> +++ b/drivers/lightnvm/pblk-init.c >>> @@ -1230,6 +1230,7 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, >>> atomic_long_set(&pblk->read_empty, 0); >>> atomic_long_set(&pblk->read_high_ecc, 0); >>> atomic_long_set(&pblk->read_failed_gc, 0); >>> + atomic_long_set(&pblk->read_ctrl_errors, 0); >>> atomic_long_set(&pblk->write_failed, 0); >>> atomic_long_set(&pblk->erase_failed, 0); >>> >>> diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c >>> index 7d8958d..922273c 100644 >>> --- a/drivers/lightnvm/pblk-sysfs.c >>> +++ b/drivers/lightnvm/pblk-sysfs.c >>> @@ -94,11 +94,12 @@ static ssize_t pblk_sysfs_stats(struct pblk *pblk, char *page) >>> ssize_t sz; >>> >>> sz = snprintf(page, PAGE_SIZE, >>> - "read_failed=%lu, read_high_ecc=%lu, read_empty=%lu, read_failed_gc=%lu, write_failed=%lu, erase_failed=%lu\n", >>> + "read_failed=%lu, read_high_ecc=%lu, read_empty=%lu, read_failed_gc=%lu, read_ctrl_errors=%lu, write_failed=%lu, erase_failed=%lu\n", >>> atomic_long_read(&pblk->read_failed), >>> atomic_long_read(&pblk->read_high_ecc), >>> atomic_long_read(&pblk->read_empty), >>> atomic_long_read(&pblk->read_failed_gc), >>> + atomic_long_read(&pblk->read_ctrl_errors), >>> atomic_long_read(&pblk->write_failed), >>> atomic_long_read(&pblk->erase_failed)); >>> >>> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h >>> index 381f074..6c82776 100644 >>> --- a/drivers/lightnvm/pblk.h >>> +++ b/drivers/lightnvm/pblk.h >>> @@ -684,6 +684,7 @@ struct pblk { >>> atomic_long_t read_empty; >>> atomic_long_t read_high_ecc; >>> atomic_long_t read_failed_gc; >>> + atomic_long_t read_ctrl_errors; >>> atomic_long_t write_failed; >>> atomic_long_t erase_failed; >>> >>> -- >>> 2.9.5 >> >> The only nitpick would be that if any user space script is relying on >> the syses output for the errors, this could break them. Maybe we could >> add a sysfs version file to mange this? Otherwise, it looks good. >> >> Reviewed-by: Javier González <javier@javigon.com> > > Agree with Javier here, as sysfs interfaces are supposed to be stable > (in the absence of a version file), so adding one would be a good > idea. > Let's drop this patch then - it is not a critical modification, we can definitely leave without it.
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c index 39280c1..64280e6 100644 --- a/drivers/lightnvm/pblk-core.c +++ b/drivers/lightnvm/pblk-core.c @@ -493,6 +493,7 @@ void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd) atomic_long_inc(&pblk->read_failed); break; default: + atomic_long_inc(&pblk->read_ctrl_errors); pblk_err(pblk, "unknown read error:%d\n", rqd->error); } #ifdef CONFIG_NVM_PBLK_DEBUG diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c index 81e8ed4..f4b6d8f2 100644 --- a/drivers/lightnvm/pblk-init.c +++ b/drivers/lightnvm/pblk-init.c @@ -1230,6 +1230,7 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, atomic_long_set(&pblk->read_empty, 0); atomic_long_set(&pblk->read_high_ecc, 0); atomic_long_set(&pblk->read_failed_gc, 0); + atomic_long_set(&pblk->read_ctrl_errors, 0); atomic_long_set(&pblk->write_failed, 0); atomic_long_set(&pblk->erase_failed, 0); diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c index 7d8958d..922273c 100644 --- a/drivers/lightnvm/pblk-sysfs.c +++ b/drivers/lightnvm/pblk-sysfs.c @@ -94,11 +94,12 @@ static ssize_t pblk_sysfs_stats(struct pblk *pblk, char *page) ssize_t sz; sz = snprintf(page, PAGE_SIZE, - "read_failed=%lu, read_high_ecc=%lu, read_empty=%lu, read_failed_gc=%lu, write_failed=%lu, erase_failed=%lu\n", + "read_failed=%lu, read_high_ecc=%lu, read_empty=%lu, read_failed_gc=%lu, read_ctrl_errors=%lu, write_failed=%lu, erase_failed=%lu\n", atomic_long_read(&pblk->read_failed), atomic_long_read(&pblk->read_high_ecc), atomic_long_read(&pblk->read_empty), atomic_long_read(&pblk->read_failed_gc), + atomic_long_read(&pblk->read_ctrl_errors), atomic_long_read(&pblk->write_failed), atomic_long_read(&pblk->erase_failed)); diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h index 381f074..6c82776 100644 --- a/drivers/lightnvm/pblk.h +++ b/drivers/lightnvm/pblk.h @@ -684,6 +684,7 @@ struct pblk { atomic_long_t read_empty; atomic_long_t read_high_ecc; atomic_long_t read_failed_gc; + atomic_long_t read_ctrl_errors; atomic_long_t write_failed; atomic_long_t erase_failed;
Currently when unknown error occurs on read path there is only dmesg information about it, but it is not counted in sysfs statistics. We would also like to track the number of such a requests, so new type of counter 'read_ctrl_errors" is added in that patch. Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> --- drivers/lightnvm/pblk-core.c | 1 + drivers/lightnvm/pblk-init.c | 1 + drivers/lightnvm/pblk-sysfs.c | 3 ++- drivers/lightnvm/pblk.h | 1 + 4 files changed, 5 insertions(+), 1 deletion(-)