svn commit: r355832 - head/sys/cam

[Warner Losh]

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 at 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);
> >   }
>
>