sata_sil24: Use memory barriers before issuing commands
in [Kernel]
|
Prev: [PATCH for-2.6.35] virtio_net: do not reschedule rx refill forever
Next: [PATCH for-2.6.35] virtio_net: fix oom handling on tx
From: Grant Grundler on 25 Jun 2010 22:40 On Thu, Jun 10, 2010 at 7:57 AM, Catalin Marinas <catalin.marinas(a)arm.com> wrote: > The data in the cmd_block buffers may reach the main memory after the > writel() to the device ports. "ia-64 Linux Kernel" (mosberger and eranian) uses exactly this sequence as an example for wmb() on page 303. I'm curious about the system that exposed this problem. I believe wmb() fixes an issue not exposed on most machines. Can any general comments be made about cache coherency, memory ordering (weak?), instruction ordering (super scalar?), etc. ? The explanation above is a bit short (most people won't understand it). > This patch introduces two calls to wmb() to ensure the relative ordering. And as Tejun asked, the comment where wmb() gets used should clearly explain which host memory writes are targetted by the wmb(). thanks, grant > > Signed-off-by: Catalin Marinas <catalin.marinas(a)arm.com> > Tested-by: Colin Tuckley <colin.tuckley(a)arm.com> > Cc: Tejun Heo <tj(a)kernel.org> > --- > drivers/ata/sata_sil24.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c > index e925051..6392fdb 100644 > --- a/drivers/ata/sata_sil24.c > +++ b/drivers/ata/sata_sil24.c > @@ -622,6 +622,7 @@ static int sil24_exec_polled_cmd(struct ata_port *ap, int pmp, > irq_enabled = readl(port + PORT_IRQ_ENABLE_SET); > writel(PORT_IRQ_COMPLETE | PORT_IRQ_ERROR, port + PORT_IRQ_ENABLE_CLR); > > + wmb(); > writel((u32)paddr, port + PORT_CMD_ACTIVATE); > writel((u64)paddr >> 32, port + PORT_CMD_ACTIVATE + 4); > > @@ -895,6 +896,7 @@ static unsigned int sil24_qc_issue(struct ata_queued_cmd *qc) > paddr = pp->cmd_block_dma + tag * sizeof(*pp->cmd_block); > activate = port + PORT_CMD_ACTIVATE + tag * 8; > > + wmb(); > writel((u32)paddr, activate); > writel((u64)paddr >> 32, activate + 4); > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ide" in > the body of a message to majordomo(a)vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Catalin Marinas on 28 Jun 2010 05:30
Hi Grant, Grant Grundler <grundler(a)google.com> wrote: > On Thu, Jun 10, 2010 at 7:57 AM, Catalin Marinas > <catalin.marinas(a)arm.com> wrote: >> The data in the cmd_block buffers may reach the main memory after the >> writel() to the device ports. > > "ia-64 Linux Kernel" (mosberger and eranian) uses exactly this sequence > as an example for wmb() on page 303. > > I'm curious about the system that exposed this problem. I believe wmb() fixes > an issue not exposed on most machines. Can any general comments be > made about cache coherency, memory ordering (weak?), instruction ordering > (super scalar?), etc. ? > > The explanation above is a bit short (most people won't understand it). I already posted a second version of this patch, though it triggered a longer discussion on whether we should do this (cross-post between LKML, linux-ide and linux-arch): http://thread.gmane.org/gmane.linux.ide/46414 I know IA-64 and a several other architectures have weak memory ordering but some of them just add barriers in the I/O accessors (with some performance penalty). Since the (new) patch is already in mainline, please comment on the other thread for memory ordering etc. Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |