Discard performance regression after "scsi: sd: Remove LBPRZ dependency for discards"

Discard performance regression after "scsi: sd: Remove LBPRZ dependency for discards"

[Wang Jianchao]

Hi Martin

We have a sata ssd with following parameters,

  max_ws_blocks = 0
  max_unmap_blocks = 262143
  lbprz = 1
  lbpu = 1
  unmap_limit_for_ws = 0

But the discard performance is very different on 3.10 and 4.18 series,
On 3.10 series, the bw of discard is ~60G/s, but on 4.18, it is only ~2.7G/s
The reason is the discard_max_bytes,
3.10 series : 4294966784
4.18 series : 134217216

This difference happen after commit 
commit bcd069bb250acf6088b60d189ab3ec3ae8dd11a5 (scsi: sd: Remove LBPRZ dependency for discards)

@@ -2842,7 +2829,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
                                sd_config_discard(sdkp, SD_LBP_WS16);
 
                } else {        /* LBP VPD page tells us what to use */
-                       if (sdkp->lbpu && sdkp->max_unmap_blocks && !sdkp->lbprz)
+                       if (sdkp->lbpu && sdkp->max_unmap_blocks)
                                sd_config_discard(sdkp, SD_LBP_UNMAP);
                        else if (sdkp->lbpws)
                                sd_config_discard(sdkp, SD_LBP_WS16);

Before this commit, it enters SD_LBP_WS16 case, but after the patch, it enters SD_LBP_UNMAP.

Which should be the correct case ?

Thanks and Best regards
Jianchao

Re: Discard performance regression after "scsi: sd: Remove LBPRZ dependency for discards"

[Martin K. Petersen]

Wang,

> We have a sata ssd with following parameters,
>
>   max_ws_blocks = 0
>   max_unmap_blocks = 262143
>   lbprz = 1
>   lbpu = 1
>   unmap_limit_for_ws = 0

The device is explicitly asking for us to use UNMAP so the older kernel
choosing WRITE SAME(16) was not correct.

I have been working on an update to the discard/zeroing heuristics that
I'll post when 5.15 opens next week. But based on what's reported it by
your device we would still choose UNMAP for discards. LBPRZ only affects
which command we use for zeroing.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: Discard performance regression after "scsi: sd: Remove LBPRZ dependency for discards"

[Wang Jianchao]

Thanks so much for your comment, Martin ;)
That's really helpful.

Jianchao

On 2021/7/8 11:23 AM, Martin K. Petersen wrote:
> 
> Wang,
> 
>> We have a sata ssd with following parameters,
>>
>>   max_ws_blocks = 0
>>   max_unmap_blocks = 262143
>>   lbprz = 1
>>   lbpu = 1
>>   unmap_limit_for_ws = 0
> 
> The device is explicitly asking for us to use UNMAP so the older kernel
> choosing WRITE SAME(16) was not correct.
> 
> I have been working on an update to the discard/zeroing heuristics that
> I'll post when 5.15 opens next week. But based on what's reported it by
> your device we would still choose UNMAP for discards. LBPRZ only affects
> which command we use for zeroing.
>