* [PATCH] block: fix bio_alloc_bioset() percpu cache fallback for non-reclaim contexts
@ 2026-07-08 1:14 Joseph Qi
2026-07-08 8:45 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Joseph Qi @ 2026-07-08 1:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: Christoph Hellwig, linux-block, linux-kernel, Baokun Li
bio_alloc_bioset() was restructured in commit b520c4eef83d ("block:
split bio_alloc_bioset more clearly into a fast and slowpath") to
separate fast and slow paths. However, the restructuring introduced a
regression for GFP_ATOMIC callers when the per-CPU bio cache is enabled.
Before the restructuring, a percpu cache miss would fall through to the
mempool allocation path, which internally performs a slab allocation.
After the restructuring, a percpu cache miss leaves bio as NULL and
falls through to:
if (unlikely(!bio)) {
if (!(saved_gfp & __GFP_DIRECT_RECLAIM))
return NULL;
...
}
This immediately returns NULL for GFP_ATOMIC callers without ever
attempting a slab allocation, even when there is plenty of free memory.
The comment says "non-blocking mempool allocations just go back to the
slab allocation", but for the percpu-cache path the slab was never tried
in the first place.
This causes virtio-pmem flush (async_pmem_flush) to fail with -ENOMEM
whenever the percpu bio cache happens to be empty (common right after
boot), making the device effectively unmountable:
Buffer I/O error on dev pmem0, logical block 0, lost sync page write
Fix this by restructuring the allocation so that the percpu cache is
tried first when applicable, and the slab allocation is always attempted
as a common fallback path when the bio is still NULL.
Fixes: b520c4eef83d ("block: split bio_alloc_bioset more clearly into a fast and slowpath")
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
block/bio.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/block/bio.c b/block/bio.c
index f2a5f4d0a9672..9e7939861b94a 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -553,6 +553,11 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs,
*/
opf |= REQ_ALLOC_CACHE;
bio = bio_alloc_percpu_cache(bs);
+ if (!bio) {
+ p = kmem_cache_alloc(bs->bio_slab, gfp);
+ if (p)
+ bio = p + bs->front_pad;
+ }
} else {
opf &= ~REQ_ALLOC_CACHE;
p = kmem_cache_alloc(bs->bio_slab, gfp);
--
2.39.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] block: fix bio_alloc_bioset() percpu cache fallback for non-reclaim contexts
2026-07-08 1:14 [PATCH] block: fix bio_alloc_bioset() percpu cache fallback for non-reclaim contexts Joseph Qi
@ 2026-07-08 8:45 ` Christoph Hellwig
2026-07-08 9:49 ` Joseph Qi
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2026-07-08 8:45 UTC (permalink / raw)
To: Joseph Qi
Cc: Jens Axboe, Christoph Hellwig, linux-block, linux-kernel,
Baokun Li
On Wed, Jul 08, 2026 at 09:14:13AM +0800, Joseph Qi wrote:
> This causes virtio-pmem flush (async_pmem_flush) to fail with -ENOMEM
> whenever the percpu bio cache happens to be empty (common right after
> boot), making the device effectively unmountable:
Please fix that to not sure GFP_ATOMIC instead. Flushes are used
in file system writeabck and must not use potential failing allocations.
Even if you slightly increase the chance of it not failing, it still
can and this code is simply broken.
> Fix this by restructuring the allocation so that the percpu cache is
> tried first when applicable, and the slab allocation is always attempted
> as a common fallback path when the bio is still NULL.
I don;
>
> Fixes: b520c4eef83d ("block: split bio_alloc_bioset more clearly into a fast and slowpath")
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
> block/bio.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/block/bio.c b/block/bio.c
> index f2a5f4d0a9672..9e7939861b94a 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -553,6 +553,11 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs,
> */
> opf |= REQ_ALLOC_CACHE;
> bio = bio_alloc_percpu_cache(bs);
> + if (!bio) {
> + p = kmem_cache_alloc(bs->bio_slab, gfp);
> + if (p)
> + bio = p + bs->front_pad;
> + }
But even if we wanted this, this code should not be duplicated by
share the common version.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] block: fix bio_alloc_bioset() percpu cache fallback for non-reclaim contexts
2026-07-08 8:45 ` Christoph Hellwig
@ 2026-07-08 9:49 ` Joseph Qi
2026-07-08 16:21 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Joseph Qi @ 2026-07-08 9:49 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, linux-kernel, Baokun Li
On 7/8/26 4:45 PM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2026 at 09:14:13AM +0800, Joseph Qi wrote:
>> This causes virtio-pmem flush (async_pmem_flush) to fail with -ENOMEM
>> whenever the percpu bio cache happens to be empty (common right after
>> boot), making the device effectively unmountable:
>
> Please fix that to not sure GFP_ATOMIC instead. Flushes are used
> in file system writeabck and must not use potential failing allocations.
>
> Even if you slightly increase the chance of it not failing, it still
> can and this code is simply broken.
>
Looks sane. So commit b520c4eef83d exposed the bug but not introduced it.
>> Fix this by restructuring the allocation so that the percpu cache is
>> tried first when applicable, and the slab allocation is always attempted
>> as a common fallback path when the bio is still NULL.
>
> I don;
>
>>
>> Fixes: b520c4eef83d ("block: split bio_alloc_bioset more clearly into a fast and slowpath")
>> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
>> ---
>> block/bio.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/block/bio.c b/block/bio.c
>> index f2a5f4d0a9672..9e7939861b94a 100644
>> --- a/block/bio.c
>> +++ b/block/bio.c
>> @@ -553,6 +553,11 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs,
>> */
>> opf |= REQ_ALLOC_CACHE;
>> bio = bio_alloc_percpu_cache(bs);
>> + if (!bio) {
>> + p = kmem_cache_alloc(bs->bio_slab, gfp);
>> + if (p)
>> + bio = p + bs->front_pad;
>> + }
>
> But even if we wanted this, this code should not be duplicated by
> share the common version.
This is because I want to keep REQ_ALLOC_CACHE set.
Thanks for comments. I'll try to fix it in virtio-pmem driver.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] block: fix bio_alloc_bioset() percpu cache fallback for non-reclaim contexts
2026-07-08 9:49 ` Joseph Qi
@ 2026-07-08 16:21 ` Christoph Hellwig
2026-07-09 1:21 ` Joseph Qi
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2026-07-08 16:21 UTC (permalink / raw)
To: Joseph Qi
Cc: Christoph Hellwig, Jens Axboe, linux-block, linux-kernel,
Baokun Li
On Wed, Jul 08, 2026 at 05:49:21PM +0800, Joseph Qi wrote:
> Looks sane. So commit b520c4eef83d exposed the bug but not introduced it.
Well, there is a bug in virtio_pmem for sure. But I also think the
bio_alloc_bioset behavior isn't quite optimal for non-sleeping
allocations. But I'd probably go more for something like this there:
diff --git a/block/bio.c b/block/bio.c
index f2a5f4d0a967..bdca2e60514b 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -555,6 +555,13 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs,
bio = bio_alloc_percpu_cache(bs);
} else {
opf &= ~REQ_ALLOC_CACHE;
+ }
+
+ /*
+ * If the percpu cache was empty, try an slab allocation with optimistic
+ * GFP_ flags ass well before falling back to the mempool.
+ */
+ if (!bio) {
p = kmem_cache_alloc(bs->bio_slab, gfp);
if (p)
bio = p + bs->front_pad;
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] block: fix bio_alloc_bioset() percpu cache fallback for non-reclaim contexts
2026-07-08 16:21 ` Christoph Hellwig
@ 2026-07-09 1:21 ` Joseph Qi
0 siblings, 0 replies; 5+ messages in thread
From: Joseph Qi @ 2026-07-09 1:21 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, linux-kernel, Baokun Li
On 7/9/26 12:21 AM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2026 at 05:49:21PM +0800, Joseph Qi wrote:
>> Looks sane. So commit b520c4eef83d exposed the bug but not introduced it.
>
> Well, there is a bug in virtio_pmem for sure. But I also think the
> bio_alloc_bioset behavior isn't quite optimal for non-sleeping
> allocations. But I'd probably go more for something like this there:
>
> diff --git a/block/bio.c b/block/bio.c
> index f2a5f4d0a967..bdca2e60514b 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -555,6 +555,13 @@ struct bio *bio_alloc_bioset(struct block_device *bdev, unsigned short nr_vecs,
> bio = bio_alloc_percpu_cache(bs);
> } else {
> opf &= ~REQ_ALLOC_CACHE;
> + }
> +
> + /*
> + * If the percpu cache was empty, try an slab allocation with optimistic
> + * GFP_ flags ass well before falling back to the mempool.
> + */
> + if (!bio) {
> p = kmem_cache_alloc(bs->bio_slab, gfp);
> if (p)
> bio = p + bs->front_pad;
I wrote like this firstly, but to not mixed the bs->cache and normal
case so I dropped it.
Anyway I'm fine with this way. I'll send v2 later.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-09 1:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 1:14 [PATCH] block: fix bio_alloc_bioset() percpu cache fallback for non-reclaim contexts Joseph Qi
2026-07-08 8:45 ` Christoph Hellwig
2026-07-08 9:49 ` Joseph Qi
2026-07-08 16:21 ` Christoph Hellwig
2026-07-09 1:21 ` Joseph Qi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox