2018-06-05 19:08:48

by Tejun Heo

[permalink] [raw]
Subject: [GIT PULL 1/2] libata changes for v4.18-rc1

Hello, Linus.

These two are fixes which missed v4.17. I tried to merge these into
libata/for-4.18 but couldn't make "git request-pull" not generate huge
spurious diffstat without merging v4.17 in the middle, so I'm sending
them out as two spearate pull request. Pulling into master should be
clean for both.

One is to remove an incorrect power management blacklist entry and the
other to fix a cdb buffer overrun which has been there for a very long
time.

The following changes since commit 4544e403eb25552aed7f0ee181a7a506b8800403:

ahci: Add PCI ID for Cannon Lake PCH-LP AHCI (2018-05-24 07:03:32 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git for-4.17-fixes

for you to fetch changes up to 2cfce3a86b64b53f0a70e92a6a659c720c319b45:

libata: Drop SanDisk SD7UB3Q*G1001 NOLPM quirk (2018-05-31 08:45:37 -0700)

----------------------------------------------------------------
Dan Carpenter (1):
libata: zpodd: small read overflow in eject_tray()

Hans de Goede (1):
libata: Drop SanDisk SD7UB3Q*G1001 NOLPM quirk

drivers/ata/libata-core.c | 3 ---
drivers/ata/libata-zpodd.c | 2 +-
2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 346b163..9bfd2f7 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4557,9 +4557,6 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
{ "SAMSUNG MZMPC128HBFU-000MV", "CXM14M1Q", ATA_HORKAGE_NOLPM, },
{ "SAMSUNG SSD PM830 mSATA *", "CXM13D1Q", ATA_HORKAGE_NOLPM, },

- /* Sandisk devices which are known to not handle LPM well */
- { "SanDisk SD7UB3Q*G1001", NULL, ATA_HORKAGE_NOLPM, },
-
/* devices that don't properly handle queued TRIM commands */
{ "Micron_M500IT_*", "MU01", ATA_HORKAGE_NO_NCQ_TRIM |
ATA_HORKAGE_ZERO_AFTER_TRIM, },
diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
index de4ddd0..b3ed8f9 100644
--- a/drivers/ata/libata-zpodd.c
+++ b/drivers/ata/libata-zpodd.c
@@ -35,7 +35,7 @@ struct zpodd {
static int eject_tray(struct ata_device *dev)
{
struct ata_taskfile tf;
- static const char cdb[] = { GPCMD_START_STOP_UNIT,
+ static const char cdb[ATAPI_CDB_LEN] = { GPCMD_START_STOP_UNIT,
0, 0, 0,
0x02, /* LoEj */
0, 0, 0, 0, 0, 0, 0,
--
tejun


2018-06-05 19:17:16

by Tejun Heo

[permalink] [raw]
Subject: [GIT PULL 2/2] libata changes for v4.18-rc1

Hello, Linus.

This is the second part of libata pull request.

* libata has always been limiting the maximum queue depth to 31, 1 set
aside mostly for historical reasons. This didn't use to make much
difference but Jens found out that modern hard drives can actually
perform measurably better with the extra one queue depth. Jens
updated libata core so that it can make use of full 32 queue depth.

* Damien updated command retry logic in error handling so that it
doesn't unnecessarily retry when upper layer (SCSI) is gonna handle
them.

* A couple misc changes.

Thanks.

The following changes since commit 75bc37fefc4471e718ba8e651aa74673d4e0a9eb:

Linux 4.17-rc4 (2018-05-06 16:57:38 -1000)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git for-4.18

for you to fetch changes up to 88e10092f6a623b88808f782b6637162b5b658fb:

sata_fsl: use the right type for tag bitshift (2018-05-14 08:14:21 -0700)

----------------------------------------------------------------
Andy Shevchenko (1):
ata: hpt37x: Convert to use match_string() helper

Christoph Hellwig (1):
sata_nv: don't use block layer bounce buffer

Damien Le Moal (5):
libata: Fix comment typo in ata_eh_analyze_tf()
libata: Fix ata_err_string()
libata: Make ata_dev_set_mode() less verbose
libata: Honor RQF_QUIET flag
libata: Fix command retry decision

Jens Axboe (10):
libata: introduce notion of separate hardware tags
libata: convert core and drivers to ->hw_tag usage
libata: bump ->qc_active to a 64-bit type
libata: use ata_tag_internal() consistently
libata: remove assumption that ATA_MAX_QUEUE - 1 is the max
sata_nv: set host can_queue count appropriately
libata: add extra internal command
libata: don't clamp queue depth to ATA_MAX_QUEUE - 1
ahci: enable full queue depth of 32
sata_fsl: use the right type for tag bitshift

drivers/ata/acard-ahci.c | 4 +-
drivers/ata/ahci.h | 2 +-
drivers/ata/libahci.c | 8 ++--
drivers/ata/libata-core.c | 61 ++++++++++++---------------
drivers/ata/libata-eh.c | 56 ++++++++++++++++++++-----
drivers/ata/libata-scsi.c | 19 +++++----
drivers/ata/pata_hpt37x.c | 13 +++---
drivers/ata/sata_dwc_460ex.c | 14 +++----
drivers/ata/sata_fsl.c | 10 ++---
drivers/ata/sata_mv.c | 26 ++++++------
drivers/ata/sata_nv.c | 98 +++++++++++++++++++-------------------------
drivers/ata/sata_sil24.c | 6 +--
include/linux/libata.h | 22 +++++-----
13 files changed, 177 insertions(+), 162 deletions(-)

--
tejun

2018-06-18 07:34:58

by Michael Ellerman

[permalink] [raw]
Subject: Constant ata messages on console with commit 28361c403683 ("libata: add extra internal command") (was Re: [GIT PULL 2/2] libata changes for v4.18-rc1)

Tejun Heo <[email protected]> writes:
...
> Jens Axboe (10):
> libata: introduce notion of separate hardware tags
> libata: convert core and drivers to ->hw_tag usage
> libata: bump ->qc_active to a 64-bit type
> libata: use ata_tag_internal() consistently
> libata: remove assumption that ATA_MAX_QUEUE - 1 is the max
> sata_nv: set host can_queue count appropriately
> libata: add extra internal command

Replying here because I can't find the original mail.

The above commit is causing one of my machines to constantly spew ata
messages on the console, according to bisect:

# first bad commit: [28361c403683c2b00d4f5e76045f3ccd299bf99d] libata: add extra internal command

To get it to boot I have to also apply:

88e10092f6a6 ("sata_fsl: use the right type for tag bitshift")


The system boots OK and seems fine, except that it's just printing
multiple of these per second:

ata2: Signature Update detected @ 0 msecs
ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata2.00: configured for UDMA/100
ata2: Signature Update detected @ 0 msecs
ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata2.00: configured for UDMA/100
ata2: Signature Update detected @ 0 msecs
ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata2.00: configured for UDMA/100
ata2: Signature Update detected @ 0 msecs
ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata2.00: configured for UDMA/100
ata2: Signature Update detected @ 0 msecs

And it never seems to stop.

The machine is a Freescale/NXP P5020ds, using the sata_fsl driver
presumably. Any ideas?

cheers

2018-06-18 14:34:22

by Jens Axboe

[permalink] [raw]
Subject: Re: Constant ata messages on console with commit 28361c403683 ("libata: add extra internal command") (was Re: [GIT PULL 2/2] libata changes for v4.18-rc1)

On 6/18/18 1:33 AM, Michael Ellerman wrote:
> Tejun Heo <[email protected]> writes:
> ...
>> Jens Axboe (10):
>> libata: introduce notion of separate hardware tags
>> libata: convert core and drivers to ->hw_tag usage
>> libata: bump ->qc_active to a 64-bit type
>> libata: use ata_tag_internal() consistently
>> libata: remove assumption that ATA_MAX_QUEUE - 1 is the max
>> sata_nv: set host can_queue count appropriately
>> libata: add extra internal command
>
> Replying here because I can't find the original mail.
>
> The above commit is causing one of my machines to constantly spew ata
> messages on the console, according to bisect:
>
> # first bad commit: [28361c403683c2b00d4f5e76045f3ccd299bf99d] libata: add extra internal command
>
> To get it to boot I have to also apply:
>
> 88e10092f6a6 ("sata_fsl: use the right type for tag bitshift")
>
>
> The system boots OK and seems fine, except that it's just printing
> multiple of these per second:
>
> ata2: Signature Update detected @ 0 msecs
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: configured for UDMA/100
> ata2: Signature Update detected @ 0 msecs
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: configured for UDMA/100
> ata2: Signature Update detected @ 0 msecs
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: configured for UDMA/100
> ata2: Signature Update detected @ 0 msecs
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: configured for UDMA/100
> ata2: Signature Update detected @ 0 msecs
>
> And it never seems to stop.
>
> The machine is a Freescale/NXP P5020ds, using the sata_fsl driver
> presumably. Any ideas?

Hmm that's odd. Can you include the boot log from a working boot as
well? Would be nice to see what devices are on the sata adapter.
The above just looks like a hardreset loop.

--
Jens Axboe


2018-06-19 07:31:59

by Michael Ellerman

[permalink] [raw]
Subject: Re: Constant ata messages on console with commit 28361c403683 ("libata: add extra internal command") (was Re: [GIT PULL 2/2] libata changes for v4.18-rc1)

Jens Axboe <[email protected]> writes:
> On 6/18/18 1:33 AM, Michael Ellerman wrote:
>> Tejun Heo <[email protected]> writes:
>> ...
>>> Jens Axboe (10):
>>> libata: introduce notion of separate hardware tags
>>> libata: convert core and drivers to ->hw_tag usage
>>> libata: bump ->qc_active to a 64-bit type
>>> libata: use ata_tag_internal() consistently
>>> libata: remove assumption that ATA_MAX_QUEUE - 1 is the max
>>> sata_nv: set host can_queue count appropriately
>>> libata: add extra internal command
>>
>> Replying here because I can't find the original mail.
>>
>> The above commit is causing one of my machines to constantly spew ata
>> messages on the console, according to bisect:
>>
>> # first bad commit: [28361c403683c2b00d4f5e76045f3ccd299bf99d] libata: add extra internal command
>>
>> To get it to boot I have to also apply:
>>
>> 88e10092f6a6 ("sata_fsl: use the right type for tag bitshift")
>>
>>
>> The system boots OK and seems fine, except that it's just printing
>> multiple of these per second:
>>
>> ata2: Signature Update detected @ 0 msecs
>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>> ata2.00: configured for UDMA/100
>> ata2: Signature Update detected @ 0 msecs
>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>> ata2.00: configured for UDMA/100
>> ata2: Signature Update detected @ 0 msecs
>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>> ata2.00: configured for UDMA/100
>> ata2: Signature Update detected @ 0 msecs
>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>> ata2.00: configured for UDMA/100
>> ata2: Signature Update detected @ 0 msecs
>>
>> And it never seems to stop.
>>
>> The machine is a Freescale/NXP P5020ds, using the sata_fsl driver
>> presumably. Any ideas?
>
> Hmm that's odd. Can you include the boot log from a working boot as
> well? Would be nice to see what devices are on the sata adapter.
> The above just looks like a hardreset loop.

Ah yep. I stupidly assumed it was working, because the machine booted,
but that's because the root disk is on ata1.

Booting the good kernel:

ba80c3a572f4 ("sata_nv: set host can_queue count appropriately")

I see:

root@p5020ds:~# ls -l /sys/class/ata_port/
total 0
lrwxrwxrwx 1 root root 0 Jun 19 06:49 ata1 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/ata_port/ata1
lrwxrwxrwx 1 root root 0 Jun 19 17:06 ata2 -> ../../devices/platform/ffe000000.soc/ffe221000.sata/ata2/ata_port/ata2

root@p5020ds:~# ls -l /sys/class/block/ | grep ata
lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda
lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda1 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda1
lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda2 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda2
lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda5 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda5
lrwxrwxrwx 1 root root 0 Jun 19 17:11 sr0 -> ../../devices/platform/ffe000000.soc/ffe221000.sata/ata2/host1/target1:0:0/1:0:0:0/block/sr0

So it's the DVD drive.

root@p5020ds:/sys/devices/platform/ffe000000.soc/ffe221000.sata/ata2/host1/target1:0:0/1:0:0:0/scsi_device/1:0:0:0/device# cat vendor
Optiarc
root@p5020ds:/sys/devices/platform/ffe000000.soc/ffe221000.sata/ata2/host1/target1:0:0/1:0:0:0/scsi_device/1:0:0:0/device# cat model
DVD RW AD-7260S


Full boot log from a good boot attached if that's helpful.

All of the above looks the same when I boot with the broken setup, it
just spams dmesg constantly.

One thing that is different, on the good kernel I see:

root@p5020ds:~# mount /dev/sr0 /mnt
mount: no medium found on /dev/sr0

vs bad (88e10092f6a6):

root@p5020ds:~# mount /dev/sr0 /mnt
mount: /dev/sr0 is already mounted or /mnt busy

cheers


Attachments:
good-boot.txt (30.87 kB)

2018-06-19 15:27:28

by Jens Axboe

[permalink] [raw]
Subject: Re: Constant ata messages on console with commit 28361c403683 ("libata: add extra internal command") (was Re: [GIT PULL 2/2] libata changes for v4.18-rc1)

On 6/19/18 1:29 AM, Michael Ellerman wrote:
> Jens Axboe <[email protected]> writes:
>> On 6/18/18 1:33 AM, Michael Ellerman wrote:
>>> Tejun Heo <[email protected]> writes:
>>> ...
>>>> Jens Axboe (10):
>>>> libata: introduce notion of separate hardware tags
>>>> libata: convert core and drivers to ->hw_tag usage
>>>> libata: bump ->qc_active to a 64-bit type
>>>> libata: use ata_tag_internal() consistently
>>>> libata: remove assumption that ATA_MAX_QUEUE - 1 is the max
>>>> sata_nv: set host can_queue count appropriately
>>>> libata: add extra internal command
>>>
>>> Replying here because I can't find the original mail.
>>>
>>> The above commit is causing one of my machines to constantly spew ata
>>> messages on the console, according to bisect:
>>>
>>> # first bad commit: [28361c403683c2b00d4f5e76045f3ccd299bf99d] libata: add extra internal command
>>>
>>> To get it to boot I have to also apply:
>>>
>>> 88e10092f6a6 ("sata_fsl: use the right type for tag bitshift")
>>>
>>>
>>> The system boots OK and seems fine, except that it's just printing
>>> multiple of these per second:
>>>
>>> ata2: Signature Update detected @ 0 msecs
>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>> ata2.00: configured for UDMA/100
>>> ata2: Signature Update detected @ 0 msecs
>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>> ata2.00: configured for UDMA/100
>>> ata2: Signature Update detected @ 0 msecs
>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>> ata2.00: configured for UDMA/100
>>> ata2: Signature Update detected @ 0 msecs
>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>> ata2.00: configured for UDMA/100
>>> ata2: Signature Update detected @ 0 msecs
>>>
>>> And it never seems to stop.
>>>
>>> The machine is a Freescale/NXP P5020ds, using the sata_fsl driver
>>> presumably. Any ideas?
>>
>> Hmm that's odd. Can you include the boot log from a working boot as
>> well? Would be nice to see what devices are on the sata adapter.
>> The above just looks like a hardreset loop.
>
> Ah yep. I stupidly assumed it was working, because the machine booted,
> but that's because the root disk is on ata1.
>
> Booting the good kernel:
>
> ba80c3a572f4 ("sata_nv: set host can_queue count appropriately")
>
> I see:
>
> root@p5020ds:~# ls -l /sys/class/ata_port/
> total 0
> lrwxrwxrwx 1 root root 0 Jun 19 06:49 ata1 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/ata_port/ata1
> lrwxrwxrwx 1 root root 0 Jun 19 17:06 ata2 -> ../../devices/platform/ffe000000.soc/ffe221000.sata/ata2/ata_port/ata2
>
> root@p5020ds:~# ls -l /sys/class/block/ | grep ata
> lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda
> lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda1 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda1
> lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda2 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda2
> lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda5 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda5
> lrwxrwxrwx 1 root root 0 Jun 19 17:11 sr0 -> ../../devices/platform/ffe000000.soc/ffe221000.sata/ata2/host1/target1:0:0/1:0:0:0/block/sr0
>
> So it's the DVD drive.
>
> root@p5020ds:/sys/devices/platform/ffe000000.soc/ffe221000.sata/ata2/host1/target1:0:0/1:0:0:0/scsi_device/1:0:0:0/device# cat vendor
> Optiarc
> root@p5020ds:/sys/devices/platform/ffe000000.soc/ffe221000.sata/ata2/host1/target1:0:0/1:0:0:0/scsi_device/1:0:0:0/device# cat model
> DVD RW AD-7260S
>
>
> Full boot log from a good boot attached if that's helpful.
>
> All of the above looks the same when I boot with the broken setup, it
> just spams dmesg constantly.
>
> One thing that is different, on the good kernel I see:
>
> root@p5020ds:~# mount /dev/sr0 /mnt
> mount: no medium found on /dev/sr0
>
> vs bad (88e10092f6a6):
>
> root@p5020ds:~# mount /dev/sr0 /mnt
> mount: /dev/sr0 is already mounted or /mnt busy

Can you try the below patch, on both 4.17 and on current -git? Might
help shed some light on this. The fsl driver does weird stuff with
the internal tag, I'm guessing that's related.


diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index b8d9cfc60374..9bac5ba36dac 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -28,6 +28,9 @@
#include <linux/of_irq.h>
#include <linux/of_platform.h>

+#undef DPRINTK
+#define DPRINTK printk
+
static unsigned int intr_coalescing_count;
module_param(intr_coalescing_count, int, S_IRUGO);
MODULE_PARM_DESC(intr_coalescing_count,
@@ -318,7 +321,7 @@ static void fsl_sata_set_irq_coalescing(struct ata_host *host,

DPRINTK("interrupt coalescing, count = 0x%x, ticks = %x\n",
intr_coalescing_count, intr_coalescing_ticks);
- DPRINTK("ICC register status: (hcr base: 0x%x) = 0x%x\n",
+ DPRINTK("ICC register status: (hcr base: 0x%p) = 0x%x\n",
hcr_base, ioread32(hcr_base + ICC));
}

@@ -1479,7 +1482,7 @@ static int sata_fsl_probe(struct platform_device *ofdev)
}

DPRINTK("@reset i/o = 0x%x\n", ioread32(csr_base + TRANSCFG));
- DPRINTK("sizeof(cmd_desc) = %d\n", sizeof(struct command_desc));
+ DPRINTK("sizeof(cmd_desc) = %d\n", (int) sizeof(struct command_desc));
DPRINTK("sizeof(#define cmd_desc) = %d\n", SATA_FSL_CMD_DESC_SIZE);

host_priv = kzalloc(sizeof(struct sata_fsl_host_priv), GFP_KERNEL);

--
Jens Axboe


2018-06-19 15:40:28

by Jens Axboe

[permalink] [raw]
Subject: Re: Constant ata messages on console with commit 28361c403683 ("libata: add extra internal command") (was Re: [GIT PULL 2/2] libata changes for v4.18-rc1)

On 6/19/18 1:29 AM, Michael Ellerman wrote:
> Jens Axboe <[email protected]> writes:
>> On 6/18/18 1:33 AM, Michael Ellerman wrote:
>>> Tejun Heo <[email protected]> writes:
>>> ...
>>>> Jens Axboe (10):
>>>> libata: introduce notion of separate hardware tags
>>>> libata: convert core and drivers to ->hw_tag usage
>>>> libata: bump ->qc_active to a 64-bit type
>>>> libata: use ata_tag_internal() consistently
>>>> libata: remove assumption that ATA_MAX_QUEUE - 1 is the max
>>>> sata_nv: set host can_queue count appropriately
>>>> libata: add extra internal command
>>>
>>> Replying here because I can't find the original mail.
>>>
>>> The above commit is causing one of my machines to constantly spew ata
>>> messages on the console, according to bisect:
>>>
>>> # first bad commit: [28361c403683c2b00d4f5e76045f3ccd299bf99d] libata: add extra internal command
>>>
>>> To get it to boot I have to also apply:
>>>
>>> 88e10092f6a6 ("sata_fsl: use the right type for tag bitshift")
>>>
>>>
>>> The system boots OK and seems fine, except that it's just printing
>>> multiple of these per second:
>>>
>>> ata2: Signature Update detected @ 0 msecs
>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>> ata2.00: configured for UDMA/100
>>> ata2: Signature Update detected @ 0 msecs
>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>> ata2.00: configured for UDMA/100
>>> ata2: Signature Update detected @ 0 msecs
>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>> ata2.00: configured for UDMA/100
>>> ata2: Signature Update detected @ 0 msecs
>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>> ata2.00: configured for UDMA/100
>>> ata2: Signature Update detected @ 0 msecs
>>>
>>> And it never seems to stop.
>>>
>>> The machine is a Freescale/NXP P5020ds, using the sata_fsl driver
>>> presumably. Any ideas?
>>
>> Hmm that's odd. Can you include the boot log from a working boot as
>> well? Would be nice to see what devices are on the sata adapter.
>> The above just looks like a hardreset loop.
>
> Ah yep. I stupidly assumed it was working, because the machine booted,
> but that's because the root disk is on ata1.
>
> Booting the good kernel:
>
> ba80c3a572f4 ("sata_nv: set host can_queue count appropriately")
>
> I see:
>
> root@p5020ds:~# ls -l /sys/class/ata_port/
> total 0
> lrwxrwxrwx 1 root root 0 Jun 19 06:49 ata1 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/ata_port/ata1
> lrwxrwxrwx 1 root root 0 Jun 19 17:06 ata2 -> ../../devices/platform/ffe000000.soc/ffe221000.sata/ata2/ata_port/ata2
>
> root@p5020ds:~# ls -l /sys/class/block/ | grep ata
> lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda
> lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda1 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda1
> lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda2 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda2
> lrwxrwxrwx 1 root root 0 Jun 19 17:11 sda5 -> ../../devices/platform/ffe000000.soc/ffe220000.sata/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda5
> lrwxrwxrwx 1 root root 0 Jun 19 17:11 sr0 -> ../../devices/platform/ffe000000.soc/ffe221000.sata/ata2/host1/target1:0:0/1:0:0:0/block/sr0
>
> So it's the DVD drive.
>
> root@p5020ds:/sys/devices/platform/ffe000000.soc/ffe221000.sata/ata2/host1/target1:0:0/1:0:0:0/scsi_device/1:0:0:0/device# cat vendor
> Optiarc
> root@p5020ds:/sys/devices/platform/ffe000000.soc/ffe221000.sata/ata2/host1/target1:0:0/1:0:0:0/scsi_device/1:0:0:0/device# cat model
> DVD RW AD-7260S
>
>
> Full boot log from a good boot attached if that's helpful.
>
> All of the above looks the same when I boot with the broken setup, it
> just spams dmesg constantly.
>
> One thing that is different, on the good kernel I see:
>
> root@p5020ds:~# mount /dev/sr0 /mnt
> mount: no medium found on /dev/sr0
>
> vs bad (88e10092f6a6):
>
> root@p5020ds:~# mount /dev/sr0 /mnt
> mount: /dev/sr0 is already mounted or /mnt busy
>
> cheers

Actually, just try this one on top of current -git.

diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index b8d9cfc60374..4007a9ae650d 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -395,12 +395,6 @@ static inline unsigned int sata_fsl_tag(unsigned int tag,
{
/* We let libATA core do actual (queue) tag allocation */

- /* all non NCQ/queued commands should have tag#0 */
- if (ata_tag_internal(tag)) {
- DPRINTK("mapping internal cmds to tag#0\n");
- return 0;
- }
-
if (unlikely(tag >= SATA_FSL_QUEUE_DEPTH)) {
DPRINTK("tag %d invalid : out of range\n", tag);
return 0;
@@ -1229,7 +1223,7 @@ static void sata_fsl_host_intr(struct ata_port *ap)

/* Workaround for data length mismatch errata */
if (unlikely(hstatus & INT_ON_DATA_LENGTH_MISMATCH)) {
- for (tag = 0; tag < ATA_MAX_QUEUE; tag++) {
+ for (tag = 0; tag <= ATA_MAX_QUEUE; tag++) {
qc = ata_qc_from_tag(ap, tag);
if (qc && ata_is_atapi(qc->tf.protocol)) {
u32 hcontrol;

--
Jens Axboe


2018-06-19 23:39:00

by Michael Ellerman

[permalink] [raw]
Subject: Re: Constant ata messages on console with commit 28361c403683 ("libata: add extra internal command") (was Re: [GIT PULL 2/2] libata changes for v4.18-rc1)

Jens Axboe <[email protected]> writes:
> On 6/19/18 1:29 AM, Michael Ellerman wrote:
>> Jens Axboe <[email protected]> writes:
>>> On 6/18/18 1:33 AM, Michael Ellerman wrote:
>>>> Tejun Heo <[email protected]> writes:
>>>> ...
>>>>> Jens Axboe (10):
>>>>> libata: introduce notion of separate hardware tags
>>>>> libata: convert core and drivers to ->hw_tag usage
>>>>> libata: bump ->qc_active to a 64-bit type
>>>>> libata: use ata_tag_internal() consistently
>>>>> libata: remove assumption that ATA_MAX_QUEUE - 1 is the max
>>>>> sata_nv: set host can_queue count appropriately
>>>>> libata: add extra internal command
>>>>
>>>> Replying here because I can't find the original mail.
>>>>
>>>> The above commit is causing one of my machines to constantly spew ata
>>>> messages on the console, according to bisect:
>>>>
>>>> # first bad commit: [28361c403683c2b00d4f5e76045f3ccd299bf99d] libata: add extra internal command
>>>>
>>>> To get it to boot I have to also apply:
>>>>
>>>> 88e10092f6a6 ("sata_fsl: use the right type for tag bitshift")
>>>>
>>>>
>>>> The system boots OK and seems fine, except that it's just printing
>>>> multiple of these per second:
>>>>
>>>> ata2: Signature Update detected @ 0 msecs
>>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>>> ata2.00: configured for UDMA/100
>>>> ata2: Signature Update detected @ 0 msecs
>>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>>> ata2.00: configured for UDMA/100
>>>> ata2: Signature Update detected @ 0 msecs
>>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>>> ata2.00: configured for UDMA/100
>>>> ata2: Signature Update detected @ 0 msecs
>>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>>> ata2.00: configured for UDMA/100
>>>> ata2: Signature Update detected @ 0 msecs
...
>
> Actually, just try this one on top of current -git.

Yep that fixes it.

No more message spam, and when I try to mount sr0 it says "no medium".

I'll have to go into the office to actually put a disc in the drive
to check it's really working, but it seems likely.

Thanks for debugging it, here's a tested-by if you like:

Tested-by: Michael Ellerman <[email protected]>

cheers

> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
> index b8d9cfc60374..4007a9ae650d 100644
> --- a/drivers/ata/sata_fsl.c
> +++ b/drivers/ata/sata_fsl.c
> @@ -395,12 +395,6 @@ static inline unsigned int sata_fsl_tag(unsigned int tag,
> {
> /* We let libATA core do actual (queue) tag allocation */
>
> - /* all non NCQ/queued commands should have tag#0 */
> - if (ata_tag_internal(tag)) {
> - DPRINTK("mapping internal cmds to tag#0\n");
> - return 0;
> - }
> -
> if (unlikely(tag >= SATA_FSL_QUEUE_DEPTH)) {
> DPRINTK("tag %d invalid : out of range\n", tag);
> return 0;
> @@ -1229,7 +1223,7 @@ static void sata_fsl_host_intr(struct ata_port *ap)
>
> /* Workaround for data length mismatch errata */
> if (unlikely(hstatus & INT_ON_DATA_LENGTH_MISMATCH)) {
> - for (tag = 0; tag < ATA_MAX_QUEUE; tag++) {
> + for (tag = 0; tag <= ATA_MAX_QUEUE; tag++) {
> qc = ata_qc_from_tag(ap, tag);
> if (qc && ata_is_atapi(qc->tf.protocol)) {
> u32 hcontrol;
>
> --
> Jens Axboe

2018-06-19 23:41:10

by Jens Axboe

[permalink] [raw]
Subject: Re: Constant ata messages on console with commit 28361c403683 ("libata: add extra internal command") (was Re: [GIT PULL 2/2] libata changes for v4.18-rc1)

On 6/19/18 5:35 PM, Michael Ellerman wrote:
> Jens Axboe <[email protected]> writes:
>> On 6/19/18 1:29 AM, Michael Ellerman wrote:
>>> Jens Axboe <[email protected]> writes:
>>>> On 6/18/18 1:33 AM, Michael Ellerman wrote:
>>>>> Tejun Heo <[email protected]> writes:
>>>>> ...
>>>>>> Jens Axboe (10):
>>>>>> libata: introduce notion of separate hardware tags
>>>>>> libata: convert core and drivers to ->hw_tag usage
>>>>>> libata: bump ->qc_active to a 64-bit type
>>>>>> libata: use ata_tag_internal() consistently
>>>>>> libata: remove assumption that ATA_MAX_QUEUE - 1 is the max
>>>>>> sata_nv: set host can_queue count appropriately
>>>>>> libata: add extra internal command
>>>>>
>>>>> Replying here because I can't find the original mail.
>>>>>
>>>>> The above commit is causing one of my machines to constantly spew ata
>>>>> messages on the console, according to bisect:
>>>>>
>>>>> # first bad commit: [28361c403683c2b00d4f5e76045f3ccd299bf99d] libata: add extra internal command
>>>>>
>>>>> To get it to boot I have to also apply:
>>>>>
>>>>> 88e10092f6a6 ("sata_fsl: use the right type for tag bitshift")
>>>>>
>>>>>
>>>>> The system boots OK and seems fine, except that it's just printing
>>>>> multiple of these per second:
>>>>>
>>>>> ata2: Signature Update detected @ 0 msecs
>>>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>>>> ata2.00: configured for UDMA/100
>>>>> ata2: Signature Update detected @ 0 msecs
>>>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>>>> ata2.00: configured for UDMA/100
>>>>> ata2: Signature Update detected @ 0 msecs
>>>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>>>> ata2.00: configured for UDMA/100
>>>>> ata2: Signature Update detected @ 0 msecs
>>>>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>>>>> ata2.00: configured for UDMA/100
>>>>> ata2: Signature Update detected @ 0 msecs
> ...
>>
>> Actually, just try this one on top of current -git.
>
> Yep that fixes it.
>
> No more message spam, and when I try to mount sr0 it says "no medium".
>
> I'll have to go into the office to actually put a disc in the drive
> to check it's really working, but it seems likely.
>
> Thanks for debugging it, here's a tested-by if you like:
>
> Tested-by: Michael Ellerman <[email protected]>

Thanks, but I packaged it a little differently, see the other
series I CC'ed you on. It'll work the same, though. It's in Tejun's
tree now, so should end up with Linus some time this week I think.

Thanks for reporting and testing the fix!

--
Jens Axboe