* [bug report] lightnvm: pblk: return NVM_ error on failed submission
@ 2020-05-11 13:50 Dan Carpenter
2020-05-12 9:03 ` Javier González
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2020-05-11 13:50 UTC (permalink / raw)
To: javier; +Cc: linux-block
Hello Javier González,
The patch b6730dd4a954: "lightnvm: pblk: return NVM_ error on failed
submission" from Jun 1, 2018, leads to the following static checker
warning:
drivers/lightnvm/pblk-recovery.c:473 pblk_recov_scan_oob()
warn: 'pblk->inflight_io.counter' not decremented on lines: 426.
drivers/lightnvm/pblk-recovery.c
417
418 for (j = 0; j < pblk->min_write_pgs; j++, i++)
419 ppa_list[i] =
420 addr_to_gen_ppa(pblk, paddr + j, line->id);
421 }
422
423 ret = pblk_submit_io_sync(pblk, rqd, data);
424 if (ret) {
425 pblk_err(pblk, "I/O submission failed: %d\n", ret);
426 return ret;
The pblk_submit_io_sync() increments the pblk->inflight_io counter but
doesn't decrement it on all error paths. It looks like something a
little bit subtle is going no but I'm not sure how it works exactly.
427 }
428
429 atomic_dec(&pblk->inflight_io);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
430
431 /* If a read fails, do a best effort by padding the line and retrying */
432 if (rqd->error && rqd->error != NVM_RSP_WARN_HIGHECC) {
433 int pad_distance, ret;
434
435 if (padded) {
436 pblk_log_read_err(pblk, rqd);
437 return -EINTR;
438 }
439
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] lightnvm: pblk: return NVM_ error on failed submission
2020-05-11 13:50 [bug report] lightnvm: pblk: return NVM_ error on failed submission Dan Carpenter
@ 2020-05-12 9:03 ` Javier González
2020-05-12 11:29 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Javier González @ 2020-05-12 9:03 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-block
On 11.05.2020 16:50, Dan Carpenter wrote:
>Hello Javier González,
>
>The patch b6730dd4a954: "lightnvm: pblk: return NVM_ error on failed
>submission" from Jun 1, 2018, leads to the following static checker
>warning:
>
> drivers/lightnvm/pblk-recovery.c:473 pblk_recov_scan_oob()
> warn: 'pblk->inflight_io.counter' not decremented on lines: 426.
>
>drivers/lightnvm/pblk-recovery.c
> 417
> 418 for (j = 0; j < pblk->min_write_pgs; j++, i++)
> 419 ppa_list[i] =
> 420 addr_to_gen_ppa(pblk, paddr + j, line->id);
> 421 }
> 422
> 423 ret = pblk_submit_io_sync(pblk, rqd, data);
> 424 if (ret) {
> 425 pblk_err(pblk, "I/O submission failed: %d\n", ret);
> 426 return ret;
>
>The pblk_submit_io_sync() increments the pblk->inflight_io counter but
>doesn't decrement it on all error paths. It looks like something a
>little bit subtle is going no but I'm not sure how it works exactly.
>
> 427 }
> 428
> 429 atomic_dec(&pblk->inflight_io);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> 430
> 431 /* If a read fails, do a best effort by padding the line and retrying */
> 432 if (rqd->error && rqd->error != NVM_RSP_WARN_HIGHECC) {
> 433 int pad_distance, ret;
> 434
> 435 if (padded) {
> 436 pblk_log_read_err(pblk, rqd);
> 437 return -EINTR;
> 438 }
> 439
>
>regards,
>dan carpenter
Hi Dan,
As I recall, we used this counter to see the total I/Os that were
submitted through pblk and wanted to keep track of how many of them
completed - so the error path did not decrement as it was I/O that had
made it to the FTL but had not reached the device.
I can see how this is confusing, as through time we introduced dedicated
counters to the failed I/Os and inflight became a counter on I/Os going
to the device.
I believe we can fix this by checking the return value on the submit
functions and decrementing in case of error.
Do you want me to send a patch or you want to fix it yourself?
Thanks,
Javier
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] lightnvm: pblk: return NVM_ error on failed submission
2020-05-12 9:03 ` Javier González
@ 2020-05-12 11:29 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2020-05-12 11:29 UTC (permalink / raw)
To: Javier González; +Cc: linux-block
On Tue, May 12, 2020 at 11:03:18AM +0200, Javier González wrote:
> On 11.05.2020 16:50, Dan Carpenter wrote:
> > Hello Javier González,
> >
> > The patch b6730dd4a954: "lightnvm: pblk: return NVM_ error on failed
> > submission" from Jun 1, 2018, leads to the following static checker
> > warning:
> >
> > drivers/lightnvm/pblk-recovery.c:473 pblk_recov_scan_oob()
> > warn: 'pblk->inflight_io.counter' not decremented on lines: 426.
> >
> > drivers/lightnvm/pblk-recovery.c
> > 417
> > 418 for (j = 0; j < pblk->min_write_pgs; j++, i++)
> > 419 ppa_list[i] =
> > 420 addr_to_gen_ppa(pblk, paddr + j, line->id);
> > 421 }
> > 422
> > 423 ret = pblk_submit_io_sync(pblk, rqd, data);
> > 424 if (ret) {
> > 425 pblk_err(pblk, "I/O submission failed: %d\n", ret);
> > 426 return ret;
> >
> > The pblk_submit_io_sync() increments the pblk->inflight_io counter but
> > doesn't decrement it on all error paths. It looks like something a
> > little bit subtle is going no but I'm not sure how it works exactly.
> >
> > 427 }
> > 428
> > 429 atomic_dec(&pblk->inflight_io);
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > 430
> > 431 /* If a read fails, do a best effort by padding the line and retrying */
> > 432 if (rqd->error && rqd->error != NVM_RSP_WARN_HIGHECC) {
> > 433 int pad_distance, ret;
> > 434
> > 435 if (padded) {
> > 436 pblk_log_read_err(pblk, rqd);
> > 437 return -EINTR;
> > 438 }
> > 439
> >
> > regards,
> > dan carpenter
>
> Hi Dan,
>
> As I recall, we used this counter to see the total I/Os that were
> submitted through pblk and wanted to keep track of how many of them
> completed - so the error path did not decrement as it was I/O that had
> made it to the FTL but had not reached the device.
>
> I can see how this is confusing, as through time we introduced dedicated
> counters to the failed I/Os and inflight became a counter on I/Os going
> to the device.
>
> I believe we can fix this by checking the return value on the submit
> functions and decrementing in case of error.
>
> Do you want me to send a patch or you want to fix it yourself?
Could you send the patch?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-12 11:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-11 13:50 [bug report] lightnvm: pblk: return NVM_ error on failed submission Dan Carpenter
2020-05-12 9:03 ` Javier González
2020-05-12 11:29 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).