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