[PATCH 1/2] libata: Expose trim capability in sysfs

[PATCH 1/2] libata: Expose trim capability in sysfs

[Martin K. Petersen]

From: "Martin K. Petersen" <martin.petersen@oracle.com>

Now that drives with support for queued trim are starting to appear, it
would be helpful to expose the chosen trim mode to userland. Create a
sysfs "trim" attribute for each ata_device that displays whether trim is
"unsupported", "unqueued" or "queued".

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Chris Samuel <chris@csamuel.org>
Cc: Marc MERLIN <marc@merlins.org>
---
 drivers/ata/libata-transport.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index e37413228228..a9a1a9055a3a 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -559,6 +559,37 @@ show_ata_dev_gscr(struct device *dev,
 
 static DEVICE_ATTR(gscr, S_IRUGO, show_ata_dev_gscr, NULL);
 
+enum {
+	ATA_TRIM_UNSUPPORTED = 0,
+	ATA_TRIM_UNQUEUED = 1,
+	ATA_TRIM_QUEUED = 2,
+};
+
+static const char *trim_mode[] = {
+	[ATA_TRIM_UNSUPPORTED] = "unsupported",
+	[ATA_TRIM_UNQUEUED] = "unqueued",
+	[ATA_TRIM_QUEUED] = "queued",
+};
+
+static ssize_t
+show_ata_dev_trim(struct device *dev,
+		  struct device_attribute *attr, char *buf)
+{
+	struct ata_device *ata_dev = transport_class_to_dev(dev);
+	unsigned int mode;
+
+	if (!ata_id_has_trim(ata_dev->id))
+		mode = ATA_TRIM_UNSUPPORTED;
+	else if (ata_fpdma_dsm_supported(ata_dev))
+		mode = ATA_TRIM_QUEUED;
+	else
+		mode = ATA_TRIM_UNQUEUED;
+
+	return snprintf(buf, 20, "%s\n", trim_mode[mode]);
+}
+
+static DEVICE_ATTR(trim, S_IRUGO, show_ata_dev_trim, NULL);
+
 static DECLARE_TRANSPORT_CLASS(ata_dev_class,
 			       "ata_device", NULL, NULL, NULL);
 
@@ -732,6 +763,7 @@ struct scsi_transport_template *ata_attach_transport(void)
 	SETUP_DEV_ATTRIBUTE(ering);
 	SETUP_DEV_ATTRIBUTE(id);
 	SETUP_DEV_ATTRIBUTE(gscr);
+	SETUP_DEV_ATTRIBUTE(trim);
 	BUG_ON(count > ATA_DEV_ATTRS);
 	i->dev_attrs[count] = NULL;
 
-- 
1.8.3.1

[PATCH 2/2] libata: Update queued trim blacklist for M5x0 drives

[Martin K. Petersen]

From: "Martin K. Petersen" <martin.petersen@oracle.com>

Crucial/Micron M500 drives properly support queued DSM TRIM starting
with firmware MU05. Update the blacklist so we only disable queued trim
for older firmware releases.

Early M550 series drives suffer from the same issue as M500. A bugfix
firmware is in the pipeline but not ready yet. Until then, blacklist
queued trim for M550.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Chris Samuel <chris@csamuel.org>
Cc: Marc MERLIN <marc@merlins.org>
---
 drivers/ata/libata-core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 34406f7fdd7a..f2a6020366e1 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4224,8 +4224,10 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	{ "PIONEER DVD-RW  DVR-216D",	NULL,	ATA_HORKAGE_NOSETXFER },
 
 	/* devices that don't properly handle queued TRIM commands */
-	{ "Micron_M500*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM, },
-	{ "Crucial_CT???M500SSD*",	NULL,	ATA_HORKAGE_NO_NCQ_TRIM, },
+	{ "Micron_M500*",		"MU0[1-4]*",	ATA_HORKAGE_NO_NCQ_TRIM, },
+	{ "Crucial_CT???M500SSD*",	"MU0[1-4]*",	ATA_HORKAGE_NO_NCQ_TRIM, },
+	{ "Micron_M550*",		NULL,		ATA_HORKAGE_NO_NCQ_TRIM, },
+	{ "Crucial_CT???M550SSD*",	NULL,		ATA_HORKAGE_NO_NCQ_TRIM, },
 
 	/*
 	 * Some WD SATA-I drives spin up and down erratically when the link
-- 
1.8.3.1

Re: [PATCH 2/2] libata: Update queued trim blacklist for M5x0 drives

[Tejun Heo]

On Tue, Apr 01, 2014 at 08:42:37PM -0400, Martin K. Petersen wrote:
> From: "Martin K. Petersen" <martin.petersen@oracle.com>
> 
> Crucial/Micron M500 drives properly support queued DSM TRIM starting
> with firmware MU05. Update the blacklist so we only disable queued trim
> for older firmware releases.
> 
> Early M550 series drives suffer from the same issue as M500. A bugfix
> firmware is in the pipeline but not ready yet. Until then, blacklist
> queued trim for M550.
> 
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Chris Samuel <chris@csamuel.org>
> Cc: Marc MERLIN <marc@merlins.org>

Cool, applied to libata/for-3.15-fixes w/ stable cc'd.

Thanks!

-- 
tejun

Re: [PATCH 1/2] libata: Expose trim capability in sysfs

[Tejun Heo]

On Tue, Apr 01, 2014 at 08:42:36PM -0400, Martin K. Petersen wrote:
> From: "Martin K. Petersen" <martin.petersen@oracle.com>
> 
> Now that drives with support for queued trim are starting to appear, it
> would be helpful to expose the chosen trim mode to userland. Create a

Can you please be a bit more elaborate with how it'd be helpful?  Do
we have actual use cases for it?

Thanks.

-- 
tejun

Re: [PATCH 1/2] libata: Expose trim capability in sysfs

[Martin K. Petersen]

>>>>> "Tejun" == Tejun Heo <tj@kernel.org> writes:

>> Now that drives with support for queued trim are starting to appear,
>> it would be helpful to expose the chosen trim mode to
>> userland. Create a

Tejun> Can you please be a bit more elaborate with how it'd be helpful?
Tejun> Do we have actual use cases for it?

There have been several threads on linux-btrfs and elsewhere where
people have asked whether queued trim is supported by their drive or
not. This information currently isn't reported anywhere. I thought it
would be convenient to have a sysfs attribute that people could inspect
to find out.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 1/2] libata: Expose trim capability in sysfs

[Marc MERLIN]

On Wed, Apr 02, 2014 at 01:26:16PM -0400, Martin K. Petersen wrote:
> >>>>> "Tejun" == Tejun Heo <tj@kernel.org> writes:
> 
> >> Now that drives with support for queued trim are starting to appear,
> >> it would be helpful to expose the chosen trim mode to
> >> userland. Create a
> 
> Tejun> Can you please be a bit more elaborate with how it'd be helpful?
> Tejun> Do we have actual use cases for it?
> 
> There have been several threads on linux-btrfs and elsewhere where
> people have asked whether queued trim is supported by their drive or
> not. This information currently isn't reported anywhere. I thought it
> would be convenient to have a sysfs attribute that people could inspect
> to find out.

Hi Tejun,

I haven't had time to pull a new kernel and install the patches I need and
this one yet (sorry), but I wanted to give a +1 to that.

If queued trim is not available, people may want to disable trim if they
want fast performance for rm.
But indeed currently it's pretty hard to know if your drive supports it.

Thanks,
Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
                                      .... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/  

Re: [PATCH 1/2] libata: Expose trim capability in sysfs

[Chris Samuel]

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

On Tue, 1 Apr 2014 08:42:36 PM Martin K. Petersen wrote:

> Now that drives with support for queued trim are starting to appear, it
> would be helpful to expose the chosen trim mode to userland. Create a
> sysfs "trim" attribute for each ata_device that displays whether trim is
> "unsupported", "unqueued" or "queued".

Thank you Martin, this is a really great idea following on from those 
discussions on the btrfs list on how to work out whether a drive supports trim 
and, if so, whether it's the original or new variant.

I'm hoping to have some time this weekend to test these patches out; should I 
test them on 3.14 or the current master for 3.15, or both? 

Much obliged!
Chris
-- 
 Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 482 bytes --]

Re: [PATCH 1/2] libata: Expose trim capability in sysfs

[Chris Samuel]

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

On Tue, 1 Apr 2014 08:42:36 PM Martin K. Petersen wrote:

> Now that drives with support for queued trim are starting to appear, it
> would be helpful to expose the chosen trim mode to userland. Create a
> sysfs "trim" attribute for each ata_device that displays whether trim is
> "unsupported", "unqueued" or "queued".

Both patches applied cleanly to 3.14, kernel now reports "unsupported" for 
everything other than the SSD in my ZaReason laptop (Crucial M4 mSATA SSD) 
which reports "unqueued" (which is expected).

Tested-By: Chris Samuel <chris@csamuel.org>

Thanks!
Chris
-- 
 Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 482 bytes --]

Re: [PATCH 1/2] libata: Expose trim capability in sysfs

[Martin K. Petersen]

>>>>> "Chris" == Chris Samuel <chris@csamuel.org> writes:

Chris> Both patches applied cleanly to 3.14, kernel now reports
Chris> "unsupported" for everything other than the SSD in my ZaReason
Chris> laptop (Crucial M4 mSATA SSD) which reports "unqueued" (which is
Chris> expected).

Chris> Tested-By: Chris Samuel <chris@csamuel.org>

Thanks for testing (both of you)!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 1/2] libata: Expose trim capability in sysfs

[Marc MERLIN]

On Tue, Apr 01, 2014 at 08:42:36PM -0400, Martin K. Petersen wrote:
> From: "Martin K. Petersen" <martin.petersen@oracle.com>
> 
> Now that drives with support for queued trim are starting to appear, it
> would be helpful to expose the chosen trim mode to userland. Create a
> sysfs "trim" attribute for each ata_device that displays whether trim is
> "unsupported", "unqueued" or "queued".
 
Ok, I'm going to sound stupid, but which sysfs property is this in?
Or more simply what file do I have to cat see this?

Thanks,
Marc

> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Chris Samuel <chris@csamuel.org>
> Cc: Marc MERLIN <marc@merlins.org>
> ---
>  drivers/ata/libata-transport.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
> index e37413228228..a9a1a9055a3a 100644
> --- a/drivers/ata/libata-transport.c
> +++ b/drivers/ata/libata-transport.c
> @@ -559,6 +559,37 @@ show_ata_dev_gscr(struct device *dev,
>  
>  static DEVICE_ATTR(gscr, S_IRUGO, show_ata_dev_gscr, NULL);
>  
> +enum {
> +	ATA_TRIM_UNSUPPORTED = 0,
> +	ATA_TRIM_UNQUEUED = 1,
> +	ATA_TRIM_QUEUED = 2,
> +};
> +
> +static const char *trim_mode[] = {
> +	[ATA_TRIM_UNSUPPORTED] = "unsupported",
> +	[ATA_TRIM_UNQUEUED] = "unqueued",
> +	[ATA_TRIM_QUEUED] = "queued",
> +};
> +
> +static ssize_t
> +show_ata_dev_trim(struct device *dev,
> +		  struct device_attribute *attr, char *buf)
> +{
> +	struct ata_device *ata_dev = transport_class_to_dev(dev);
> +	unsigned int mode;
> +
> +	if (!ata_id_has_trim(ata_dev->id))
> +		mode = ATA_TRIM_UNSUPPORTED;
> +	else if (ata_fpdma_dsm_supported(ata_dev))
> +		mode = ATA_TRIM_QUEUED;
> +	else
> +		mode = ATA_TRIM_UNQUEUED;
> +
> +	return snprintf(buf, 20, "%s\n", trim_mode[mode]);
> +}
> +
> +static DEVICE_ATTR(trim, S_IRUGO, show_ata_dev_trim, NULL);
> +
>  static DECLARE_TRANSPORT_CLASS(ata_dev_class,
>  			       "ata_device", NULL, NULL, NULL);
>  
> @@ -732,6 +763,7 @@ struct scsi_transport_template *ata_attach_transport(void)
>  	SETUP_DEV_ATTRIBUTE(ering);
>  	SETUP_DEV_ATTRIBUTE(id);
>  	SETUP_DEV_ATTRIBUTE(gscr);
> +	SETUP_DEV_ATTRIBUTE(trim);
>  	BUG_ON(count > ATA_DEV_ATTRS);
>  	i->dev_attrs[count] = NULL;
>  
> -- 
> 1.8.3.1
> 
> 

-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
                                      .... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/                         | PGP 1024R/763BE901

Re: [PATCH 1/2] libata: Expose trim capability in sysfs

[Marc MERLIN]

On Sun, Apr 06, 2014 at 03:05:51PM +1000, Chris Samuel wrote:
> On Sat, 5 Apr 2014 09:03:12 PM you wrote:
> 
> > Ok, I'm going to sound stupid, but which sysfs property is this in?
> > Or more simply what file do I have to cat see this?
> 
> I found them with a simple:
> 
> find /sys -name trim
> 
> I looked at my dmesg info from boot to correlate with devices.

Ah, that helps, thanks.

So here's another
Tested-By: Chris Samuel <marc@merlins.org>

legolas [mc]# cat /sys/devices/pci0000:00/0000:00:1f.2/ata1/link1/dev1.0/ata_device/dev1.0/trim
unqueued

:(

So much for the Samsung 840 EVO then.
Then again, it does confirm why deleting a kernel tree takes 1.5
seconds.

legolas [mc]# smartctl -i /dev/sda
smartctl 6.2 2013-07-26 r3841 [x86_64-linux-3.14.0-amd64-i915-preempt-20140216] (local build)
Copyright (C) 2002-13, Bruce Allen, Christian Franke, www.smartmontools.org

=== START OF INFORMATION SECTION ===
Device Model:     Samsung SSD 840 EVO 1TB
Serial Number:    S1D9NEAD934600N
LU WWN Device Id: 5 002538 85009a8ff
Firmware Version: EXT0BB0Q
User Capacity:    1,000,204,886,016 bytes [1.00 TB]
Sector Size:      512 bytes logical/physical
Rotation Rate:    Solid State Device
Device is:        Not in smartctl database [for details use: -P showall]
ATA Version is:   ACS-2, ATA8-ACS T13/1699-D revision 4c
SATA Version is:  SATA 3.1, 6.0 Gb/s (current: 6.0 Gb/s)
Local Time is:    Sat Apr  5 23:18:08 2014 PDT
SMART support is: Available - device has SMART capability.
SMART support is: Enabled

Thanks for the new code.

Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
                                      .... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/                         | PGP 1024R/763BE901

Re: [PATCH 1/2] libata: Expose trim capability in sysfs

[Tejun Heo]

Hello,

On Tue, Apr 01, 2014 at 08:42:36PM -0400, Martin K. Petersen wrote:
> +static ssize_t
> +show_ata_dev_trim(struct device *dev,
> +		  struct device_attribute *attr, char *buf)
> +{
> +	struct ata_device *ata_dev = transport_class_to_dev(dev);
> +	unsigned int mode;

Let's please just do

	const char *mode = "unsupported";

	if (ata_id_has_trim(ata_dev->id)) {
		if (ata_fpdma_dsm_supported(ata_dev))
			mode = "queued";
		else
			mode = "unqueued";
	}

	return sprintf(buf, "%s\n", mode);

Thanks.

-- 
tejun