svn commit: r326036 - head/sys/cam/scsi

[Justin T. Gibbs]

I'm sure it was to improve the performance of some SSD we were testing at Spectra.

--
Justin

> On Nov 22, 2017, at 9:16 AM, Alan Somers <asomers at freebsd.org> wrote:
> 
> I believe the original motivation was to reduce FLUSH CACHE EXT
> activity with some old SSDs that handled that command very slowly.
> gibbs might know more.
> 
> On Tue, Nov 21, 2017 at 1:14 PM, Ronald Klop <ronald-lists at klop.ws> wrote:
>> Out of curiosity, What is the use case which this improves? Does happen in
>> daily usage or mostly in testing?
>> 
>> Regards,
>> Ronald.
>> 
>> 
>> 
>> On Mon, 20 Nov 2017 23:27:34 +0100, Alan Somers <asomers at freebsd.org> wrote:
>> 
>>> Author: asomers
>>> Date: Mon Nov 20 22:27:33 2017
>>> New Revision: 326036
>>> URL: https://svnweb.freebsd.org/changeset/base/326036
>>> 
>>> Log:
>>>  da(4): Short-circuit unnecessary BIO_FLUSH commands
>>> sys/cam/scsi/scsi_da.c
>>>        Complete BIO_FLUSH commands immediately if the da(4) device hasn't
>>>        been written to since the last flush. If we haven't written to the
>>>        device, there is no reason to send a flush.
>>> Submitted by:  gibbs
>>>  Reviewed by:  imp
>>>  MFC after:    3 weeks
>>>  Sponsored by: Spectra Logic Corp
>>>  Differential Revision:        https://reviews.freebsd.org/D13106
>>> 
>>> Modified:
>>>  head/sys/cam/scsi/scsi_da.c
>>> 
>>> Modified: head/sys/cam/scsi/scsi_da.c
>>> 
>>> ==============================================================================
>>> --- head/sys/cam/scsi/scsi_da.c Mon Nov 20 22:18:24 2017        (r326035)
>>> +++ head/sys/cam/scsi/scsi_da.c Mon Nov 20 22:27:33 2017        (r326036)
>>> @@ -3038,6 +3038,18 @@ more:
>>>                }
>>>                case BIO_FLUSH:
>>>                        /*
>>> +                        * If we don't support sync cache, or the disk
>>> +                        * isn't dirty, FLUSH is a no-op.  Use the
>>> +                        * allocated * CCB for the next bio if one is
>>> +                        * available.
>>> +                        */
>>> +                       if ((softc->quirks & DA_Q_NO_SYNC_CACHE) != 0 ||
>>> +                           (softc->flags & DA_FLAG_DIRTY) == 0) {
>>> +                               biodone(bp);
>>> +                               goto skipstate;
>>> +                       }
>>> +
>>> +                       /*
>>>                         * BIO_FLUSH doesn't currently communicate
>>>                         * range data, so we synchronize the cache
>>>                         * over the whole disk.  We also force
>>> @@ -3052,6 +3064,15 @@ more:
>>>                                               /*lb_count*/0,
>>>                                               SSD_FULL_SIZE,
>>>                                               da_default_timeout*1000);
>>> +                       /*
>>> +                        * Clear the dirty flag before sending the
>>> command.
>>> +                        * Either this sync cache will be successful, or
>>> it
>>> +                        * will fail after a retry.  If it fails, it is
>>> +                        * unlikely to be successful if retried later, so
>>> +                        * we'll save ourselves time by just marking the
>>> +                        * device clean.
>>> +                        */
>>> +                       softc->flags &= ~DA_FLAG_DIRTY;
>>>                        break;
>>>                case BIO_ZONE: {
>>>                        int error, queue_ccb;
>>> _______________________________________________
>>> svn-src-all at freebsd.org mailing list
>>> https://lists.freebsd.org/mailman/listinfo/svn-src-all
>>> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"
>