Hi, Zoltan Boszormenyi írta:
I will test the code without this locking. Can you give me a better idea besides beating both my disks with separate requests? Say, bonnie++ and simultaneous hdparm -tT on both?
I tested the driver without locking with the above test, it survived nicely. Patch is attached which deletes locking, enables swncq by default and a correction to my previous readability cleanup. Best regards, Zoltán Böszörményi
--- linux-2.6.23-rc3-mm1/drivers/ata/sata_nv.c.committed 2007-09-21 08:32:04.000000000 +0200 +++ linux-2.6.23-rc3-mm1/drivers/ata/sata_nv.c 2007-09-21 09:27:49.000000000 +0200 @@ -279,7 +279,6 @@ unsigned int last_issue_tag; - spinlock_t lock; /* fifo circular queue to store deferral command */ struct defer_queue defer_queue; @@ -637,7 +636,7 @@ MODULE_VERSION(DRV_VERSION); static int adma_enabled = 1; -static int swncq_enabled; +static int swncq_enabled = 1; static void nv_adma_register_mode(struct ata_port *ap) { @@ -1965,7 +1964,6 @@ pp->sactive_block = ap->ioaddr.scr_addr + 4 * SCR_ACTIVE; pp->irq_block = mmio + NV_INT_STATUS_MCP55 + ap->port_no * 2; pp->tag_block = mmio + NV_NCQ_REG_MCP55 + ap->port_no * 2; - spin_lock_init(&pp->lock); return 0; } @@ -2051,18 +2049,15 @@ { struct ata_port *ap = qc->ap; struct nv_swncq_port_priv *pp = ap->private_data; - unsigned long flags; if (qc->tf.protocol != ATA_PROT_NCQ) return ata_qc_issue_prot(qc); DPRINTK("Enter\n"); - spin_lock_irqsave(&pp->lock, flags); if (!pp->qc_active) nv_swncq_issue_atacmd(ap, qc); else nv_swncq_qc_to_dq(ap, qc); /* add qc to defer queue */ - spin_unlock_irqrestore(&pp->lock, flags); return 0; } @@ -2265,7 +2260,6 @@ return; } - spin_lock(&pp->lock); if (fis & NV_SWNCQ_IRQ_BACKOUT) { /* If the IRQ is backout, driver must issue * the new command again some time later. @@ -2290,8 +2284,7 @@ */ pp->dhfis_bits |= (0x1 << pp->last_issue_tag); pp->ncq_flags |= ncq_saw_d2h; - if ((pp->ncq_flags & ncq_saw_sdb) || - (pp->ncq_flags & ncq_saw_backout)) { + if (pp->ncq_flags & (ncq_saw_sdb | ncq_saw_backout)) { ata_ehi_push_desc(ehi, "illegal fis transaction"); ehi->err_mask |= AC_ERR_HSM; ehi->action |= ATA_EH_HARDRESET; @@ -2302,7 +2295,7 @@ !(pp->ncq_flags & ncq_saw_dmas)) { ata_stat = ap->ops->check_status(ap); if (ata_stat & ATA_BUSY) - goto irq_exit; + return; if (pp->defer_queue.defer_bits) { DPRINTK("send next command\n"); @@ -2321,13 +2314,11 @@ rc = nv_swncq_dmafis(ap); } -irq_exit: - spin_unlock(&pp->lock); return; + irq_error: ata_ehi_push_desc(ehi, "fis:0x%x", fis); ata_port_freeze(ap); - spin_unlock(&pp->lock); return; }