Re: [PATCH 05/13] lightnvm: pblk: Count all read errors in stats

Javier González
2019-03-04T09:23:38Z

[-- Attachment #1: Type: text/plain, Size: 2361 bytes --]

> On 4 Mar 2019, at 10.02, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote:
>
> Igor: Have you seen this happening in real life?
>
> I think it would be better to count all expected errors and put them
> in the right bucket (without spamming dmesg). If we need a new bucket
> for i.e. vendor-specific-errors, let's do that instead.
>
> Someone wiser than me told me that every error print in the log is a
> potential customer call.
>
> Javier: Yeah, I think S.M.A.R.T is the way to deliver this
> information. Why can't we let the drives expose this info and remove
> this from pblk? What's blocking that?

Until now the spec. We added some new log information in Denali exactly
for this. But since pblk supports OCSSD 1.2 and 2.0 I think it is needed to
have it here, at least for debugging.

>
> Thanks,
> Hans
>
> On Mon, Mar 4, 2019 at 8:42 AM Javier González <javier@javigon.com> wrote:
>>> On 27 Feb 2019, at 18.14, 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. Since this is
>>> still an error we should also count it there.
>>>
>>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com>
>>> ---
>>> drivers/lightnvm/pblk-core.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
>>> index eabcbc119681..a98b2255f963 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_failed);
>>> pblk_err(pblk, "unknown read error:%d\n", rqd->error);
>>> }
>>> #ifdef CONFIG_NVM_PBLK_DEBUG
>>> --
>>> 2.17.1
>>
>> I left this out intentionally so that we could correlate the logs from
>> the controller and the errors in the read path. Since we do not have an
>> standard way to correlate this on SMART yet, let’s add this now (I
>> assume that you are using it for something?) and we can separate the
>> error stats in the future.
>>
>> Reviewed-by: Javier González <javier@javigon.com>

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]