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@xxxxxxxxxxxxxx>
---
drivers/scsi/sd.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a1a2ac09066f..526e4c93ceb4 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -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);
--
2.36.1
