Include da87ca4d4ca101f177fffd84f1f0a5e4c0343557 (ioat: fix tasklet tear down) for 3.2, 3.5, 3.8
Brad Figg
brad.figg at canonical.com
Mon Mar 31 16:17:15 UTC 2014
On 03/31/2014 08:43 AM, Tim Gardner wrote:
> 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
Yes, I think we should go with Ben's version.
Brad
>
> 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);
>> }
>>
>>
>>
>
>
--
Brad Figg brad.figg at canonical.com http://www.canonical.com
More information about the kernel-team
mailing list