On Wed, 5 Dec 2012, Jim Harris wrote:



On Tue, Dec 4, 2012 at 2:36 PM, Jeff Roberson <jrober...@jroberson.net>
wrote:
      http://people.freebsd.org/~jeff/loadccb.diff

      This patch consolidates all of the functions that map cam
      control blocks for DMA into one central function.  This change
      is a precursor to adding new features to the I/O stack.  It is
      mostly mechanical.  If you are running current on a raid or scsi
      card, especially if it is a lesser used one, I would really like
      you to apply this patch and report back any problems.  If it
      works you should notice nothing.  If it doesn't work you will
      probably panic immediately on I/O or otherwise no I/O will
      happen.


+int
+bus_dmamap_load_ccb(bus_dma_tag_t dmat, bus_dmamap_t map, union ccb *ccb,
+            bus_dmamap_callback_t *callback, void *callback_arg,
+            int flags)
+{
+    struct ccb_ataio *ataio;
+    struct ccb_scsiio *csio;
+    struct ccb_hdr *ccb_h;
+    void *data_ptr;
+    uint32_t dxfer_len;
+    uint16_t sglist_cnt;
+
+    ccb_h = &ccb->ccb_h;
+    if ((ccb_h->flags & CAM_DIR_MASK) == CAM_DIR_NONE) {
+        callback(callback_arg, NULL, 0, 0);
+    }
+

I think you need to return here after invoking the callback.  Otherwise you
drop through and then either invoke the callback again or call
bus_dmamap_load (which will in turn invoke the callback again).

This fix allows the ahci.c change to go back to:


Thanks Jim. That was silly of me. I have decided to move this work to a branch and keep expanding on it. I'll solicit more testing once the branch is closer to the ultimate goal.

Thanks,
Jeff

Index: sys/dev/ahci/ahci.c
===================================================================
--- sys/dev/ahci/ahci.c (revision 243900)
+++ sys/dev/ahci/ahci.c (working copy)
@@ -1667,23 +1667,9 @@
            (ccb->ataio.cmd.flags & (CAM_ATAIO_CONTROL |
CAM_ATAIO_NEEDRESULT)))
                ch->aslots |= (1 << slot->slot);
        slot->dma.nsegs = 0;
-       /* If request moves data, setup and load SG list */
-       if ((ccb->ccb_h.flags & CAM_DIR_MASK) != CAM_DIR_NONE) {
-               void *buf;
-               bus_size_t size;
-
-               slot->state = AHCI_SLOT_LOADING;
-               if (ccb->ccb_h.func_code == XPT_ATA_IO) {
-                       buf = ccb->ataio.data_ptr;
-                       size = ccb->ataio.dxfer_len;
-               } else {
-                       buf = ccb->csio.data_ptr;
-                       size = ccb->csio.dxfer_len;
-               }
-               bus_dmamap_load(ch->dma.data_tag, slot->dma.data_map,
-                   buf, size, ahci_dmasetprd, slot, 0);
-       } else
-               ahci_execute_transaction(slot);
+       slot->state = AHCI_SLOT_LOADING;
+       bus_dmamap_load_ccb(ch->dma.data_tag, slot->dma.data_map, ccb,
+           ahci_dmasetprd, slot, 0);
 }
 
 /* Locked by busdma engine. */

This is almost what you head earlier, but adding back setting of the slot's
state to AHCI_SLOT_LOADING, to cover the case where the load is deferred. 
It seems OK to do this even in case where no load is actually happening
(i.e. CAM_DIR_NONE).

-Jim





_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to