PERFORCE change 170571 for review

[Alexander Motin-3]

http://p4web.freebsd.org/chv.cgi?CH=170571

Change 170571 by mav@mav_mavbook on 2009/11/12 20:14:54

        Fix several device freeze counting bugs.
        Assert correct reference counting instead of blindly ignoring problems.

Affected files ...

.. //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#43 edit
.. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#128 edit
.. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_cd.c#29 edit
.. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_ch.c#20 edit

Differences ...

==== //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#43 (text+ko) ====

@@ -981,15 +981,21 @@
 {
  union ccb      *saved_ccb;
  cam_status status;
- int frozen;
+ int frozen = 0;
  int sense;
  struct scsi_start_stop_unit *scsi_cmd;
  u_int32_t relsim_flags, timeout;
- int xpt_done_ccb;
+ int xpt_done_ccb = FALSE;
 
- xpt_done_ccb = FALSE;
  status = done_ccb->ccb_h.status;
- frozen = (status & CAM_DEV_QFRZN) != 0;
+ if (status & CAM_DEV_QFRZN) {
+ frozen = 1;
+ /*
+ * Clear freeze flag now for case of retry,
+ * freeze will be dropped later.
+ */
+ done_ccb->ccb_h.status &= ~CAM_DEV_QFRZN;
+ }
  sense  = (status & CAM_AUTOSNS_VALID) != 0;
  status &= CAM_STATUS_MASK;
 
@@ -997,17 +1003,6 @@
  relsim_flags = 0;
  saved_ccb = (union ccb *)done_ccb->ccb_h.saved_ccb_ptr;
 
- /*
- * Unfreeze the queue once if it is already frozen..
- */
- if (frozen != 0) {
- cam_release_devq(done_ccb->ccb_h.path,
- /*relsim_flags*/0,
- /*openings*/0,
- /*timeout*/0,
- /*getcount_only*/0);
- }
-
  switch (status) {
  case CAM_REQ_CMP:
  {
@@ -1184,14 +1179,33 @@
  */
  if (done_ccb->ccb_h.retry_count > 0)
  done_ccb->ccb_h.retry_count--;
-
+ /*
+ * Drop freeze taken due to CAM_DEV_QFREEZE flag set on recovery
+ * request.
+ */
  cam_release_devq(done_ccb->ccb_h.path,
  /*relsim_flags*/relsim_flags,
  /*openings*/0,
  /*timeout*/timeout,
  /*getcount_only*/0);
- if (xpt_done_ccb == TRUE)
+ if (xpt_done_ccb == TRUE) {
+ /*
+ * Copy frozen flag from recovery request if it is set there
+ * for some reason.
+ */
+ if (frozen != 0)
+ done_ccb->ccb_h.status |= CAM_DEV_QFRZN;
  (*done_ccb->ccb_h.cbfcnp)(periph, done_ccb);
+ } else {
+ /* Drop freeze taken, if this recovery request got error. */
+ if (frozen != 0) {
+ cam_release_devq(done_ccb->ccb_h.path,
+ /*relsim_flags*/0,
+ /*openings*/0,
+ /*timeout*/0,
+ /*getcount_only*/0);
+ }
+ }
 }
 
 /*
@@ -1451,6 +1465,11 @@
  action_string = "No recovery CCB supplied";
  goto sense_error_done;
  }
+ /*
+ * Clear freeze flag for original request here, as
+ * this freeze will be dropped as part of ERESTART.
+ */
+ ccb->ccb_h.status &= ~CAM_DEV_QFRZN;
  bcopy(ccb, save_ccb, sizeof(*save_ccb));
  print_ccb = save_ccb;
  periph->flags |= CAM_PERIPH_RECOVERY_INPROG;

==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#128 (text+ko) ====

@@ -4131,45 +4131,36 @@
 static void
 xpt_release_devq_device(struct cam_ed *dev, u_int count, int run_queue)
 {
- int rundevq;
 
- rundevq = 0;
- if (dev->ccbq.queue.qfrozen_cnt > 0) {
-
- count = (count > dev->ccbq.queue.qfrozen_cnt) ?
-    dev->ccbq.queue.qfrozen_cnt : count;
- dev->ccbq.queue.qfrozen_cnt -= count;
- if (dev->ccbq.queue.qfrozen_cnt == 0) {
-
- /*
- * No longer need to wait for a successful
- * command completion.
- */
- dev->flags &= ~CAM_DEV_REL_ON_COMPLETE;
-
- /*
- * Remove any timeouts that might be scheduled
- * to release this queue.
- */
- if ((dev->flags & CAM_DEV_REL_TIMEOUT_PENDING) != 0) {
- callout_stop(&dev->callout);
- dev->flags &= ~CAM_DEV_REL_TIMEOUT_PENDING;
- }
-
- /*
- * Now that we are unfrozen schedule the
- * device so any pending transactions are
- * run.
- */
- if ((dev->ccbq.queue.entries > 0)
- && (xpt_schedule_dev_sendq(dev->target->bus, dev))
- && (run_queue != 0)) {
- rundevq = 1;
- }
+ KASSERT(count <= dev->ccbq.queue.qfrozen_cnt,
+    ("xpt_release_devq: requested %u > present %u\n",
+    count, dev->ccbq.queue.qfrozen_cnt));
+ dev->ccbq.queue.qfrozen_cnt -= count;
+ if (dev->ccbq.queue.qfrozen_cnt == 0) {
+ /*
+ * No longer need to wait for a successful
+ * command completion.
+ */
+ dev->flags &= ~CAM_DEV_REL_ON_COMPLETE;
+ /*
+ * Remove any timeouts that might be scheduled
+ * to release this queue.
+ */
+ if ((dev->flags & CAM_DEV_REL_TIMEOUT_PENDING) != 0) {
+ callout_stop(&dev->callout);
+ dev->flags &= ~CAM_DEV_REL_TIMEOUT_PENDING;
+ }
+ /*
+ * Now that we are unfrozen schedule the
+ * device so any pending transactions are
+ * run.
+ */
+ if ((dev->ccbq.queue.entries > 0)
+ && (xpt_schedule_dev_sendq(dev->target->bus, dev))
+ && (run_queue != 0)) {
+ xpt_run_dev_sendq(dev->target->bus);
  }
  }
- if (rundevq != 0)
- xpt_run_dev_sendq(dev->target->bus);
 }
 
 void
@@ -4178,32 +4169,30 @@
  struct camq *sendq;
 
  mtx_assert(sim->mtx, MA_OWNED);
-
  sendq = &(sim->devq->send_queue);
- if (sendq->qfrozen_cnt > 0) {
+ KASSERT(sendq->qfrozen_cnt > 0,
+    ("xpt_release_simq: requested 1 > present %u\n",
+    sendq->qfrozen_cnt));
+ sendq->qfrozen_cnt--;
+ if (sendq->qfrozen_cnt == 0) {
+ /*
+ * If there is a timeout scheduled to release this
+ * sim queue, remove it.  The queue frozen count is
+ * already at 0.
+ */
+ if ((sim->flags & CAM_SIM_REL_TIMEOUT_PENDING) != 0){
+ callout_stop(&sim->callout);
+ sim->flags &= ~CAM_SIM_REL_TIMEOUT_PENDING;
+ }
+ if (run_queue) {
+ struct cam_eb *bus;
 
- sendq->qfrozen_cnt--;
- if (sendq->qfrozen_cnt == 0) {
  /*
- * If there is a timeout scheduled to release this
- * sim queue, remove it.  The queue frozen count is
- * already at 0.
+ * Now that we are unfrozen run the send queue.
  */
- if ((sim->flags & CAM_SIM_REL_TIMEOUT_PENDING) != 0){
- callout_stop(&sim->callout);
- sim->flags &= ~CAM_SIM_REL_TIMEOUT_PENDING;
- }
-
- if (run_queue) {
- struct cam_eb *bus;
-
- /*
- * Now that we are unfrozen run the send queue.
- */
- bus = xpt_find_bus(sim->path_id);
- xpt_run_dev_sendq(bus);
- xpt_release_bus(bus);
- }
+ bus = xpt_find_bus(sim->path_id);
+ xpt_run_dev_sendq(bus);
+ xpt_release_bus(bus);
  }
  }
 }

==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_cd.c#29 (text+ko) ====

@@ -1570,7 +1570,8 @@
  bp->bio_resid = bp->bio_bcount;
  bp->bio_error = error;
  bp->bio_flags |= BIO_ERROR;
- cam_release_devq(done_ccb->ccb_h.path,
+ if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0)
+ cam_release_devq(done_ccb->ccb_h.path,
  /*relsim_flags*/0,
  /*reduction*/0,
  /*timeout*/0,
@@ -1658,7 +1659,8 @@
  struct ccb_getdev cgd;
 
  /* Don't wedge this device's queue */
- cam_release_devq(done_ccb->ccb_h.path,
+ if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0)
+ cam_release_devq(done_ccb->ccb_h.path,
  /*relsim_flags*/0,
  /*reduction*/0,
  /*timeout*/0,

==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_ch.c#20 (text+ko) ====

@@ -606,7 +606,8 @@
  retry_scheduled = 0;
 
  /* Don't wedge this device's queue */
- cam_release_devq(done_ccb->ccb_h.path,
+ if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0)
+ cam_release_devq(done_ccb->ccb_h.path,
  /*relsim_flags*/0,
  /*reduction*/0,
  /*timeout*/0,
_______________________________________________
p4-projects@... mailing list
http://lists.freebsd.org/mailman/listinfo/p4-projects
To unsubscribe, send any mail to "p4-projects-unsubscribe@..."