From owner-svn-src-head@freebsd.org Tue Dec 17 00:43:58 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 CC7591D3496 for ; Tue, 17 Dec 2019 00:43:58 +0000 (UTC) (envelope-from steven.hartland@multiplay.co.uk) Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) (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 47cKC20Dh9z4XcC for ; Tue, 17 Dec 2019 00:43:57 +0000 (UTC) (envelope-from steven.hartland@multiplay.co.uk) Received: by mail-wr1-x42e.google.com with SMTP id y11so9432973wrt.6 for ; Mon, 16 Dec 2019 16:43:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=multiplay-co-uk.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-transfer-encoding:content-language; bh=jDm8P9swc3dTndiNZsXulqiaAb9O+072DRsswfSxPSI=; b=qtMfh18+r+GEvxdi4FF8PzQx7tiENbiMDxiQfS4acnWY9pD4tJNMflbmNL/YahKszR 97fY7dBzy24Fb3j3fivSf5Fepzdc0QQ7y9w9gaLaSYlywlmhL1+IUqzxYab8NYNkpgpD oc55P+SkC/wjGeokKDnDleYqRBj1rcvLEHwPg7e7HmfKs3EGWeRXi5wXUTKxcMsRYJ4F 1y3Mmnou0gTn/sRbOrAZi3K2tWWKKItpPML1FXYDdeyUqIWTaYerSWlmua/KBc3JePcx 7W3XaMP/fUInXlYQ5dH/KNiIbS6Sd/i7V0rUE3lP69guPqbZAJqx1l4q3W6MGszUmZ0B eZkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=jDm8P9swc3dTndiNZsXulqiaAb9O+072DRsswfSxPSI=; b=Q9inOkukPUJZC1WzW5aA2JSogrvTgkzTzQKlx7ek2tOYQPXmM36KWtcv/Q8k5f1AiC 63Pl/5HQzXikAYNZzO1c3Hwhue5qmgtbpSzAq6QFLPS7JdWy/GdyQNliiJPBi/j4ULAT vNJrqtwL50sBsIfjWEr/cXZ62bOLmffyHWHoHIvjq5PY4KQDdA4KobsvRQYkfx+pYdio r8bYx7Zh4YEAeGcBZVtdR1FqxcW07WgbFwahuZAEUPKFABlqLJkVUfSQdZeSxhQqi+ES kCFwaderlol7bU7phFPRXZK083QFuNbWIbeiRPCA6CfJyZOlmr/qssacwEh3Lz1m7ON8 sFJw== X-Gm-Message-State: APjAAAW4nSc4eq2Wk3pqR1KVEztvCr3NzY2EEby/pPg5tKyi3UqKJ3Zm P/MNR2vK3Ld+pI6Mfz/TwU0sSFQcLTw= X-Google-Smtp-Source: APXvYqwkuU+PMU4/Lxx8vCV6qZNvJMrLIKdYCEzKAfjMNsZn9ThkpHqGHusnKTcdRTeUB//iXAnVuw== X-Received: by 2002:a5d:6408:: with SMTP id z8mr34156721wru.122.1576543435416; Mon, 16 Dec 2019 16:43:55 -0800 (PST) Received: from [10.44.128.75] ([193.117.175.106]) by smtp.gmail.com with ESMTPSA id z11sm23825291wrt.82.2019.12.16.16.43.54 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 16 Dec 2019 16:43:54 -0800 (PST) Subject: Re: svn commit: r355837 - head/sys/cam To: Warner Losh , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201912170013.xBH0DjZY090730@repo.freebsd.org> From: Steven Hartland Message-ID: <5b944742-51ff-861b-10bc-c01d67c02933@multiplay.co.uk> Date: Tue, 17 Dec 2019 00:43:54 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.1 MIME-Version: 1.0 In-Reply-To: <201912170013.xBH0DjZY090730@repo.freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-Rspamd-Queue-Id: 47cKC20Dh9z4XcC X-Spamd-Bar: ----- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=multiplay-co-uk.20150623.gappssmtp.com header.s=20150623 header.b=qtMfh18+; dmarc=pass (policy=none) header.from=multiplay.co.uk; spf=pass (mx1.freebsd.org: domain of steven.hartland@multiplay.co.uk designates 2a00:1450:4864:20::42e as permitted sender) smtp.mailfrom=steven.hartland@multiplay.co.uk X-Spamd-Result: default: False [-5.73 / 15.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_DKIM_ALLOW(-0.20)[multiplay-co-uk.20150623.gappssmtp.com:s=20150623]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; R_SPF_ALLOW(-0.20)[+ip6:2a00:1450:4000::/36:c]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; MIME_GOOD(-0.10)[text/plain]; PREVIOUSLY_DELIVERED(0.00)[svn-src-head@freebsd.org]; TO_DN_SOME(0.00)[]; RCVD_COUNT_THREE(0.00)[3]; TO_MATCH_ENVRCPT_SOME(0.00)[]; DKIM_TRACE(0.00)[multiplay-co-uk.20150623.gappssmtp.com:+]; DMARC_POLICY_ALLOW(-0.50)[multiplay.co.uk,none]; RCVD_IN_DNSWL_NONE(0.00)[e.2.4.0.0.0.0.0.0.0.0.0.0.0.0.0.0.2.0.0.4.6.8.4.0.5.4.1.0.0.a.2.list.dnswl.org : 127.0.5.0]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; IP_SCORE(-2.73)[ip: (-9.04), ipnet: 2a00:1450::/32(-2.67), asn: 15169(-1.90), country: US(-0.05)]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_TLS_ALL(0.00)[] 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 00:43:58 -0000 Sticky keyboard there Warner? 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. 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 completing > as successful BIO_DELETE commands that are pending, up to the length > 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 to > allocate and write to the blocks that were about to be trimmed. Since > FreeBSD implements TRIMSs as advisory, we can eliminliminate them and > 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 > ============================================================================== > --- 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 the > + * hardware, though. We have to wait for them to complete. > + */ > + if (bp->bio_cmd == BIO_SPEEDUP) { > + off_t len; > + struct bio *nbp; > + > + len = 0; > + while (bioq_first(&isc->trim_queue) && > + (bp->bio_length == 0 || len < bp->bio_length)) { > + nbp = bioq_takefirst(&isc->trim_queue); > + len += nbp->bio_length; > + nbp->bio_error = 0; > + biodone(nbp); > + } > + if (bp->bio_length > 0) { > + if (bp->bio_length > len) > + bp->bio_resid = bp->bio_length - len; > + else > + bp->bio_resid = 0; > + } > + bp->bio_error = 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