On 1/30/2020 10:42 PM, Christoph Hellwig wrote:
On Thu, Jan 30, 2020 at 07:23:42PM -0800, Bijan Mottahedeh wrote:
Get a reference to a bio, so it won't be freed if end_io() gets to
it before submit_io() returns. Defer the release of the first bio
in a mult-bio request until the last end_io() since the first bio is
embedded in the dio structure and must therefore persist through an
entire multi-bio request.
Can you explain the issue a little more?
The initial bio is embedded into the dio, and will have a reference
until the bio_put call at the end of the function, so we can't have
a race for that one and won't ever need the refcount for the single
bio case. Avoiding the atomic is pretty important for aio/uring
performance.
I think the problem is that in the async case, bio_get() is not called
for the initial bio *before* the submit_bio() call for that bio:
if (dio->is_sync) {
dio->waiter = current;
bio_get(bio);
} else {
dio->iocb = iocb;
}
The bio_get() call for the async case happens too late, after the
submit_bio() call:
if (!dio->multi_bio) {
/*
* AIO needs an extra reference to ensure the dio
* structure which is embedded into the first bio
* stays around.
*/
if (!is_sync)
bio_get(bio);
dio->multi_bio = true;
atomic_set(&dio->ref, 2);
} else {
atomic_inc(&dio->ref);
}
See my previous message on the mailing list titled "io_uring: acquire
ctx->uring_lock before calling io_issue_sqe()" for the more details but
basically blkdev_bio_end_io() can be called before submit_bio() returns
and therefore free the initial bio. I think it is the unconditional
bio_put() at the end that does it.
