>>>>> "Mike" == Mike Snitzer <snitzer@xxxxxxxxxx> writes: Mike> lim->discard_zeroes_data = -1; was suspect to me too. Mike> But why default to 1 here? Because otherwise DM would default to having dzd to "unsupported", meaning the feature would never be turned on regardless of the bottom device capabilities. The old approach used the -1 value to indicate "has not been set". That was only really intended as a value for the stacking drivers, not for the LLDs. It was a bit of a hack and I'd rather deal with dzd the same way as we do with clustering. >> @@ -166,6 +166,7 @@ void blk_queue_make_request(struct request_queue >> *q, make_request_fn *mfn) >> >> blk_set_default_limits(&q->limits); >> blk_queue_max_hw_sectors(q, BLK_SAFE_MAX_SECTORS); >> + q->limits.discard_zeroes_data = 0; >> >> /* >> * by default assume old behaviour and bounce for any highmem page Mike> Only to then reset to 0 here? Shouldn't we default to 0 and only Mike> set to 1 where applicable (e.g. sd_config_discard)? My first approach was to set it in dm-table.c before stacking. But I thought it was icky to have the stacking driver ask for defaults and then have to tweak them for things to work correctly. The other option is to have blk_set_default_stacking_limits(). Or we could add a flag to blk_set_default_limits to indicate whether this is a LLD or a stacking driver. We already special-case BLK_SAFE_MAX_SECTORS when setting the request function. And that's the only non-stacking user of the default limits call. So that's why I disabled dzd there. Since this is a stable bugfix I also wanted to keep it small and simple. But I'm totally open to suggestions. -- Martin K. Petersen Oracle Linux Engineering -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
