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 <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#16687
  • Loading branch information
robn authored and behlendorf committed Nov 1, 2024
1 parent 19a8dd4 commit 86b5853
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 @@ -801,21 +801,13 @@ vbio_completion(struct bio *bio)
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 @@ -1451,6 +1443,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 86b5853

Please sign in to comment.