Hi Keith,
On Mon, Aug 08, 2016 at 09:47:51AM -0600, Keith Busch wrote:
> We can't initialize the list head on deletion as this causes the node
> to point to itself, looping infinitely if the vmd IRQ handler happened
> to be servicing that node.
>
> The list initialization was trying fix a bug from multiple calls to
> disable the same IRQ. We can fix this instead by having the vmd driver
> track if the interrupt is enabled.
>
> Signed-off-by: Keith Busch <keith.busch@xxxxxxxxx>
> Cc: Jon Derrick <jonathan.derrick@xxxxxxxxx>
> Reported-by: Grzegorz Koczot <grzegorz.koczot@xxxxxxxxx>
> Tested-by: Miroslaw Drost <miroslaw.drost@xxxxxxxxx>
I assume you want
Fixes: 97e923063575 ("x86/PCI: VMD: Initialize list item in IRQ disable")
and you want this in for-linus so it goes in v4.8 instead of v4.9.
I did that for you, so let me know if it's not right.
> ---
> v1 -> v2:
>
> Removed check for poisioned list pointer. This is abusing knowledge
> internal to the list api, so we need another way to know if the child
> irq is enabled. This patch uses a driver controlled boolean for this.
>
> Added tags for the bug reporter and tester.
>
> arch/x86/pci/vmd.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c
> index e88b417..4b85837 100644
> --- a/arch/x86/pci/vmd.c
> +++ b/arch/x86/pci/vmd.c
> @@ -41,6 +41,7 @@ static DEFINE_RAW_SPINLOCK(list_lock);
> * @node: list item for parent traversal.
> * @rcu: RCU callback item for freeing.
> * @irq: back pointer to parent.
> + * @enabled: true if driver enabled irq
> * @virq: the virtual IRQ value provided to the requesting driver.
> *
> * Every MSI/MSI-X IRQ requested for a device in a VMD domain will be mapped to
> @@ -50,6 +51,7 @@ struct vmd_irq {
> struct list_head node;
> struct rcu_head rcu;
> struct vmd_irq_list *irq;
> + bool enabled;
> unsigned int virq;
> };
>
> @@ -122,7 +124,9 @@ static void vmd_irq_enable(struct irq_data *data)
> unsigned long flags;
>
> raw_spin_lock_irqsave(&list_lock, flags);
> + WARN_ON(vmdirq->enabled);
> list_add_tail_rcu(&vmdirq->node, &vmdirq->irq->irq_list);
> + vmdirq->enabled = true;
> raw_spin_unlock_irqrestore(&list_lock, flags);
>
> data->chip->irq_unmask(data);
> @@ -136,8 +140,10 @@ static void vmd_irq_disable(struct irq_data *data)
> data->chip->irq_mask(data);
>
> raw_spin_lock_irqsave(&list_lock, flags);
> - list_del_rcu(&vmdirq->node);
> - INIT_LIST_HEAD_RCU(&vmdirq->node);
> + if (vmdirq->enabled) {
> + list_del_rcu(&vmdirq->node);
> + vmdirq->enabled = false;
> + }
> raw_spin_unlock_irqrestore(&list_lock, flags);
> }
>
> --
> 2.7.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
