From owner-svn-src-projects@FreeBSD.ORG Fri Mar 6 00:51:14 2015 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id BD175A3F for ; Fri, 6 Mar 2015 00:51:14 +0000 (UTC) Received: from mail-wi0-f173.google.com (mail-wi0-f173.google.com [209.85.212.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 50C28251 for ; Fri, 6 Mar 2015 00:51:13 +0000 (UTC) Received: by wibhm9 with SMTP id hm9so48671wib.2 for ; Thu, 05 Mar 2015 16:51:06 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:message-id:date:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=KWiRZUSumtLHv5OAzHZhatCRRG8v2MnjndWvQ+svAYI=; b=D0y4lqxu+tswOjfI2vf5UXrIXFyYti6jMH+gf0aQFtsB13o6wWO2z0dE4ya6caZqhC zZ+ELbx1hsg0IXb9G0hLbKcHg+8Ktk0JGyiY/UePi32ERDwBx/+jYWtkgmp4bzp0sZVW IvZGLj4Gsfty7alGIy1366Fl9RYRQmGtbh0WgoplGCluHcpAYJCHPORdv5FmzeoBtQsU tmJ50VNBI86jOR7C7kqsK8UMR/mGPdrfXP5+gyZkdRBdRJqVtpZTRm2wVusPPsj23XOe K/sQOLp1flOiSua0HH8ZpZ+G00OyIHsZRX/YV2IZZzzVvV4uFfBXCSOIgMyjBlQYaHGf Q+Gg== X-Gm-Message-State: ALoCoQmQPxxeqazYjRAsMt2TU2H+KtBudjK0rhhJyrvrhqgayO2G/EyC0uJQGAqajV5x+FG9xof0 X-Received: by 10.194.86.194 with SMTP id r2mr24435032wjz.41.1425603065954; Thu, 05 Mar 2015 16:51:05 -0800 (PST) Received: from [10.10.1.68] (82-69-141-170.dsl.in-addr.zen.co.uk. [82.69.141.170]) by mx.google.com with ESMTPSA id ev7sm12683958wjb.47.2015.03.05.16.51.04 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 05 Mar 2015 16:51:05 -0800 (PST) From: Steven Hartland X-Google-Original-From: Steven Hartland Message-ID: <54F8F9F7.8080605@freebsd.org> Date: Fri, 06 Mar 2015 00:51:03 +0000 User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Warner Losh , src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: Re: svn commit: r279678 - in projects/iosched/sys: cam cam/ata dev/ahci References: <201503060023.t260NOA8076981@svn.freebsd.org> In-Reply-To: <201503060023.t260NOA8076981@svn.freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Mar 2015 00:51:14 -0000 Any reason why you don't retry the failed NCQ TRIM as a none NCQ TRIM instead of returning success then it failed? Regards Steve On 06/03/2015 00:23, Warner Losh wrote: > Author: imp > Date: Fri Mar 6 00:23:23 2015 > New Revision: 279678 > URL: https://svnweb.freebsd.org/changeset/base/279678 > > Log: > Implement "NCQ TRIM" by using Send First Party DMA commands with a > Dataset Management subcommand and setting the TRIM bit. For drives > that support this method of TRIM, better latency is expected because > TRIMs no longer act as a gating event forcing the queue to drain > before they are executed and holding off new commands while the TRIM > finishes. We still impose a one-at-a-time limit of the TRIMs, though > that could be relaxed (though you start running into fairness issues > if you have a lot of TRIMs competing with other I/Os), so that > optimization is left for the future. > > Add a flag for supporting the NCQ DSM TRIM extention to the ata_cmd > structure. > > Add a flag to adversize that the SIM knows how to do said kludge. > > When NCQ DSM Trim fails, fall back to normal DSM Trim. However, treat > the original trim as advisory and don't reissue it. The stack is too > fragile at this point in the code to risk it. > > Modified: > projects/iosched/sys/cam/ata/ata_all.h > projects/iosched/sys/cam/ata/ata_da.c > projects/iosched/sys/cam/cam_ccb.h > projects/iosched/sys/dev/ahci/ahci.c > > Modified: projects/iosched/sys/cam/ata/ata_all.h > ============================================================================== > --- projects/iosched/sys/cam/ata/ata_all.h Thu Mar 5 21:41:58 2015 (r279677) > +++ projects/iosched/sys/cam/ata/ata_all.h Fri Mar 6 00:23:23 2015 (r279678) > @@ -46,6 +46,7 @@ struct ata_cmd { > #define CAM_ATAIO_CONTROL 0x04 /* Control, not a command */ > #define CAM_ATAIO_NEEDRESULT 0x08 /* Request requires result. */ > #define CAM_ATAIO_DMA 0x10 /* DMA command */ > +#define CAM_ATAIO_AUX_HACK 0x20 /* Kludge to make FPDMA DSM TRIM work */ > > u_int8_t command; > u_int8_t features; > > Modified: projects/iosched/sys/cam/ata/ata_da.c > ============================================================================== > --- projects/iosched/sys/cam/ata/ata_da.c Thu Mar 5 21:41:58 2015 (r279677) > +++ projects/iosched/sys/cam/ata/ata_da.c Fri Mar 6 00:23:23 2015 (r279678) > @@ -87,7 +87,8 @@ typedef enum { > ADA_FLAG_CAN_CFA = 0x0400, > ADA_FLAG_CAN_POWERMGT = 0x0800, > ADA_FLAG_CAN_DMA48 = 0x1000, > - ADA_FLAG_DIRTY = 0x2000 > + ADA_FLAG_DIRTY = 0x2000, > + ADA_FLAG_CAN_NCQ_TRIM = 0x4000 /* CAN_TRIM also set */ > } ada_flags; > > typedef enum { > @@ -1018,10 +1019,22 @@ adaasync(void *callback_arg, u_int32_t c > else > softc->flags &= ~ADA_FLAG_CAN_NCQ; > if ((cgd.ident_data.support_dsm & ATA_SUPPORT_DSM_TRIM) && > - (cgd.inq_flags & SID_DMA)) > + (cgd.inq_flags & SID_DMA)) { > softc->flags |= ADA_FLAG_CAN_TRIM; > - else > - softc->flags &= ~ADA_FLAG_CAN_TRIM; > + /* > + * If we can do RCVSND_FPDMA_QUEUED commands, we may be able to do > + * NCQ trims, if we support trims at all. We also need support from > + * the sim do do things properly. Perhaps we should look at log 13 > + * dword 0 bit 0 and dword 1 bit 0 are set too... > + */ > + if (/* (cpi.hba_misc & PIM_NCQ_KLUDGE) != 0 && */ /* Don't know how to do this here */ > + (cgd.ident_data.satacapabilities2 & ATA_SUPPORT_RCVSND_FPDMA_QUEUED) != 0 && > + (softc->flags & ADA_FLAG_CAN_TRIM) != 0) > + softc->flags |= ADA_FLAG_CAN_NCQ_TRIM; > + else > + softc->flags &= ~ADA_FLAG_CAN_NCQ_TRIM; > + } else > + softc->flags &= ~(ADA_FLAG_CAN_TRIM | ADA_FLAG_CAN_NCQ_TRIM); > > cam_periph_async(periph, code, path, arg); > break; > @@ -1293,6 +1306,17 @@ adaregister(struct cam_periph *periph, v > softc->disk->d_delmaxsize = maxio; > if ((cpi.hba_misc & PIM_UNMAPPED) != 0) > softc->disk->d_flags |= DISKFLAG_UNMAPPED_BIO; > + /* > + * If we can do RCVSND_FPDMA_QUEUED commands, we may be able to do > + * NCQ trims, if we support trims at all. We also need support from > + * the sim do do things properly. Perhaps we should look at log 13 > + * dword 0 bit 0 and dword 1 bit 0 are set too... > + */ > + if ((cpi.hba_misc & PIM_NCQ_KLUDGE) != 0 && > + (cgd->ident_data.satacapabilities2 & > + ATA_SUPPORT_RCVSND_FPDMA_QUEUED) != 0 && > + (softc->flags & ADA_FLAG_CAN_TRIM) != 0) > + softc->flags |= ADA_FLAG_CAN_NCQ_TRIM; > strlcpy(softc->disk->d_descr, cgd->ident_data.model, > MIN(sizeof(softc->disk->d_descr), sizeof(cgd->ident_data.model))); > strlcpy(softc->disk->d_ident, cgd->ident_data.serial, > @@ -1410,10 +1434,9 @@ adaregister(struct cam_periph *periph, v > return(CAM_REQ_CMP); > } > > -static void > -ada_dsmtrim(struct ada_softc *softc, struct bio *bp, struct ccb_ataio *ataio) > +static int > +ada_dsmtrim_req_create(struct ada_softc *softc, struct bio *bp, struct trim_request *req) > { > - struct trim_request *req = &softc->trim_req; > uint64_t lastlba = (uint64_t)-1; > int c, lastcount = 0, off, ranges = 0; > > @@ -1466,6 +1489,17 @@ ada_dsmtrim(struct ada_softc *softc, str > (softc->trim_max_ranges - ranges) * ATA_DSM_RANGE_MAX) > break; > } while (1); > + > + return (ranges); > +} > + > +static void > +ada_dsmtrim(struct ada_softc *softc, struct bio *bp, struct ccb_ataio *ataio) > +{ > + struct trim_request *req = &softc->trim_req; > + int ranges; > + > + ranges = ada_dsmtrim_req_create(softc, bp, req); > cam_fill_ataio(ataio, > ada_retry_count, > adadone, > @@ -1481,6 +1515,30 @@ ada_dsmtrim(struct ada_softc *softc, str > } > > static void > +ada_ncq_dsmtrim(struct ada_softc *softc, struct bio *bp, struct ccb_ataio *ataio) > +{ > + struct trim_request *req = &softc->trim_req; > + int ranges; > + > + ranges = ada_dsmtrim_req_create(softc, bp, req); > + cam_fill_ataio(ataio, > + ada_retry_count, > + adadone, > + CAM_DIR_OUT, > + 0, > + req->data, > + ((ranges + ATA_DSM_BLK_RANGES - 1) / > + ATA_DSM_BLK_RANGES) * ATA_DSM_BLK_SIZE, > + ada_default_timeout * 1000); > + ata_ncq_cmd(ataio, > + ATA_SEND_FPDMA_QUEUED, > + 0, > + (ranges + ATA_DSM_BLK_RANGES - 1) / ATA_DSM_BLK_RANGES); > + ataio->cmd.sector_count_exp = ATA_SFPDMA_DSM; > + ataio->cmd.flags |= CAM_ATAIO_AUX_HACK; > +} > + > +static void > ada_cfaerase(struct ada_softc *softc, struct bio *bp, struct ccb_ataio *ataio) > { > struct trim_request *req = &softc->trim_req; > @@ -1523,7 +1581,9 @@ adastart(struct cam_periph *periph, unio > /* Run TRIM if not running yet. */ > if (!softc->trim_running && > (bp = bioq_first(&softc->trim_queue)) != 0) { > - if (softc->flags & ADA_FLAG_CAN_TRIM) { > + if (softc->flags & ADA_FLAG_CAN_NCQ_TRIM) { > + ada_ncq_dsmtrim(softc, bp, ataio); > + } else if (softc->flags & ADA_FLAG_CAN_TRIM) { > ada_dsmtrim(softc, bp, ataio); > } else if ((softc->flags & ADA_FLAG_CAN_CFA) && > !(softc->flags & ADA_FLAG_CAN_48BIT)) { > @@ -1749,6 +1809,7 @@ adadone(struct cam_periph *periph, union > int error; > > cam_periph_lock(periph); > + bp = (struct bio *)done_ccb->ccb_h.ccb_bp; > if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) { > error = adaerror(done_ccb, 0, 0); > if (error == ERESTART) { > @@ -1762,12 +1823,24 @@ adadone(struct cam_periph *periph, union > /*reduction*/0, > /*timeout*/0, > /*getcount_only*/0); > + /* > + * If we get an error on an NCQ DSM TRIM, fall back > + * to a non-NCQ DSM TRIM forever. Please note that if > + * CAN_NCQ_TRIM is set, CAN_TRIM is necessarily set too. > + * However, for this one trim, we treat it as advisory > + * and return success up the stack. > + */ > + if (state == ADA_CCB_TRIM && > + error != 0 && > + (softc->flags & ADA_FLAG_CAN_NCQ_TRIM) != 0) { > + softc->flags &= ~ADA_FLAG_CAN_NCQ_TRIM; > + error = 0; > + } > } else { > if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) > panic("REQ_CMP with QFRZN"); > error = 0; > } > - bp = (struct bio *)done_ccb->ccb_h.ccb_bp; > bp->bio_error = error; > if (error != 0) { > bp->bio_resid = bp->bio_bcount; > > Modified: projects/iosched/sys/cam/cam_ccb.h > ============================================================================== > --- projects/iosched/sys/cam/cam_ccb.h Thu Mar 5 21:41:58 2015 (r279677) > +++ projects/iosched/sys/cam/cam_ccb.h Fri Mar 6 00:23:23 2015 (r279678) > @@ -573,6 +573,7 @@ typedef enum { > } pi_tmflag; > > typedef enum { > + PIM_NCQ_KLUDGE = 0x200, /* Supports the sata ncq trim kludge */ > PIM_EXTLUNS = 0x100,/* 64bit extended LUNs supported */ > PIM_SCANHILO = 0x80, /* Bus scans from high ID to low ID */ > PIM_NOREMOVE = 0x40, /* Removeable devices not included in scan */ > > Modified: projects/iosched/sys/dev/ahci/ahci.c > ============================================================================== > --- projects/iosched/sys/dev/ahci/ahci.c Thu Mar 5 21:41:58 2015 (r279677) > +++ projects/iosched/sys/dev/ahci/ahci.c Fri Mar 6 00:23:23 2015 (r279678) > @@ -2361,6 +2361,9 @@ ahci_setup_fis(struct ahci_channel *ch, > fis[13] = ccb->ataio.cmd.sector_count_exp; > } > fis[15] = ATA_A_4BIT; > + /* Gross and vile hack -- makes ncq trim work w/o changing ataio size */ > + if (ccb->ataio.cmd.flags & CAM_ATAIO_AUX_HACK) > + fis[16] = 1; > } else { > fis[15] = ccb->ataio.cmd.control; > } > @@ -2618,7 +2621,7 @@ ahciaction(struct cam_sim *sim, union cc > if (ch->caps & AHCI_CAP_SPM) > cpi->hba_inquiry |= PI_SATAPM; > cpi->target_sprt = 0; > - cpi->hba_misc = PIM_SEQSCAN | PIM_UNMAPPED; > + cpi->hba_misc = PIM_SEQSCAN | PIM_UNMAPPED | PIM_NCQ_KLUDGE; > cpi->hba_eng_cnt = 0; > if (ch->caps & AHCI_CAP_SPM) > cpi->max_target = 15; >