[PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation

[PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation

[Joseph Qi]

async_pmem_flush() allocates a child bio for the flush with GFP_ATOMIC.
This runs from pmem_submit_bio(), a ->submit_bio callback that executes
in a sleepable context, so there is no atomicity requirement here.

bio_alloc() only guarantees success when __GFP_DIRECT_RECLAIM is set,
because that is what lets it fall back to the mempool reserve. With
GFP_ATOMIC the reclaim bit is absent, so the allocation can fail and
return -ENOMEM whenever the fast paths (percpu cache and slab) are
exhausted, which is common right after boot. A flush is issued from
filesystem writeback and must not fail on a transient allocation
shortage, otherwise the device can appear unmountable:

  Buffer I/O error on dev pmem0, logical block 0, lost sync page write

Use GFP_NOIO instead. It keeps __GFP_DIRECT_RECLAIM so the mempool and
rescuer machinery guarantee forward progress, while avoiding recursion
back into the filesystem and block layer during writeback.

Fixes: 6e84200c0a29 ("virtio-pmem: Add virtio pmem driver")
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 drivers/nvdimm/nd_virtio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 4176046627beb..081370aac6317 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
 	if (bio && bio->bi_iter.bi_sector != -1) {
 		struct bio *child = bio_alloc(bio->bi_bdev, 0,
 					      REQ_OP_WRITE | REQ_PREFLUSH,
-					      GFP_ATOMIC);
+					      GFP_NOIO);
 
 		if (!child)
 			return -ENOMEM;
-- 
2.39.3

Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation

[Christoph Hellwig]

On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> index 4176046627beb..081370aac6317 100644
> --- a/drivers/nvdimm/nd_virtio.c
> +++ b/drivers/nvdimm/nd_virtio.c
> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
>  	if (bio && bio->bi_iter.bi_sector != -1) {
>  		struct bio *child = bio_alloc(bio->bi_bdev, 0,
>  					      REQ_OP_WRITE | REQ_PREFLUSH,
> -					      GFP_ATOMIC);
> +					      GFP_NOIO);
>  
>  		if (!child)
>  			return -ENOMEM;

This NULL  check can go away now, and probaby should to avoid confusion.

Also bio_alloc allocates from fs_bio_set, so if the incoming bio
is from that, we can still deadlock.  We'll need a separate bio_set
for this to be deadlock free.  disk->bio_split isn't otherwise
used for the drivers/nvdimm/ driverss, so you might be able to
repurpose that if you want to be creative.

Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation

[Joseph Qi]

On 7/9/26 12:24 AM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
>> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
>> index 4176046627beb..081370aac6317 100644
>> --- a/drivers/nvdimm/nd_virtio.c
>> +++ b/drivers/nvdimm/nd_virtio.c
>> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
>>  	if (bio && bio->bi_iter.bi_sector != -1) {
>>  		struct bio *child = bio_alloc(bio->bi_bdev, 0,
>>  					      REQ_OP_WRITE | REQ_PREFLUSH,
>> -					      GFP_ATOMIC);
>> +					      GFP_NOIO);
>>  
>>  		if (!child)
>>  			return -ENOMEM;
> 
> This NULL  check can go away now, and probaby should to avoid confusion.
> 
> Also bio_alloc allocates from fs_bio_set, so if the incoming bio
> is from that, we can still deadlock.  We'll need a separate bio_set
> for this to be deadlock free.  disk->bio_split isn't otherwise
> used for the drivers/nvdimm/ driverss, so you might be able to
> repurpose that if you want to be creative.

Thanks for pointing this out.

Seems it is more proper to use a driver-private bio_set instead of
repurposing bd_disk->bio_split, like raid5-ppl flush_bs.

Pankaj, what's your opinion?

Thanks,
Joseph

Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation

[Pankaj Gupta]

> On 7/9/26 12:24 AM, Christoph Hellwig wrote:
> > On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
> >> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> >> index 4176046627beb..081370aac6317 100644
> >> --- a/drivers/nvdimm/nd_virtio.c
> >> +++ b/drivers/nvdimm/nd_virtio.c
> >> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
> >>      if (bio && bio->bi_iter.bi_sector != -1) {
> >>              struct bio *child = bio_alloc(bio->bi_bdev, 0,
> >>                                            REQ_OP_WRITE | REQ_PREFLUSH,
> >> -                                          GFP_ATOMIC);
> >> +                                          GFP_NOIO);
> >>
> >>              if (!child)
> >>                      return -ENOMEM;
> >
> > This NULL  check can go away now, and probaby should to avoid confusion.
> >
> > Also bio_alloc allocates from fs_bio_set, so if the incoming bio
> > is from that, we can still deadlock.  We'll need a separate bio_set
> > for this to be deadlock free.  disk->bio_split isn't otherwise
> > used for the drivers/nvdimm/ driverss, so you might be able to
> > repurpose that if you want to be creative.
>
> Thanks for pointing this out.
>
> Seems it is more proper to use a driver-private bio_set instead of
> repurposing bd_disk->bio_split, like raid5-ppl flush_bs.
>
> Pankaj, what's your opinion?

Hi Joseph,

Thank you for the patch!

I am not very familiar with the internals of fs_bio_set, so I do not
have a strong opinion on the implementation details. However, a
driver-private bio_set sounds reasonable to me, and I would go with
Christoph's suggestion.

Best regards,
Pankaj
>
> Thanks,
> Joseph

Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation

[Joseph Qi]

On 7/9/26 6:58 PM, Pankaj Gupta wrote:
>> On 7/9/26 12:24 AM, Christoph Hellwig wrote:
>>> On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
>>>> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
>>>> index 4176046627beb..081370aac6317 100644
>>>> --- a/drivers/nvdimm/nd_virtio.c
>>>> +++ b/drivers/nvdimm/nd_virtio.c
>>>> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
>>>>      if (bio && bio->bi_iter.bi_sector != -1) {
>>>>              struct bio *child = bio_alloc(bio->bi_bdev, 0,
>>>>                                            REQ_OP_WRITE | REQ_PREFLUSH,
>>>> -                                          GFP_ATOMIC);
>>>> +                                          GFP_NOIO);
>>>>
>>>>              if (!child)
>>>>                      return -ENOMEM;
>>>
>>> This NULL  check can go away now, and probaby should to avoid confusion.
>>>
>>> Also bio_alloc allocates from fs_bio_set, so if the incoming bio
>>> is from that, we can still deadlock.  We'll need a separate bio_set
>>> for this to be deadlock free.  disk->bio_split isn't otherwise
>>> used for the drivers/nvdimm/ driverss, so you might be able to
>>> repurpose that if you want to be creative.
>>
>> Thanks for pointing this out.
>>
>> Seems it is more proper to use a driver-private bio_set instead of
>> repurposing bd_disk->bio_split, like raid5-ppl flush_bs.
>>
>> Pankaj, what's your opinion?
> 
> Hi Joseph,
> 
> Thank you for the patch!
> 
> I am not very familiar with the internals of fs_bio_set, so I do not
> have a strong opinion on the implementation details. However, a
> driver-private bio_set sounds reasonable to me, and I would go with
> Christoph's suggestion.
> 
Fine, I'll send out v2 later to address this.

Thanks,
Joseph