Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid vq_lock drop in vdev_queue_aggregate(). #12297

Merged
merged 1 commit into from
Aug 17, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 34 additions & 29 deletions module/zfs/vdev_queue.c
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,6 @@ static zio_t *
vdev_queue_aggregate(vdev_queue_t *vq, zio_t *zio)
{
zio_t *first, *last, *aio, *dio, *mandatory, *nio;
zio_link_t *zl = NULL;
uint64_t maxgap = 0;
uint64_t size;
uint64_t limit;
Expand Down Expand Up @@ -797,19 +796,12 @@ vdev_queue_aggregate(vdev_queue_t *vq, zio_t *zio)
ASSERT3U(abd_get_size(aio->io_abd), ==, aio->io_size);

/*
* We need to drop the vdev queue's lock during zio_execute() to
* avoid a deadlock that we could encounter due to lock order
* reversal between vq_lock and io_lock in zio_change_priority().
* Callers must call zio_vdev_io_bypass() and zio_execute() for
* aggregated (parent) I/Os so that we could avoid dropping the
* queue's lock here to avoid a deadlock that we could encounter
* due to lock order reversal between vq_lock and io_lock in
* zio_change_priority().
*/
mutex_exit(&vq->vq_lock);
while ((dio = zio_walk_parents(aio, &zl)) != NULL) {
ASSERT3U(dio->io_type, ==, aio->io_type);

zio_vdev_io_bypass(dio);
zio_execute(dio);
}
mutex_enter(&vq->vq_lock);

return (aio);
}

Expand Down Expand Up @@ -847,23 +839,24 @@ vdev_queue_io_to_issue(vdev_queue_t *vq)
ASSERT3U(zio->io_priority, ==, p);

aio = vdev_queue_aggregate(vq, zio);
if (aio != NULL)
if (aio != NULL) {
zio = aio;
else
} else {
vdev_queue_io_remove(vq, zio);

/*
* If the I/O is or was optional and therefore has no data, we need to
* simply discard it. We need to drop the vdev queue's lock to avoid a
* deadlock that we could encounter since this I/O will complete
* immediately.
*/
if (zio->io_flags & ZIO_FLAG_NODATA) {
mutex_exit(&vq->vq_lock);
zio_vdev_io_bypass(zio);
zio_execute(zio);
mutex_enter(&vq->vq_lock);
goto again;
/*
* If the I/O is or was optional and therefore has no data, we
* need to simply discard it. We need to drop the vdev queue's
* lock to avoid a deadlock that we could encounter since this
* I/O will complete immediately.
*/
if (zio->io_flags & ZIO_FLAG_NODATA) {
mutex_exit(&vq->vq_lock);
zio_vdev_io_bypass(zio);
zio_execute(zio);
mutex_enter(&vq->vq_lock);
goto again;
}
}

vdev_queue_pending_add(vq, zio);
Expand All @@ -876,7 +869,8 @@ zio_t *
vdev_queue_io(zio_t *zio)
{
vdev_queue_t *vq = &zio->io_vd->vdev_queue;
zio_t *nio;
zio_t *dio, *nio;
zio_link_t *zl = NULL;

if (zio->io_flags & ZIO_FLAG_DONT_QUEUE)
return (zio);
Expand Down Expand Up @@ -923,6 +917,11 @@ vdev_queue_io(zio_t *zio)
return (NULL);

if (nio->io_done == vdev_queue_agg_io_done) {
while ((dio = zio_walk_parents(nio, &zl)) != NULL) {
ASSERT3U(dio->io_type, ==, nio->io_type);
zio_vdev_io_bypass(dio);
zio_execute(dio);
}
zio_nowait(nio);
return (NULL);
}
Expand All @@ -934,7 +933,8 @@ void
vdev_queue_io_done(zio_t *zio)
{
vdev_queue_t *vq = &zio->io_vd->vdev_queue;
zio_t *nio;
zio_t *dio, *nio;
zio_link_t *zl = NULL;

mutex_enter(&vq->vq_lock);

Expand All @@ -947,6 +947,11 @@ vdev_queue_io_done(zio_t *zio)
while ((nio = vdev_queue_io_to_issue(vq)) != NULL) {
mutex_exit(&vq->vq_lock);
if (nio->io_done == vdev_queue_agg_io_done) {
while ((dio = zio_walk_parents(nio, &zl)) != NULL) {
ASSERT3U(dio->io_type, ==, nio->io_type);
zio_vdev_io_bypass(dio);
zio_execute(dio);
}
zio_nowait(nio);
} else {
zio_vdev_io_reissue(nio);
Expand Down