bad sectors, suspicious behaviour

42 messages, 8 authors, 2008-08-29 · open the first message on its own page

bad sectors, suspicious behaviour

From: Artem Bokhan <hidden>
Date: 2008-08-08 10:02:57

(sorry for dup, incorrect system clock was set)

Hello!

I'm not sure  I chose correct maillist, so point me somewhere in case of mistake :)

I'm trying to emulate OS behaviour when something goes wrong with sata hard drive, for example, unrecoverable "bad blocks". By some reason I do not want to use any sw/hw raid.
I took new hard drive, because it should contain (and it contains) unreadable (not reallocated yet) sectros, and did


'dd if=/dev/sda of=/dev/null bs=1M'.

first run dd log (errors1.txt) looks OK, drive recovers, as I suppose, approximately at time

cat
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout
30

but when running dd second time, log looks strange (errors2.txt)


running
echo 1 > /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/delete
echo "scsi add-single-device 4 0 0 0" > /proc/scsi/scsi

and then running dd solve situation until next dd

It is vanilla 2.6.26.2 kernel and sata_mv (MV88SX6081 8-port SATA II PCI-X)

Re: bad sectors, suspicious behaviour

From: Mark Lord <hidden>
Date: 2008-08-08 13:34:37

Artem Bokhan wrote:
..
I'm trying to emulate OS behaviour when something goes wrong with sata 
hard drive, for example, unrecoverable "bad blocks". By some reason I do 
not want to use any sw/hw raid.
..

Note that you can create/remove *real* bad sectors on most drives
by using "hdparm --make-bad-sector" and "hdparm --repair-sector".
I took new hard drive, because it should contain (and it contains) 
unreadable (not reallocated yet) sectros, and did

'dd if=/dev/sda of=/dev/null bs=1M'.

first run dd log (errors1.txt) looks OK, drive recovers, as I suppose, 
approximately at time

cat
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout 

30

but when running dd second time, log looks strange (errors2.txt)
..
[75702.039300] ata5.00: NCQ disabled due to excessive errors
[75702.039382]          res 41/00:08:00:a8:36/00:00:01:00:00/40 Emask 0x1 (device error)
[75702.039452]          res 41/00:00:01:00:00/00:00:01:00:00/40 Emask 0x1 (device error)
[75702.039522] ata5: hard resetting link
[75702.936061] ata5: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[75702.996080] ata5.00: max_sectors limited to 256 for NCQ
[75703.296058] ata5.00: max_sectors limited to 256 for NCQ
[75703.296061] ata5.00: configured for UDMA/133
[75703.296069] ata5: EH complete
[75703.296098] ------------[ cut here ]------------
[75703.296100] WARNING: at drivers/ata/libata-core.c:4732 ata_qc_issue+0x1ca/0x230 [libata]()
[75703.296102] Modules linked in: dm_mod autofs4 ipv6 iptable_filter ip_tables x_tables lp loop af_packet sr_mod cdrom container e1000e button parport_pc parport iTCO_wdt iTCO_vendor_support pcspkr shpchp pci_hotplug psmouse evdev ext3 jbd mbcache sg sd_mod pata_it8213 ata_piix pata_acpi floppy ata_generic sata_mv libata scsi_mod dock ehci_hcd uhci_hcd usbcore thermal processor fan thermal_sys fuse
[75703.296131] Pid: 2235, comm: scsi_eh_4 Tainted: G        W 2.6.26.2.030320 #1
[75703.296133]
[75703.296133] Call Trace:
[75703.296140]  [ext3:warn_on_slowpath+0x64/0x320] warn_on_slowpath+0x64/0xa0
[75703.296145]  [sched_clock_cpu+0xc1/0x150] sched_clock_cpu+0xc1/0x150
[75703.296148]  [usbcore:up+0x16/0xc0] up+0x16/0x50
[75703.296150]  [release_console_sem+0x19d/0x1e0] release_console_sem+0x19d/0x1e0
[75703.296156]  [sata_mv:mv_qc_issue+0x3c0/0x500] :sata_mv:mv_qc_issue+0x3c0/0x500
[75703.296159]  [nommu_map_sg+0x6b/0xe0] nommu_map_sg+0x6b/0xe0
[75703.296170]  [libata:ata_build_rw_tf+0xc1/0x290] :libata:ata_build_rw_tf+0xc1/0x290
[75703.296181]  [libata:ata_qc_issue+0x1ca/0x230] :libata:ata_qc_issue+0x1ca/0x230
[75703.296193]  [scsi_mod:scsi_done+0x0/0x20] :scsi_mod:scsi_done+0x0/0x20
[75703.296203]  [libata:ata_scsi_translate+0xf8/0x190] :libata:ata_scsi_translate+0xf8/0x190
[75703.296213]  [scsi_mod:scsi_done+0x0/0x20] :scsi_mod:scsi_done+0x0/0x20
[75703.296224]  [sata_mv:ata_scsi_queuecmd+0xbb/0x240] :libata:ata_scsi_queuecmd+0xbb/0x240
[75703.296235]  [scsi_mod:scsi_dispatch_cmd+0x11e/0x2e0] :scsi_mod:scsi_dispatch_cmd+0x11e/0x2e0
[75703.296246]  [scsi_mod:scsi_request_fn+0x213/0x3c0] :scsi_mod:scsi_request_fn+0x213/0x3c0
[75703.296251]  [scsi_mod:__blk_run_queue+0x43/0xd0] __blk_run_queue+0x43/0xd0
[75703.296253]  [scsi_mod:blk_run_queue+0x28/0x50] blk_run_queue+0x28/0x50
[75703.296264]  [scsi_mod:scsi_error_handler+0x0/0x570] :scsi_mod:scsi_error_handler+0x0/0x570
[75703.296274]  [scsi_mod:scsi_run_host_queues+0x19/0x40] :scsi_mod:scsi_run_host_queues+0x19/0x40
[75703.296284]  [scsi_mod:scsi_error_handler+0x1d9/0x570] :scsi_mod:scsi_error_handler+0x1d9/0x570
[75703.296296]  [scsi_mod:scsi_error_handler+0x0/0x570] :scsi_mod:scsi_error_handler+0x0/0x570
[75703.296299]  [kthread+0x4b/0x80] kthread+0x4b/0x80
[75703.296303]  [child_rip+0xa/0x12] child_rip+0xa/0x12
[75703.296307]  [kthread+0x0/0x80] kthread+0x0/0x80
[75703.296309]  [child_rip+0x0/0x12] child_rip+0x0/0x12
[75703.296311]
[75703.296312] ---[ end trace 98cca564749c28e3 ]---
Mmmm.. something bad there, and also here:
[76898.778051] ------------[ cut here ]------------
[76898.778051] WARNING: at drivers/ata/libata-core.c:4732 ata_qc_issue+0x1ca/0x230 [libata]()
[76898.778051] Modules linked in: dm_mod autofs4 ipv6 iptable_filter ip_tables x_tables lp loop af_packet sr_mod cdrom container e1000e button parport_pc parport iTCO_wdt iTCO_vendor_support pcspkr shpchp pci_hotplug psmouse evdev ext3 jbd mbcache sg sd_mod pata_it8213 ata_piix pata_acpi floppy ata_generic sata_mv libata scsi_mod dock ehci_hcd uhci_hcd usbcore thermal processor fan thermal_sys fuse
[76898.778051] Pid: 4150, comm: dd Tainted: G        W 2.6.26.2.030320 #1
[76898.778051]
[76898.778051] Call Trace:
[76898.778051]  [ext3:warn_on_slowpath+0x64/0x320] warn_on_slowpath+0x64/0xa0
[76898.778051]  [sg:sg_init_table+0x20/0x663e0] sg_init_table+0x20/0x50
[76898.778051]  [scsi_mod:blk_rq_map_sg+0x7c/0x25e0] blk_rq_map_sg+0x7c/0x2b0
[76898.778051]  [scsi_mod:scsi_sg_alloc+0x0/0x50] :scsi_mod:scsi_sg_alloc+0x0/0x50
[76898.778051]  [scsi_mod:scsi_init_sgtable+0x54/0xb0] :scsi_mod:scsi_init_sgtable+0x54/0xb0
[76898.778051]  [libata:ata_build_rw_tf+0xc1/0x290] :libata:ata_build_rw_tf+0xc1/0x290
[76898.778051]  [libata:ata_qc_issue+0x1ca/0x230] :libata:ata_qc_issue+0x1ca/0x230
[76898.778051]  [scsi_mod:scsi_done+0x0/0x20] :scsi_mod:scsi_done+0x0/0x20
[76898.778051]  [libata:ata_scsi_translate+0xf8/0x190] :libata:ata_scsi_translate+0xf8/0x190
[76898.778051]  [scsi_mod:scsi_done+0x0/0x20] :scsi_mod:scsi_done+0x0/0x20
[76898.778051]  [sata_mv:ata_scsi_queuecmd+0xbb/0x240] :libata:ata_scsi_queuecmd+0xbb/0x240
[76898.778051]  [scsi_mod:scsi_dispatch_cmd+0x11e/0x2e0] :scsi_mod:scsi_dispatch_cmd+0x11e/0x2e0
[76898.778051]  [scsi_mod:scsi_request_fn+0x213/0x3c0] :scsi_mod:scsi_request_fn+0x213/0x3c0
[76898.778051]  [sync_page_killable+0x0/0x40] sync_page_killable+0x0/0x40
[76898.778051]  [floppy:generic_unplug_device+0x24/0xa0] generic_unplug_device+0x24/0x30
[76898.778051]  [blk_unplug+0x35/0x70] blk_unplug+0x35/0x70
[76898.778051]  [sync_page+0x2e/0x50] sync_page+0x2e/0x50
[76898.778051]  [sync_page_killable+0x9/0x40] sync_page_killable+0x9/0x40
[76898.778051]  [__wait_on_bit_lock+0x4a/0x80] __wait_on_bit_lock+0x4a/0x80
[76898.778051]  [__lock_page_killable+0x5f/0x70] __lock_page_killable+0x5f/0x70
[76898.778051]  [<ffffffff8024ac30>] wake_bit_function+0x0/0x30
[76898.778051]  [fuse:generic_file_aio_read+0x36a/0x710] generic_file_aio_read+0x36a/0x570
[76898.778051]  [ext3:do_sync_read+0xd9/0xbc0] do_sync_read+0xd9/0x120
[76898.778051]  [<ffffffff8024ac00>] autoremove_wake_function+0x0/0x30
[76898.778051]  [handle_mm_fault+0x575/0x810] handle_mm_fault+0x575/0x810
[76898.778051]  [block_ioctl+0x1b/0x30] block_ioctl+0x1b/0x30
[76898.778051]  [vfs_read+0xc5/0x180] vfs_read+0xc5/0x180
[76898.778051]  [sys_read+0x53/0x90] sys_read+0x53/0x90
[76898.778051]  [system_call_after_swapgs+0x7b/0x80] system_call_after_swapgs+0x7b/0x80
[76898.778051]
[76898.778051] ---[ end trace 98cca564749c28e3 ]---

