(Adding David and Ben to CC list) On Sat, Mar 15, 2014 at 03:05:33PM -0600, Tim Gardner wrote: > Dan - attached are 2 patches for stable inclusion in kernel versions > 3.2, 3.5, and 3.8. The first patch > 8a52b9ff1154a68b6a2a8da9a31a87e52f5f6418 (ioatdma: channel reset > scheme fixup on Intel Atom S1200 platforms) is a partial backport > sufficient to allow a clean cherry-pick of the second patch > da87ca4d4ca101f177fffd84f1f0a5e4c0343557 (ioat: fix tasklet tear > down). > > Please advise if you think this is correct. I should get test > results in http://bugs.launchpad.net/bugs/1291113 sometime soon. >
This definitely seems to be stable material. There is positive feedback on the bug report so, if there are no objections, I would like to queue these patches for the 3.5 kernel. Cheers, -- Luís > Stable releases 3.11.10.6, 3.12.14, and 3.13.6 are up to date wrt to > da87ca4d4ca101f177fffd84f1f0a5e4c0343557 (ioat: fix tasklet tear > down). > > rtg > -- > Tim Gardner [email protected] > From e2ff589a895d119146b46ec5579cc51792641218 Mon Sep 17 00:00:00 2001 > From: Dave Jiang <[email protected]> > Date: Tue, 26 Mar 2013 15:42:47 -0700 > Subject: [PATCH 1/2] ioatdma: channel reset scheme fixup on Intel Atom S1200 > platforms > > BugLink: http://bugs.launchpad.net/bugs/1291113 > > The Intel Atom S1200 family ioatdma changed the channel reset behavior. > It does a reset similar to PCI FLR by resetting all the MSIX > registers. We have to re-init msix interrupts because of this. This > workaround is only specific to this platform and is not expected to carry > over to the later generations. > > Signed-off-by: Dave Jiang <[email protected]> > Acked-by: Dan Williams <[email protected]> > Signed-off-by: Vinod Koul <[email protected]> > (back ported from commit 8a52b9ff1154a68b6a2a8da9a31a87e52f5f6418) > > Back port notes: I implemented enough of this patch to support > device->irq_mode storage, upon which da87ca4d4ca101f177fffd84f1f0a5e4c0343557 > (ioat: fix tasklet tear down) is dependent. > > Signed-off-by: Tim Gardner <[email protected]> > --- > drivers/dma/ioat/dma.c | 8 +++++++- > drivers/dma/ioat/dma.h | 10 ++++++++++ > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c > index 6595180..fe5152a 100644 > --- a/drivers/dma/ioat/dma.c > +++ b/drivers/dma/ioat/dma.c > @@ -890,7 +890,7 @@ MODULE_PARM_DESC(ioat_interrupt_style, > * ioat_dma_setup_interrupts - setup interrupt handler > * @device: ioat device > */ > -static int ioat_dma_setup_interrupts(struct ioatdma_device *device) > +int ioat_dma_setup_interrupts(struct ioatdma_device *device) > { > struct ioat_chan_common *chan; > struct pci_dev *pdev = device->pdev; > @@ -939,6 +939,7 @@ msix: > } > } > intrctrl |= IOAT_INTRCTRL_MSIX_VECTOR_CONTROL; > + device->irq_mode = IOAT_MSIX; > goto done; > > msix_single_vector: > @@ -954,6 +955,7 @@ msix_single_vector: > pci_disable_msix(pdev); > goto msi; > } > + device->irq_mode = IOAT_MSIX_SINGLE; > goto done; > > msi: > @@ -967,6 +969,7 @@ msi: > pci_disable_msi(pdev); > goto intx; > } > + device->irq_mode = IOAT_MSIX; > goto done; > > intx: > @@ -975,6 +978,7 @@ intx: > if (err) > goto err_no_irq; > > + device->irq_mode = IOAT_INTX; > done: > if (device->intr_quirk) > device->intr_quirk(device); > @@ -985,9 +989,11 @@ done: > err_no_irq: > /* Disable all interrupt generation */ > writeb(0, device->reg_base + IOAT_INTRCTRL_OFFSET); > + device->irq_mode = IOAT_NOIRQ; > dev_err(dev, "no usable interrupts\n"); > return err; > } > +EXPORT_SYMBOL(ioat_dma_setup_interrupts); > > static void ioat_disable_interrupts(struct ioatdma_device *device) > { > diff --git a/drivers/dma/ioat/dma.h b/drivers/dma/ioat/dma.h > index 8bebddd..148883e 100644 > --- a/drivers/dma/ioat/dma.h > +++ b/drivers/dma/ioat/dma.h > @@ -48,6 +48,14 @@ > */ > #define NULL_DESC_BUFFER_SIZE 1 > > +enum ioat_irq_mode { > + IOAT_NOIRQ = 0, > + IOAT_MSIX, > + IOAT_MSIX_SINGLE, > + IOAT_MSI, > + IOAT_INTX > +}; > + > /** > * struct ioatdma_device - internal representation of a IOAT device > * @pdev: PCI-Express device > @@ -77,6 +85,7 @@ struct ioatdma_device { > struct msix_entry msix_entries[4]; > struct ioat_chan_common *idx[4]; > struct dca_provider *dca; > + enum ioat_irq_mode irq_mode; > void (*intr_quirk)(struct ioatdma_device *device); > int (*enumerate_channels)(struct ioatdma_device *device); > int (*reset_hw)(struct ioat_chan_common *chan); > @@ -344,6 +353,7 @@ bool ioat_cleanup_preamble(struct ioat_chan_common *chan, > dma_addr_t *phys_complete); > void ioat_kobject_add(struct ioatdma_device *device, struct kobj_type *type); > void ioat_kobject_del(struct ioatdma_device *device); > +int ioat_dma_setup_interrupts(struct ioatdma_device *device); > extern const struct sysfs_ops ioat_sysfs_ops; > extern struct ioat_sysfs_entry ioat_version_attr; > extern struct ioat_sysfs_entry ioat_cap_attr; > -- > 1.7.9.5 > > From 48fbb694150ff1ae04d622bfb0771b7e03a068cd Mon Sep 17 00:00:00 2001 > From: Dan Williams <[email protected]> > Date: Wed, 19 Feb 2014 16:19:35 -0800 > Subject: [PATCH 2/2] ioat: fix tasklet tear down > > BugLink: http://bugs.launchpad.net/bugs/1291113 > > Since commit 77873803363c "net_dma: mark broken" we no longer pin dma > engines active for the network-receive-offload use case. As a result > the ->free_chan_resources() that occurs after the driver self test no > longer has a NET_DMA induced ->alloc_chan_resources() to back it up. A > late firing irq can lead to ksoftirqd spinning indefinitely due to the > tasklet_disable() performed by ->free_chan_resources(). Only > ->alloc_chan_resources() can clear this condition in affected kernels. > > This problem has been present since commit 3e037454bcfa "I/OAT: Add > support for MSI and MSI-X" in 2.6.24, but is now exposed. Given the > NET_DMA use case is deprecated we can revisit moving the driver to use > threaded irqs. For now, just tear down the irq and tasklet properly by: > > 1/ Disable the irq from triggering the tasklet > > 2/ Disable the irq from re-arming > > 3/ Flush inflight interrupts > > 4/ Flush the timer > > 5/ Flush inflight tasklets > > References: > https://lkml.org/lkml/2014/1/27/282 > https://lkml.org/lkml/2014/2/19/672 > > Cc: Ingo Molnar <[email protected]> > Cc: Steven Rostedt <[email protected]> > Cc: <[email protected]> > Reported-by: Mike Galbraith <[email protected]> > Reported-by: Stanislav Fomichev <[email protected]> > Tested-by: Mike Galbraith <[email protected]> > Tested-by: Stanislav Fomichev <[email protected]> > Reviewed-by: Thomas Gleixner <[email protected]> > Signed-off-by: Dan Williams <[email protected]> > (cherry picked from commit da87ca4d4ca101f177fffd84f1f0a5e4c0343557) > > Signed-off-by: Tim Gardner <[email protected]> > --- > drivers/dma/ioat/dma.c | 52 > +++++++++++++++++++++++++++++++++++++++------ > drivers/dma/ioat/dma.h | 1 + > drivers/dma/ioat/dma_v2.c | 11 +++++----- > drivers/dma/ioat/dma_v3.c | 3 +++ > 4 files changed, 54 insertions(+), 13 deletions(-) > > diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c > index fe5152a..1bb65da 100644 > --- a/drivers/dma/ioat/dma.c > +++ b/drivers/dma/ioat/dma.c > @@ -75,7 +75,8 @@ static irqreturn_t ioat_dma_do_interrupt(int irq, void > *data) > attnstatus = readl(instance->reg_base + IOAT_ATTNSTATUS_OFFSET); > for_each_set_bit(bit, &attnstatus, BITS_PER_LONG) { > chan = ioat_chan_by_index(instance, bit); > - tasklet_schedule(&chan->cleanup_task); > + if (test_bit(IOAT_RUN, &chan->state)) > + tasklet_schedule(&chan->cleanup_task); > } > > writeb(intrctrl, instance->reg_base + IOAT_INTRCTRL_OFFSET); > @@ -91,7 +92,8 @@ static irqreturn_t ioat_dma_do_interrupt_msix(int irq, void > *data) > { > struct ioat_chan_common *chan = data; > > - tasklet_schedule(&chan->cleanup_task); > + if (test_bit(IOAT_RUN, &chan->state)) > + tasklet_schedule(&chan->cleanup_task); > > return IRQ_HANDLED; > } > @@ -113,7 +115,6 @@ void ioat_init_channel(struct ioatdma_device *device, > struct ioat_chan_common *c > chan->timer.function = device->timer_fn; > chan->timer.data = data; > tasklet_init(&chan->cleanup_task, device->cleanup_fn, data); > - tasklet_disable(&chan->cleanup_task); > } > > /** > @@ -356,13 +357,49 @@ static int ioat1_dma_alloc_chan_resources(struct > dma_chan *c) > writel(((u64) chan->completion_dma) >> 32, > chan->reg_base + IOAT_CHANCMP_OFFSET_HIGH); > > - tasklet_enable(&chan->cleanup_task); > + set_bit(IOAT_RUN, &chan->state); > ioat1_dma_start_null_desc(ioat); /* give chain to dma device */ > dev_dbg(to_dev(chan), "%s: allocated %d descriptors\n", > __func__, ioat->desccount); > return ioat->desccount; > } > > +void ioat_stop(struct ioat_chan_common *chan) > +{ > + struct ioatdma_device *device = chan->device; > + struct pci_dev *pdev = device->pdev; > + int chan_id = chan_num(chan); > + struct msix_entry *msix; > + > + /* 1/ stop irq from firing tasklets > + * 2/ stop the tasklet from re-arming irqs > + */ > + clear_bit(IOAT_RUN, &chan->state); > + > + /* flush inflight interrupts */ > + switch (device->irq_mode) { > + case IOAT_MSIX: > + msix = &device->msix_entries[chan_id]; > + synchronize_irq(msix->vector); > + break; > + case IOAT_MSI: > + case IOAT_INTX: > + synchronize_irq(pdev->irq); > + break; > + default: > + break; > + } > + > + /* flush inflight timers */ > + del_timer_sync(&chan->timer); > + > + /* flush inflight tasklet runs */ > + tasklet_kill(&chan->cleanup_task); > + > + /* final cleanup now that everything is quiesced and can't re-arm */ > + device->cleanup_fn((unsigned long) &chan->common); > +} > + > /** > * ioat1_dma_free_chan_resources - release all the descriptors > * @chan: the channel to be cleaned > @@ -381,9 +418,7 @@ static void ioat1_dma_free_chan_resources(struct dma_chan > *c) > if (ioat->desccount == 0) > return; > > - tasklet_disable(&chan->cleanup_task); > - del_timer_sync(&chan->timer); > - ioat1_cleanup(ioat); > + ioat_stop(chan); > > /* Delay 100ms after reset to allow internal DMA logic to quiesce > * before removing DMA descriptor resources. > @@ -528,8 +563,11 @@ ioat1_dma_prep_memcpy(struct dma_chan *c, dma_addr_t > dma_dest, > static void ioat1_cleanup_event(unsigned long data) > { > struct ioat_dma_chan *ioat = to_ioat_chan((void *) data); > + struct ioat_chan_common *chan = &ioat->base; > > ioat1_cleanup(ioat); > + if (!test_bit(IOAT_RUN, &chan->state)) > + return; > writew(IOAT_CHANCTRL_RUN, ioat->base.reg_base + IOAT_CHANCTRL_OFFSET); > } > > diff --git a/drivers/dma/ioat/dma.h b/drivers/dma/ioat/dma.h > index 148883e..d70124b 100644 > --- a/drivers/dma/ioat/dma.h > +++ b/drivers/dma/ioat/dma.h > @@ -354,6 +354,7 @@ bool ioat_cleanup_preamble(struct ioat_chan_common *chan, > void ioat_kobject_add(struct ioatdma_device *device, struct kobj_type *type); > void ioat_kobject_del(struct ioatdma_device *device); > int ioat_dma_setup_interrupts(struct ioatdma_device *device); > +void ioat_stop(struct ioat_chan_common *chan); > extern const struct sysfs_ops ioat_sysfs_ops; > extern struct ioat_sysfs_entry ioat_version_attr; > extern struct ioat_sysfs_entry ioat_cap_attr; > diff --git a/drivers/dma/ioat/dma_v2.c b/drivers/dma/ioat/dma_v2.c > index cb8864d..e60933e 100644 > --- a/drivers/dma/ioat/dma_v2.c > +++ b/drivers/dma/ioat/dma_v2.c > @@ -189,8 +189,11 @@ static void ioat2_cleanup(struct ioat2_dma_chan *ioat) > void ioat2_cleanup_event(unsigned long data) > { > struct ioat2_dma_chan *ioat = to_ioat2_chan((void *) data); > + struct ioat_chan_common *chan = &ioat->base; > > ioat2_cleanup(ioat); > + if (!test_bit(IOAT_RUN, &chan->state)) > + return; > writew(IOAT_CHANCTRL_RUN, ioat->base.reg_base + IOAT_CHANCTRL_OFFSET); > } > > @@ -542,10 +545,10 @@ int ioat2_alloc_chan_resources(struct dma_chan *c) > ioat->issued = 0; > ioat->tail = 0; > ioat->alloc_order = order; > + set_bit(IOAT_RUN, &chan->state); > spin_unlock_bh(&ioat->prep_lock); > spin_unlock_bh(&chan->cleanup_lock); > > - tasklet_enable(&chan->cleanup_task); > ioat2_start_null_desc(ioat); > > /* check that we got off the ground */ > @@ -555,7 +558,6 @@ int ioat2_alloc_chan_resources(struct dma_chan *c) > } while (i++ < 20 && !is_ioat_active(status) && !is_ioat_idle(status)); > > if (is_ioat_active(status) || is_ioat_idle(status)) { > - set_bit(IOAT_RUN, &chan->state); > return 1 << ioat->alloc_order; > } else { > u32 chanerr = readl(chan->reg_base + IOAT_CHANERR_OFFSET); > @@ -798,11 +800,8 @@ void ioat2_free_chan_resources(struct dma_chan *c) > if (!ioat->ring) > return; > > - tasklet_disable(&chan->cleanup_task); > - del_timer_sync(&chan->timer); > - device->cleanup_fn((unsigned long) c); > + ioat_stop(chan); > device->reset_hw(chan); > - clear_bit(IOAT_RUN, &chan->state); > > spin_lock_bh(&chan->cleanup_lock); > spin_lock_bh(&ioat->prep_lock); > diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c > index 714560f..8680031 100644 > --- a/drivers/dma/ioat/dma_v3.c > +++ b/drivers/dma/ioat/dma_v3.c > @@ -325,8 +325,11 @@ static void ioat3_cleanup(struct ioat2_dma_chan *ioat) > static void ioat3_cleanup_event(unsigned long data) > { > struct ioat2_dma_chan *ioat = to_ioat2_chan((void *) data); > + struct ioat_chan_common *chan = &ioat->base; > > ioat3_cleanup(ioat); > + if (!test_bit(IOAT_RUN, &chan->state)) > + return; > writew(IOAT_CHANCTRL_RUN, ioat->base.reg_base + IOAT_CHANCTRL_OFFSET); > } > > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
