testers needed for CAM INVARIANTS fix

Kenneth D. Merry ken at kdm.org
Mon Sep 1 15:13:53 PDT 2003


I've got a fix for the panic from the cd(4)/da(4) drivers when INVARIANTS
is turned on in -stable.

The fix is to create a task queue that runs in a thread context and use
that to create the sysctl variables needed by cd(4) and da(4).  The
eventual fix will be to move the CAM transport layer probe code into a
kernel thread.

Anyway, these patches work for me, but if I could get some feedback from
people running with INVARIANTS turned on in -stable, I'd appreciate it.

If it seems to work for people, I'll talk to re@ about getting this in
before 4.9 goes out the door.

Ken
-- 
Kenneth Merry
ken at kdm.org
-------------- next part --------------
==== //depot/FreeBSD-ken-RELENG_4/src/sys/cam/scsi/scsi_cd.c#9 - /usr/home/ken/perforce2/FreeBSD-ken-RELENG_4/src/sys/cam/scsi/scsi_cd.c ====
*** /tmp/tmp.5698.0	Mon Sep  1 16:06:53 2003
--- /usr/home/ken/perforce2/FreeBSD-ken-RELENG_4/src/sys/cam/scsi/scsi_cd.c	Mon Sep  1 15:51:14 2003
***************
*** 60,65 ****
--- 60,66 ----
  #include <sys/dvdio.h>
  #include <sys/devicestat.h>
  #include <sys/sysctl.h>
+ #include <sys/taskqueue.h>
  
  #include <cam/cam.h>
  #include <cam/cam_ccb.h>
***************
*** 152,157 ****
--- 153,159 ----
  	int			bufs_left;
  	struct cam_periph	*periph;
  	int			minimum_command_size;
+ 	struct task		sysctl_task;
  	struct sysctl_ctx_list	sysctl_ctx;
  	struct sysctl_oid	*sysctl_tree;
  	STAILQ_HEAD(, cd_mode_params)	mode_queue;
***************
*** 614,619 ****
--- 616,659 ----
  	}
  }
  
+ static void
+ cdsysctlinit(void *context, int pending)
+ {
+ 	struct cam_periph *periph;
+ 	struct cd_softc *softc;
+ 	char tmpstr[80], tmpstr2[80];
+ 	int s;
+ 
+ 	periph = (struct cam_periph *)context;
+ 	softc = (struct cd_softc *)periph->softc;
+ 
+ 	snprintf(tmpstr, sizeof(tmpstr), "CAM CD unit %d", periph->unit_number);
+ 	snprintf(tmpstr2, sizeof(tmpstr2), "%d", periph->unit_number);
+ 
+ 	s = splcam();
+ 
+ 	sysctl_ctx_init(&softc->sysctl_ctx);
+ 	softc->sysctl_tree = SYSCTL_ADD_NODE(&softc->sysctl_ctx,
+ 		SYSCTL_STATIC_CHILDREN(_kern_cam_cd), OID_AUTO,
+ 		tmpstr2, CTLFLAG_RD, 0, tmpstr);
+ 
+ 	if (softc->sysctl_tree == NULL) {
+ 		printf("cdsysctlinit: unable to allocate sysctl tree\n");
+ 		return;
+ 	}
+ 
+ 	/*
+ 	 * Now register the sysctl handler, so the user can the value on
+ 	 * the fly.
+ 	 */
+ 	SYSCTL_ADD_PROC(&softc->sysctl_ctx,SYSCTL_CHILDREN(softc->sysctl_tree),
+ 		OID_AUTO, "minimum_cmd_size", CTLTYPE_INT | CTLFLAG_RW,
+ 		&softc->minimum_command_size, 0, cdcmdsizesysctl, "I",
+ 		"Minimum CDB size");
+ 
+ 	splx(s);
+ }
+ 
  /*
   * We have a handler function for this so we can check the values when the
   * user sets them, instead of every time we look at them.
***************
*** 658,664 ****
  	struct ccb_setasync csa;
  	struct ccb_pathinq cpi;
  	struct ccb_getdev *cgd;
! 	char tmpstr[80], tmpstr2[80];
  	caddr_t match;
  
  	cgd = (struct ccb_getdev *)arg;
--- 698,704 ----
  	struct ccb_setasync csa;
  	struct ccb_pathinq cpi;
  	struct ccb_getdev *cgd;
! 	char tmpstr[80];
  	caddr_t match;
  
  	cgd = (struct ccb_getdev *)arg;
***************
*** 714,730 ****
  	if (cpi.ccb_h.status == CAM_REQ_CMP && (cpi.hba_misc & PIM_NO_6_BYTE))
  		softc->quirks |= CD_Q_10_BYTE_ONLY;
  
! 	snprintf(tmpstr, sizeof(tmpstr), "CAM CD unit %d", periph->unit_number);
! 	snprintf(tmpstr2, sizeof(tmpstr2), "%d", periph->unit_number);
! 	sysctl_ctx_init(&softc->sysctl_ctx);
! 	softc->sysctl_tree = SYSCTL_ADD_NODE(&softc->sysctl_ctx,
! 		SYSCTL_STATIC_CHILDREN(_kern_cam_cd), OID_AUTO,
! 		tmpstr2, CTLFLAG_RD, 0, tmpstr);
! 	if (softc->sysctl_tree == NULL) {
! 		printf("cdregister: unable to allocate sysctl tree\n");
! 		free(softc, M_DEVBUF);
! 		return (CAM_REQ_CMP_ERR);
! 	}
  
  	/* The default is 6 byte commands, unless quirked otherwise */
  	if (softc->quirks & CD_Q_10_BYTE_ONLY)