Re: bad sectors, suspicious behaviour

From: Mark Lord <hidden>
Date: 2008-08-08 13:50:31

Mark Lord wrote:
Artem Bokhan wrote:
..
quoted
I'm trying to emulate OS behaviour when something goes wrong with sata 
hard drive, for example, unrecoverable "bad blocks". By some reason I 
do not want to use any sw/hw raid.
..

Note that you can create/remove *real* bad sectors on most drives
by using "hdparm --make-bad-sector" and "hdparm --repair-sector".
quoted
I took new hard drive, because it should contain (and it contains) 
unreadable (not reallocated yet) sectros, and did

'dd if=/dev/sda of=/dev/null bs=1M'.

first run dd log (errors1.txt) looks OK, drive recovers, as I suppose, 
approximately at time

cat
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout 

30

but when running dd second time, log looks strange (errors2.txt)
..
quoted
[75702.039300] ata5.00: NCQ disabled due to excessive errors
[75702.039382]          res 41/00:08:00:a8:36/00:00:01:00:00/40 Emask 
0x1 (device error)
[75702.039452]          res 41/00:00:01:00:00/00:00:01:00:00/40 Emask 
0x1 (device error)
[75702.039522] ata5: hard resetting link
[75702.936061] ata5: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[75702.996080] ata5.00: max_sectors limited to 256 for NCQ
[75703.296058] ata5.00: max_sectors limited to 256 for NCQ
[75703.296061] ata5.00: configured for UDMA/133
[75703.296069] ata5: EH complete
[75703.296098] ------------[ cut here ]------------
[75703.296100] WARNING: at drivers/ata/libata-core.c:4732 ata_qc_issue+0x1ca/0x230 [libata]()
..
That line is this one (linux-2.6.26.2):

        WARN_ON(ap->ops->error_handler && ata_tag_valid(link->active_tag));

So this should trigger only when link->active_tag is valid, which doesn't normally happen.
But the convoluted traceback shows that this code path came from the EH,
so something in libata EH is likely neglecting to clear link->active_tag
before issuing a new command.  

Tejun?

Re: bad sectors, suspicious behaviour

From: Mark Lord <hidden>
Date: 2008-08-08 14:14:40

Mark Lord wrote:
..
quoted
quoted
[75703.296100] WARNING: at drivers/ata/libata-core.c:4732 
ata_qc_issue+0x1ca/0x230 [libata]()
..
That line is this one (linux-2.6.26.2):

       WARN_ON(ap->ops->error_handler && ata_tag_valid(link->active_tag));

So this should trigger only when link->active_tag is valid, which 
doesn't normally happen.
But the convoluted traceback shows that this code path came from the EH,
so something in libata EH is likely neglecting to clear link->active_tag
before issuing a new command. 

Tejun?
..

Mmm.. since it happens only once in a while, and not on every EH action,
one might assume that it's a race of some kind.

One possibility, might be due to .qc_defer.

The stock ata_qc_defer relies heavily on ata_tag_valid(),
which matches what the above WARN_ON uses.

But sata_mv doesn't use ata_tag_valid, because it wants to know
about the entire port and not just a single individual link on the port.
So instead, it uses ap->nr_active_links for the test.

My guess is that these two items are not kept in sync during EH.

Tejun?

Re: bad sectors, suspicious behaviour

From: Bokhan Artem <hidden>
Date: 2008-08-11 11:12:31

So, is there any chance to test patch from someone on this issue?

ps. I'm very far from kernel development.

