From owner-svn-src-head@freebsd.org Tue Dec 17 03:13:11 2019 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 14F611D97C5 for ; Tue, 17 Dec 2019 03:13:11 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qt1-x832.google.com (mail-qt1-x832.google.com [IPv6:2607:f8b0:4864:20::832]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 47cNWB15ZSz3FFp for ; Tue, 17 Dec 2019 03:13:10 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qt1-x832.google.com with SMTP id l12so7594884qtq.12 for ; Mon, 16 Dec 2019 19:13:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=i/BKBrF5PhTZF4Xqvh7YgLqsbqD1n1JcNvwJ52VuORY=; b=N2lcW/vJanuzV3Q7uMB3IePpT7rdnFvqi/hmaUZr1ik19Hr/hixJZNkVpaXowJh3TH 2Qx5uLFwX69ST+EsVi0IucH/RP3pd+mq7vB2/tOyPPj0u/RO26Ortw1/1v/GDlpW8AMI OF87wrIvKWbVRjFABxPik0dEbGYt1CtRZ3xzlsVqxWW2AZ+e1aKqFRtWDEA2oLejvBRd lmd36yC6CBE3djip50yNYyRYBUrtZ02AkQP8kVVwXfjAZCjdvM/OsN5VJLrpn5AYKUUY KPKsUf7cUUYQB3kAMroSMhFTFF01l1FiPlGpWCdxbDScq4AbPnecxNp/UX+4akVgkdfZ Jv6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=i/BKBrF5PhTZF4Xqvh7YgLqsbqD1n1JcNvwJ52VuORY=; b=HVc0VDjO2nGTGym/W3jK0Nz0ovQUpnHEqLSvnzLwqtDJ/pSNZ9mKtZsUsb/ZlLFiUR Cq9XHwCp7K2iuKvEQtln+ibdz+klQMJvJJy+RUIpsGGNMUQyC9AJkxKMotPt0LTEKMPQ LLyOO0jSDwXxHp1GRUTgVWLU4k01C6X3dqL3N5ApFbCNG7nW01pfNGg3TR+dryb2vaWn ujmXalr3yaPFjyPUMRwjSOTop2CeT2ZewwM2mTBHZSSH5WXwZXRoMsirGsKXYdiyxuJZ mIKZ7+aWXJdBcUG+yytKsz/N3YonAncRSrEyQtPvw6/XEGrLfEPAW/IwA9EQvJiu+x1u JiVg== X-Gm-Message-State: APjAAAWZSUcbXWyK7SFEjm46GwG9sV1HCRi4wDUMZDUcSur4oLQIUDhw GmHEM5wKTSY9kqWJ9BnpZvAFbAamOALVXgsAOJNYyw== X-Google-Smtp-Source: APXvYqwxUfHopNoUtCs8wtC9dcCVpsk8H4z7dcKrx7Ou2QSQQSmCj+OCScn9Vhz19ScOLq0Jm7bEDtNMu2xie115ptU= X-Received: by 2002:ac8:68c:: with SMTP id f12mr2511839qth.187.1576552389104; Mon, 16 Dec 2019 19:13:09 -0800 (PST) MIME-Version: 1.0 References: <201912170013.xBH0DjZY090730@repo.freebsd.org> <5b944742-51ff-861b-10bc-c01d67c02933@multiplay.co.uk> In-Reply-To: From: Warner Losh Date: Mon, 16 Dec 2019 20:12:58 -0700 Message-ID: Subject: Re: svn commit: r355837 - head/sys/cam To: Kevin Bowling Cc: Steven Hartland , Warner Losh , src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 47cNWB15ZSz3FFp X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bsdimp-com.20150623.gappssmtp.com header.s=20150623 header.b=N2lcW/vJ; dmarc=none; spf=none (mx1.freebsd.org: domain of wlosh@bsdimp.com has no SPF policy when checking 2607:f8b0:4864:20::832) smtp.mailfrom=wlosh@bsdimp.com X-Spamd-Result: default: False [-3.70 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; R_DKIM_ALLOW(-0.20)[bsdimp-com.20150623.gappssmtp.com:s=20150623]; FROM_HAS_DN(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; PREVIOUSLY_DELIVERED(0.00)[svn-src-head@freebsd.org]; DMARC_NA(0.00)[bsdimp.com]; URI_COUNT_ODD(1.00)[9]; RCPT_COUNT_FIVE(0.00)[6]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[bsdimp-com.20150623.gappssmtp.com:+]; RCVD_IN_DNSWL_NONE(0.00)[2.3.8.0.0.0.0.0.0.0.0.0.0.0.0.0.0.2.0.0.4.6.8.4.0.b.8.f.7.0.6.2.list.dnswl.org : 127.0.5.0]; R_SPF_NA(0.00)[]; FORGED_SENDER(0.30)[imp@bsdimp.com,wlosh@bsdimp.com]; MIME_TRACE(0.00)[0:+,1:+,2:~]; IP_SCORE(-2.70)[ip: (-9.35), ipnet: 2607:f8b0::/32(-2.19), asn: 15169(-1.90), country: US(-0.05)]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; FROM_NEQ_ENVFROM(0.00)[imp@bsdimp.com,wlosh@bsdimp.com]; RCVD_TLS_ALL(0.00)[]; RCVD_COUNT_TWO(0.00)[2] Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Dec 2019 03:13:11 -0000 On Mon, Dec 16, 2019 at 5:54 PM Kevin Bowling wrote: > > > On Mon, Dec 16, 2019 at 5:44 PM Steven Hartland < > steven.hartland@multiplay.co.uk> wrote: > >> Sticky keyboard there Warner? > > > LOL > Yea. I have a mac with a keyboard with a stuck delete key. I tried to edit the commit message on a flight to california last week. It screwed up a few of the commit messages so I quit for the night. I fixed all the others, but missed this one :(. On a more serious note the fact that the controllers lie about the >> underlying >> location of data, the impact of skipping the TRIM requests can have a >> much more >> serious impact than one might think depending on the drive, so this type >> of >> optimisation can significantly harm performance instead of increasing it= . >> >> This was the main reasons we sponsored the initial ZFS TRIM >> implementation; as >> drive performance go so bad with no TRIM that SSD's performed worse than >> HDD's. > > > Have you been able to test the new OpenZFS/ZoF TRIM? > > I notice the current FBSD one gets quite beleaguered with highly > concurrent poudriere as the snapshots are being reaped, I.e TRIMs totally > swamp r/w ops on the Plextor PCIe SSD I have. I haven=E2=80=99t tried Zo= F on this > machine yet since it is my main workstation but will do so once it is rea= dy > for mainline. > Trims totally swamping r/w ops is why I started this work in the first place. I'd wager that the new ZoF TRIM code may not be as well tuned as FreeBSD and/or makes performance assumptions that are unwise in practice. I've not looked at it to know, but I suspect it is combining adjacent trims less. If you are using nvd it will shot gun all requests into the device's queue, which on less than high end enterprise drives can lead to issues like you described... I'm willing to help people characterize what's going on, but I won't have time to look into this until sometime in January. In general, at least for the drives we use, fewer trims that are larger work a lot better. Also, biasing your I/O selection towards reads by some factor helps mitigate the trim factor a bit, though they can still swamp if there's no reads for a short while to keep the trims at bay (which is why I wrote the pacing code). Warner > >> Now obviously this was some time ago, but I wouldn't be surprised if >> there's bad >> hardware / firmware like this still being produced. >> >> Given that might be a good idea to make this optional, possibly even opt >> in not opt >> out? >> >> Regards >> Steve >> >> On 17/12/2019 00:13, Warner Losh wrote: >> > Author: imp >> > Date: Tue Dec 17 00:13:45 2019 >> > New Revision: 355837 >> > URL: https://svnweb.freebsd.org/changeset/base/355837 >> > >> > Log: >> > Implement bio_speedup >> > >> > React to the BIO_SPEED command in the cam io scheduler by completin= g >> > as successful BIO_DELETE commands that are pending, up to the lengt= h >> > passed down in the BIO_SPEEDUP cmomand. The length passed down is a >> > hint for how much space on the drive needs to be recovered. By >> > completing the BIO_DELETE comomands, this allows the upper layers t= o >> > allocate and write to the blocks that were about to be trimmed. Sin= ce >> > FreeBSD implements TRIMSs as advisory, we can eliminliminate them a= nd >> > go directly to writing. >> > >> > The biggest benefit from TRIMS coomes ffrom the drive being able t >> > ooptimize its free block pool inthe log run. There's little nto no >> > bene3efit in the shoort term. , sepeciall whn the trim is followed = by >> > a write. Speedup lets us make this tradeoff. >> > >> > Reviewed by: kirk, kib >> > Sponsored by: Netflix >> > Differential Revision: https://reviews.freebsd.org/D18351 >> > >> > Modified: >> > head/sys/cam/cam_iosched.c >> > >> > Modified: head/sys/cam/cam_iosched.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 >> > --- head/sys/cam/cam_iosched.c Tue Dec 17 00:13:40 2019 >> (r355836) >> > +++ head/sys/cam/cam_iosched.c Tue Dec 17 00:13:45 2019 >> (r355837) >> > @@ -1534,6 +1534,41 @@ cam_iosched_queue_work(struct cam_iosched_softc >> *isc, >> > { >> > >> > /* >> > + * A BIO_SPEEDUP from the uppper layers means that they have a >> block >> > + * shortage. At the present, this is only sent when we're trying >> to >> > + * allocate blocks, but have a shortage before giving up. >> bio_length is >> > + * the size of their shortage. We will complete just enough >> BIO_DELETEs >> > + * in the queue to satisfy the need. If bio_length is 0, we'll >> complete >> > + * them all. This allows the scheduler to delay BIO_DELETEs to >> improve >> > + * read/write performance without worrying about the upper >> layers. When >> > + * it's possibly a problem, we respond by pretending the >> BIO_DELETEs >> > + * just worked. We can't do anything about the BIO_DELETEs in th= e >> > + * hardware, though. We have to wait for them to complete. >> > + */ >> > + if (bp->bio_cmd =3D=3D BIO_SPEEDUP) { >> > + off_t len; >> > + struct bio *nbp; >> > + >> > + len =3D 0; >> > + while (bioq_first(&isc->trim_queue) && >> > + (bp->bio_length =3D=3D 0 || len < bp->bio_length)) { >> > + nbp =3D bioq_takefirst(&isc->trim_queue); >> > + len +=3D nbp->bio_length; >> > + nbp->bio_error =3D 0; >> > + biodone(nbp); >> > + } >> > + if (bp->bio_length > 0) { >> > + if (bp->bio_length > len) >> > + bp->bio_resid =3D bp->bio_length - len; >> > + else >> > + bp->bio_resid =3D 0; >> > + } >> > + bp->bio_error =3D 0; >> > + biodone(bp); >> > + return; >> > + } >> > + >> > + /* >> > * If we get a BIO_FLUSH, and we're doing delayed BIO_DELETEs >> then we >> > * set the last tick time to one less than the current ticks >> minus the >> > * delay to force the BIO_DELETEs to be presented to the client >> driver. >> > @@ -1919,8 +1954,8 @@ DB_SHOW_COMMAND(iosched, cam_iosched_db_show) >> > db_printf("Trim Q len %d\n", biolen(&isc->trim_queue)); >> > db_printf("read_bias: %d\n", isc->read_bias); >> > db_printf("current_read_bias: %d\n", isc->current_read_bias); >> > - db_printf("Trims active %d\n", isc->pend_trim); >> > - db_printf("Max trims active %d\n", isc->max_trim); >> > + db_printf("Trims active %d\n", isc->pend_trims); >> > + db_printf("Max trims active %d\n", isc->max_trims); >> > } >> > #endif >> > #endif >> >> _______________________________________________ >> svn-src-head@freebsd.org mailing list >> https://lists.freebsd.org/mailman/listinfo/svn-src-head >> To unsubscribe, send any mail to "svn-src-head-unsubscribe@freebsd.org" >> >