From 1b500975aaacf8b5d0e18c9a117bf5560069ffc3 Mon Sep 17 00:00:00 2001 From: Mike Gerdts Date: Sun, 15 Dec 2019 04:05:04 +0000 Subject: 4454 ldi notifications trigger vdev_disk_free() without spa_config_lock() Portions contributed by: Jason King Reviewed by: Jerry Jelinek Approved by: Dan McDonald --- usr/src/uts/common/fs/zfs/vdev_disk.c | 67 ++++++++++------------------------- 1 file changed, 18 insertions(+), 49 deletions(-) (limited to 'usr/src') diff --git a/usr/src/uts/common/fs/zfs/vdev_disk.c b/usr/src/uts/common/fs/zfs/vdev_disk.c index b046431527..dcb61064eb 100644 --- a/usr/src/uts/common/fs/zfs/vdev_disk.c +++ b/usr/src/uts/common/fs/zfs/vdev_disk.c @@ -22,7 +22,7 @@ * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2012, 2018 by Delphix. All rights reserved. * Copyright 2016 Nexenta Systems, Inc. All rights reserved. - * Copyright 2019 Joyent, Inc. + * Copyright 2020 Joyent, Inc. */ #include @@ -127,10 +127,9 @@ vdev_disk_free(vdev_t *vd) vd->vdev_tsd = NULL; } -/* ARGSUSED */ static int -vdev_disk_off_notify(ldi_handle_t lh, ldi_ev_cookie_t ecookie, void *arg, - void *ev_data) +vdev_disk_off_notify(ldi_handle_t lh __unused, ldi_ev_cookie_t ecookie, + void *arg, void *ev_data __unused) { vdev_t *vd = (vdev_t *)arg; vdev_disk_t *dvd = vd->vdev_tsd; @@ -142,19 +141,15 @@ vdev_disk_off_notify(ldi_handle_t lh, ldi_ev_cookie_t ecookie, void *arg, return (LDI_EV_SUCCESS); /* - * All LDI handles must be closed for the state change to succeed, so - * call on vdev_disk_close() to do this. - * - * We inform vdev_disk_close that it is being called from offline - * notify context so it will defer cleanup of LDI event callbacks and - * freeing of vd->vdev_tsd to the offline finalize or a reopen. + * Tell any new threads that stumble upon this vdev that they should not + * try to do I/O. */ dvd->vd_ldi_offline = B_TRUE; - vdev_disk_close(vd); /* - * Now that the device is closed, request that the spa_async_thread - * mark the device as REMOVED and notify FMA of the removal. + * Request that the spa_async_thread mark the device as REMOVED and + * notify FMA of the removal. This should also trigger a vdev_close() + * in the async thread. */ zfs_post_remove(vd->vdev_spa, vd); vd->vdev_remove_wanted = B_TRUE; @@ -163,10 +158,9 @@ vdev_disk_off_notify(ldi_handle_t lh, ldi_ev_cookie_t ecookie, void *arg, return (LDI_EV_SUCCESS); } -/* ARGSUSED */ static void -vdev_disk_off_finalize(ldi_handle_t lh, ldi_ev_cookie_t ecookie, - int ldi_result, void *arg, void *ev_data) +vdev_disk_off_finalize(ldi_handle_t lh __unused, ldi_ev_cookie_t ecookie, + int ldi_result, void *arg, void *ev_data __unused) { vdev_t *vd = (vdev_t *)arg; @@ -176,12 +170,6 @@ vdev_disk_off_finalize(ldi_handle_t lh, ldi_ev_cookie_t ecookie, if (strcmp(ldi_ev_get_type(ecookie), LDI_EV_OFFLINE) != 0) return; - /* - * We have already closed the LDI handle in notify. - * Clean up the LDI event callbacks and free vd->vdev_tsd. - */ - vdev_disk_free(vd); - /* * Request that the vdev be reopened if the offline state change was * unsuccessful. @@ -198,10 +186,9 @@ static ldi_ev_callback_t vdev_disk_off_callb = { .cb_finalize = vdev_disk_off_finalize }; -/* ARGSUSED */ static void -vdev_disk_dgrd_finalize(ldi_handle_t lh, ldi_ev_cookie_t ecookie, - int ldi_result, void *arg, void *ev_data) +vdev_disk_dgrd_finalize(ldi_handle_t lh __unused, ldi_ev_cookie_t ecookie, + int ldi_result, void *arg, void *ev_data __unused) { vdev_t *vd = (vdev_t *)arg; @@ -326,17 +313,8 @@ vdev_disk_open(vdev_t *vd, uint64_t *psize, uint64_t *max_psize, * just update the physical size of the device. */ if (dvd != NULL) { - if (dvd->vd_ldi_offline && dvd->vd_lh == NULL) { - /* - * If we are opening a device in its offline notify - * context, the LDI handle was just closed. Clean - * up the LDI event callbacks and free vd->vdev_tsd. - */ - vdev_disk_free(vd); - } else { - ASSERT(vd->vdev_reopening); - goto skip_open; - } + ASSERT(vd->vdev_reopening); + goto skip_open; } /* @@ -768,14 +746,6 @@ vdev_disk_close(vdev_t *vd) } vd->vdev_delayed_close = B_FALSE; - /* - * If we closed the LDI handle due to an offline notify from LDI, - * don't free vd->vdev_tsd or unregister the callbacks here; - * the offline finalize callback or a reopen will take care of it. - */ - if (dvd->vd_ldi_offline) - return; - vdev_disk_free(vd); } @@ -809,7 +779,8 @@ vdev_disk_ldi_physio(ldi_handle_t vd_lh, caddr_t data, static int vdev_disk_dumpio(vdev_t *vd, caddr_t data, size_t size, - uint64_t offset, uint64_t origoffset, boolean_t doread, boolean_t isdump) + uint64_t offset, uint64_t origoffset __unused, boolean_t doread, + boolean_t isdump) { vdev_disk_t *dvd = vd->vdev_tsd; int flags = doread ? B_READ : B_WRITE; @@ -817,11 +788,9 @@ vdev_disk_dumpio(vdev_t *vd, caddr_t data, size_t size, /* * If the vdev is closed, it's likely in the REMOVED or FAULTED state. * Nothing to be done here but return failure. - * - * XXX-mg there is still a race here with off_notify */ if (dvd == NULL || dvd->vd_ldi_offline) { - return (EIO); + return (SET_ERROR(ENXIO)); } ASSERT(vd->vdev_ops == &vdev_disk_ops); @@ -905,7 +874,7 @@ vdev_disk_io_start(zio_t *zio) * If the vdev is closed, it's likely in the REMOVED or FAULTED state. * Nothing to be done here but return failure. */ - if (dvd == NULL || (dvd->vd_ldi_offline && dvd->vd_lh == NULL)) { + if (dvd == NULL || dvd->vd_ldi_offline) { zio->io_error = ENXIO; zio_interrupt(zio); return; -- cgit v1.2.3