Attachment 226787 Details for Bug 254437 – Drop enc_cache_lock before cam_periph_sleep

[patch] Drop enc_cache_lock before cam_periph_sleep

setelmstat6.diff (text/plain), 3.70 KB, created by Alan Somers on 2021-07-29 21:57:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alan Somers

Created: 2021-07-29 21:57:26 UTC

Size: 3.70 KB

patch

obsolete

>diff --git a/sys/cam/scsi/scsi_enc.c b/sys/cam/scsi/scsi_enc.c
>index 30cbf108fd44..23d8ce8b9859 100644
>--- a/sys/cam/scsi/scsi_enc.c
>+++ b/sys/cam/scsi/scsi_enc.c
>@@ -435,13 +435,14 @@ enc_ioctl(struct cdev *dev, u_long cmd, caddr_t arg_addr, int flag,
> 	 *     configuration generation.  We need these ioctls
> 	 *     to also pass in/out a generation number.
> 	 */
>-	sx_slock(&enc->enc_cache_lock);
> 	switch (cmd) {
> 	case ENCIOC_GETNELM:
>+		sx_slock(&enc->enc_cache_lock);
> 		error = copyout(&cache->nelms, addr, sizeof (cache->nelms));
> 		break;
> 		
> 	case ENCIOC_GETELMMAP:
>+		sx_slock(&enc->enc_cache_lock);
> 		for (uelm = addr, i = 0; i != cache->nelms; i++) {
> 			encioc_element_t kelm;
> 			kelm.elm_idx = i;
>@@ -454,6 +455,7 @@ enc_ioctl(struct cdev *dev, u_long cmd, caddr_t arg_addr, int flag,
> 		break;
> 
> 	case ENCIOC_GETENCSTAT:
>+		sx_slock(&enc->enc_cache_lock);
> 		cam_periph_lock(periph);
> 		error = enc->enc_vec.get_enc_status(enc, 1);
> 		if (error) {
>@@ -463,22 +465,24 @@ enc_ioctl(struct cdev *dev, u_long cmd, caddr_t arg_addr, int flag,
> 		tmp = cache->enc_status;
> 		cam_periph_unlock(periph);
> 		error = copyout(&tmp, addr, sizeof(tmp));
>+		// XXX Why this next line?
> 		cache->enc_status = tmp;
> 		break;
> 
> 	case ENCIOC_SETENCSTAT:
> 		error = copyin(addr, &tmp, sizeof(tmp));
> 		if (error)
>-			break;
>+			return (error);
> 		cam_periph_lock(periph);
> 		error = enc->enc_vec.set_enc_status(enc, tmp, 1);
> 		cam_periph_unlock(periph);
>-		break;
>+		return (error);	/* Return early so as not to double-unlock */
> 
> 	case ENCIOC_GETSTRING:
> 	case ENCIOC_SETSTRING:
> 	case ENCIOC_GETENCNAME:
> 	case ENCIOC_GETENCID:
>+		sx_slock(&enc->enc_cache_lock);
> 		if (enc->enc_vec.handle_string == NULL) {
> 			error = EINVAL;
> 			break;
>@@ -496,6 +500,7 @@ enc_ioctl(struct cdev *dev, u_long cmd, caddr_t arg_addr, int flag,
> 		break;
> 
> 	case ENCIOC_GETELMSTAT:
>+		sx_slock(&enc->enc_cache_lock);
> 		error = copyin(addr, &elms, sizeof(elms));
> 		if (error)
> 			break;
>@@ -512,6 +517,7 @@ enc_ioctl(struct cdev *dev, u_long cmd, caddr_t arg_addr, int flag,
> 		break;
> 
> 	case ENCIOC_GETELMDESC:
>+		sx_slock(&enc->enc_cache_lock);
> 		error = copyin(addr, &elmd, sizeof(elmd));
> 		if (error)
> 			break;
>@@ -529,6 +535,7 @@ enc_ioctl(struct cdev *dev, u_long cmd, caddr_t arg_addr, int flag,
> 		break;
> 
> 	case ENCIOC_GETELMDEVNAMES:
>+		sx_slock(&enc->enc_cache_lock);
> 		if (enc->enc_vec.get_elm_devnames == NULL) {
> 			error = EINVAL;
> 			break;
>@@ -549,6 +556,7 @@ enc_ioctl(struct cdev *dev, u_long cmd, caddr_t arg_addr, int flag,
> 		break;
> 
> 	case ENCIOC_SETELMSTAT:
>+		sx_slock(&enc->enc_cache_lock);
> 		error = copyin(addr, &elms, sizeof(elms));
> 		if (error)
> 			break;
>@@ -557,14 +565,15 @@ enc_ioctl(struct cdev *dev, u_long cmd, caddr_t arg_addr, int flag,
> 			error = EINVAL;
> 			break;
> 		}
>+		sx_sunlock(&enc->enc_cache_lock);
> 		cam_periph_lock(periph);
> 		error = enc->enc_vec.set_elm_status(enc, &elms, 1);
> 		cam_periph_unlock(periph);
> 
>-		break;
>+		return (error);	/* Return early so as not to double-unlock */
> 
> 	case ENCIOC_INIT:
>-
>+		sx_slock(&enc->enc_cache_lock);
> 		cam_periph_lock(periph);
> 		error = enc->enc_vec.init_enc(enc);
> 		cam_periph_unlock(periph);
>@@ -574,7 +583,7 @@ enc_ioctl(struct cdev *dev, u_long cmd, caddr_t arg_addr, int flag,
> 		cam_periph_lock(periph);
> 		error = cam_periph_ioctl(periph, cmd, arg_addr, enc_error);
> 		cam_periph_unlock(periph);
>-		break;
>+		return (error);
> 	}
> 	sx_sunlock(&enc->enc_cache_lock);
> 	return (error);
>@@ -802,6 +811,8 @@ enc_fsm_step(enc_softc_t *enc)
> 	} else
> 		xfer_len = 0;
> 
>+	// Why unlock enc->periph here?  I think it's responsible for missed
>+	// wakeups.
> 	cam_periph_unlock(enc->periph);
> 	cur_state->done(enc, cur_state, ccb, &buf, error, xfer_len);
> 	cam_periph_lock(enc->periph);

Actions: View | Diff

Attachments on bug 254437: 226787