Mark Lord пишет:
Mark Lord wrote:
..
quoted
quoted
quoted
[75703.296100] WARNING: at drivers/ata/libata-core.c:4732 
ata_qc_issue+0x1ca/0x230 [libata]()
..
That line is this one (linux-2.6.26.2):

       WARN_ON(ap->ops->error_handler && 
ata_tag_valid(link->active_tag));

So this should trigger only when link->active_tag is valid, which 
doesn't normally happen.
But the convoluted traceback shows that this code path came from the EH,
so something in libata EH is likely neglecting to clear link->active_tag
before issuing a new command.
Tejun?
..

Mmm.. since it happens only once in a while, and not on every EH action,
one might assume that it's a race of some kind.

One possibility, might be due to .qc_defer.

The stock ata_qc_defer relies heavily on ata_tag_valid(),
which matches what the above WARN_ON uses.

But sata_mv doesn't use ata_tag_valid, because it wants to know
about the entire port and not just a single individual link on the port.
So instead, it uses ap->nr_active_links for the test.

My guess is that these two items are not kept in sync during EH.

Tejun?
-- 
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: bad sectors, suspicious behaviour

From: Tejun Heo <hidden>
Date: 2008-08-13 08:41:31

Hello, Mark, Artem.

Mark Lord wrote:
Mmm.. since it happens only once in a while, and not on every EH action,
one might assume that it's a race of some kind.

One possibility, might be due to .qc_defer.

The stock ata_qc_defer relies heavily on ata_tag_valid(),
which matches what the above WARN_ON uses.

But sata_mv doesn't use ata_tag_valid, because it wants to know
about the entire port and not just a single individual link on the port.
So instead, it uses ap->nr_active_links for the test.

My guess is that these two items are not kept in sync during EH.
The culprit is mv_qc_defer().  If the controller is in host queueing
mode, it allows multiple non-NCQ commands to be queued, which
currently isn't allowed by the current libata core.  Allowing it
shouldn't be too difficult but it would incur some core layer changes.
For the time being, can you please test whether the following patch
fixes the problem?
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index ad169ff..80c655f 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1134,30 +1134,16 @@ static int mv_qc_defer(struct ata_queued_cmd *qc)
 	if (ap->nr_active_links == 0)
 		return 0;
 
-	if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
-		/*
-		 * The port is operating in host queuing mode (EDMA).
-		 * It can accomodate a new qc if the qc protocol
-		 * is compatible with the current host queue mode.
-		 */
-		if (pp->pp_flags & MV_PP_FLAG_NCQ_EN) {
-			/*
-			 * The host queue (EDMA) is in NCQ mode.
-			 * If the new qc is also an NCQ command,
-			 * then allow the new qc.
-			 */
-			if (qc->tf.protocol == ATA_PROT_NCQ)
-				return 0;
-		} else {
-			/*
-			 * The host queue (EDMA) is in non-NCQ, DMA mode.
-			 * If the new qc is also a non-NCQ, DMA command,
-			 * then allow the new qc.
-			 */
-			if (qc->tf.protocol == ATA_PROT_DMA)
-				return 0;
-		}
-	}
+	/*
+	 * The port is operating in host queuing mode (EDMA) with NCQ
+	 * enabled, allow multiple NCQ commands.  EDMA also allows
+	 * queueing multiple DMA commands but libata core currently
+	 * doesn't allow it.
+	 */
+	if ((pp->pp_flags & MV_PP_FLAG_EDMA_EN) &&
+	    (pp->pp_flags & MV_PP_FLAG_NCQ_EN) && ata_is_ncq(qc->tf.protocol))
+		return 0;
+
 	return ATA_DEFER_PORT;
 }

Re: bad sectors, suspicious behaviour

From: Artem Bokhan <hidden>
Date: 2008-08-13 10:47:51

patch did not help,  but I found additional necessary condition (I 
didn't take it into account before system reboot):

to reproduce the problem from scratch it is necessary to

echo 1 >  
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout

returning the value to 30 do not help.

With default 30 secs the problem can not be reproduced


Tejun Heo пишет:
quoted hunk
Hello, Mark, Artem.

Mark Lord wrote:
  
quoted
Mmm.. since it happens only once in a while, and not on every EH action,
one might assume that it's a race of some kind.

One possibility, might be due to .qc_defer.

The stock ata_qc_defer relies heavily on ata_tag_valid(),
which matches what the above WARN_ON uses.

But sata_mv doesn't use ata_tag_valid, because it wants to know
about the entire port and not just a single individual link on the port.
So instead, it uses ap->nr_active_links for the test.

My guess is that these two items are not kept in sync during EH.
    
The culprit is mv_qc_defer().  If the controller is in host queueing
mode, it allows multiple non-NCQ commands to be queued, which
currently isn't allowed by the current libata core.  Allowing it
shouldn't be too difficult but it would incur some core layer changes.
For the time being, can you please test whether the following patch
fixes the problem?
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index ad169ff..80c655f 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1134,30 +1134,16 @@ static int mv_qc_defer(struct ata_queued_cmd *qc)
 	if (ap->nr_active_links == 0)
 		return 0;
 
-	if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
-		/*
-		 * The port is operating in host queuing mode (EDMA).
-		 * It can accomodate a new qc if the qc protocol
-		 * is compatible with the current host queue mode.
-		 */
-		if (pp->pp_flags & MV_PP_FLAG_NCQ_EN) {
-			/*
-			 * The host queue (EDMA) is in NCQ mode.
-			 * If the new qc is also an NCQ command,
-			 * then allow the new qc.
-			 */
-			if (qc->tf.protocol == ATA_PROT_NCQ)
-				return 0;
-		} else {
-			/*
-			 * The host queue (EDMA) is in non-NCQ, DMA mode.
-			 * If the new qc is also a non-NCQ, DMA command,
-			 * then allow the new qc.
-			 */
-			if (qc->tf.protocol == ATA_PROT_DMA)
-				return 0;
-		}
-	}
+	/*
+	 * The port is operating in host queuing mode (EDMA) with NCQ
+	 * enabled, allow multiple NCQ commands.  EDMA also allows
+	 * queueing multiple DMA commands but libata core currently
+	 * doesn't allow it.
+	 */
+	if ((pp->pp_flags & MV_PP_FLAG_EDMA_EN) &&
+	    (pp->pp_flags & MV_PP_FLAG_NCQ_EN) && ata_is_ncq(qc->tf.protocol))
+		return 0;
+
 	return ATA_DEFER_PORT;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Re: bad sectors, suspicious behaviour

From: Tejun Heo <hidden>
Date: 2008-08-13 10:50:43

