Date: Wed, 5 Dec 2012 10:56:34 -1000 (HST) From: Jeff Roberson <jroberson@jroberson.net> To: Jim Harris <jim.harris@gmail.com> Cc: current@freebsd.org Subject: Re: Call for testers, users with scsi cards Message-ID: <alpine.BSF.2.00.1212051055480.4081@desktop> In-Reply-To: <CAJP=Hc_GqHdyhVPBngbQqkjxTV_mvf_1QS29J9qfkabnmy=9xg@mail.gmail.com> References: <alpine.BSF.2.00.1212041131340.4081@desktop> <CAJP=Hc_GqHdyhVPBngbQqkjxTV_mvf_1QS29J9qfkabnmy=9xg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --2547152148-1429842545-1354740996=:4081 Content-Type: TEXT/PLAIN; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8BIT On Wed, 5 Dec 2012, Jim Harris wrote: > > > On Tue, Dec 4, 2012 at 2:36 PM, Jeff Roberson <jroberson@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 > > > > > > --2547152148-1429842545-1354740996=:4081--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.1212051055480.4081>