configurable discard parameters

* configurable discard parameters
@ 2015-06-20 17:12 Tom Yan
  2015-06-21  0:20 ` Martin K. Petersen
  0 siblings, 1 reply; 18+ messages in thread
From: Tom Yan @ 2015-06-20 17:12 UTC (permalink / raw)
  To: linux-scsi, linux-ide

[-- Attachment #1: Type: text/plain, Size: 2716 bytes --]

Today I check if blkdiscard really does a full device trim/wipe for my
Intel 530 SSD (240gb) with hexdump. I end up found that it fail to do
so because it report garbage info on its block limits VPD.

[tom@localhost ~]$ sudo sg_vpd -p 0xb0 /dev/sda
Block limits VPD page (SBC):
  Write same non-zero (WSNZ): 0
  Maximum compare and write length: 0 blocks
  Optimal transfer length granularity: 1 blocks
  Maximum transfer length: 0 blocks
  Optimal transfer length: 0 blocks
  Maximum prefetch length: 0 blocks
  Maximum unmap LBA count: 0
  Maximum unmap block descriptor count: 0
  Optimal unmap granularity: 1
  Unmap granularity alignment valid: 0
  Unmap granularity alignment: 0
  Maximum write same length: 0x3fffc0 blocks
  Maximum atomic transfer length: 0
  Atomic alignment: 0
  Atomic transfer length granularity: 0

The fact is, in each iteration, for this drive, blkdiscard can only
trim a maximum of 65528 sectors, which is the largest multiple of 8
sectors, which is the minimum possible. (In fact 65535 sectors seems
to be some sort of limit of ATA TRIM commands. It's also the maximum
of WRITE SAME (10).)

With `blkdiscard`, I can still workaround this with the "--step"
option, but for discard mount options, I don't think I have any way to
deal with it. Although it's basically the reponsibility of Intel (Well
I tried to find a way to complain to them but even their website is
borked as always), but still I would like to know why the kernel
doesn't allow "discard_granularity" and "discard_max_bytes" to be
configurable for users. Also wanna share about this in case nobody
ever noticed.


Another case is, I have a SanDisk Extreme USB (32gb, w/o UASP), which
is sort of a SATA SSD with USB bridge. I can basically TRIM the drive
with hdparm (although there's a minor issue:
https://sourceforge.net/p/hdparm/bugs/63/), but I can't do it with
blkdiscard. At first I thought it was only because its VPD doesn't
provide limits of discard:

[tom@localhost ~]$ sudo sg_vpd -p 0xb0 /dev/sdc
Block limits VPD page (SBC):
  Write same non-zero (WSNZ): 0
  Maximum compare and write length: 0 blocks
  Optimal transfer length granularity: 0 blocks
  Maximum transfer length: 8388607 blocks
  Optimal transfer length: 8388607 blocks
  Maximum prefetch length: 0 blocks

but then I try to use the attached patch to force some value for it,
blkdiscard still gives me "I/O error" instead of "NOT SUPPROTED". So
what's the requirement of the current discard way in the kernel? Does
it work differently on SATA drives than on USB drives because of the
controller (like involvement of libata)? Does it require capability of
one of the three scsi unmap ways? Or did I just miss some other dirty
hacking bits?

[-- Attachment #2: discard.patch --]
[-- Type: text/x-patch, Size: 1570 bytes --]

diff --git a/drivers/scsi/sd.c~ b/drivers/scsi/sd.c
index a661d33..a9cdfb6 100644
--- a/drivers/scsi/sd.c~
+++ b/drivers/scsi/sd.c
@@ -99,6 +99,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC);
 #endif
 
 static void sd_config_discard(struct scsi_disk *, unsigned int);
+static void sd_config_discard_novpd(struct scsi_disk *);
 static void sd_config_write_same(struct scsi_disk *);
 static int  sd_revalidate_disk(struct gendisk *);
 static void sd_unlock_native_capacity(struct gendisk *disk);
@@ -700,6 +701,22 @@ static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode)
 	queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 }
 
+static void sd_config_discard_novpd(struct scsi_disk *sdkp)
+{
+	struct request_queue *q = sdkp->disk->queue;
+	unsigned int logical_block_size = sdkp->device->sector_size;
+	unsigned int max_blocks = (u32)SD_MAX_WS16_BLOCKS;
+
+	q->limits.discard_zeroes_data = 0;
+	q->limits.discard_alignment = 0;
+	q->limits.discard_granularity = sdkp->physical_block_size;
+
+	sdkp->provisioning_mode = SD_LBP_WS16;
+
+	q->limits.max_discard_sectors = max_blocks * (logical_block_size >> 9);
+	queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
+}
+
 /**
  * sd_setup_discard_cmnd - unmap blocks on thinly provisioned device
  * @sdp: scsi device to operate one
@@ -2776,6 +2793,9 @@ static int sd_revalidate_disk(struct gendisk *disk)
 			sd_read_block_limits(sdkp);
 			sd_read_block_characteristics(sdkp);
 		}
+		else {
+			sd_config_discard_novpd(sdkp);
+		}
 
 		sd_read_write_protect_flag(sdkp, buffer);
 		sd_read_cache_type(sdkp, buffer);

^ permalink raw reply related	[flat|nested] 18+ messages in thread