From owner-freebsd-current@FreeBSD.ORG Wed Dec 5 20:57:56 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 D3FFA94E for ; Wed, 5 Dec 2012 20:57:56 +0000 (UTC) (envelope-from jroberson@jroberson.net) Received: from mail-pa0-f54.google.com (mail-pa0-f54.google.com [209.85.220.54]) by mx1.freebsd.org (Postfix) with ESMTP id 9956A8FC0C for ; Wed, 5 Dec 2012 20:57:56 +0000 (UTC) Received: by mail-pa0-f54.google.com with SMTP id bi5so3956324pad.13 for ; Wed, 05 Dec 2012 12:57:55 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:x-x-sender:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version:content-type:x-gm-message-state; bh=Zd1CMKAfjxNHYu+7KNz4OXp1nuEhqtmq9WIimZbhlts=; b=NUzBZ0gzAFun+oQOBGNRFJ0VlHjQvH8sKYktfD1REmnS+knr6ti9DfDVfvWsmHMEaM lrZukDYlYIqV2Mh7VTRTVhwGdUxoe5GxVDBlLE2Ju9P7eb25YhgrzaZtrtRimdYQgqml AJSISB62hljf7nHtvZsk2oHRQf2m/Hgvl1u+yDVPLIO+IVB5Abfpxy1+PLMBtm3CEp7f 7b9bIOidz4FPLjwM0ixjdiEJBGdOVnIeP1WbrUUUGOunn4hK7fmLwyEnXRWGd5/Zb+k3 GUZPoBbXC4aU/m1t1EmL4iX3KYgbmM9pF2T/kESn4qDhsNrDFQiUipSUxrXcofSpagSy sVzg== Received: by 10.66.88.129 with SMTP id bg1mr47868986pab.71.1354741075796; Wed, 05 Dec 2012 12:57:55 -0800 (PST) Received: from rrcs-66-91-135-210.west.biz.rr.com (rrcs-66-91-135-210.west.biz.rr.com. [66.91.135.210]) by mx.google.com with ESMTPS id n11sm3437116pby.67.2012.12.05.12.57.54 (version=SSLv3 cipher=OTHER); Wed, 05 Dec 2012 12:57:54 -0800 (PST) Date: Wed, 5 Dec 2012 10:56:34 -1000 (HST) From: Jeff Roberson X-X-Sender: jroberson@desktop To: Jim Harris Subject: Re: Call for testers, users with scsi cards In-Reply-To: Message-ID: References: User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="2547152148-1429842545-1354740996=:4081" X-Gm-Message-State: ALoCoQlyjab6eHwfc17Bf8fH+i4nnrBG3nzcxWHOJN0VJv3lE0g7UeQokagCwW4Bd2H3uAbx/YoE 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 20:57:56 -0000 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 > 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--