> On 4 Mar 2019, at 14.04, Matias Bjørling <mb@xxxxxxxxxxx> wrote: > > On 3/4/19 10:48 AM, Javier González wrote: >>> On 4 Mar 2019, at 10.35, Hans Holmberg <hans.ml.holmberg@xxxxxxxxxxxxx> wrote: >>> >>> On Mon, Mar 4, 2019 at 9:03 AM Javier González <javier@xxxxxxxxxxx> wrote: >>>>> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@xxxxxxxxx> wrote: >>>>> >>>>> Currently in case of read errors, bi_status is not >>>>> set properly which leads to returning inproper data >>>>> to higher layer. This patch fix that by setting proper >>>>> status in case of read errors >>>>> >>>>> Patch also removes unnecessary warn_once(), which does >>>>> not make sense in that place, since user bio is not used >>>>> for interation with drive and thus bi_status will not be >>>>> set here. >>>>> >>>>> Signed-off-by: Igor Konopko <igor.j.konopko@xxxxxxxxx> >>>>> --- >>>>> drivers/lightnvm/pblk-read.c | 11 +++++------ >>>>> 1 file changed, 5 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c >>>>> index 3789185144da..39c1d6ccaedb 100644 >>>>> --- a/drivers/lightnvm/pblk-read.c >>>>> +++ b/drivers/lightnvm/pblk-read.c >>>>> @@ -175,11 +175,10 @@ static void pblk_read_check_rand(struct pblk *pblk, struct nvm_rq *rqd, >>>>> WARN_ONCE(j != rqd->nr_ppas, "pblk: corrupted random request\n"); >>>>> } >>>>> >>>>> -static void pblk_end_user_read(struct bio *bio) >>>>> +static void pblk_end_user_read(struct bio *bio, int error) >>>>> { >>>>> -#ifdef CONFIG_NVM_PBLK_DEBUG >>>>> - WARN_ONCE(bio->bi_status, "pblk: corrupted read bio\n"); >>>>> -#endif >>>>> + if (error && error != NVM_RSP_WARN_HIGHECC) >>>>> + bio_io_error(bio); >>>>> bio_endio(bio); >>>>> } >>>>> >>>>> @@ -219,7 +218,7 @@ static void pblk_end_io_read(struct nvm_rq *rqd) >>>>> struct pblk_g_ctx *r_ctx = nvm_rq_to_pdu(rqd); >>>>> struct bio *bio = (struct bio *)r_ctx->private; >>>>> >>>>> - pblk_end_user_read(bio); >>>>> + pblk_end_user_read(bio, rqd->error); >>>>> __pblk_end_io_read(pblk, rqd, true); >>>>> } >>>>> >>>>> @@ -292,7 +291,7 @@ static void pblk_end_partial_read(struct nvm_rq *rqd) >>>>> rqd->bio = NULL; >>>>> rqd->nr_ppas = nr_secs; >>>>> >>>>> - bio_endio(bio); >>>>> + pblk_end_user_read(bio, rqd->error); >>>>> __pblk_end_io_read(pblk, rqd, false); >>>>> } >>>>> >>>>> -- >>>>> 2.17.1 >>>> >>>> This is by design. We do not report the read errors as in any other >>>> block device - this is why we clone the read bio. >>> >>> Could you elaborate on why not reporting read errors is a good thing in pblk? >> Normal block devices do not report read errors on the completion path >> unless it is a fatal error. This is actually not well understood by the >> upper layers, which tend to assume that the device is completely broken. >> This is a challenge for OCSSD / Denali / Zone devices as there are cases >> where reads can fail. Unfortunately at this point, we need to mask these >> errors and deal with them in the different layers. > > Please don't include zone devices in that list. ZAC/ZBC are > well-behaved, and an error is a real error. They have worked around this for years. AFAIK the read path still returns predefined data, not an error. So, as I see it is the same Javier
Attachment:
signature.asc
Description: Message signed with OpenPGP