Artem Bokhan wrote:
patch did not help,  but I found additional necessary condition (I
didn't take it into account before system reboot):

to reproduce the problem from scratch it is necessary to

echo 1 > 
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout


returning the value to 30 do not help.

With default 30 secs the problem can not be reproduced
Can you please post full kernel log?

-- 
tejun

Re: bad sectors, suspicious behaviour

From: Artem Bokhan <hidden>
Date: 2008-08-13 11:19:51

Tejun, I am very sorry, necessary initrd image was not updated. Now I 
can not reproduce the problem, thx a lot.

Tejun Heo пишет:
Artem Bokhan wrote:
  
quoted
patch did not help,  but I found additional necessary condition (I
didn't take it into account before system reboot):

to reproduce the problem from scratch it is necessary to

echo 1 > 
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout


returning the value to 30 do not help.

With default 30 secs the problem can not be reproduced
    
Can you please post full kernel log?

  

[PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Tejun Heo <hidden>
Date: 2008-08-13 11:25:14

sata_mv allowed issuing two DMA commands concurrently which the
hardware allows.  Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
Don't allow concurrent DMA commands for now.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
No problem, Artem.

Mark, does this look good to you?

 drivers/ata/sata_mv.c |   34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index ad169ff..80c655f 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1134,30 +1134,16 @@ static int mv_qc_defer(struct ata_queued_cmd *qc)
 	if (ap->nr_active_links == 0)
 		return 0;
 
-	if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
-		/*
-		 * The port is operating in host queuing mode (EDMA).
-		 * It can accomodate a new qc if the qc protocol
-		 * is compatible with the current host queue mode.
-		 */
-		if (pp->pp_flags & MV_PP_FLAG_NCQ_EN) {
-			/*
-			 * The host queue (EDMA) is in NCQ mode.
-			 * If the new qc is also an NCQ command,
-			 * then allow the new qc.
-			 */
-			if (qc->tf.protocol == ATA_PROT_NCQ)
-				return 0;
-		} else {
-			/*
-			 * The host queue (EDMA) is in non-NCQ, DMA mode.
-			 * If the new qc is also a non-NCQ, DMA command,
-			 * then allow the new qc.
-			 */
-			if (qc->tf.protocol == ATA_PROT_DMA)
-				return 0;
-		}
-	}
+	/*
+	 * The port is operating in host queuing mode (EDMA) with NCQ
+	 * enabled, allow multiple NCQ commands.  EDMA also allows
+	 * queueing multiple DMA commands but libata core currently
+	 * doesn't allow it.
+	 */
+	if ((pp->pp_flags & MV_PP_FLAG_EDMA_EN) &&
+	    (pp->pp_flags & MV_PP_FLAG_NCQ_EN) && ata_is_ncq(qc->tf.protocol))
+		return 0;
+
 	return ATA_DEFER_PORT;
 }
 

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Artem Bokhan <hidden>
Date: 2008-08-13 11:37:15

Now I have about 10 seconds of "disk restart" when hitting bad block 
(with /sys/...../timeout=1).
Can somebody point me, if there any way to reduce it to 1/2/3 secs?

Tejun Heo пишет:
quoted hunk
sata_mv allowed issuing two DMA commands concurrently which the
hardware allows.  Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
Don't allow concurrent DMA commands for now.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
No problem, Artem.

Mark, does this look good to you?

 drivers/ata/sata_mv.c |   34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index ad169ff..80c655f 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1134,30 +1134,16 @@ static int mv_qc_defer(struct ata_queued_cmd *qc)
 	if (ap->nr_active_links == 0)
 		return 0;
 
-	if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
-		/*
-		 * The port is operating in host queuing mode (EDMA).
-		 * It can accomodate a new qc if the qc protocol
-		 * is compatible with the current host queue mode.
-		 */
-		if (pp->pp_flags & MV_PP_FLAG_NCQ_EN) {
-			/*
-			 * The host queue (EDMA) is in NCQ mode.
-			 * If the new qc is also an NCQ command,
-			 * then allow the new qc.
-			 */
-			if (qc->tf.protocol == ATA_PROT_NCQ)
-				return 0;
-		} else {
-			/*
-			 * The host queue (EDMA) is in non-NCQ, DMA mode.
-			 * If the new qc is also a non-NCQ, DMA command,
-			 * then allow the new qc.
-			 */
-			if (qc->tf.protocol == ATA_PROT_DMA)
-				return 0;
-		}
-	}
+	/*
+	 * The port is operating in host queuing mode (EDMA) with NCQ
+	 * enabled, allow multiple NCQ commands.  EDMA also allows
+	 * queueing multiple DMA commands but libata core currently
+	 * doesn't allow it.
+	 */
+	if ((pp->pp_flags & MV_PP_FLAG_EDMA_EN) &&
+	    (pp->pp_flags & MV_PP_FLAG_NCQ_EN) && ata_is_ncq(qc->tf.protocol))
+		return 0;
+
 	return ATA_DEFER_PORT;
 }
 
  

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Artem Bokhan <hidden>
Date: 2008-08-13 11:47:02

Tejun Heo пишет:
Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
  
Does this affect performance/functionality of controller/hdd someway?
Don't allow concurrent DMA commands for now.

  

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Tejun Heo <hidden>
Date: 2008-08-13 11:53:17

Artem Bokhan wrote:
Tejun Heo пишет:
quoted
Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
When NCQ is disabled, it will probably add tiny bit to inter command
latency thus reducing performance a bit but I don't think it will stand
out in any way.  When you count in the seek time and all, the inter
command latency should be negligible in most cases.

-- 
tejun

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Tejun Heo <hidden>
Date: 2008-08-13 11:53:57

Artem Bokhan wrote:
Now I have about 10 seconds of "disk restart" when hitting bad block
(with /sys/...../timeout=1).
Can somebody point me, if there any way to reduce it to 1/2/3 secs?
I don't really follow what you're saying.  Can you please elaborate?

-- 
tejun

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Artem Bokhan <hidden>
Date: 2008-08-13 12:05:23

When I run 'dd if=/dev/sda of=/dev/null skip=9700 bs=1M' and it meets 
bad block, it takes about 
'/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout 
+ about 10 seconds' to determine I/O error. Until that IO to device is 
locked, as I understand. I want to reduce the time of lock as  much as 
possible.

dd: reading `/dev/sda': Input/output error
240+1 records in
240+1 records out
252575744 bytes (253 MB) copied, 13.7833 s, 18.3 MB/s



Tejun Heo пишет:
Artem Bokhan wrote:
  
quoted
Now I have about 10 seconds of "disk restart" when hitting bad block
(with /sys/...../timeout=1).
Can somebody point me, if there any way to reduce it to 1/2/3 secs?
    
I don't really follow what you're saying.  Can you please elaborate?

  

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Tejun Heo <hidden>
Date: 2008-08-13 12:22:42

Artem Bokhan wrote:
When I run 'dd if=/dev/sda of=/dev/null skip=9700 bs=1M' and it meets
bad block, it takes about
'/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout
+ about 10 seconds' to determine I/O error. Until that IO to device is
locked, as I understand. I want to reduce the time of lock as  much as
possible.

dd: reading `/dev/sda': Input/output error
240+1 records in
240+1 records out
252575744 bytes (253 MB) copied, 13.7833 s, 18.3 MB/s
Can you post timestamped dmesg?

-- 
tejun

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Artem Bokhan <hidden>
Date: 2008-08-13 12:32:06

Last part of it with two examples is in attachment.

Tejun Heo пишет:
Artem Bokhan wrote:
  
quoted
When I run 'dd if=/dev/sda of=/dev/null skip=9700 bs=1M' and it meets
bad block, it takes about
'/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout
+ about 10 seconds' to determine I/O error. Until that IO to device is
locked, as I understand. I want to reduce the time of lock as  much as
possible.

