Skip site navigation (1)Skip section navigation (2)
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>