--- 754,760 ----
  	if (cpi.ccb_h.status == CAM_REQ_CMP && (cpi.hba_misc & PIM_NO_6_BYTE))
  		softc->quirks |= CD_Q_10_BYTE_ONLY;
  
! 	TASK_INIT(&softc->sysctl_task, 0, cdsysctlinit, periph);
  
  	/* The default is 6 byte commands, unless quirked otherwise */
  	if (softc->quirks & CD_Q_10_BYTE_ONLY)
***************
*** 746,760 ****
  		softc->minimum_command_size = 10;
  
  	/*
- 	 * Now register the sysctl handler, so the user can the value on
- 	 * the fly.
- 	 */
- 	SYSCTL_ADD_PROC(&softc->sysctl_ctx,SYSCTL_CHILDREN(softc->sysctl_tree),
- 		OID_AUTO, "minimum_cmd_size", CTLTYPE_INT | CTLFLAG_RW,
- 		&softc->minimum_command_size, 0, cdcmdsizesysctl, "I",
- 		"Minimum CDB size");
- 
- 	/*
  	 * We need to register the statistics structure for this device,
  	 * but we don't have the blocksize yet for it.  So, we register
  	 * the structure and indicate that we don't have the blocksize
--- 776,781 ----
***************
*** 1891,1896 ****
--- 1912,1922 ----
  			xpt_announce_periph(periph, announce_buf);
  			if (softc->flags & CD_FLAG_CHANGER)
  				cdchangerschedule(softc);
+ 			/*
+ 			 * Create our sysctl variables, now that we know
+ 			 * we have successfully attached.
+ 			 */
+ 			taskqueue_enqueue(taskqueue_thread,&softc->sysctl_task);
  		}
  		softc->state = CD_STATE_NORMAL;		
  		/*
==== //depot/FreeBSD-ken-RELENG_4/src/sys/cam/scsi/scsi_da.c#19 - /usr/home/ken/perforce2/FreeBSD-ken-RELENG_4/src/sys/cam/scsi/scsi_da.c ====
*** /tmp/tmp.5698.1	Mon Sep  1 16:06:53 2003
--- /usr/home/ken/perforce2/FreeBSD-ken-RELENG_4/src/sys/cam/scsi/scsi_da.c	Mon Sep  1 15:50:53 2003
***************
*** 48,53 ****
--- 48,54 ----
  #include <sys/eventhandler.h>
  #include <sys/malloc.h>
  #include <sys/cons.h>
+ #include <sys/taskqueue.h>
  
  #include <machine/md_var.h>
  
***************
*** 130,135 ****
--- 131,137 ----
  	struct	 disk_params params;
  	struct	 disk disk;
  	union	 ccb saved_ccb;
+ 	struct task		sysctl_task;
  	struct sysctl_ctx_list	sysctl_ctx;
  	struct sysctl_oid	*sysctl_tree;
  };
***************
*** 397,402 ****
--- 399,405 ----
  static	periph_init_t	dainit;
  static	void		daasync(void *callback_arg, u_int32_t code,
  				struct cam_path *path, void *arg);
+ static	int		dacmdsizesysctl(SYSCTL_HANDLER_ARGS);
  static	periph_ctor_t	daregister;
  static	periph_dtor_t	dacleanup;
  static	periph_start_t	dastart;
***************
*** 1123,1128 ****
--- 1126,1167 ----
  	}
  }
  
+ static void
+ dasysctlinit(void *context, int pending)
+ {
+ 	struct cam_periph *periph;
+ 	struct da_softc *softc;
+ 	char tmpstr[80], tmpstr2[80];
+ 	int s;
+ 
+ 	periph = (struct cam_periph *)context;
+ 	softc = (struct da_softc *)periph->softc;
+ 
+ 	snprintf(tmpstr, sizeof(tmpstr), "CAM DA unit %d", periph->unit_number);
+ 	snprintf(tmpstr2, sizeof(tmpstr2), "%d", periph->unit_number);
+ 
+ 	s = splcam();
+ 	sysctl_ctx_init(&softc->sysctl_ctx);
+ 	softc->sysctl_tree = SYSCTL_ADD_NODE(&softc->sysctl_ctx,
+ 		SYSCTL_STATIC_CHILDREN(_kern_cam_da), OID_AUTO, tmpstr2,
+ 		CTLFLAG_RD, 0, tmpstr);
+ 	if (softc->sysctl_tree == NULL) {
+ 		printf("dasysctlinit: unable to allocate sysctl tree\n");
+ 		return;
+ 	}
+ 
+ 	/*
+ 	 * Now register the sysctl handler, so the user can the value on
+ 	 * the fly.
+ 	 */
+ 	SYSCTL_ADD_PROC(&softc->sysctl_ctx,SYSCTL_CHILDREN(softc->sysctl_tree),
+ 		OID_AUTO, "minimum_cmd_size", CTLTYPE_INT | CTLFLAG_RW,
+ 		&softc->minimum_cmd_size, 0, dacmdsizesysctl, "I",
+ 		"Minimum CDB size");
+ 
+ 	splx(s);
+ }
+ 
  static int
  dacmdsizesysctl(SYSCTL_HANDLER_ARGS)
  {
***************
*** 1163,1169 ****
  	struct ccb_setasync csa;
  	struct ccb_pathinq cpi;
  	struct ccb_getdev *cgd;
! 	char tmpstr[80], tmpstr2[80];
  	caddr_t match;
  
  	cgd = (struct ccb_getdev *)arg;
--- 1202,1208 ----
  	struct ccb_setasync csa;
  	struct ccb_pathinq cpi;
  	struct ccb_getdev *cgd;
! 	char tmpstr[80];
  	caddr_t match;
  
  	cgd = (struct ccb_getdev *)arg;
***************
*** 1211,1227 ****
  	else
  		softc->quirks = DA_Q_NONE;
  
! 	snprintf(tmpstr, sizeof(tmpstr), "CAM DA unit %d", periph->unit_number);
! 	snprintf(tmpstr2, sizeof(tmpstr2), "%d", periph->unit_number);
! 	sysctl_ctx_init(&softc->sysctl_ctx);
! 	softc->sysctl_tree = SYSCTL_ADD_NODE(&softc->sysctl_ctx,
! 		SYSCTL_STATIC_CHILDREN(_kern_cam_da), OID_AUTO, tmpstr2,
! 		CTLFLAG_RD, 0, tmpstr);
! 	if (softc->sysctl_tree == NULL) {
! 		printf("daregister: unable to allocate sysctl tree\n");
! 		free(softc, M_DEVBUF);
! 		return (CAM_REQ_CMP_ERR);
! 	}
  
  	/* Check if the SIM does not want 6 byte commands */
  	xpt_setup_ccb(&cpi.ccb_h, periph->path, /*priority*/1);
--- 1250,1256 ----
  	else
  		softc->quirks = DA_Q_NONE;
  
! 	TASK_INIT(&softc->sysctl_task, 0, dasysctlinit, periph);
  
  	/* Check if the SIM does not want 6 byte commands */
  	xpt_setup_ccb(&cpi.ccb_h, periph->path, /*priority*/1);
***************
*** 1257,1271 ****
  		softc->minimum_cmd_size = 12;
  
  	/*
- 	 * Now register the sysctl handler, so the user can the value on
- 	 * the fly.
- 	 */
- 	SYSCTL_ADD_PROC(&softc->sysctl_ctx,SYSCTL_CHILDREN(softc->sysctl_tree),
- 		OID_AUTO, "minimum_cmd_size", CTLTYPE_INT | CTLFLAG_RW,
- 		&softc->minimum_cmd_size, 0, dacmdsizesysctl, "I",
- 		"Minimum CDB size");
- 
- 	/*
  	 * Block our timeout handler while we
  	 * add this softc to the dev list.
  	 */
--- 1286,1291 ----
***************
*** 1694,1701 ****
  			}
  		}
  		free(rdcap, M_TEMP);