dd: reading `/dev/sda': Input/output error
240+1 records in
240+1 records out
252575744 bytes (253 MB) copied, 13.7833 s, 18.3 MB/s
    
Can you post timestamped dmesg?

  

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Mark Lord <hidden>
Date: 2008-08-13 16:10:16

Tejun Heo wrote:
sata_mv allowed issuing two DMA commands concurrently which the
hardware allows.  Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
Don't allow concurrent DMA commands for now.

Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Mark Lord <redacted>
---
No problem, Artem.

Mark, does this look good to you?
Yeah, looks fine.  For some odd reason, I must have been thinking
that libata-{scsi,core} would not even attempt multiple simultaneous non-NCQ.
Silly me!
quoted hunk
 drivers/ata/sata_mv.c |   34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index ad169ff..80c655f 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1134,30 +1134,16 @@ static int mv_qc_defer(struct ata_queued_cmd *qc)
 	if (ap->nr_active_links == 0)
 		return 0;
 
-	if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
-		/*
-		 * The port is operating in host queuing mode (EDMA).
-		 * It can accomodate a new qc if the qc protocol
-		 * is compatible with the current host queue mode.
-		 */
-		if (pp->pp_flags & MV_PP_FLAG_NCQ_EN) {
-			/*
-			 * The host queue (EDMA) is in NCQ mode.
-			 * If the new qc is also an NCQ command,
-			 * then allow the new qc.
-			 */
-			if (qc->tf.protocol == ATA_PROT_NCQ)
-				return 0;
-		} else {
-			/*
-			 * The host queue (EDMA) is in non-NCQ, DMA mode.
-			 * If the new qc is also a non-NCQ, DMA command,
-			 * then allow the new qc.
-			 */
-			if (qc->tf.protocol == ATA_PROT_DMA)
-				return 0;
-		}
-	}
+	/*
+	 * The port is operating in host queuing mode (EDMA) with NCQ
+	 * enabled, allow multiple NCQ commands.  EDMA also allows
+	 * queueing multiple DMA commands but libata core currently
+	 * doesn't allow it.
+	 */
+	if ((pp->pp_flags & MV_PP_FLAG_EDMA_EN) &&
+	    (pp->pp_flags & MV_PP_FLAG_NCQ_EN) && ata_is_ncq(qc->tf.protocol))
+		return 0;
+
 	return ATA_DEFER_PORT;
 }
 

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Mark Lord <hidden>
Date: 2008-08-13 16:17:36

Artem Bokhan wrote:
When I run 'dd if=/dev/sda of=/dev/null skip=9700 bs=1M' and it meets 
bad block, it takes about 
'/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout 
+ about 10 seconds' to determine I/O error. Until that IO to device is 
locked, as I understand. I want to reduce the time of lock as  much as 
possible.
..

Tell the SCSI layer to stop flogging the drive with unnecessary retries.
A quick and dirty way, is this patch below, but it really should be more
intelligent than this for some situations:
--- linux/include/scsi/sd.h.orig	2008-06-09 14:27:19.000000000 -0400
+++ linux/include/scsi/sd.h	2008-08-13 12:16:01.000000000 -0400
@@ -23,7 +23,7 @@
 /*
  * Number of allowed retries
  */
-#define SD_MAX_RETRIES		5
+#define SD_MAX_RETRIES		1
 #define SD_PASSTHROUGH_RETRIES	1
 
 /*

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Greg Freemyer <hidden>
Date: 2008-08-13 16:57:58

On Wed, Aug 13, 2008 at 8:05 AM, Artem Bokhan [off-list ref] wrote:
When I run 'dd if=/dev/sda of=/dev/null skip=9700 bs=1M' and it meets bad
block, it takes about
'/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout
+ about 10 seconds' to determine I/O error. Until that IO to device is
locked, as I understand. I want to reduce the time of lock as  much as
possible.

dd: reading `/dev/sda': Input/output error
240+1 records in
240+1 records out
252575744 bytes (253 MB) copied, 13.7833 s, 18.3 MB/s
Artem,

I'm not sure what your goal is but you might find it interesting that
"Enterprise" versions of some of the drives have fast fail logic built
into them.  ie. They don't internally retry on medium errors.

As I understand it, the concept is if you have a raid setup, you don't
want to waste time retrying a read.  It is better to fail the read
ASAP and get the data from another source.

Greg
-- 
Greg Freemyer
Litigation Triage Solutions Specialist
http://www.linkedin.com/in/gregfreemyer
First 99 Days Litigation White Paper -
http://www.norcrossgroup.com/forms/whitepapers/99%20Days%20whitepaper.pdf

The Norcross Group
The Intersection of Evidence & Technology
http://www.norcrossgroup.com

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Bokhan Artem <hidden>
Date: 2008-08-13 17:29:56

I have particular fast, simple and cheap controller(s) (MV88SX6081 
8-port SATA II PCI-X) with a bunch of disks w/o sw/hw raid.
I need to return an error to application ASAP if media error detected.

Greg Freemyer ?????:

Artem,

I'm not sure what your goal is but you might find it interesting that
"Enterprise" versions of some of the drives have fast fail logic built
into them.  ie. They don't internally retry on medium errors.

As I understand it, the concept is if you have a raid setup, you don't
want to waste time retrying a read.  It is better to fail the read
ASAP and get the data from another source.

Greg
  

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Bokhan Artem <hidden>
Date: 2008-08-13 17:37:35

It's very strange for me that it is not runtime configurable... OK, thnx.

Also I'm not very close to ata protocols, so :
1. Why there is need to reset link if sector can not be read?
2. Why device is locked until hard reset?

Aug 13 08:45:45 ubuntu kernel: [ 5634.080692] sd 4:0:0:0: [sde] 
1465149168 512-byte hardware sectors (750156 MB)
Aug 13 08:45:46 ubuntu kernel: [ 5635.088067]          res 
40/00:08:e0:a7:36/01:00:01:00:00/00 Emask 0x4 (timeout)
Aug 13 08:45:46 ubuntu kernel: [ 5635.088136] ata5: hard resetting link



Mark Lord пишет:
quoted hunk
Artem Bokhan wrote:
quoted
When I run 'dd if=/dev/sda of=/dev/null skip=9700 bs=1M' and it meets 
bad block, it takes about 
'/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout 
+ about 10 seconds' to determine I/O error. Until that IO to device 
is locked, as I understand. I want to reduce the time of lock as  
much as possible.
..

Tell the SCSI layer to stop flogging the drive with unnecessary retries.
A quick and dirty way, is this patch below, but it really should be more
intelligent than this for some situations:
--- linux/include/scsi/sd.h.orig    2008-06-09 14:27:19.000000000 -0400
+++ linux/include/scsi/sd.h    2008-08-13 12:16:01.000000000 -0400
@@ -23,7 +23,7 @@
/*
 * Number of allowed retries
 */
-#define SD_MAX_RETRIES        5
+#define SD_MAX_RETRIES        1
#define SD_PASSTHROUGH_RETRIES    1

