On 8/23/25 05:15, Ritesh Harjani (IBM) wrote:
Matthew Wilcox <willy@xxxxxxxxxxxxx> writes:
On Fri, Aug 22, 2025 at 09:37:32PM +0530, Ritesh Harjani wrote:
Matthew Wilcox <willy@xxxxxxxxxxxxx> writes:
On Fri, Aug 22, 2025 at 08:05:50AM -0700, Darrick J. Wong wrote:
Is there a reason /not/ to use the per-cpu bio cache unconditionally?
AIUI it's not safe because completions might happen on a different CPU
from the submission.
At max the bio de-queued from cpu X can be returned to cpu Y cache, this
shouldn't be unsafe right? e.g. bio_put_percpu_cache().
Not optimal for performance though.
Also even for io-uring the IRQ completions (non-polling requests) can
get routed to a different cpu then the submitting cpu, correct?
Then the completions (bio completion processing) are handled via IPIs on
the submtting cpu or based on the cache topology, right?
At least, there's nowhere that sets REQ_ALLOC_CACHE unconditionally.
This could do with some better documentation ..
Agreed. Looking at the history this got added for polling mode first but
later got enabled for even irq driven io-uring rw requests [1]. So it
make sense to understand if this can be added unconditionally for DIO
requests or not.
So why does the flag now exist at all? Why not use the cache
unconditionally?
I am hoping the author of this patch or folks with io-uring expertise
(which added the per-cpu bio cache in the first place) could answer
this better. i.e.
CC'ing would help :)
Now that per-cpu bio cache is being used by io-uring rw requests for
both polled and non-polled I/O. Does that mean, we can kill
IOCB_ALLOC_CACHE check from iomap dio path completely and use per-cpu
bio cache unconditionally by passing REQ_ALLOC_CACHE flag? That means
all DIO requests via iomap can now use this per-cpu bio cache and not
just the one initiated via io-uring path.
Or are there still restrictions in using this per-cpu bio cache, which
limits it to be only used via io-uring path? If yes, what are they? And
can this be documented somewhere?
It should be safe to use for task context allocations (struct
bio_alloc_cache::free_list is [soft]irq unsafe)
IOCB_ALLOC_CACHE shouldn't be needed, but IIRC I played it
conservatively to not impact paths I didn't specifically benchmark.
FWIW, I couldn't measure any negative impact with io_uring at the
time for requests completed on a different CPU (same NUMA), but if
it's a problem, to offset the effect we can probably add a CPU
check => bio_free and/or try batch de-allocate when the cache is
full.
--
Pavel Begunkov
