From owner-freebsd-current@FreeBSD.ORG Wed Dec 5 17:55:30 2012 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 5D623F00 for ; Wed, 5 Dec 2012 17:55:30 +0000 (UTC) (envelope-from jim.harris@gmail.com) Received: from mail-wg0-f42.google.com (mail-wg0-f42.google.com [74.125.82.42]) by mx1.freebsd.org (Postfix) with ESMTP id D7F2E8FC14 for ; Wed, 5 Dec 2012 17:55:29 +0000 (UTC) Received: by mail-wg0-f42.google.com with SMTP id dr1so1114092wgb.1 for ; Wed, 05 Dec 2012 09:55:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=jLHgCxCovT9V4XHys5Xh8txCD0YrFjDri+XzcfYu5jU=; b=RkaoC/kYMViXjEyr+15ZZqU4KPWffOBGwAqQTL1a+o22STCKwHO41kax4CjlyWGsSq 7WcCIsJDLbCjCOn4eB2/qO3JyfvBF+QxH5kxPOyqD5+fEQDNPuqh9b8mSQw2BwA8CHfY hqe2DGWAqzVGDbY6xUoJkHHSW1BSz2DhXhirdsF8PRHkynC16ImmPYQrlgnG2WhKbMcO qhPACTrR5B3FK/ZG2wA3b2UHXLq4odSd81oCzGij6TjRyrWzMBAragDDljhb7Ho4dPlw C8xXpsqaKQEYgIAqzmD1WMkwnfFjCjiuGgyoH6/TGCwztxYZ/PIiwFdKPNL4RzmgllqO Vd/A== MIME-Version: 1.0 Received: by 10.180.109.132 with SMTP id hs4mr4662793wib.1.1354730123335; Wed, 05 Dec 2012 09:55:23 -0800 (PST) Received: by 10.217.57.4 with HTTP; Wed, 5 Dec 2012 09:55:23 -0800 (PST) In-Reply-To: References: Date: Wed, 5 Dec 2012 10:55:23 -0700 Message-ID: Subject: Re: Call for testers, users with scsi cards From: Jim Harris To: Jeff Roberson Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.14 Cc: current@freebsd.org X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Dec 2012 17:55:30 -0000 On Tue, Dec 4, 2012 at 2:36 PM, Jeff Roberson 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: 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