/*

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Greg Freemyer <hidden>
Date: 2008-08-13 17:50:53

On Wed, Aug 13, 2008 at 1:29 PM, Bokhan Artem [off-list ref] wrote:
I have particular fast, simple and cheap controller(s) (MV88SX6081 8-port
SATA II PCI-X) with a bunch of disks w/o sw/hw raid.
I need to return an error to application ASAP if media error detected.
My thought was that if speed is more important than reliability in
your case, then you could buy a "Enterprise" version hard drive and
eliminate the time it takes the drive to internally do retries.

ie. The concept for which Enterprise drives exist is raid, but that
does not mean you have to use them that way.

Greg
-- 
Greg Freemyer
Litigation Triage Solutions Specialist
http://www.linkedin.com/in/gregfreemyer
First 99 Days Litigation White Paper -
http://www.norcrossgroup.com/forms/whitepapers/99%20Days%20whitepaper.pdf

The Norcross Group
The Intersection of Evidence & Technology
http://www.norcrossgroup.com

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Bokhan Artem <hidden>
Date: 2008-08-13 18:04:15

But as I understand my problem, all retries and delays are made inside 
linux kernel, not inside hardware...

Greg Freemyer ?????:
On Wed, Aug 13, 2008 at 1:29 PM, Bokhan Artem [off-list ref] wrote:
  
quoted
I have particular fast, simple and cheap controller(s) (MV88SX6081 8-port
SATA II PCI-X) with a bunch of disks w/o sw/hw raid.
I need to return an error to application ASAP if media error detected.

    
My thought was that if speed is more important than reliability in
your case, then you could buy a "Enterprise" version hard drive and
eliminate the time it takes the drive to internally do retries.

ie. The concept for which Enterprise drives exist is raid, but that
does not mean you have to use them that way.

Greg
  

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Greg Freemyer <hidden>
Date: 2008-08-13 18:13:08

On Wed, Aug 13, 2008 at 2:04 PM, Bokhan Artem [off-list ref] wrote:
But as I understand my problem, all retries and delays are made inside linux
kernel, not inside hardware...
AIUI you have both.

And they multiply, so if the hardware is doing X retries per attempt
and the kernel is doing Y retries, then the total retries is close to
X*(Y+1).

To get the fastest possible failure mode you need to reduce both X and Y.

Greg
-- 
Greg Freemyer
Litigation Triage Solutions Specialist
http://www.linkedin.com/in/gregfreemyer
First 99 Days Litigation White Paper -
http://www.norcrossgroup.com/forms/whitepapers/99%20Days%20whitepaper.pdf

The Norcross Group
The Intersection of Evidence & Technology
http://www.norcrossgroup.com

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Bokhan Artem <hidden>
Date: 2008-08-13 19:58:23

As it named RETRY, I set it to 0, "define SD_MAX_RETRIES        0"
But still got two retries. Any suggestions?

dd if=/dev/sda of=/dev/null bs=512 skip=20359137 count=1

Aug 13 15:50:02 ubuntu kernel: [  244.688030]          res 
51/40:08:e0:a7:36/01:00:01:00:00/00 Emask 0x409 (media error) <F>
Aug 13 15:50:02 ubuntu kernel: [  244.688206] ata5: hard resetting link
Aug 13 15:50:02 ubuntu kernel: [  245.226248] ata5: SATA link up 3.0 
Gbps (SStatus 123 SControl 300)
Aug 13 15:50:02 ubuntu kernel: [  245.306267] ata5.00: max_sectors 
limited to 256 for NCQ
Aug 13 15:50:02 ubuntu kernel: [  245.388635] ata5.00: max_sectors 
limited to 256 for NCQ
Aug 13 15:50:02 ubuntu kernel: [  245.388637] ata5.00: configured for 
UDMA/133
Aug 13 15:50:02 ubuntu kernel: [  245.388643] sd 4:0:0:0: [sda] Result: 
hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
Aug 13 15:50:02 ubuntu kernel: [  245.388646] sd 4:0:0:0: [sda] Sense 
Key : Medium Error [current] [descriptor]
Aug 13 15:50:02 ubuntu kernel: [  245.388649] Descriptor sense data with 
sense descriptors (in hex):
Aug 13 15:50:02 ubuntu kernel: [  245.388651]         72 03 11 04 00 00 
00 0c 00 0a 80 00 00 00 00 00
Aug 13 15:50:02 ubuntu kernel: [  245.388658]         01 36 a7 e0
Aug 13 15:50:02 ubuntu kernel: [  245.388661] sd 4:0:0:0: [sda] Add. 
Sense: Unrecovered read error - auto reallocate failed
Aug 13 15:50:02 ubuntu kernel: [  245.388783] ata5: EH complete
Aug 13 15:50:02 ubuntu kernel: [  245.388722] sd 4:0:0:0: [sda] 
1465149168 512-byte hardware sectors (750156 MB)
Aug 13 15:50:02 ubuntu kernel: [  245.388722] sd 4:0:0:0: [sda] Write 
Protect is off
Aug 13 15:50:02 ubuntu kernel: [  245.388722] sd 4:0:0:0: [sda] Write 
cache: enabled, read cache: enabled, doesn't support DPO or FUA

Aug 13 15:50:04 ubuntu kernel: [  247.117648]          res 
51/40:08:e0:a7:36/01:00:01:00:00/00 Emask 0x409 (media error) <F>
Aug 13 15:50:04 ubuntu kernel: [  247.117824] ata5: hard resetting link
Aug 13 15:50:04 ubuntu kernel: [  247.659679] ata5: SATA link up 3.0 
Gbps (SStatus 123 SControl 300)
Aug 13 15:50:04 ubuntu kernel: [  247.747372] ata5.00: max_sectors 
limited to 256 for NCQ
Aug 13 15:50:04 ubuntu kernel: [  247.825139] ata5.00: max_sectors 
limited to 256 for NCQ
Aug 13 15:50:04 ubuntu kernel: [  247.825141] ata5.00: configured for 
UDMA/133
Aug 13 15:50:04 ubuntu kernel: [  247.825145] sd 4:0:0:0: [sda] Result: 
hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
Aug 13 15:50:04 ubuntu kernel: [  247.825148] sd 4:0:0:0: [sda] Sense 
Key : Medium Error [current] [descriptor]
Aug 13 15:50:04 ubuntu kernel: [  247.825151] Descriptor sense data with 
sense descriptors (in hex):
Aug 13 15:50:04 ubuntu kernel: [  247.825153]         72 03 11 04 00 00 
00 0c 00 0a 80 00 00 00 00 00
Aug 13 15:50:04 ubuntu kernel: [  247.825160]         01 36 a7 e0
Aug 13 15:50:04 ubuntu kernel: [  247.825163] sd 4:0:0:0: [sda] Add. 
Sense: Unrecovered read error - auto reallocate failed
Aug 13 15:50:04 ubuntu kernel: [  247.825281] ata5: EH complete
Aug 13 15:50:04 ubuntu kernel: [  247.825223] sd 4:0:0:0: [sda] 
1465149168 512-byte hardware sectors (750156 MB)
Aug 13 15:50:04 ubuntu kernel: [  247.825223] sd 4:0:0:0: [sda] Write 
Protect is off
Aug 13 15:50:04 ubuntu kernel: [  247.825223] sd 4:0:0:0: [sda] Write 
cache: enabled, read cache: enabled, doesn't support DPO or FUA



Mark Lord пишет:
quoted hunk
Artem Bokhan wrote:
quoted
When I run 'dd if=/dev/sda of=/dev/null skip=9700 bs=1M' and it meets 
bad block, it takes about 
'/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout 
+ about 10 seconds' to determine I/O error. Until that IO to device 
is locked, as I understand. I want to reduce the time of lock as  
much as possible.
..

Tell the SCSI layer to stop flogging the drive with unnecessary retries.
A quick and dirty way, is this patch below, but it really should be more
intelligent than this for some situations:
--- linux/include/scsi/sd.h.orig    2008-06-09 14:27:19.000000000 -0400
+++ linux/include/scsi/sd.h    2008-08-13 12:16:01.000000000 -0400
@@ -23,7 +23,7 @@
/*
 * Number of allowed retries
 */
