patch "libata: Add support for SCT Write Same" breaks system

patch "libata: Add support for SCT Write Same" breaks system

[Mike Krinkin]

Hi,

i tried recent linux-next on my laptop, and after boot system is
almost unusable because most of apps just crash with segfaults and
in dmesg output there are a lot of errors like this:

[   17.641510] do_general_protection: 51 callbacks suppressed
[   17.641514] traps: dpkg[2831] general protection ip:40be90 sp:7ffd6daad9b8 error:0 in dpkg[400000+43000]
[   17.648882] apport[2832]: segfault at 800000000 ip 0000000000485896 sp 00007ffece3b6c40 error 6 in python3.5[400000+3a7000]
[   17.648965] Process 2832(apport) has RLIMIT_CORE set to 1
[   17.648967] Aborting core
[   17.653463] gpu-manager[2818]: segfault at 0 ip 00007f77c4b8eb5a sp 00007ffc7a01b348 error 4 in libc-2.23.so[7f77c4af0000+1c0000]
[   17.660760] apport[2833]: segfault at 800000000 ip 0000000000485896 sp 00007ffddfe2a650 error 6 in python3.5[400000+3a7000]
[   17.660841] Process 2833(apport) has RLIMIT_CORE set to 1
[   17.660843] Aborting core
[   18.192604] apport[2847]: segfault at 800000000 ip 0000000000485896 sp 00007ffd2d4a82c0 error 6 in python3.5[400000+3a7000]
[   18.192630] Process 2847(apport) has RLIMIT_CORE set to 1
[   18.192633] Aborting core
[   19.089476] traps: dpkg[2861] general protection ip:40be90 sp:7ffcabc70fa8 error:0 in dpkg[400000+43000]
[   19.096747] apport[2862]: segfault at 800000000 ip 0000000000485896 sp 00007ffeda308490 error 6 in python3.5[400000+3a7000]
[   19.096827] Process 2862(apport) has RLIMIT_CORE set to 1
[   19.096829] Aborting core
[   19.101197] gpu-manager[2849]: segfault at 0 ip 00007fdccfef8b5a sp 00007ffeb4758488 error 4 in libc-2.23.so[7fdccfe5a000+1c0000]
[   19.106683] apport[2863]: segfault at 800000000 ip 0000000000485896 sp 00007ffcf8955d10 error 6 in python3.5[400000+3a7000]
[   19.106708] Process 2863(apport) has RLIMIT_CORE set to 1
[   19.106709] Aborting core
[   19.616176] apport[2898]: segfault at 800000000 ip 0000000000485896 sp 00007ffd12c29180 error 6 in python3.5[400000+3a7000]
[   19.616216] Process 2898(apport) has RLIMIT_CORE set to 1
[   19.616217] Aborting core
[   23.182904] lsb_release[2911]: segfault at 800000000 ip 0000000000485896 sp 00007ffca2f6bec0 error 6 in python3.5[400000+3a7000]
[   23.185672] apport[2913]: segfault at 800000000 ip 0000000000485896 sp 00007ffcf199e160 error 6 in python3.5[400000+3a7000]
[   23.185698] Process 2913(apport) has RLIMIT_CORE set to 1
[   23.185698] Aborting core
[   23.212363] apport[2921]: segfault at 800000000 ip 0000000000485896 sp 00007ffe934094c0 error 6 in python3.5[400000+3a7000]
[   23.212389] Process 2921(apport) has RLIMIT_CORE set to 1
[   23.212390] Aborting core
[   23.223112] apport[2926]: segfault at 800000000 ip 0000000000485896 sp 00007ffcc68e1240 error 6 in python3.5[400000+3a7000]
[   23.223137] Process 2926(apport) has RLIMIT_CORE set to 1
[   23.223137] Aborting core

