On Tue, 6 Mar 2007, Jeff Garzik wrote:
> Dave Dillow wrote:
> > BUG: at drivers/ata/sata_mv.c:1245 mv_qc_issue()
> > BUG: at drivers/ata/sata_mv.c:1245 mv_qc_issue()
> > BUG: at drivers/ata/sata_mv.c:1245 mv_qc_issue()
> > BUG: at drivers/ata/sata_mv.c:1245 mv_qc_issue()
> > BUG: at drivers/ata/sata_mv.c:1291 mv_get_crpb_status()
>
> Well, all these seem to be WARN_ON() statements that will fire if NCQ queueing
> is enabled, which it should not be.
>
> I also note that my test disk is not NCQ-capable, but the testers like you
> getting the BUG: output are attaching NCQ-capable disks.
>
> So it sounds like something changes for the worst in NCQ land, which is area I
> did not touch for the new-EH work. However, I do remember cleaning up the
> EDMA configuration in a patch (also sent upstream).
>
> Does the attached patch fix things? It should apply on top of linux-2.6.git
> upstream, or libata-dev.git#mv-eh. This patch merely reverts commit
> e728eabea110da90e69c05855e3a11174edb77ef.
no luck... this actually made it worse :) now it's spewing BUGs a lot
more rapidly (without the revert it would error once a minute or so and
didn't make other progress). my tree is 2.6.21-rc1 + your "RFT, v2" patch
+ this revert.
so i threw in some extra printks (see patch)... here's some samples:
[ 87.798423] mv_qc_issue: in_ptr = 0x37fe0060, in_index = 0x3, EDMA_REQ_Q_OUT_PTR_OFS = 0x40
[ 87.806974] BUG: at drivers/ata/sata_mv.c:1223 mv_qc_issue()
[ 88.039421] mv_get_crpb_status: out_index = 0x2, EDMA_RSP_Q_IN_PTR_OFS = 0x18
[ 88.047103] BUG: at drivers/ata/sata_mv.c:1273 mv_get_crpb_status()
[ 88.312250] mv_get_crpb_status: out_index = 0x2, EDMA_RSP_Q_IN_PTR_OFS = 0x20
[ 88.319925] BUG: at drivers/ata/sata_mv.c:1273 mv_get_crpb_status()
[ 88.585094] mv_get_crpb_status: out_index = 0x3, EDMA_RSP_Q_IN_PTR_OFS = 0x20
[ 88.592771] BUG: at drivers/ata/sata_mv.c:1273 mv_get_crpb_status()
[ 89.152245] mv_get_crpb_status: out_index = 0x4, EDMA_RSP_Q_IN_PTR_OFS = 0x28
[ 89.159547] BUG: at drivers/ata/sata_mv.c:1273 mv_get_crpb_status()
[ 89.543206] mv_get_crpb_status: out_index = 0x5, EDMA_RSP_Q_IN_PTR_OFS = 0x30
[ 89.550516] BUG: at drivers/ata/sata_mv.c:1273 mv_get_crpb_status()
[ 90.382613] mv_get_crpb_status: out_index = 0x6, EDMA_RSP_Q_IN_PTR_OFS = 0x38
[ 90.389933] BUG: at drivers/ata/sata_mv.c:1273 mv_get_crpb_status()
...
hope that helps...
-dean
Index: linux/drivers/ata/sata_mv.c
===================================================================
--- linux.orig/drivers/ata/sata_mv.c 2007-03-05 22:27:51.000000000 -0800
+++ linux/drivers/ata/sata_mv.c 2007-03-05 22:38:26.000000000 -0800
@@ -1201,6 +1201,7 @@
struct mv_port_priv *pp = qc->ap->private_data;
unsigned in_index;
u32 in_ptr;
+ unsigned tmp;
if (ATA_PROT_DMA != qc->tf.protocol) {
/* We're about to send a non-EDMA capable command to the
@@ -1215,8 +1216,12 @@
in_index = (in_ptr >> EDMA_REQ_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK;
/* until we do queuing, the queue should be empty at this point */
- WARN_ON(in_index != ((readl(port_mmio + EDMA_REQ_Q_OUT_PTR_OFS)
- >> EDMA_REQ_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK));
+ tmp = readl(port_mmio + EDMA_REQ_Q_OUT_PTR_OFS);
+ if (in_index != ((tmp >> EDMA_REQ_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK)) {
+ printk(KERN_INFO "mv_qc_issue: in_ptr = 0x%x, in_index = 0x%x, EDMA_REQ_Q_OUT_PTR_OFS = 0x%x\n",
+ in_ptr, in_index, tmp);
+ WARN_ON(1);
+ }
in_index = mv_inc_q_index(in_index); /* now incr producer index */
@@ -1250,6 +1255,7 @@
unsigned out_index;
u32 out_ptr;
u8 ata_status;
+ unsigned tmp;
out_ptr = readl(port_mmio + EDMA_RSP_Q_OUT_PTR_OFS);
out_index = (out_ptr >> EDMA_RSP_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK;
@@ -1261,8 +1267,11 @@
out_index = mv_inc_q_index(out_index);
/* and, until we do NCQ, there should only be 1 CRPB waiting */
- WARN_ON(out_index != ((readl(port_mmio + EDMA_RSP_Q_IN_PTR_OFS)
- >> EDMA_RSP_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK));
+ tmp = readl(port_mmio + EDMA_RSP_Q_IN_PTR_OFS);
+ if (out_index != ((tmp >> EDMA_RSP_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK)) {
+ printk(KERN_INFO "mv_get_crpb_status: out_index = 0x%x, EDMA_RSP_Q_IN_PTR_OFS = 0x%x\n", out_index, tmp);
+ WARN_ON(1);
+ }
/* write out our inc'd consumer index so EDMA knows we're caught up */
out_ptr &= EDMA_RSP_Q_BASE_LO_MASK;