-#define SD_MAX_RETRIES        5
+#define SD_MAX_RETRIES        1
#define SD_PASSTHROUGH_RETRIES    1

/*

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Mark Lord <hidden>
Date: 2008-08-13 23:36:08

Bokhan Artem wrote:
As it named RETRY, I set it to 0, "define SD_MAX_RETRIES        0"
But still got two retries. Any suggestions?
..

I only see two failures (eg. one retry) in the log you showed.
But it probably was not a retry, but rather a second I/O after
the first one.  The filesystem layer often does that.

Cheers

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Artem Bokhan <hidden>
Date: 2008-08-14 07:42:50

Mark Lord пишет:
I only see two failures (eg. one retry) in the log you showed.
Yes, I mean 2 tries, so 1 unnecessary rerty.
But it probably was not a retry, but rather a second I/O after
the first one.  The filesystem layer often does that.
If kernel tries to read the same sector more then one time, I call that 
retry :)

I found that it depend on vm someway... First 'dd if=/dev/sdb 
of=/dev/null skip=300000 count=1' causes 2 failures (1 retry), but next 
'dd's only 1failure. After I drop vm cache 'sysctl vm.drop_caches=1', I 
get two failures again.

running 'dd if=/dev/sdb of=/dev/null skip=300000 count=1 iflag=direct' 
always causes only 1 failure

IMHO, looks like cavity somewhere... Any ideas?

Cheers

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Mark Lord <hidden>
Date: 2008-08-14 12:40:34

Artem Bokhan wrote:
..
IMHO, looks like cavity somewhere... Any ideas?
..

Have you tried to disable fs read-ahead?

Something like "hdparm -a0 /dev/sd*" ?

-ml

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Artem Bokhan <hidden>
Date: 2008-08-14 12:58:40

That work, thanks, no retries at all now. But is behavior with readahead 
on and double reads correct?

Mark Lord пишет:
Artem Bokhan wrote:
..
quoted
IMHO, looks like cavity somewhere... Any ideas?
..

Have you tried to disable fs read-ahead?

Something like "hdparm -a0 /dev/sd*" ?

-ml

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Artem Bokhan <hidden>
Date: 2008-08-14 13:17:01

Now I spend ~1.9 seconds to serve one bad sector. That's great.
0.75 of 1.9 seconds is a time spent on hard reset, 1.15 seconds are 
spent on reallocation try.
Is hard reset real necessary to return disk to life after media error?



time dd if=/dev/sda of=/dev/null count=1 skip=300000
dd: reading `/dev/sda': Input/output error
0+0 records in
0+0 records out
0 bytes (0 B) copied, 1.87432 s, 0.0 kB/s

real    0m1.876s

[63107.038255] ata6.00: exception Emask 0x0 SAct 0x1 SErr 0x0 action 0x6
[63107.038287] ata6.00: edma_err_cause=00000084 pp_flags=00000003, dev 
error, EDMA self-disable
[63107.038335] ata6.00: cmd 60/08:00:e0:93:04/00:00:00:00:00/40 tag 0 
ncq 4096 in
[63107.038336]          res 51/40:08:e0:93:04/01:00:00:00:00/00 Emask 
0x409 (media error) <F>
[63107.038380] ata6.00: status: { DRDY ERR }
[63107.038403] ata6.00: error: { UNC }
[63107.038430] ata6: hard resetting link
[63107.608304] ata6: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[63107.692432] ata6.00: max_sectors limited to 256 for NCQ
[63107.772479] ata6.00: max_sectors limited to 256 for NCQ
[63107.772481] ata6.00: configured for UDMA/133
[63107.772488] sd 5:0:0:0: [sda] Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE,SUGGEST_OK
[63107.772491] sd 5:0:0:0: [sda] Sense Key : Medium Error [current] 
[descriptor]
[63107.772495] Descriptor sense data with sense descriptors (in hex):
[63107.772497]         72 03 11 04 00 00 00 0c 00 0a 80 00 00 00 00 00
[63107.772504]         00 04 93 e0
[63107.772507] sd 5:0:0:0: [sda] Add. Sense: Unrecovered read error - 
auto reallocate failed
[63107.772512] end_request: I/O error, dev sda, sector 300000
[63107.772540] Buffer I/O error on device sda, logical block 37500
[63107.772572] ata6: EH complete
[63107.790290] sd 5:0:0:0: [sda] 1465149168 512-byte hardware sectors 
(750156 MB)
[63107.790290] sd 5:0:0:0: [sda] Write Protect is off
[63107.790290] sd 5:0:0:0: [sda] Mode Sense: 00 3a 00 00
[63107.790290] sd 5:0:0:0: [sda] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA




Mark Lord пишет:
Artem Bokhan wrote:
..
quoted
IMHO, looks like cavity somewhere... Any ideas?
..

Have you tried to disable fs read-ahead?

Something like "hdparm -a0 /dev/sd*" ?

-ml

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Mark Lord <hidden>
Date: 2008-08-14 19:49:04

Artem Bokhan wrote:
Now I spend ~1.9 seconds to serve one bad sector. That's great.
0.75 of 1.9 seconds is a time spent on hard reset, 1.15 seconds are 
spent on reallocation try.
Is hard reset real necessary to return disk to life after media error?
..

It is necessary in many cases, but not in all.
But the observed behaviour (reset) is the safest way to
guarantee that things will nearly always work by default.

If the drive is NOT using NCQ, and the host controller does not get confused
by the incomplete data transfers (many do), then the reset is not necessary.

You can try hacking it out, if you want to.

Cheers

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Artem Bokhan <hidden>
Date: 2008-08-15 05:35:21

It uses ncq. Thanks, Mark.

Mark Lord пишет:
Artem Bokhan wrote:
quoted
Now I spend ~1.9 seconds to serve one bad sector. That's great.
0.75 of 1.9 seconds is a time spent on hard reset, 1.15 seconds are 
spent on reallocation try.
Is hard reset real necessary to return disk to life after media error?
..

It is necessary in many cases, but not in all.
But the observed behaviour (reset) is the safest way to
guarantee that things will nearly always work by default.

If the drive is NOT using NCQ, and the host controller does not get 
confused
by the incomplete data transfers (many do), then the reset is not 
necessary.

You can try hacking it out, if you want to.

Cheers

-- 
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Mark Lord <hidden>
Date: 2008-08-15 12:27:26

Artem Bokhan wrote:
It uses ncq. Thanks, Mark.
..

Okay, so the ATA specs explicitly require a reset there.
Not much to do about that.

If you were super-keen, though, you could crawl through libata
and try reducing various delays and polling intervals that are
associated with a hard reset for that specific (ahci?) driver
and the core libata routines.

Might shave that 0.75sec down to perhaps 100ms or so.
But again, the optimal numbers in your specific case
will not work for the general case.  It might be possible
to parameterize it all, though, permitting different
values for differing hardware.

Cheers

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Jeff Garzik <hidden>
Date: 2008-08-22 06:11:29

Tejun Heo wrote:
sata_mv allowed issuing two DMA commands concurrently which the
hardware allows.  Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
Don't allow concurrent DMA commands for now.

Signed-off-by: Tejun Heo <tj@kernel.org>
applied

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Grant Grundler <hidden>
Date: 2008-08-22 16:28:55

On Wed, Aug 13, 2008 at 4:52 AM, Tejun Heo [off-list ref] wrote:
Artem Bokhan wrote:
quoted
Tejun Heo пишет:
quoted
Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
When NCQ is disabled, it will probably add tiny bit to inter command
latency thus reducing performance a bit but I don't think it will stand
out in any way.  When you count in the seek time and all, the inter
command latency should be negligible in most cases.
When there is no seek time it does matter (e.g. SSD). To test
command perf with a normal HD, excercise the disk cache with
this fio command line:
/root/fio --runtime=30 --time_based --bwavgtime=5000 --thread
--numjobs=1 --iodepth=1 -rw=randrw --norandommap -overwrite=1
--direct=1 --ioengine=sync --ioscheduler=noop --bs=4k --size=4k
--name=hd_sdc --filename=/dev/sdc

and will get output which contains something like:
hd_sdc: (g=0): rw=randrw, bs=4K-4K/4K-4K, ioengine=sync, iodepth=1
Starting 1 thread
Jobs: 1 (f=1): [m] [96.7% done] [ 18381/ 17705 kb/s] [eta 00m:01s]
hd_sdc: (groupid=0, jobs=1): err= 0: pid=17079
  read : io=528344KiB, bw=18476KiB/s, iops=4510, runt= 29281msec
    clat (usec): min=82, max=131, avg=93.11, stdev= 1.67
  write: io=527792KiB, bw=18235KiB/s, iops=4452, runt= 29637msec
    clat (usec): min=84, max=209, avg=93.88, stdev= 1.60
...
     lat (usec): 100=99.60%, 250=0.40%
...

The drive/controller config reports almost 9000 iops from a single drive.
This compares well with existing SSDs on the market.

And looks like fio needs better granularity in it's latency buckets.

hth,
grant
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Martin Michlmayr <hidden>
Date: 2008-08-22 17:46:22

* Jeff Garzik [off-list ref] [2008-08-22 02:11]:
Tejun Heo wrote:
quoted
sata_mv allowed issuing two DMA commands concurrently which the
hardware allows.  Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
Don't allow concurrent DMA commands for now.

Signed-off-by: Tejun Heo <tj@kernel.org>
applied
Is this -stable material?
-- 
Martin Michlmayr
http://www.cyrius.com/

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Gwendal Grignou <hidden>
Date: 2008-08-26 01:25:15

What happen if there is a SATA port multiplier behing the marvell
controller and NCQ is disabled?

I have the impression that with this patch we are able to only send
one DMA command at a time to all the drives behind the PM, which has
an impact on performance.

Gwendal.

On Wed, Aug 13, 2008 at 4:24 AM, Tejun Heo [off-list ref] wrote:
quoted hunk
sata_mv allowed issuing two DMA commands concurrently which the
hardware allows.  Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
Don't allow concurrent DMA commands for now.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
No problem, Artem.

Mark, does this look good to you?

 drivers/ata/sata_mv.c |   34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index ad169ff..80c655f 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1134,30 +1134,16 @@ static int mv_qc_defer(struct ata_queued_cmd *qc)
       if (ap->nr_active_links == 0)
               return 0;

-       if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
-               /*
-                * The port is operating in host queuing mode (EDMA).
-                * It can accomodate a new qc if the qc protocol
-                * is compatible with the current host queue mode.
-                */
-               if (pp->pp_flags & MV_PP_FLAG_NCQ_EN) {
-                       /*
-                        * The host queue (EDMA) is in NCQ mode.
-                        * If the new qc is also an NCQ command,
-                        * then allow the new qc.
-                        */
-                       if (qc->tf.protocol == ATA_PROT_NCQ)
-                               return 0;
-               } else {
-                       /*
-                        * The host queue (EDMA) is in non-NCQ, DMA mode.
-                        * If the new qc is also a non-NCQ, DMA command,
-                        * then allow the new qc.
-                        */
-                       if (qc->tf.protocol == ATA_PROT_DMA)
-                               return 0;
-               }
-       }
+       /*
+        * The port is operating in host queuing mode (EDMA) with NCQ
+        * enabled, allow multiple NCQ commands.  EDMA also allows
+        * queueing multiple DMA commands but libata core currently
+        * doesn't allow it.
+        */
+       if ((pp->pp_flags & MV_PP_FLAG_EDMA_EN) &&
+           (pp->pp_flags & MV_PP_FLAG_NCQ_EN) && ata_is_ncq(qc->tf.protocol))
+               return 0;
+
       return ATA_DEFER_PORT;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Tejun Heo <hidden>
