Date: Tue, 17 Dec 2019 00:32:15 +0000 From: Steven Hartland <steven.hartland@multiplay.co.uk> To: Warner Losh <imp@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r355832 - head/sys/cam Message-ID: <135a3dcc-5e32-153d-6398-822e106bd79d@multiplay.co.uk> In-Reply-To: <201912170013.xBH0DLnh090458@repo.freebsd.org> References: <201912170013.xBH0DLnh090458@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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); > }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?135a3dcc-5e32-153d-6398-822e106bd79d>