git bisect points at commit 7b20309428598df00ffe ("libata: Add support for SCT
Write Same". I temporary fixed problem with the following change:

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 08d9c8e..0dcd59e 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1159,6 +1159,7 @@ static void ata_scsi_sdev_config(struct scsi_device *sdev)
 {
        sdev->use_10_for_rw = 1;
        sdev->use_10_for_ms = 1;
+       sdev->no_write_same = 1;
 
        /* Schedule policy is determined by ->qc_defer() callback and
         * it needs to see every deferred qc.  Set dev_blocked to 1 to

Don't know whether it will help or not, but here is my hdparm output:

sudo hdparm -I /dev/sda

/dev/sda:

ATA device, with non-removable media
	Model Number:       SAMSUNG MZNLN512HCJH-000L1              
	Serial Number:      S201NX0H404301      
	Firmware Revision:  EMT22L0Q
	Transport:          Serial, ATA8-AST, SATA 1.0a, SATA II Extensions, SATA Rev 2.5, SATA Rev 2.6, SATA Rev 3.0
Standards:
	Used: unknown (minor revision code 0x0039) 
	Supported: 9 8 7 6 5 
	Likely used: 9
Configuration:
	Logical		max	current
	cylinders	16383	16383
	heads		16	16
	sectors/track	63	63
	--
	CHS current addressable sectors:   16514064
	LBA    user addressable sectors:  268435455
	LBA48  user addressable sectors: 1000215216
	Logical  Sector size:                   512 bytes
	Physical Sector size:                   512 bytes
	Logical Sector-0 offset:                  0 bytes
	device size with M = 1024*1024:      488386 MBytes
	device size with M = 1000*1000:      512110 MBytes (512 GB)
	cache/buffer size  = unknown
	Nominal Media Rotation Rate: Solid State Device
Capabilities:
	LBA, IORDY(can be disabled)
	Queue depth: 32
	Standby timer values: spec'd by Standard, no device specific minimum
	R/W multiple sector transfer: Max = 16	Current = 16
	DMA: mdma0 mdma1 mdma2 udma0 udma1 udma2 udma3 udma4 udma5 *udma6 
	     Cycle time: min=120ns recommended=120ns
	PIO: pio0 pio1 pio2 pio3 pio4 
	     Cycle time: no flow control=120ns  IORDY flow control=120ns
Commands/features:
	Enabled	Supported:
	   *	SMART feature set
	    	Security Mode feature set
	   *	Power Management feature set
	   *	Write cache
	   *	Look-ahead
	   *	Host Protected Area feature set
	   *	WRITE_BUFFER command
	   *	READ_BUFFER command
	   *	NOP cmd
	   *	DOWNLOAD_MICROCODE
	    	SET_MAX security extension
	   *	48-bit Address feature set
	   *	Device Configuration Overlay feature set
	   *	Mandatory FLUSH_CACHE
	   *	FLUSH_CACHE_EXT
	   *	SMART error logging
	   *	SMART self-test
	   *	General Purpose Logging feature set
	   *	WRITE_{DMA|MULTIPLE}_FUA_EXT
	   *	64-bit World wide name
	    	Write-Read-Verify feature set
	   *	WRITE_UNCORRECTABLE_EXT command
	   *	{READ,WRITE}_DMA_EXT_GPL commands
	   *	Segmented DOWNLOAD_MICROCODE
	   *	Gen1 signaling speed (1.5Gb/s)
	   *	Gen2 signaling speed (3.0Gb/s)
	   *	Gen3 signaling speed (6.0Gb/s)
	   *	Native Command Queueing (NCQ)
	   *	Phy event counters
	   *	READ_LOG_DMA_EXT equivalent to READ_LOG_EXT
	   *	DMA Setup Auto-Activate optimization
	    	Device-initiated interface power management
	   *	Asynchronous notification (eg. media change)
	   *	Software settings preservation
	    	Device Sleep (DEVSLP)
	   *	SMART Command Transport (SCT) feature set
	   *	SCT Write Same (AC2)
	   *	SCT Error Recovery Control (AC3)
	   *	SCT Features Control (AC4)
	   *	SCT Data Tables (AC5)
	   *	DOWNLOAD MICROCODE DMA command
	   *	SET MAX SETPASSWORD/UNLOCK DMA commands
	   *	WRITE BUFFER DMA command
	   *	READ BUFFER DMA command
	   *	Data Set Management TRIM supported (limit 8 blocks)
Security: 
	Master password revision code = 65534
		supported
	not	enabled
	not	locked
		frozen
	not	expired: security count
		supported: enhanced erase
	2min for SECURITY ERASE UNIT. 8min for ENHANCED SECURITY ERASE UNIT. 
Logical Unit WWN Device Identifier: 5002538d00000000
	NAA		: 5
	IEEE OUI	: 002538
	Unique ID	: d00000000
Device Sleep:
	DEVSLP Exit Timeout (DETO): 50 ms (drive)
	Minimum DEVSLP Assertion Time (MDAT): 30 ms (drive)
Checksum: correct

Re: patch "libata: Add support for SCT Write Same" breaks system

[Tejun Heo]

Hello,

On Thu, Sep 08, 2016 at 10:27:37PM +0300, Mike Krinkin wrote:
> Hi,
> 
> i tried recent linux-next on my laptop, and after boot system is
> almost unusable because most of apps just crash with segfaults and
> in dmesg output there are a lot of errors like this:
...
> git bisect points at commit 7b20309428598df00ffe ("libata: Add support for SCT
> Write Same". I temporary fixed problem with the following change:

Shaun, any ideas?  If this isn't an easy fix, I'm gonna disable
write_same for all devices for now.

Thanks.

-- 
tejun

Re: patch "libata: Add support for SCT Write Same" breaks system

[Tejun Heo]

Hello, Shaun.

On Fri, Sep 09, 2016 at 10:26:44AM -0500, Shaun Tancheff wrote:
> I'm looking into it now. Let me see if I can reproduce this on any of my
> hardware.
> 
> If not there are a couple of options ... one is to only enable for ZBC
> devices
> where this explicitly required by the spec.
> 
> Or disable for devices that report support trim?

I'd much prefer enabling this only on ZBC devices.  There isn't any
real benefits to !ZBC devices, right?  Using non-essential features on
ATA never goes well.

Thanks.

-- 
tejun

Re: patch "libata: Add support for SCT Write Same" breaks system

[Shaun Tancheff]

On Fri, Sep 9, 2016 at 10:36 AM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Shaun.
>
> On Fri, Sep 09, 2016 at 10:26:44AM -0500, Shaun Tancheff wrote:
>> I'm looking into it now. Let me see if I can reproduce this on any of my
>> hardware.
>>
>> If not there are a couple of options ... one is to only enable for ZBC
>> devices
>> where this explicitly required by the spec.
>>
>> Or disable for devices that report support trim?
>
> I'd much prefer enabling this only on ZBC devices.  There isn't any
> real benefits to !ZBC devices, right?  Using non-essential features on
> ATA never goes well.

Sure I'm fine with that.

I'll move the WRITE SAME support to be conditional on ZBC.
Sending a patch as soon as it's tested.

> Thanks.
>
> --
> tejun

Thanks
--
Shaun

[PATCH] Some drives failing on SCT Write Same

[Shaun Tancheff]

Restrict support SCT Write Same to devices which also support ZAC where 
support is required.

Reported-by: Mike Krinkin <krinkin.m.u@gmail.com>
Signed-off-by: Shaun Tancheff <shaun.tancheff@seagate.com>
---
 drivers/ata/libata-scsi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2f5487f..9cceb4a 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3562,9 +3562,9 @@ static unsigned int ata_scsiop_maint_in(struct ata_scsi_args *args, u8 *rbuf)
 		supported = 3;
 		break;
 	case WRITE_SAME_16:
-		if (ata_id_sct_write_same(dev->id))
-			supported = 3;
-		break;
+		if (!ata_id_sct_write_same(dev->id))
+			break;
+		/* fallthrough: if SCT ... only enable for ZBC */
 	case ZBC_IN:
 	case ZBC_OUT:
 		if (ata_id_zoned_cap(dev->id) ||
-- 
2.9.3

Re: patch "libata: Add support for SCT Write Same" breaks system

[Shaun Tancheff]

On Fri, Sep 9, 2016 at 10:36 AM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Shaun.
>
> On Fri, Sep 09, 2016 at 10:26:44AM -0500, Shaun Tancheff wrote:
>> I'm looking into it now. Let me see if I can reproduce this on any of my
>> hardware.
>>
>> If not there are a couple of options ... one is to only enable for ZBC
>> devices
>> where this explicitly required by the spec.
>>
>> Or disable for devices that report support trim?
>
> I'd much prefer enabling this only on ZBC devices.  There isn't any
> real benefits to !ZBC devices, right?  Using non-essential features on
> ATA never goes well.

I've posted a patch for !ZBC.

Mike, can you confirm if this works for you?

>
> Thanks.
>
> --
> tejun

Thanks!

(Apologies for the html reply earlier)
-- 
Shaun Tancheff

Re: [PATCH] Some drives failing on SCT Write Same

[Mike Krinkin]

On Fri, Sep 09, 2016 at 11:48:51AM -0500, Shaun Tancheff wrote:
> Mike,
> 
> Can you confirm if this fix works for you?
> 
> I tested the patch is tested on top of next-20160909.

Works for me too, thank you.

> 
> Thanks!
> --Shaun
> 
> On Fri, Sep 9, 2016 at 11:44 AM, Shaun Tancheff <shaun@tancheff.com> wrote:
> 
> > Restrict support SCT Write Same to devices which also support ZAC where
> > support is required.
> >
> > Reported-by: Mike Krinkin <krinkin.m.u@gmail.com>
> > Signed-off-by: Shaun Tancheff <shaun.tancheff@seagate.com>
> > ---
> >  drivers/ata/libata-scsi.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> > index 2f5487f..9cceb4a 100644
> > --- a/drivers/ata/libata-scsi.c
> > +++ b/drivers/ata/libata-scsi.c
> > @@ -3562,9 +3562,9 @@ static unsigned int ata_scsiop_maint_in(struct
> > ata_scsi_args *args, u8 *rbuf)
> >                 supported = 3;
> >                 break;
> >         case WRITE_SAME_16:
> > -               if (ata_id_sct_write_same(dev->id))
> > -                       supported = 3;
> > -               break;
> > +               if (!ata_id_sct_write_same(dev->id))
> > +                       break;
> > +               /* fallthrough: if SCT ... only enable for ZBC */
> >         case ZBC_IN:
> >         case ZBC_OUT:
> >                 if (ata_id_zoned_cap(dev->id) ||
> > --
> > 2.9.3
> >
> >
> 
> 
> -- 
> Shaun Tancheff

Re: [PATCH] Some drives failing on SCT Write Same

[Tejun Heo]

On Fri, Sep 09, 2016 at 11:44:19AM -0500, Shaun Tancheff wrote:
> Restrict support SCT Write Same to devices which also support ZAC where 
> support is required.
> 
> Reported-by: Mike Krinkin <krinkin.m.u@gmail.com>
> Signed-off-by: Shaun Tancheff <shaun.tancheff@seagate.com>

Applied to libata/for-4.9.

Thanks.

-- 
tejun