Date: 2008-08-26 07:05:40

Gwendal Grignou wrote:
What happen if there is a SATA port multiplier behing the marvell
controller and NCQ is disabled?

I have the impression that with this patch we are able to only send
one DMA command at a time to all the drives behind the PM, which has
an impact on performance.
Yeah, right. :-( The simplest way to implement this would be just
calling ata_std_qc_defer() which computes per-link restrictions after
ruling out all non-standard restrictions.  I wasn't really sure how
EDMA/NCQ_EN flags are supposed to work so just sticked to the original
condition tests.  Mark, can you please help us out here?  :-)

Thanks.

-- 
tejun

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Mark Lord <hidden>
Date: 2008-08-26 13:54:25

Martin Michlmayr wrote:
* Jeff Garzik [off-list ref] [2008-08-22 02:11]:
quoted
Tejun Heo wrote:
quoted
sata_mv allowed issuing two DMA commands concurrently which the
hardware allows.  Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
Don't allow concurrent DMA commands for now.

Signed-off-by: Tejun Heo <tj@kernel.org>
applied
Is this -stable material?
Absolutely yes.

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Mark Lord <hidden>
Date: 2008-08-26 13:58:26

Tejun Heo wrote:
Gwendal Grignou wrote:
quoted
What happen if there is a SATA port multiplier behing the marvell
controller and NCQ is disabled?

I have the impression that with this patch we are able to only send
one DMA command at a time to all the drives behind the PM, which has
an impact on performance.
Yeah, right. :-( The simplest way to implement this would be just
calling ata_std_qc_defer() which computes per-link restrictions after
ruling out all non-standard restrictions.  I wasn't really sure how
EDMA/NCQ_EN flags are supposed to work so just sticked to the original
condition tests.  Mark, can you please help us out here?  :-)
..

The hardware is both tricky and buggy, depending upon the version,
and my memory is fading -- I haven't looked at the code since early June,
when Marvell stopped paying invoices (administrative "issues").

The only 100% safe scenario is to issue only a single non-NCQ command at a time.
But there are many situations where it is safe to allow one command/port,
provided that there are no PIO commands anywhere in the mix.

Messy.  I'll have another walk through it whenever Marvell gets over
their "administrative issues" on this.

Cheers

Re: [PATCH #upstream-fixes] sata_mv: don't issue two DMA commands concurrently

From: Martin Michlmayr <hidden>
Date: 2008-08-29 07:12:22

* Mark Lord [off-list ref] [2008-08-26 09:54]:
quoted
quoted
quoted
sata_mv allowed issuing two DMA commands concurrently which the
hardware allows.  Unfortunately, libata core layer isn't ready for
this yet and spews ugly warning message and malfunctions on this.
Don't allow concurrent DMA commands for now.

Signed-off-by: Tejun Heo <tj@kernel.org>
applied
Is this -stable material?
Absolutely yes.
Adding stable@kernel.org.  This is commit 4bdee6c5103696a2729d3db2f235d202191788e4

-- 
Martin Michlmayr
http://www.cyrius.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help