Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Dynamic allocations with `GFP_NOIO` on the block IO submission path 
without a mempool cause spurious IO failures under memory pressure.
--

commit 74fa63081d89bf3b9f9f1dec68f5b9b9595e7390
Author: Li Chen <[email protected]>

nvdimm: virtio_pmem: use GFP_NOIO for flush requests
    
virtio_pmem_flush() can run from pmem_submit_bio() while filesystem IO
is waiting on the flush completion. The request object allocation can
sleep, but it should not enter filesystem or block IO reclaim from this
flush path.

> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> index 8e16b7780be1a..a35044afddf34 100644
> --- a/drivers/nvdimm/nd_virtio.c
> +++ b/drivers/nvdimm/nd_virtio.c
> @@ -61,7 +61,7 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
>               return -EIO;
>       }
>  
> -     req_data = kmalloc_obj(*req_data);
> +     req_data = kmalloc_obj(*req_data, GFP_NOIO);

[Severity: High]
This is a pre-existing issue, but does this allocation lack a mempool?

Since this dynamically allocates memory on the block IO submission path
without a mempool, plain kmalloc with GFP_NOIO can fail under high
memory pressure because it does not wait indefinitely for reclaim.

When the allocation fails, virtio_pmem_flush() returns -ENOMEM, which
propagates up to pmem_submit_bio() as BLK_STS_RESOURCE. Since the block
layer does not retry BLK_STS_RESOURCE for bio-based drivers like pmem,
the error immediately propagates to the filesystem, potentially causing
unexpected data loss, read-only remounts, or journal aborts.

Should the request structure allocations here (and similarly in
async_pmem_flush()) be backed by a mempool_t to guarantee forward progress?

>       if (!req_data)
>               return -ENOMEM;
>

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=5

Reply via email to