From owner-svn-src-all@freebsd.org Tue Dec 17 02:17:31 2019 Return-Path: Delivered-To: svn-src-all@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 AFDA61D7BC0 for ; Tue, 17 Dec 2019 02:17:31 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qk1-x72b.google.com (mail-qk1-x72b.google.com [IPv6:2607:f8b0:4864:20::72b]) (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 47cMGz04r7z4fpC for ; Tue, 17 Dec 2019 02:17:30 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qk1-x72b.google.com with SMTP id r14so6602862qke.13 for ; Mon, 16 Dec 2019 18:17:30 -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=TY0mfLzoOPXrELVd80NxVe6kdVj8PAX1j5xzDtR3S3g=; b=Ju29o93hWPKp8BAqeu4VQa84+ZQKbqGBV1wSFR4rIoVEyqb1NSADRZAeLj3x0J96ou HxSHbuiUbgbU8n3J3xcU3KFfKFvF9IyvmyVMnxXEluKO82obin7WYFgeQilSXU7c70Kw jj4Y9CaK0jRfoaEAmqTXS1jJIeln6iR//L5o0S3pDYO96Z11Q9O/eOxwvAmdHVT54Zl6 6L8pqYJiLWu8elSMJRWn+C0+v9rytKqKYGZWpcZhf7lNmsiDx2F5L9w2+rakJXH1g+T4 TwXH/0eUidNkwDE04h0u6S2HBkEekhoPnR3fljJkBtURgm46OX2TscyfXozI3M6gbdeP Wyow== 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=TY0mfLzoOPXrELVd80NxVe6kdVj8PAX1j5xzDtR3S3g=; b=hMxILFMx9f5w5IQTwiW4iMQxdcj08bxCmOCyBs/JA5j1Qg8drKaLcSb2lzIaT8BglD ZXhyHrOg02G6c5d1B1rRHG1IJO5WbHGFH95/n3OMPytGOd4N6cEA90/Mvy2EkZmJoYEZ 0/dpnpyziTTqEBvUzwzjjtRYIxskAfD8CM9Qe4l2Nuv0kLReiEDlI6JRrFQbir3kqzTE f7rB/Gu8P+mQqg+QFzXTJhXyZ5rx8A7XW0uI+KholcTAJE26bAkdLeQnahPeNi2E48t1 Gnbc7s48fX/j9uAjRHqlM/pMV/WI3WYej5goSUmRRFc071feAKW/WoweHXApVNdROYt9 vsZQ== X-Gm-Message-State: APjAAAVy7kOQ5EneofVCV3lKCECQhQ9i6EAe2EOCgtiy6+P7zdsiNVbz ciUpRPah8vU2OXTH/DdL+m1aXal7tQM5jtKwwZRPwg24 X-Google-Smtp-Source: APXvYqw0/dtls9EKwsh4zqr15giUfElvfEH9Gm1q7xVf7//R9VXWj/5YmNiCNMUjltve6luKoUo7+/fqIm1peGpP8eE= X-Received: by 2002:a05:620a:94f:: with SMTP id w15mr2655229qkw.380.1576549049108; Mon, 16 Dec 2019 18:17:29 -0800 (PST) MIME-Version: 1.0 References: <201912170013.xBH0DLnh090458@repo.freebsd.org> <135a3dcc-5e32-153d-6398-822e106bd79d@multiplay.co.uk> In-Reply-To: <135a3dcc-5e32-153d-6398-822e106bd79d@multiplay.co.uk> From: Warner Losh Date: Mon, 16 Dec 2019 19:17:10 -0700 Message-ID: Subject: Re: svn commit: r355832 - head/sys/cam To: Steven Hartland Cc: Warner Losh , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org X-Rspamd-Queue-Id: 47cMGz04r7z4fpC X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bsdimp-com.20150623.gappssmtp.com header.s=20150623 header.b=Ju29o93h; dmarc=none; spf=none (mx1.freebsd.org: domain of wlosh@bsdimp.com has no SPF policy when checking 2607:f8b0:4864:20::72b) smtp.mailfrom=wlosh@bsdimp.com X-Spamd-Result: default: False [-3.71 / 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)[]; TO_DN_SOME(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; PREVIOUSLY_DELIVERED(0.00)[svn-src-all@freebsd.org]; DMARC_NA(0.00)[bsdimp.com]; URI_COUNT_ODD(1.00)[3]; RCPT_COUNT_FIVE(0.00)[5]; TO_MATCH_ENVRCPT_SOME(0.00)[]; DKIM_TRACE(0.00)[bsdimp-com.20150623.gappssmtp.com:+]; RCVD_IN_DNSWL_NONE(0.00)[b.2.7.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.71)[ip: (-9.39), 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" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Dec 2019 02:17:31 -0000 The order is relaxed. Nothing in the system depends on ordering wrt BIO_DELETE requests. Apart from flushes, and one softdep thing in UFS, nothing does ordered writes. In a multithreaded world they don't really make sense because multiple upper layer consumers race each other to submit requests anyway... Warner On Mon, Dec 16, 2019, 5:32 PM Steven Hartland < steven.hartland@multiplay.co.uk> wrote: > What if any is the impact on request ordering with this new delayed TRIM? > > On 17/12/2019 00:13, Warner Losh wrote: > > Author: imp > > Date: Tue Dec 17 00:13:21 2019 > > New Revision: 355832 > > URL: https://svnweb.freebsd.org/changeset/base/355832 > > > > Log: > > Add rate limiters to TRIM. > > > > Add rate limiters to trims. Trims are a bit different than reads or > > writes in that they can be combined, so some care needs to be taken > > where we rate limit them. Additional work will be needed to push the > > working rate limit below the I/O quanta rate for things like IOPS. > > > > Sponsored by: Netflix > > > > Modified: > > head/sys/cam/cam_iosched.c > > > > Modified: head/sys/cam/cam_iosched.c > > > ============================================================================== > > --- head/sys/cam/cam_iosched.c Tue Dec 17 00:11:48 2019 > (r355831) > > +++ head/sys/cam/cam_iosched.c Tue Dec 17 00:13:21 2019 > (r355832) > > @@ -755,7 +755,20 @@ cam_iosched_has_io(struct cam_iosched_softc *isc) > > static inline bool > > cam_iosched_has_more_trim(struct cam_iosched_softc *isc) > > { > > + struct bio *bp; > > > > + bp = bioq_first(&isc->trim_queue); > > +#ifdef CAM_IOSCHED_DYNAMIC > > + if (do_dynamic_iosched) { > > + /* > > + * If we're limiting trims, then defer action on trims > > + * for a bit. > > + */ > > + if (bp == NULL || > cam_iosched_limiter_caniop(&isc->trim_stats, bp) != 0) > > + return false; > > + } > > +#endif > > + > > /* > > * If we've set a trim_goal, then if we exceed that allow trims > > * to be passed back to the driver. If we've also set a tick > timeout > > @@ -771,8 +784,8 @@ cam_iosched_has_more_trim(struct cam_iosched_softc > *is > > return false; > > } > > > > - return !(isc->flags & CAM_IOSCHED_FLAG_TRIM_ACTIVE) && > > - bioq_first(&isc->trim_queue); > > + /* NB: Should perhaps have a max trim active independent of I/O > limiters */ > > + return !(isc->flags & CAM_IOSCHED_FLAG_TRIM_ACTIVE) && bp != NULL; > > } > > > > #define cam_iosched_sort_queue(isc) ((isc)->sort_io_queue >= 0 ? \ > > @@ -1389,10 +1402,17 @@ cam_iosched_next_trim(struct cam_iosched_softc > *isc) > > struct bio * > > cam_iosched_get_trim(struct cam_iosched_softc *isc) > > { > > +#ifdef CAM_IOSCHED_DYNAMIC > > + struct bio *bp; > > +#endif > > > > if (!cam_iosched_has_more_trim(isc)) > > return NULL; > > #ifdef CAM_IOSCHED_DYNAMIC > > + bp = bioq_first(&isc->trim_queue); > > + if (bp == NULL) > > + return NULL; > > + > > /* > > * If pending read, prefer that based on current read bias > setting. The > > * read bias is shared for both writes and TRIMs, but on TRIMs the > bias > > @@ -1414,6 +1434,26 @@ cam_iosched_get_trim(struct cam_iosched_softc > *isc) > > */ > > isc->current_read_bias = isc->read_bias; > > } > > + > > + /* > > + * See if our current limiter allows this I/O. Because we only > call this > > + * here, and not in next_trim, the 'bandwidth' limits for trims > won't > > + * work, while the iops or max queued limits will work. It's tricky > > + * because we want the limits to be from the perspective of the > > + * "commands sent to the device." To make iops work, we need to > check > > + * only here (since we want all the ops we combine to count as > one). To > > + * make bw limits work, we'd need to check in next_trim, but that > would > > + * have the effect of limiting the iops as seen from the upper > layers. > > + */ > > + if (cam_iosched_limiter_iop(&isc->trim_stats, bp) != 0) { > > + if (iosched_debug) > > + printf("Can't trim because limiter says no.\n"); > > + isc->trim_stats.state_flags |= IOP_RATE_LIMITED; > > + return NULL; > > + } > > + isc->current_read_bias = isc->read_bias; > > + isc->trim_stats.state_flags &= ~IOP_RATE_LIMITED; > > + /* cam_iosched_next_trim below keeps proper book */ > > #endif > > return cam_iosched_next_trim(isc); > > } > >