Skip to content

Commit

Permalink
vdev_disk: move abd return and free off the interrupt handler
Browse files Browse the repository at this point in the history
Freeing an ABD can take sleeping locks to update various stats. We
aren't allowed to sleep on an interrupt handler. So, move the free off
to the io_done callback.

We should never have been freeing things in the interrupt handler, but
we got away with it because we were usually freeing a linear ABD, which
at most is returning two objects to a cache and never sleeping. Scatter
ABDs can be used now, and those have more complex locking.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes #16687
  • Loading branch information
robn authored and tonyhutter committed Nov 6, 2024
1 parent 4197af4 commit f4e66db
Showing 1 changed file with 27 additions and 13 deletions.
40 changes: 27 additions & 13 deletions module/os/linux/zfs/vdev_disk.c
Original file line number Diff line number Diff line change
Expand Up @@ -828,21 +828,13 @@ BIO_END_IO_PROTO(vbio_completion, bio, error)
bio_put(bio);

/*
* If we copied the ABD before issuing it, clean up and return the copy
* to the ADB, with changes if appropriate.
* We're likely in an interrupt context so we can't do ABD/memory work
* here; instead we stash vbio on the zio and take care of it in the
* done callback.
*/
if (vbio->vbio_abd != NULL) {
if (zio->io_type == ZIO_TYPE_READ)
abd_copy(zio->io_abd, vbio->vbio_abd, zio->io_size);
ASSERT3P(zio->io_bio, ==, NULL);
zio->io_bio = vbio;

abd_free(vbio->vbio_abd);
vbio->vbio_abd = NULL;
}

/* Final cleanup */
kmem_free(vbio, sizeof (vbio_t));

/* All done, submit for processing */
zio_delay_interrupt(zio);
}

Expand Down Expand Up @@ -1496,6 +1488,28 @@ vdev_disk_io_start(zio_t *zio)
static void
vdev_disk_io_done(zio_t *zio)
{
/* If this was a read or write, we need to clean up the vbio */
if (zio->io_bio != NULL) {
vbio_t *vbio = zio->io_bio;
zio->io_bio = NULL;

/*
* If we copied the ABD before issuing it, clean up and return
* the copy to the ADB, with changes if appropriate.
*/
if (vbio->vbio_abd != NULL) {
if (zio->io_type == ZIO_TYPE_READ)
abd_copy(zio->io_abd, vbio->vbio_abd,
zio->io_size);

abd_free(vbio->vbio_abd);
vbio->vbio_abd = NULL;
}

/* Final cleanup */
kmem_free(vbio, sizeof (vbio_t));
}

/*
* If the device returned EIO, we revalidate the media. If it is
* determined the media has changed this triggers the asynchronous
Expand Down

0 comments on commit f4e66db

Please sign in to comment.