On Wed, Jul 15 2015 at 6:14pm -0400, Jens Axboe <[email protected]> wrote:
> On 07/15/2015 10:29 AM, Mike Snitzer wrote: > >On Wed, Jul 15 2015 at 11:30am -0400, > >Jens Axboe <[email protected]> wrote: > > > >>On 07/15/2015 05:46 AM, Austin S Hemmelgarn wrote: > >>>On 2015-07-14 17:48, Jens Axboe wrote: > >>>>On 07/14/2015 02:45 PM, Jens Axboe wrote: > >>>>>On 07/14/2015 02:44 PM, Mike Snitzer wrote: > >>>>>>On Tue, Jul 14 2015 at 2:48pm -0400, > >>>>>>Jens Axboe <[email protected]> wrote: > >>>>>> > >>>>>>>Lots of devices exhibit very high latencies for big discards, hurting > >>>>>>>reads and writes. By default, limit the max discard we will build to > >>>>>>>64MB. This value has shown good results across a number of devices. > >>>>>>> > >>>>>>>This will potentially hurt discard throughput, from a provisioning > >>>>>>>point of view (when the user does mkfs.xfs, for instance, and mkfs > >>>>>>>issues a full device discard). If that becomes an issue, we could > >>>>>>>have different behavior for provisioning vs runtime discards. > >>>>>>> > >>>>>>>Signed-off-by: Jens Axboe <[email protected]> > >>>>>> > >>>>>>Christoph suggested you impose this default for the specific > >>>>>>drivers/devices that benefit. I'm not following why imposing a 64MB > >>>>>>default is the right thing to do for all devices. > >>>>> > >>>>>I'd argue that's most of them... But the testing we did was on NVMe. I > >>>>>can limit it to NVMe, no big deal. > >>>> > >>>>Oh, and LSI flash too, so not just NVMe. > >>>> > >>>While I don't have time to test it, I have a feeling that such a limit > >>>would help with many of the consumer SSD's out there. Secondarily, once > >>>this gets in and discard is fixed for BTRFS, I'll have some performance > >>>testing to do WRT dm-thinp. > >> > >>Right, that was the point of it. After more consideration, a default > >>"sane" limit should be applied to any non-stacked device. > > > >Sounds good. > > > >For DM thinp, it can handle really large discards efficiently (without > >passing discards down to the underlying data device). But if/when > >discard passdown is enabled it'll obviously split those larger discards > >based on this new "sane" limit of the underlying data device. > > > >Which would then potentially usher in the problem of discard latency > >being high for DM thinp (if discard passdown is enabled). But in > >practice I doubt that will be much of a concern. I'll keep both pieces > >if I'm wrong ;) > > Lets focus on patch 1+2 for now, so I can queue those up. > Acked/reviewed-by's welcome. Then we can tackle the "what is a sane > default and for whom" patch 3 later, it's orthogonal to exposing the > knob. Hey Jens, I happened upon the 2 patches you staged for 4.3: http://git.kernel.dk/?p=linux-block.git;a=commit;h=2bb4cd5cc472b191a46938becb7dafdd44644329 http://git.kernel.dk/?p=linux-block.git;a=commit;h=0034af036554c39eefd14d835a8ec3496ac46712 I noticed that none of DM is touched by the first wrapper patch (e.g. thin_io_hints should use it). I guess I should go through and prepare DM changes for 4.3 that go along with your patches (rebase dm-4.3 ontop of your for-4.3-core)? But that aside, I also checked with Joe Thornber about your desire to have the soft limit default to 64MB. Joe said he'd be fine with that; so you could go ahead and queue up that change if you like. Mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
