svn commit: r355837 - head/sys/cam

[Kevin Bowling]

On Mon, Dec 16, 2019 at 5:44 PM Steven Hartland <
steven.hartland at multiplay.co.uk> wrote:

> Sticky keyboard there Warner?


LOL


> 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.


Have you been able to test the new OpenZFS/ZoF TRIM?

I notice the current FBSD one gets quite beleaguered with highly concurrent
poudriere as the snapshots are being reaped, I.e TRIMs totally swamp r/w
ops on the Plextor PCIe SSD I have.  I haven’t tried ZoF on this machine
yet since it is my main workstation but will do so once it is ready for
mainline.


> 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
>
> _______________________________________________
> svn-src-head at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-head
> To unsubscribe, send any mail to "svn-src-head-unsubscribe at freebsd.org"
>