@@ -785,11 +785,6 @@ static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode)
unsigned int logical_block_size = sdkp->device->sector_size;
unsigned int max_blocks = 0;
- q->limits.discard_alignment =
- sdkp->unmap_alignment * logical_block_size;
- q->limits.discard_granularity =
- max(sdkp->physical_block_size,
- sdkp->unmap_granularity * logical_block_size);
sdkp->provisioning_mode = mode;
switch (mode) {
@@ -3260,6 +3255,12 @@ static int sd_revalidate_disk(struct gendisk *disk)
sd_print_capacity(sdkp, old_capacity);
+ q->limits.discard_alignment =
+ sdkp->unmap_alignment * sdkp->device->sector_size;
+ q->limits.discard_granularity =
+ max(sdkp->physical_block_size,
+ sdkp->unmap_granularity * sdkp->device->sector_size);
+
sd_read_write_protect_flag(sdkp, buffer);
sd_read_cache_type(sdkp, buffer);
sd_read_app_tag_own(sdkp, buffer);
Previously, the discard_alignment and discard_granularity attributes of queue->limits were calculated in sd_config_discard(). However, it uses the logical block size, physical block size, unmap alignment and unmap granularity, which were not necessarily already known (and therefore set to 0) at that point. Instead, the calculations should be done after these values are known. The reason for finding this bug was the following observable behavior: I use an external SSD which supports the unmap command but sets lbpme=0. To make the kernel send the unmap command, I added the following udev rule: ACTION=="add|change", ATTRS{idVendor}=="0634", ATTRS{idProduct}=="5600", SUBSYSTEM=="scsi_disk", ATTR{provisioning_mode}="unmap" When running blkdiscard on the disk after plugging it in, it still failed. In dmesg, an error “Error: discard_granularity is 0.” was shown. Setting provisioning_mode=unmap again fixed the error. Looking at the code, I saw that the discard_granularity can be 0 only if sdkp->physical_block_size is 0. Therefore, I concluded that the physical block size was not yet known (set to 0) at the point when udev set provisioning_mode=unmap, which calls sd_config_discard(). The block sizes are set in sd_read_capacity() and the unmap alignment and granularity are set in sd_read_block_limits(). Therefore, I moved the calculations after control flow after calling these functions joined. The moved code sets the attributes even if the disk doesn’t support discard at all. Before, this happened only if sd_config_discard() was called (even if to disable discard). Now, it’s always set if media is present. So before and after this change, the attributes could not be used to determine whether discard is enabled or not. Signed-off-by: Manuel Jacob <me@manueljacob.de> --- drivers/scsi/sd.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)