From owner-svn-src-projects@FreeBSD.ORG Fri Mar 6 01:50:44 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 8E209484 for ; Fri, 6 Mar 2015 01:50:44 +0000 (UTC) Received: from mail-pa0-f47.google.com (mail-pa0-f47.google.com [209.85.220.47]) (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 597259E7 for ; Fri, 6 Mar 2015 01:50:44 +0000 (UTC) Received: by padet14 with SMTP id et14so23420895pad.11 for ; Thu, 05 Mar 2015 17:50:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:mime-version:content-type:from :in-reply-to:date:cc:message-id:references:to; bh=YcP3cpk0HAarnqyPd1PF/MfzS+waN3Xm+0azwY1KxdM=; b=N4xk3Sd2CgqOmhCEEYR735K4aEQPyGk15oZv1Ea8HiZh/kdhqpbAdkiCztVgHoicCM FZA2JuQqwdEz5Jn+e8LpyvCNnSuYKtm9YiIpzn2wR9e2RbEhVQx/UXiYYRNFphNPbi6v kMpVDA2SlYEnIM29eyOZZ9CJd0tcZxqSpoliBamDNGGx79lo5NEJS/LXCSGPun9F/hlA WoBfhfBtB07wNa0aTB40tmfV3u+hnMnte20joMm9ey6+mWPcPyqsmxoFYstm4blO07hc XP2Jo/uYcQoULs/jciAk+Iso5OZcsLUr4tYy5iUMzcTLclr2bvAZUKm5StixgnA3UkVU jEIQ== X-Gm-Message-State: ALoCoQnAZPTMlM1UnFyg2kDo1WXUkpZixehL48B1C++xZMUXKuIY2lvXdprCxeDZMpqi7p1VC2RY X-Received: by 10.70.96.204 with SMTP id du12mr12103313pdb.151.1425606638451; Thu, 05 Mar 2015 17:50:38 -0800 (PST) Received: from [10.64.24.161] ([69.53.236.236]) by mx.google.com with ESMTPSA id ki2sm8127597pdb.33.2015.03.05.17.50.35 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 05 Mar 2015 17:50:37 -0800 (PST) Sender: Warner Losh Subject: Re: svn commit: r279678 - in projects/iosched/sys: cam cam/ata dev/ahci Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2070.6\)) Content-Type: multipart/signed; boundary="Apple-Mail=_BCDC3C1D-6567-4014-BC2A-916B7D97BBFE"; protocol="application/pgp-signature"; micalg=pgp-sha512 X-Pgp-Agent: GPGMail 2.5b5 From: Warner Losh In-Reply-To: <54F8F9F7.8080605@freebsd.org> Date: Thu, 5 Mar 2015 18:50:30 -0700 Message-Id: <5637B469-F10D-4D7A-8036-DE417368F71E@bsdimp.com> References: <201503060023.t260NOA8076981@svn.freebsd.org> <54F8F9F7.8080605@freebsd.org> To: Steven Hartland X-Mailer: Apple Mail (2.2070.6) Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org, Warner Losh 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 01:50:44 -0000 --Apple-Mail=_BCDC3C1D-6567-4014-BC2A-916B7D97BBFE Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Pure laziness on my part. I should be retrying a failed NCQ DSM TRIM as = a DSM TRIM. This is one reason why I haven=E2=80=99t pushed these = changes into -head. That level of detail doesn=E2=80=99t matter for the = I/O Scheduler which this branch exists for, but bits of that code depend = on these changes, alas. Warner > On Mar 5, 2015, at 5:51 PM, Steven Hartland = wrote: >=20 > Any reason why you don't retry the failed NCQ TRIM as a none NCQ TRIM = instead of returning success then it failed? >=20 > Regards > Steve >=20 > 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 >>=20 >> 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. >>=20 >> 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 >>=20 >> Modified: projects/iosched/sys/cam/ata/ata_all.h >> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> --- 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; >>=20 >> Modified: projects/iosched/sys/cam/ata/ata_da.c >> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> --- 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 =3D 0x0400, >> ADA_FLAG_CAN_POWERMGT =3D 0x0800, >> ADA_FLAG_CAN_DMA48 =3D 0x1000, >> - ADA_FLAG_DIRTY =3D 0x2000 >> + ADA_FLAG_DIRTY =3D 0x2000, >> + ADA_FLAG_CAN_NCQ_TRIM =3D 0x4000 /* CAN_TRIM also set */ >> } ada_flags; >> typedef enum { >> @@ -1018,10 +1019,22 @@ adaasync(void *callback_arg, u_int32_t c >> else >> softc->flags &=3D ~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 |=3D ADA_FLAG_CAN_TRIM; >> - else >> - softc->flags &=3D ~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) !=3D 0 && = */ /* Don't know how to do this here */ >> + (cgd.ident_data.satacapabilities2 & = ATA_SUPPORT_RCVSND_FPDMA_QUEUED) !=3D 0 && >> + (softc->flags & ADA_FLAG_CAN_TRIM) !=3D 0) >> + softc->flags |=3D ADA_FLAG_CAN_NCQ_TRIM; >> + else >> + softc->flags &=3D = ~ADA_FLAG_CAN_NCQ_TRIM; >> + } else >> + softc->flags &=3D ~(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 =3D maxio; >> if ((cpi.hba_misc & PIM_UNMAPPED) !=3D 0) >> softc->disk->d_flags |=3D 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) !=3D 0 && >> + (cgd->ident_data.satacapabilities2 & >> + ATA_SUPPORT_RCVSND_FPDMA_QUEUED) !=3D 0 && >> + (softc->flags & ADA_FLAG_CAN_TRIM) !=3D 0) >> + softc->flags |=3D 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 =3D &softc->trim_req; >> uint64_t lastlba =3D (uint64_t)-1; >> int c, lastcount =3D 0, off, ranges =3D 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 =3D &softc->trim_req; >> + int ranges; >> + >> + ranges =3D 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 =3D &softc->trim_req; >> + int ranges; >> + >> + ranges =3D 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 =3D ATA_SFPDMA_DSM; >> + ataio->cmd.flags |=3D CAM_ATAIO_AUX_HACK; >> +} >> + >> +static void >> ada_cfaerase(struct ada_softc *softc, struct bio *bp, struct = ccb_ataio *ataio) >> { >> struct trim_request *req =3D &softc->trim_req; >> @@ -1523,7 +1581,9 @@ adastart(struct cam_periph *periph, unio >> /* Run TRIM if not running yet. */ >> if (!softc->trim_running && >> (bp =3D bioq_first(&softc->trim_queue)) !=3D 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 =3D (struct bio *)done_ccb->ccb_h.ccb_bp; >> if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) !=3D = CAM_REQ_CMP) { >> error =3D adaerror(done_ccb, 0, 0); >> if (error =3D=3D 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 =3D=3D ADA_CCB_TRIM && >> + error !=3D 0 && >> + (softc->flags & ADA_FLAG_CAN_NCQ_TRIM) !=3D = 0) { >> + softc->flags &=3D = ~ADA_FLAG_CAN_NCQ_TRIM; >> + error =3D 0; >> + } >> } else { >> if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) !=3D = 0) >> panic("REQ_CMP with QFRZN"); >> error =3D 0; >> } >> - bp =3D (struct bio *)done_ccb->ccb_h.ccb_bp; >> bp->bio_error =3D error; >> if (error !=3D 0) { >> bp->bio_resid =3D bp->bio_bcount; >>=20 >> Modified: projects/iosched/sys/cam/cam_ccb.h >> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> --- 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 =3D 0x200, /* Supports the sata ncq trim kludge = */ >> PIM_EXTLUNS =3D 0x100,/* 64bit extended LUNs supported */ >> PIM_SCANHILO =3D 0x80, /* Bus scans from high ID to low = ID */ >> PIM_NOREMOVE =3D 0x40, /* Removeable devices not = included in scan */ >>=20 >> Modified: projects/iosched/sys/dev/ahci/ahci.c >> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> --- 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] =3D ccb->ataio.cmd.sector_count_exp; >> } >> fis[15] =3D 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] =3D 1; >> } else { >> fis[15] =3D ccb->ataio.cmd.control; >> } >> @@ -2618,7 +2621,7 @@ ahciaction(struct cam_sim *sim, union cc >> if (ch->caps & AHCI_CAP_SPM) >> cpi->hba_inquiry |=3D PI_SATAPM; >> cpi->target_sprt =3D 0; >> - cpi->hba_misc =3D PIM_SEQSCAN | PIM_UNMAPPED; >> + cpi->hba_misc =3D PIM_SEQSCAN | PIM_UNMAPPED | = PIM_NCQ_KLUDGE; >> cpi->hba_eng_cnt =3D 0; >> if (ch->caps & AHCI_CAP_SPM) >> cpi->max_target =3D 15; >>=20 >=20 --Apple-Mail=_BCDC3C1D-6567-4014-BC2A-916B7D97BBFE Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP using GPGMail -----BEGIN PGP SIGNATURE----- Comment: GPGTools - https://gpgtools.org iQIcBAEBCgAGBQJU+QfmAAoJEGwc0Sh9sBEAK8kP/3d+IhMb3+EDshU/MdsfpFy3 XWB7+WtEilrWRcYCBJGGFDZm0Nq8byS2PQHY76OEjWDwIN82gWymaNDAMCs6FFE3 +Jy2aDjUVIL1Fp4xkMiFGXIThOXzb1rK4Gd7QWz1J8K+3BRHczBtxPtjWyTRAhnS ou+niJyJnW3sFZ7Vt9pW3Fm81xIElf5jE68z+AP82Aekw/fG5yRjkJDmTeE8BOIQ vdV7VnoyrpCbjlj1yGNqQud2KGOC49QbcLKnMPn3IvNrvyYotEIz1+OzwAdFkmSV mIkg/GpGmIQF14F3/9W5gzdbVTVmGV3PZNVqoPn5xG6ZZrzJ2vR91cdPWUAY+gon BXi3LBbSydcuXUaBmOg9LjmQwJzZ6E6k4ppd1+sbJ6uspZpdXqix+55wZmCb7eyI cjnVSrkPYGrMwPUiJasGJosS5ZqjMAiYoQEhtcDgiaqnp1qhGZUl4+00wmGUdjjf aQSg6VoeLUUhhTztwZBUAs8ywY6hSlXylsSJsJAqZlXoUtdPF04XFuxBU6fWmVOU /XpM0LA7Qy7ifFuFYEC80sMgOqpy5UgF52zU2r6Qq2oIMRlxfxpizAEF7O3Adcpf n1S+purMpqqenrZ1sMQ50LJQyEfYdrHa6wgDiBotUiA6ukSfV5Fi6UtEqla6zMW+ Pq7XhtzaV1X7Dj2ZqyjM =aNHU -----END PGP SIGNATURE----- --Apple-Mail=_BCDC3C1D-6567-4014-BC2A-916B7D97BBFE--