! 		if (announce_buf[0] != '\0')
  			xpt_announce_periph(periph, announce_buf);
  		softc->state = DA_STATE_NORMAL;		
  		/*
  		 * Since our peripheral may be invalidated by an error
--- 1714,1727 ----
  			}
  		}
  		free(rdcap, M_TEMP);
! 		if (announce_buf[0] != '\0') {
  			xpt_announce_periph(periph, announce_buf);
+ 			/*
+ 			 * Create our sysctl variables, now that we know
+ 			 * we have successfully attached.
+ 			 */
+ 			taskqueue_enqueue(taskqueue_thread,&softc->sysctl_task);
+ 		}
  		softc->state = DA_STATE_NORMAL;		
  		/*
  		 * Since our peripheral may be invalidated by an error
==== //depot/FreeBSD-ken-RELENG_4/src/sys/kern/subr_taskqueue.c#2 - /usr/home/ken/perforce2/FreeBSD-ken-RELENG_4/src/sys/kern/subr_taskqueue.c ====
*** /tmp/tmp.5698.3	Mon Sep  1 16:06:53 2003
--- /usr/home/ken/perforce2/FreeBSD-ken-RELENG_4/src/sys/kern/subr_taskqueue.c	Mon Sep  1 15:55:22 2003
***************
*** 33,43 ****
--- 33,46 ----
  #include <sys/taskqueue.h>
  #include <sys/interrupt.h>
  #include <sys/malloc.h>
+ #include <sys/kthread.h>
+ 
  #include <machine/ipl.h>
  
  MALLOC_DEFINE(M_TASKQUEUE, "taskqueue", "Task Queues");
  
  static STAILQ_HEAD(taskqueue_list, taskqueue) taskqueue_queues;
+ static struct proc *taskqueue_thread_proc;
  
  struct taskqueue {
  	STAILQ_ENTRY(taskqueue)	tq_link;
***************
*** 199,203 ****
--- 202,224 ----
  	taskqueue_run(taskqueue_swi);
  }
  
+ static void
+ taskqueue_kthread(void *arg)
+ {
+ 	for (;;) {
+ 		taskqueue_run(taskqueue_thread);
+ 		tsleep(&taskqueue_thread, PWAIT, "tqthr", 0);
+ 	}
+ }
+ 
+ static void
+ taskqueue_thread_enqueue(void *context)
+ {
+ 	wakeup(&taskqueue_thread);
+ }
+ 
  TASKQUEUE_DEFINE(swi, taskqueue_swi_enqueue, 0,
  		 register_swi(SWI_TQ, taskqueue_swi_run));
+ TASKQUEUE_DEFINE(thread, taskqueue_thread_enqueue, 0,
+ 		 kthread_create(taskqueue_kthread, NULL,
+ 		 &taskqueue_thread_proc, "taskqueue"));
==== //depot/FreeBSD-ken-RELENG_4/src/sys/sys/taskqueue.h#1 - /usr/home/ken/perforce2/FreeBSD-ken-RELENG_4/src/sys/sys/taskqueue.h ====
*** /tmp/tmp.5698.4	Mon Sep  1 16:06:53 2003
--- /usr/home/ken/perforce2/FreeBSD-ken-RELENG_4/src/sys/sys/taskqueue.h	Mon Sep  1 15:49:15 2003
***************
*** 111,115 ****
--- 111,116 ----
   * a task, call taskqueue_enqueue(taskqueue_swi, &task).
   */
  TASKQUEUE_DECLARE(swi);
+ TASKQUEUE_DECLARE(thread);
  
  #endif /* !_SYS_TASKQUEUE_H_ */


More information about the freebsd-stable mailing list