Include da87ca4d4ca101f177fffd84f1f0a5e4c0343557 (ioat: fix tasklet tear down) for 3.2, 3.5, 3.8
Tim Gardner
tim.gardner at canonical.com
Mon Mar 31 15:43:00 UTC 2014
Brad - shall we replace the 2 Precise ioat patches in favour of upstream
3.2 stable ? You haven't turned the crank on packaging yet.
rtg
On 03/30/2014 03:29 PM, Ben Hutchings wrote:
> On Sat, 2014-03-15 at 15:05 -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.
>>
>> 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).
>
> I don't think it's necessary to add the irq_mode field. The fix relies
> on knowing whether the device is using MSI-X interrupts or not, and that
> can be checked quite easily. The version I ended up with for 3.2 is
> below.
>
> Ben.
>
> ---
> From: Dan Williams <dan.j.williams at intel.com>
> Date: Wed, 19 Feb 2014 16:19:35 -0800
> Subject: ioat: fix tasklet tear down
>
> commit da87ca4d4ca101f177fffd84f1f0a5e4c0343557 upstream.
>
> 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 <mingo at elte.hu>
> Cc: Steven Rostedt <rostedt at goodmis.org>
> Reported-by: Mike Galbraith <bitbucket at online.de>
> Reported-by: Stanislav Fomichev <stfomichev at yandex-team.ru>
> Tested-by: Mike Galbraith <bitbucket at online.de>
> Tested-by: Stanislav Fomichev <stfomichev at yandex-team.ru>
> Reviewed-by: Thomas Gleixner <tglx at linutronix.de>
> Signed-off-by: Dan Williams <dan.j.williams at intel.com>
> [bwh: Backported to 3.2:
> - Adjust context
> - As there is no ioatdma_device::irq_mode member, check
> pci_dev::msix_enabled instead]
> Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
> ---
> --- a/drivers/dma/ioat/dma.c
> +++ b/drivers/dma/ioat/dma.c
> @@ -75,7 +75,8 @@ static irqreturn_t ioat_dma_do_interrupt
> 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
> {
> 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_de
> 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,43 @@ static int ioat1_dma_alloc_chan_resource
> 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);
> +
> + /* 1/ stop irq from firing tasklets
> + * 2/ stop the tasklet from re-arming irqs
> + */
> + clear_bit(IOAT_RUN, &chan->state);
> +
> + /* flush inflight interrupts */
> +#ifdef CONFIG_PCI_MSI
> + if (pdev->msix_enabled) {
> + struct msix_entry *msix = &device->msix_entries[chan_id];
> + synchronize_irq(msix->vector);
> + } else
> +#endif
> + synchronize_irq(pdev->irq);
> +
> + /* 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 +412,7 @@ static void ioat1_dma_free_chan_resource
> 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 +557,11 @@ ioat1_dma_prep_memcpy(struct dma_chan *c
> 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);
> }
>
> --- a/drivers/dma/ioat/dma.h
> +++ b/drivers/dma/ioat/dma.h
> @@ -344,6 +344,7 @@ bool ioat_cleanup_preamble(struct ioat_c
> 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);
> +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;
> --- a/drivers/dma/ioat/dma_v2.c
> +++ b/drivers/dma/ioat/dma_v2.c
> @@ -189,8 +189,11 @@ static void ioat2_cleanup(struct ioat2_d
> 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 dm
> 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 dm
> } 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 dm
> 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);
> --- a/drivers/dma/ioat/dma_v3.c
> +++ b/drivers/dma/ioat/dma_v3.c
> @@ -325,8 +325,11 @@ static void ioat3_cleanup(struct ioat2_d
> 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);
> }
>
>
>
--
Tim Gardner tim.gardner at canonical.com
More information about the kernel-team
mailing list