Re: [PATCH v6 01/12] nvdimm: preserve flush callback errors

[Pankaj Gupta]

> nvdimm_flush() currently converts any non-zero provider flush error to
> -EIO. That loses useful errno values from provider callbacks.
>
> A local virtio-pmem mkfs sanity test showed the masking clearly:
>
>   wipefs: /dev/pmem0: cannot flush modified buffers: Input/output error
>   mkfs.ext4: Input/output error while writing out and closing file system
>   nd_region region0: dbg: nvdimm_flush rc=-5
>
> The virtio-pmem callback can return -ENOMEM when async_pmem_flush() fails
> to allocate a child flush bio, but nvdimm_flush() hides that as -EIO before
> pmem_submit_bio() converts it to a block status.
>
> Return the provider callback error directly. The generic flush path still
> returns 0, and pmem_submit_bio() already handles errno-to-blk_status
> conversion for bio completion.
>
> Signed-off-by: Li Chen <me@linux.beauty>
> ---
> v3->v4:
> - New patch.
>
>  drivers/nvdimm/region_devs.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index e35c2e18518f0..0cd96503c0596 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -1114,10 +1114,8 @@ int nvdimm_flush(struct nd_region *nd_region, struct bio *bio)
>
>         if (!nd_region->flush)
>                 rc = generic_nvdimm_flush(nd_region);
> -       else {
> -               if (nd_region->flush(nd_region, bio))
> -                       rc = -EIO;
> -       }
> +       else
> +               rc = nd_region->flush(nd_region, bio);

IIRC this was introduced as a generic populate error type since a
failed flush can also propagate host-side errors, which may not be
relevant to the guest.
That said, we could still consider handling specific cases like
-ENOMEM, unless there is a better approach to address this.

Thanks,
Pankaj
>
>         return rc;
>  }
> --
> 2.52.0