Date: Tue, 19 May 2020 14:42:10 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r361256 - stable/11/sys/cam/ctl Message-ID: <202005191442.04JEgAZ2051912@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mav Date: Tue May 19 14:42:09 2020 New Revision: 361256 URL: https://svnweb.freebsd.org/changeset/base/361256 Log: MFC r360564: Cleanup LUN addition/removal. - Make ctl_add_lun() synchronous. Asynchronous addition was used by Copan's proprietary code long ago and never for upstream FreeBSD. - Move LUN enable/disable calls from backends to CTL core. - Serialize LUN modification and partially removal to avoid double frees. - Slightly unify backends code. Modified: stable/11/sys/cam/ctl/ctl.c stable/11/sys/cam/ctl/ctl_backend.c stable/11/sys/cam/ctl/ctl_backend.h stable/11/sys/cam/ctl/ctl_backend_block.c stable/11/sys/cam/ctl/ctl_backend_ramdisk.c stable/11/sys/cam/ctl/ctl_private.h Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/cam/ctl/ctl.c ============================================================================== --- stable/11/sys/cam/ctl/ctl.c Tue May 19 14:31:47 2020 (r361255) +++ stable/11/sys/cam/ctl/ctl.c Tue May 19 14:42:09 2020 (r361256) @@ -461,10 +461,9 @@ static void ctl_ioctl_fill_ooa(struct ctl_lun *lun, ui struct ctl_ooa_entry *kern_entries); static int ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag, struct thread *td); -static int ctl_alloc_lun(struct ctl_softc *ctl_softc, struct ctl_lun *lun, - struct ctl_be_lun *be_lun); +static int ctl_enable_lun(struct ctl_lun *lun); +static int ctl_disable_lun(struct ctl_lun *lun); static int ctl_free_lun(struct ctl_lun *lun); -static void ctl_create_lun(struct ctl_be_lun *be_lun); static int ctl_do_mode_select(union ctl_io *io); static int ctl_pro_preempt(struct ctl_softc *softc, struct ctl_lun *lun, @@ -538,7 +537,6 @@ static int ctl_datamove_remote_xfer(union ctl_io *io, static void ctl_datamove_remote_read(union ctl_io *io); static void ctl_datamove_remote(union ctl_io *io); static void ctl_process_done(union ctl_io *io); -static void ctl_lun_thread(void *arg); static void ctl_thresh_thread(void *arg); static void ctl_work_thread(void *arg); static void ctl_enqueue_incoming(union ctl_io *io); @@ -1935,7 +1933,6 @@ ctl_init(void) "HA link state (0 - offline, 1 - unknown, 2 - online)"); STAILQ_INIT(&softc->lun_list); - STAILQ_INIT(&softc->pending_lun_queue); STAILQ_INIT(&softc->fe_list); STAILQ_INIT(&softc->port_list); STAILQ_INIT(&softc->be_list); @@ -1963,12 +1960,6 @@ ctl_init(void) return (error); } } - error = kproc_kthread_add(ctl_lun_thread, softc, - &softc->ctl_proc, &softc->lun_thread, 0, 0, "ctl", "lun"); - if (error != 0) { - printf("error creating CTL lun thread!\n"); - return (error); - } error = kproc_kthread_add(ctl_thresh_thread, softc, &softc->ctl_proc, &softc->thresh_thread, 0, 0, "ctl", "thresh"); if (error != 0) { @@ -2009,11 +2000,6 @@ ctl_shutdown(void) } mtx_destroy(&thr->queue_lock); } - while (softc->lun_thread != NULL) { - wakeup(&softc->pending_lun_queue); - if (softc->lun_thread != NULL) - pause("CTL thr shutdown", 1); - } while (softc->thresh_thread != NULL) { wakeup(softc->thresh_thread); if (softc->thresh_thread != NULL) @@ -4512,32 +4498,23 @@ hex2bin(const char *str, uint8_t *buf, int buf_size) } /* - * LUN allocation. + * Add LUN. * - * Requirements: - * - caller allocates and zeros LUN storage, or passes in a NULL LUN if he - * wants us to allocate the LUN and he can block. - * - ctl_softc is always set - * - be_lun is set if the LUN has a backend (needed for disk LUNs) - * * Returns 0 for success, non-zero (errno) for failure. */ -static int -ctl_alloc_lun(struct ctl_softc *ctl_softc, struct ctl_lun *ctl_lun, - struct ctl_be_lun *const be_lun) +int +ctl_add_lun(struct ctl_be_lun *be_lun) { + struct ctl_softc *ctl_softc = control_softc; struct ctl_lun *nlun, *lun; struct scsi_vpd_id_descriptor *desc; struct scsi_vpd_id_t10 *t10id; const char *eui, *naa, *scsiname, *uuid, *vendor, *value; - int lun_number, lun_malloced; + int lun_number; int devidlen, idlen1, idlen2 = 0, len; - if (be_lun == NULL) - return (EINVAL); - /* - * We currently only support Direct Access or Processor LUN types. + * We support only Direct Access, CD-ROM or Processor LUN types. */ switch (be_lun->lun_type) { case T_DIRECT: @@ -4547,22 +4524,10 @@ ctl_alloc_lun(struct ctl_softc *ctl_softc, struct ctl_ case T_SEQUENTIAL: case T_CHANGER: default: - be_lun->lun_config_status(be_lun->be_lun, - CTL_LUN_CONFIG_FAILURE); - break; + return (EINVAL); } - if (ctl_lun == NULL) { - lun = malloc(sizeof(*lun), M_CTL, M_WAITOK); - lun_malloced = 1; - } else { - lun_malloced = 0; - lun = ctl_lun; - } + lun = malloc(sizeof(*lun), M_CTL, M_WAITOK | M_ZERO); - memset(lun, 0, sizeof(*lun)); - if (lun_malloced) - lun->flags = CTL_LUN_MALLOCED; - lun->pending_sense = malloc(sizeof(struct scsi_sense_data *) * ctl_max_ports, M_DEVBUF, M_WAITOK | M_ZERO); lun->pending_ua = malloc(sizeof(ctl_ua_type *) * ctl_max_ports, @@ -4673,10 +4638,7 @@ ctl_alloc_lun(struct ctl_softc *ctl_softc, struct ctl_ } fail: free(lun->lun_devid, M_CTL); - if (lun->flags & CTL_LUN_MALLOCED) - free(lun, M_CTL); - be_lun->lun_config_status(be_lun->be_lun, - CTL_LUN_CONFIG_FAILURE); + free(lun, M_CTL); return (ENOSPC); } lun_number = be_lun->req_lun_id; @@ -4702,7 +4664,6 @@ fail: lun->backend = be_lun->be; be_lun->ctl_lun = lun; be_lun->lun_id = lun_number; - atomic_add_int(&be_lun->be->num_luns, 1); if (be_lun->flags & CTL_LUN_FLAG_EJECTED) lun->flags |= CTL_LUN_EJECTED; if (be_lun->flags & CTL_LUN_FLAG_NO_MEDIA) @@ -4769,15 +4730,27 @@ fail: ctl_softc->num_luns++; mtx_unlock(&ctl_softc->ctl_lock); - lun->be_lun->lun_config_status(lun->be_lun->be_lun, CTL_LUN_CONFIG_OK); + /* + * We successfully added the LUN, attempt to enable it. + */ + if (ctl_enable_lun(lun) != 0) { + printf("%s: ctl_enable_lun() failed!\n", __func__); + mtx_lock(&ctl_softc->ctl_lock); + STAILQ_REMOVE(&ctl_softc->lun_list, lun, ctl_lun, links); + ctl_clear_mask(ctl_softc->ctl_lun_mask, lun_number); + ctl_softc->ctl_luns[lun_number] = NULL; + ctl_softc->num_luns--; + mtx_unlock(&ctl_softc->ctl_lock); + free(lun->lun_devid, M_CTL); + free(lun, M_CTL); + return (EIO); + } + return (0); } /* - * Delete a LUN. - * Assumptions: - * - LUN has already been marked invalid and any pending I/O has been taken - * care of. + * Free LUN that has no active requests. */ static int ctl_free_lun(struct ctl_lun *lun) @@ -4804,7 +4777,6 @@ ctl_free_lun(struct ctl_lun *lun) /* * Tell the backend to free resources, if this LUN has a backend. */ - atomic_subtract_int(&lun->be_lun->be->num_luns, 1); lun->be_lun->lun_shutdown(lun->be_lun->be_lun); lun->ie_reportcnt = UINT32_MAX; @@ -4820,57 +4792,24 @@ ctl_free_lun(struct ctl_lun *lun) free(lun->pr_keys, M_DEVBUF); free(lun->write_buffer, M_CTL); free(lun->prevent, M_CTL); - if (lun->flags & CTL_LUN_MALLOCED) - free(lun, M_CTL); + free(lun, M_CTL); return (0); } -static void -ctl_create_lun(struct ctl_be_lun *be_lun) +static int +ctl_enable_lun(struct ctl_lun *lun) { - - /* - * ctl_alloc_lun() should handle all potential failure cases. - */ - ctl_alloc_lun(control_softc, NULL, be_lun); -} - -int -ctl_add_lun(struct ctl_be_lun *be_lun) -{ - struct ctl_softc *softc = control_softc; - - mtx_lock(&softc->ctl_lock); - STAILQ_INSERT_TAIL(&softc->pending_lun_queue, be_lun, links); - mtx_unlock(&softc->ctl_lock); - wakeup(&softc->pending_lun_queue); - - return (0); -} - -int -ctl_enable_lun(struct ctl_be_lun *be_lun) -{ struct ctl_softc *softc; struct ctl_port *port, *nport; - struct ctl_lun *lun; int retval; - lun = (struct ctl_lun *)be_lun->ctl_lun; softc = lun->ctl_softc; mtx_lock(&softc->ctl_lock); mtx_lock(&lun->lun_lock); - if ((lun->flags & CTL_LUN_DISABLED) == 0) { - /* - * eh? Why did we get called if the LUN is already - * enabled? - */ - mtx_unlock(&lun->lun_lock); - mtx_unlock(&softc->ctl_lock); - return (0); - } + KASSERT((lun->flags & CTL_LUN_DISABLED) != 0, + ("%s: LUN not disabled", __func__)); lun->flags &= ~CTL_LUN_DISABLED; mtx_unlock(&lun->lun_lock); @@ -4901,24 +4840,19 @@ ctl_enable_lun(struct ctl_be_lun *be_lun) return (0); } -int -ctl_disable_lun(struct ctl_be_lun *be_lun) +static int +ctl_disable_lun(struct ctl_lun *lun) { struct ctl_softc *softc; struct ctl_port *port; - struct ctl_lun *lun; int retval; - lun = (struct ctl_lun *)be_lun->ctl_lun; softc = lun->ctl_softc; mtx_lock(&softc->ctl_lock); mtx_lock(&lun->lun_lock); - if (lun->flags & CTL_LUN_DISABLED) { - mtx_unlock(&lun->lun_lock); - mtx_unlock(&softc->ctl_lock); - return (0); - } + KASSERT((lun->flags & CTL_LUN_DISABLED) == 0, + ("%s: LUN not enabled", __func__)); lun->flags |= CTL_LUN_DISABLED; mtx_unlock(&lun->lun_lock); @@ -5049,8 +4983,14 @@ ctl_lun_secondary(struct ctl_be_lun *be_lun) return (0); } +/* + * Remove LUN. If there are active requests, wait for completion. + * + * Returns 0 for success, non-zero (errno) for failure. + * Completion is reported to backed via the lun_shutdown() method. + */ int -ctl_invalidate_lun(struct ctl_be_lun *be_lun) +ctl_remove_lun(struct ctl_be_lun *be_lun) { struct ctl_softc *softc; struct ctl_lun *lun; @@ -5058,18 +4998,9 @@ ctl_invalidate_lun(struct ctl_be_lun *be_lun) lun = (struct ctl_lun *)be_lun->ctl_lun; softc = lun->ctl_softc; - mtx_lock(&lun->lun_lock); + ctl_disable_lun(lun); - /* - * The LUN needs to be disabled before it can be marked invalid. - */ - if ((lun->flags & CTL_LUN_DISABLED) == 0) { - mtx_unlock(&lun->lun_lock); - return (-1); - } - /* - * Mark the LUN invalid. - */ + mtx_lock(&lun->lun_lock); lun->flags |= CTL_LUN_INVALID; /* @@ -13296,35 +13227,6 @@ ctl_work_thread(void *arg) mtx_sleep(thr, &thr->queue_lock, PDROP, "-", 0); } thr->thread = NULL; - kthread_exit(); -} - -static void -ctl_lun_thread(void *arg) -{ - struct ctl_softc *softc = (struct ctl_softc *)arg; - struct ctl_be_lun *be_lun; - - CTL_DEBUG_PRINT(("ctl_lun_thread starting\n")); - thread_lock(curthread); - sched_prio(curthread, PUSER - 1); - thread_unlock(curthread); - - while (!softc->shutdown) { - mtx_lock(&softc->ctl_lock); - be_lun = STAILQ_FIRST(&softc->pending_lun_queue); - if (be_lun != NULL) { - STAILQ_REMOVE_HEAD(&softc->pending_lun_queue, links); - mtx_unlock(&softc->ctl_lock); - ctl_create_lun(be_lun); - continue; - } - - /* Sleep until we have something to do. */ - mtx_sleep(&softc->pending_lun_queue, &softc->ctl_lock, - PDROP, "-", 0); - } - softc->lun_thread = NULL; kthread_exit(); } Modified: stable/11/sys/cam/ctl/ctl_backend.c ============================================================================== --- stable/11/sys/cam/ctl/ctl_backend.c Tue May 19 14:31:47 2020 (r361255) +++ stable/11/sys/cam/ctl/ctl_backend.c Tue May 19 14:42:09 2020 (r361256) @@ -81,7 +81,6 @@ ctl_backend_register(struct ctl_backend_driver *be) #ifdef CS_BE_CONFIG_MOVE_DONE_IS_NOT_USED be->config_move_done = ctl_config_move_done; #endif - be->num_luns = 0; /* Call the backend's initialization routine. */ if (be->init != NULL) { Modified: stable/11/sys/cam/ctl/ctl_backend.h ============================================================================== --- stable/11/sys/cam/ctl/ctl_backend.h Tue May 19 14:31:47 2020 (r361255) +++ stable/11/sys/cam/ctl/ctl_backend.h Tue May 19 14:42:09 2020 (r361256) @@ -77,14 +77,7 @@ typedef enum { MODULE_DEPEND(name, cam, 1, 1, 1) -typedef enum { - CTL_LUN_CONFIG_OK, - CTL_LUN_CONFIG_FAILURE -} ctl_lun_config_status; - typedef void (*be_callback_t)(void *be_lun); -typedef void (*be_lun_config_t)(void *be_lun, - ctl_lun_config_status status); /* * The lun_type field is the SCSI device type of this particular LUN. In @@ -133,16 +126,11 @@ typedef void (*be_lun_config_t)(void *be_lun, * should be padded with ASCII spaces. This field should NOT be NULL * terminated. * - * The lun_shutdown() method is the callback for the ctl_invalidate_lun() + * The lun_shutdown() method is the callback for the ctl_remove_lun() * call. It is called when all outstanding I/O for that LUN has been * completed and CTL has deleted the resources for that LUN. When the CTL * backend gets this call, it can safely free its per-LUN resources. * - * The lun_config_status() method is the callback for the ctl_add_lun() - * call. It is called when the LUN is successfully added, or when LUN - * addition fails. If the LUN is successfully added, the backend may call - * the ctl_enable_lun() method to enable the LUN. - * * The be field is a pointer to the ctl_backend_driver structure, which * contains the backend methods to be called by CTL. * @@ -170,7 +158,6 @@ struct ctl_be_lun { uint8_t serial_num[CTL_SN_LEN]; /* passed to CTL */ uint8_t device_id[CTL_DEVID_LEN];/* passed to CTL */ be_callback_t lun_shutdown; /* passed to CTL */ - be_lun_config_t lun_config_status; /* passed to CTL */ struct ctl_backend_driver *be; /* passed to CTL */ void *ctl_lun; /* used by CTL */ ctl_options_t options; /* passed to CTL */ @@ -209,7 +196,6 @@ struct ctl_backend_driver { #if 0 be_vfunc_t config_write_done; /* passed to backend */ #endif - u_int num_luns; /* used by CTL */ STAILQ_ENTRY(ctl_backend_driver) links; /* used by CTL */ }; @@ -218,22 +204,16 @@ int ctl_backend_deregister(struct ctl_backend_driver * struct ctl_backend_driver *ctl_backend_find(char *backend_name); /* - * To add a LUN, first call ctl_add_lun(). You will get the lun_config_status() - * callback when the LUN addition has either succeeded or failed. - * - * Once you get that callback, you can then call ctl_enable_lun() to enable - * the LUN. + * To add a LUN, call ctl_add_lun(). */ int ctl_add_lun(struct ctl_be_lun *be_lun); -int ctl_enable_lun(struct ctl_be_lun *be_lun); /* - * To delete a LUN, first call ctl_disable_lun(), then - * ctl_invalidate_lun(). You will get the lun_shutdown() callback when all + * To remove a LUN, first call ctl_remove_lun(). + * You will get the lun_shutdown() callback when all * I/O to the LUN has completed and the LUN has been deleted. */ -int ctl_disable_lun(struct ctl_be_lun *be_lun); -int ctl_invalidate_lun(struct ctl_be_lun *be_lun); +int ctl_remove_lun(struct ctl_be_lun *be_lun); /* * To start a LUN (transition from powered off to powered on state) call Modified: stable/11/sys/cam/ctl/ctl_backend_block.c ============================================================================== --- stable/11/sys/cam/ctl/ctl_backend_block.c Tue May 19 14:31:47 2020 (r361255) +++ stable/11/sys/cam/ctl/ctl_backend_block.c Tue May 19 14:42:09 2020 (r361256) @@ -76,6 +76,7 @@ __FBSDID("$FreeBSD$"); #include <sys/sdt.h> #include <sys/devicestat.h> #include <sys/sysctl.h> +#include <sys/sx.h> #include <geom/geom.h> @@ -117,7 +118,6 @@ SDT_PROVIDER_DEFINE(cbb); typedef enum { CTL_BE_BLOCK_LUN_UNCONFIGURED = 0x01, - CTL_BE_BLOCK_LUN_CONFIG_ERR = 0x02, CTL_BE_BLOCK_LUN_WAITING = 0x04, } ctl_be_block_lun_flags; @@ -149,7 +149,6 @@ typedef uint64_t (*cbb_getattr_t)(struct ctl_be_block_ */ struct ctl_be_block_lun { struct ctl_lun_create_params params; - char lunname[32]; char *dev_path; ctl_be_block_type dev_type; struct vnode *vn; @@ -165,7 +164,7 @@ struct ctl_be_block_lun { struct ctl_be_block_softc *softc; struct devstat *disk_stats; ctl_be_block_lun_flags flags; - STAILQ_ENTRY(ctl_be_block_lun) links; + SLIST_ENTRY(ctl_be_block_lun) links; struct ctl_be_lun cbe_lun; struct taskqueue *io_taskqueue; struct task io_task; @@ -182,10 +181,11 @@ struct ctl_be_block_lun { * Overall softc structure for the block backend module. */ struct ctl_be_block_softc { + struct sx modify_lock; struct mtx lock; uma_zone_t beio_zone; int num_luns; - STAILQ_HEAD(, ctl_be_block_lun) lun_list; + SLIST_HEAD(, ctl_be_block_lun) lun_list; }; static struct ctl_be_block_softc backend_block_softc; @@ -268,8 +268,6 @@ static int ctl_be_block_rm(struct ctl_be_block_softc * static int ctl_be_block_modify(struct ctl_be_block_softc *softc, struct ctl_lun_req *req); static void ctl_be_block_lun_shutdown(void *be_lun); -static void ctl_be_block_lun_config_status(void *be_lun, - ctl_lun_config_status status); static int ctl_be_block_config_write(union ctl_io *io); static int ctl_be_block_config_read(union ctl_io *io); static int ctl_be_block_lun_info(void *be_lun, struct sbuf *sb); @@ -292,7 +290,7 @@ static struct ctl_backend_driver ctl_be_block_driver = .lun_attr = ctl_be_block_lun_attr }; -MALLOC_DEFINE(M_CTLBLK, "ctlblk", "Memory used for CTL block backend"); +MALLOC_DEFINE(M_CTLBLK, "ctlblock", "Memory used for CTL block backend"); CTL_BACKEND_DECLARE(cbb, ctl_be_block_driver); static struct ctl_be_block_io * @@ -1769,13 +1767,10 @@ static int ctl_be_block_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag, struct thread *td) { - struct ctl_be_block_softc *softc; + struct ctl_be_block_softc *softc = &backend_block_softc; int error; - softc = &backend_block_softc; - error = 0; - switch (cmd) { case CTL_LUN_REQ: { struct ctl_lun_req *lun_req; @@ -2238,12 +2233,11 @@ ctl_be_block_create(struct ctl_be_block_softc *softc, STAILQ_INIT(&be_lun->config_read_queue); STAILQ_INIT(&be_lun->config_write_queue); STAILQ_INIT(&be_lun->datamove_queue); - sprintf(be_lun->lunname, "cblk%d", softc->num_luns); - mtx_init(&be_lun->io_lock, "cblk io lock", NULL, MTX_DEF); - mtx_init(&be_lun->queue_lock, "cblk queue lock", NULL, MTX_DEF); + mtx_init(&be_lun->io_lock, "ctlblock io", NULL, MTX_DEF); + mtx_init(&be_lun->queue_lock, "ctlblock queue", NULL, MTX_DEF); ctl_init_opts(&cbe_lun->options, req->num_be_args, req->kern_be_args); - be_lun->lun_zone = uma_zcreate(be_lun->lunname, CTLBLK_MAX_SEG, + be_lun->lun_zone = uma_zcreate("ctlblock", CTLBLK_MAX_SEG, NULL, NULL, NULL, NULL, /*align*/ 0, /*flags*/0); if (be_lun->lun_zone == NULL) { snprintf(req->error_str, sizeof(req->error_str), @@ -2255,7 +2249,7 @@ ctl_be_block_create(struct ctl_be_block_softc *softc, cbe_lun->lun_type = params->device_type; else cbe_lun->lun_type = T_DIRECT; - be_lun->flags = CTL_BE_BLOCK_LUN_UNCONFIGURED; + be_lun->flags = 0; cbe_lun->flags = 0; value = ctl_get_opt(&cbe_lun->options, "ha_role"); if (value != NULL) { @@ -2320,7 +2314,6 @@ ctl_be_block_create(struct ctl_be_block_softc *softc, cbe_lun->req_lun_id = 0; cbe_lun->lun_shutdown = ctl_be_block_lun_shutdown; - cbe_lun->lun_config_status = ctl_be_block_lun_config_status; cbe_lun->be = &ctl_be_block_driver; if ((params->flags & CTL_LUN_FLAG_SERIAL_NUM) == 0) { @@ -2353,7 +2346,7 @@ ctl_be_block_create(struct ctl_be_block_softc *softc, TASK_INIT(&be_lun->io_task, /*priority*/0, ctl_be_block_worker, be_lun); - be_lun->io_taskqueue = taskqueue_create(be_lun->lunname, M_WAITOK, + be_lun->io_taskqueue = taskqueue_create("ctlblocktq", M_WAITOK, taskqueue_thread_enqueue, /*context*/&be_lun->io_taskqueue); if (be_lun->io_taskqueue == NULL) { @@ -2379,27 +2372,15 @@ ctl_be_block_create(struct ctl_be_block_softc *softc, retval = taskqueue_start_threads(&be_lun->io_taskqueue, /*num threads*/num_threads, /*priority*/PUSER, - /*thread name*/ - "%s taskq", be_lun->lunname); + /*thread name*/"block"); if (retval != 0) goto bailout_error; be_lun->num_threads = num_threads; - mtx_lock(&softc->lock); - softc->num_luns++; - STAILQ_INSERT_TAIL(&softc->lun_list, be_lun, links); - - mtx_unlock(&softc->lock); - retval = ctl_add_lun(&be_lun->cbe_lun); if (retval != 0) { - mtx_lock(&softc->lock); - STAILQ_REMOVE(&softc->lun_list, be_lun, ctl_be_block_lun, - links); - softc->num_luns--; - mtx_unlock(&softc->lock); snprintf(req->error_str, sizeof(req->error_str), "ctl_add_lun() returned error %d, see dmesg for " "details", retval); @@ -2407,42 +2388,20 @@ ctl_be_block_create(struct ctl_be_block_softc *softc, goto bailout_error; } - mtx_lock(&softc->lock); - - /* - * Tell the config_status routine that we're waiting so it won't - * clean up the LUN in the event of an error. - */ - be_lun->flags |= CTL_BE_BLOCK_LUN_WAITING; - - while (be_lun->flags & CTL_BE_BLOCK_LUN_UNCONFIGURED) { - retval = msleep(be_lun, &softc->lock, PCATCH, "ctlblk", 0); - if (retval == EINTR) - break; - } - be_lun->flags &= ~CTL_BE_BLOCK_LUN_WAITING; - - if (be_lun->flags & CTL_BE_BLOCK_LUN_CONFIG_ERR) { - snprintf(req->error_str, sizeof(req->error_str), - "LUN configuration error, see dmesg for details"); - STAILQ_REMOVE(&softc->lun_list, be_lun, ctl_be_block_lun, - links); - softc->num_luns--; - mtx_unlock(&softc->lock); - goto bailout_error; - } else { - params->req_lun_id = cbe_lun->lun_id; - } - - mtx_unlock(&softc->lock); - - be_lun->disk_stats = devstat_new_entry("cbb", params->req_lun_id, + be_lun->disk_stats = devstat_new_entry("cbb", cbe_lun->lun_id, cbe_lun->blocksize, DEVSTAT_ALL_SUPPORTED, cbe_lun->lun_type | DEVSTAT_TYPE_IF_OTHER, DEVSTAT_PRIORITY_OTHER); + mtx_lock(&softc->lock); + softc->num_luns++; + SLIST_INSERT_HEAD(&softc->lun_list, be_lun, links); + mtx_unlock(&softc->lock); + + params->req_lun_id = cbe_lun->lun_id; + return (retval); bailout_error: @@ -2473,12 +2432,18 @@ ctl_be_block_rm(struct ctl_be_block_softc *softc, stru params = &req->reqdata.rm; + sx_xlock(&softc->modify_lock); mtx_lock(&softc->lock); - STAILQ_FOREACH(be_lun, &softc->lun_list, links) { - if (be_lun->cbe_lun.lun_id == params->lun_id) + SLIST_FOREACH(be_lun, &softc->lun_list, links) { + if (be_lun->cbe_lun.lun_id == params->lun_id) { + SLIST_REMOVE(&softc->lun_list, be_lun, + ctl_be_block_lun, links); + softc->num_luns--; break; + } } mtx_unlock(&softc->lock); + sx_xunlock(&softc->modify_lock); if (be_lun == NULL) { snprintf(req->error_str, sizeof(req->error_str), "LUN %u is not managed by the block backend", @@ -2487,14 +2452,6 @@ ctl_be_block_rm(struct ctl_be_block_softc *softc, stru } cbe_lun = &be_lun->cbe_lun; - retval = ctl_disable_lun(cbe_lun); - if (retval != 0) { - snprintf(req->error_str, sizeof(req->error_str), - "error %d returned from ctl_disable_lun() for " - "LUN %d", retval, params->lun_id); - goto bailout_error; - } - if (be_lun->vn != NULL) { cbe_lun->flags |= CTL_LUN_FLAG_NO_MEDIA; ctl_lun_no_media(cbe_lun); @@ -2502,49 +2459,36 @@ ctl_be_block_rm(struct ctl_be_block_softc *softc, stru ctl_be_block_close(be_lun); } - retval = ctl_invalidate_lun(cbe_lun); + mtx_lock(&softc->lock); + be_lun->flags |= CTL_BE_BLOCK_LUN_WAITING; + mtx_unlock(&softc->lock); + + retval = ctl_remove_lun(cbe_lun); if (retval != 0) { snprintf(req->error_str, sizeof(req->error_str), - "error %d returned from ctl_invalidate_lun() for " + "error %d returned from ctl_remove_lun() for " "LUN %d", retval, params->lun_id); + mtx_lock(&softc->lock); + be_lun->flags &= ~CTL_BE_BLOCK_LUN_WAITING; + mtx_unlock(&softc->lock); goto bailout_error; } mtx_lock(&softc->lock); - be_lun->flags |= CTL_BE_BLOCK_LUN_WAITING; while ((be_lun->flags & CTL_BE_BLOCK_LUN_UNCONFIGURED) == 0) { - retval = msleep(be_lun, &softc->lock, PCATCH, "ctlblk", 0); - if (retval == EINTR) - break; - } + retval = msleep(be_lun, &softc->lock, PCATCH, "ctlblockrm", 0); + if (retval == EINTR) + break; + } be_lun->flags &= ~CTL_BE_BLOCK_LUN_WAITING; - - if ((be_lun->flags & CTL_BE_BLOCK_LUN_UNCONFIGURED) == 0) { - snprintf(req->error_str, sizeof(req->error_str), - "interrupted waiting for LUN to be freed"); + if (be_lun->flags & CTL_BE_BLOCK_LUN_UNCONFIGURED) { mtx_unlock(&softc->lock); - goto bailout_error; + free(be_lun, M_CTLBLK); + } else { + mtx_unlock(&softc->lock); + return (EINTR); } - STAILQ_REMOVE(&softc->lun_list, be_lun, ctl_be_block_lun, links); - - softc->num_luns--; - mtx_unlock(&softc->lock); - - taskqueue_drain_all(be_lun->io_taskqueue); - taskqueue_free(be_lun->io_taskqueue); - - if (be_lun->disk_stats != NULL) - devstat_remove_entry(be_lun->disk_stats); - - uma_zdestroy(be_lun->lun_zone); - - ctl_free_opts(&cbe_lun->options); - free(be_lun->dev_path, M_CTLBLK); - mtx_destroy(&be_lun->queue_lock); - mtx_destroy(&be_lun->io_lock); - free(be_lun, M_CTLBLK); - req->status = CTL_LUN_OK; return (0); @@ -2565,8 +2509,9 @@ ctl_be_block_modify(struct ctl_be_block_softc *softc, params = &req->reqdata.modify; + sx_xlock(&softc->modify_lock); mtx_lock(&softc->lock); - STAILQ_FOREACH(be_lun, &softc->lun_list, links) { + SLIST_FOREACH(be_lun, &softc->lun_list, links) { if (be_lun->cbe_lun.lun_id == params->lun_id) break; } @@ -2639,66 +2584,41 @@ ctl_be_block_modify(struct ctl_be_block_softc *softc, /* Tell the user the exact size we ended up using */ params->lun_size_bytes = be_lun->size_bytes; + sx_xunlock(&softc->modify_lock); req->status = error ? CTL_LUN_WARNING : CTL_LUN_OK; return (0); bailout_error: + sx_xunlock(&softc->modify_lock); req->status = CTL_LUN_ERROR; return (0); } static void -ctl_be_block_lun_shutdown(void *be_lun) +ctl_be_block_lun_shutdown(void *lun) { - struct ctl_be_block_lun *lun = be_lun; - struct ctl_be_block_softc *softc = lun->softc; + struct ctl_be_block_lun *be_lun = lun; + struct ctl_be_block_softc *softc = be_lun->softc; - mtx_lock(&softc->lock); - lun->flags |= CTL_BE_BLOCK_LUN_UNCONFIGURED; - if (lun->flags & CTL_BE_BLOCK_LUN_WAITING) - wakeup(lun); - mtx_unlock(&softc->lock); -} + taskqueue_drain_all(be_lun->io_taskqueue); + taskqueue_free(be_lun->io_taskqueue); + if (be_lun->disk_stats != NULL) + devstat_remove_entry(be_lun->disk_stats); + uma_zdestroy(be_lun->lun_zone); + ctl_free_opts(&be_lun->cbe_lun.options); + free(be_lun->dev_path, M_CTLBLK); + mtx_destroy(&be_lun->queue_lock); + mtx_destroy(&be_lun->io_lock); -static void -ctl_be_block_lun_config_status(void *be_lun, ctl_lun_config_status status) -{ - struct ctl_be_block_lun *lun; - struct ctl_be_block_softc *softc; - - lun = (struct ctl_be_block_lun *)be_lun; - softc = lun->softc; - - if (status == CTL_LUN_CONFIG_OK) { - mtx_lock(&softc->lock); - lun->flags &= ~CTL_BE_BLOCK_LUN_UNCONFIGURED; - if (lun->flags & CTL_BE_BLOCK_LUN_WAITING) - wakeup(lun); - mtx_unlock(&softc->lock); - - /* - * We successfully added the LUN, attempt to enable it. - */ - if (ctl_enable_lun(&lun->cbe_lun) != 0) { - printf("%s: ctl_enable_lun() failed!\n", __func__); - if (ctl_invalidate_lun(&lun->cbe_lun) != 0) { - printf("%s: ctl_invalidate_lun() failed!\n", - __func__); - } - } - - return; - } - - mtx_lock(&softc->lock); - lun->flags &= ~CTL_BE_BLOCK_LUN_UNCONFIGURED; - lun->flags |= CTL_BE_BLOCK_LUN_CONFIG_ERR; - wakeup(lun); + be_lun->flags |= CTL_BE_BLOCK_LUN_UNCONFIGURED; + if (be_lun->flags & CTL_BE_BLOCK_LUN_WAITING) + wakeup(be_lun); + else + free(be_lun, M_CTLBLK); mtx_unlock(&softc->lock); } - static int ctl_be_block_config_write(union ctl_io *io) { @@ -2862,10 +2782,11 @@ ctl_be_block_init(void) { struct ctl_be_block_softc *softc = &backend_block_softc; + sx_init(&softc->modify_lock, "ctlblock modify"); mtx_init(&softc->lock, "ctlblock", NULL, MTX_DEF); softc->beio_zone = uma_zcreate("beio", sizeof(struct ctl_be_block_io), NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, 0); - STAILQ_INIT(&softc->lun_list); + SLIST_INIT(&softc->lun_list); return (0); } @@ -2874,23 +2795,24 @@ static int ctl_be_block_shutdown(void) { struct ctl_be_block_softc *softc = &backend_block_softc; - struct ctl_be_block_lun *lun, *next_lun; + struct ctl_be_block_lun *lun; mtx_lock(&softc->lock); - STAILQ_FOREACH_SAFE(lun, &softc->lun_list, links, next_lun) { + while ((lun = SLIST_FIRST(&softc->lun_list)) != NULL) { + SLIST_REMOVE_HEAD(&softc->lun_list, links); + softc->num_luns--; /* - * Drop our lock here. Since ctl_invalidate_lun() can call + * Drop our lock here. Since ctl_remove_lun() can call * back into us, this could potentially lead to a recursive * lock of the same mutex, which would cause a hang. */ mtx_unlock(&softc->lock); - ctl_disable_lun(&lun->cbe_lun); - ctl_invalidate_lun(&lun->cbe_lun); + ctl_remove_lun(&lun->cbe_lun); mtx_lock(&softc->lock); } mtx_unlock(&softc->lock); - uma_zdestroy(softc->beio_zone); mtx_destroy(&softc->lock); + sx_destroy(&softc->modify_lock); return (0); } Modified: stable/11/sys/cam/ctl/ctl_backend_ramdisk.c ============================================================================== --- stable/11/sys/cam/ctl/ctl_backend_ramdisk.c Tue May 19 14:31:47 2020 (r361255) +++ stable/11/sys/cam/ctl/ctl_backend_ramdisk.c Tue May 19 14:42:09 2020 (r361256) @@ -98,13 +98,11 @@ typedef enum { typedef enum { CTL_BE_RAMDISK_LUN_UNCONFIGURED = 0x01, - CTL_BE_RAMDISK_LUN_CONFIG_ERR = 0x02, CTL_BE_RAMDISK_LUN_WAITING = 0x04 } ctl_be_ramdisk_lun_flags; struct ctl_be_ramdisk_lun { struct ctl_lun_create_params params; - char lunname[32]; int indir; uint8_t **pages; uint8_t *zero_page; @@ -117,7 +115,7 @@ struct ctl_be_ramdisk_lun { uint64_t cap_used; struct ctl_be_ramdisk_softc *softc; ctl_be_ramdisk_lun_flags flags; - STAILQ_ENTRY(ctl_be_ramdisk_lun) links; + SLIST_ENTRY(ctl_be_ramdisk_lun) links; struct ctl_be_lun cbe_lun; struct taskqueue *io_taskqueue; struct task io_task; @@ -126,9 +124,10 @@ struct ctl_be_ramdisk_lun { }; struct ctl_be_ramdisk_softc { + struct sx modify_lock; struct mtx lock; int num_luns; - STAILQ_HEAD(, ctl_be_ramdisk_lun) lun_list; + SLIST_HEAD(, ctl_be_ramdisk_lun) lun_list; }; static struct ctl_be_ramdisk_softc rd_softc; @@ -153,8 +152,6 @@ static int ctl_backend_ramdisk_create(struct ctl_be_ra static int ctl_backend_ramdisk_modify(struct ctl_be_ramdisk_softc *softc, struct ctl_lun_req *req); static void ctl_backend_ramdisk_lun_shutdown(void *be_lun); -static void ctl_backend_ramdisk_lun_config_status(void *be_lun, - ctl_lun_config_status status); static struct ctl_backend_driver ctl_be_ramdisk_driver = { @@ -170,7 +167,7 @@ static struct ctl_backend_driver ctl_be_ramdisk_driver .lun_attr = ctl_backend_ramdisk_lun_attr, }; -MALLOC_DEFINE(M_RAMDISK, "ramdisk", "Memory used for CTL RAMdisk"); +MALLOC_DEFINE(M_RAMDISK, "ctlramdisk", "Memory used for CTL RAMdisk"); CTL_BACKEND_DECLARE(cbr, ctl_be_ramdisk_driver); static int @@ -179,8 +176,9 @@ ctl_backend_ramdisk_init(void) struct ctl_be_ramdisk_softc *softc = &rd_softc; memset(softc, 0, sizeof(*softc)); - mtx_init(&softc->lock, "ctlramdisk", NULL, MTX_DEF); - STAILQ_INIT(&softc->lun_list); + sx_init(&softc->modify_lock, "ctlrammod"); + mtx_init(&softc->lock, "ctlram", NULL, MTX_DEF); + SLIST_INIT(&softc->lun_list); return (0); } @@ -188,22 +186,24 @@ static int ctl_backend_ramdisk_shutdown(void) { struct ctl_be_ramdisk_softc *softc = &rd_softc; - struct ctl_be_ramdisk_lun *lun, *next_lun; + struct ctl_be_ramdisk_lun *lun; mtx_lock(&softc->lock); - STAILQ_FOREACH_SAFE(lun, &softc->lun_list, links, next_lun) { + while ((lun = SLIST_FIRST(&softc->lun_list)) != NULL) { + SLIST_REMOVE_HEAD(&softc->lun_list, links); + softc->num_luns--; /* - * Drop our lock here. Since ctl_invalidate_lun() can call + * Drop our lock here. Since ctl_remove_lun() can call * back into us, this could potentially lead to a recursive * lock of the same mutex, which would cause a hang. */ mtx_unlock(&softc->lock); - ctl_disable_lun(&lun->cbe_lun); - ctl_invalidate_lun(&lun->cbe_lun); + ctl_remove_lun(&lun->cbe_lun); mtx_lock(&softc->lock); } mtx_unlock(&softc->lock); mtx_destroy(&softc->lock); + sx_destroy(&softc->modify_lock); return (0); } @@ -885,12 +885,18 @@ ctl_backend_ramdisk_rm(struct ctl_be_ramdisk_softc *so int retval; params = &req->reqdata.rm; + sx_xlock(&softc->modify_lock); mtx_lock(&softc->lock); - STAILQ_FOREACH(be_lun, &softc->lun_list, links) { - if (be_lun->cbe_lun.lun_id == params->lun_id) + SLIST_FOREACH(be_lun, &softc->lun_list, links) { + if (be_lun->cbe_lun.lun_id == params->lun_id) { + SLIST_REMOVE(&softc->lun_list, be_lun, + ctl_be_ramdisk_lun, links); + softc->num_luns--; break; + } } mtx_unlock(&softc->lock); + sx_xunlock(&softc->modify_lock); if (be_lun == NULL) { snprintf(req->error_str, sizeof(req->error_str), "%s: LUN %u is not managed by the ramdisk backend", *** DIFF OUTPUT TRUNCATED AT 1000 LINES ***
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202005191442.04JEgAZ2051912>