Re: [tip:irq/irqdomain] irqdomain: Introduce helper function irq_domain_add_hierarchy()

From: Jiang Liu
Date: Sat Nov 29 2014 - 09:29:45 EST

Next message: SF Markus Elfring: "[PATCH 1/1] HID: Wacom: Deletion of unnecessary checks before the function call "input_free_device""
Previous message: SF Markus Elfring: "[PATCH 1/1] net: cassini: Deletion of an unnecessary check before the function call "vfree""
In reply to: Borislav Petkov: "Re: [tip:irq/irqdomain] irqdomain: Introduce helper function irq_domain_add_hierarchy()"
Next in thread: Borislav Petkov: "Re: [tip:irq/irqdomain] irqdomain: Introduce helper function irq_domain_add_hierarchy()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 2014/11/29 20:53, Borislav Petkov wrote:
> On Sun, Nov 23, 2014 at 10:11:44AM -0800, tip-bot for Jiang Liu wrote:
>> Commit-ID: afb7da83b9f476728623130703acb553d7c7c4d9
>> Gitweb: http://git.kernel.org/tip/afb7da83b9f476728623130703acb553d7c7c4d9
>> Author: Jiang Liu <jiang.liu@xxxxxxxxxxxxxxx>
>> AuthorDate: Sat, 15 Nov 2014 22:24:02 +0800
>> Committer: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> CommitDate: Sun, 23 Nov 2014 13:01:46 +0100
>>
>> irqdomain: Introduce helper function irq_domain_add_hierarchy()
>>
>> Introduce helper function irq_domain_add_hierarchy(), which creates
>> a linear irqdomain if parameter 'size' is not zero, otherwise creates
>> a tree irqdomain.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@xxxxxxxxxxxxxxx>
>> Cc: Tony Luck <tony.luck@xxxxxxxxx>
>> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>> Cc: Grant Likely <grant.likely@xxxxxxxxxx>
>> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
>> Cc: Yijing Wang <wangyijing@xxxxxxxxxx>
>> Cc: Yingjoe Chen <yingjoe.chen@xxxxxxxxxxxx>
>> Cc: Borislav Petkov <bp@xxxxxxxxx>
>> Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
>> Cc: Matthias Brugger <matthias.bgg@xxxxxxxxx>
>> Link: http://lkml.kernel.org/r/1416061447-9472-5-git-send-email-jiang.liu@xxxxxxxxxxxxxxx
>> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> ---
>> include/linux/irqdomain.h | 4 ++++
>> kernel/irq/irqdomain.c | 36 ++++++++++++++++++++++++++++++++++++
>> 2 files changed, 40 insertions(+)
>>
>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>> index dd2709b..676d730 100644
>> --- a/include/linux/irqdomain.h
>> +++ b/include/linux/irqdomain.h
>> @@ -259,6 +259,10 @@ int irq_domain_xlate_onetwocell(struct irq_domain *d, struct device_node *ctrlr,
>> extern struct irq_data *irq_domain_get_irq_data(struct irq_domain *domain,
>> unsigned int virq);
>> #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
>> +extern struct irq_domain *irq_domain_add_hierarchy(struct irq_domain *parent,
>> + unsigned int flags, unsigned int size,
>> + struct device_node *node,
>> + const struct irq_domain_ops *ops, void *host_data);
>> extern int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
>> unsigned int nr_irqs, int node, void *arg,
>> bool realloc);
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index 9c88db7..7fac311 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -742,6 +742,42 @@ static int irq_domain_alloc_descs(int virq, unsigned int cnt,
>> }
>>
>> #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
>> +/**
>> + * irq_domain_add_hierarchy - Add a irqdomain into the hierarchy
>> + * @parent: Parent irq domain to associate with the new domain
>> + * @flags: Irq domain flags associated to the domain
>> + * @size: Size of the domain. See below
>> + * @node: Optional device-tree node of the interrupt controller
>> + * @ops: Pointer to the interrupt domain callbacks
>> + * @host_data: Controller private data pointer
>> + *
>> + * If @size is 0 a tree domain is created, otherwise a linear domain.
>> + *
>> + * If successful the parent is associated to the new domain and the
>> + * domain flags are set.
>> + * Returns pointer to IRQ domain, or NULL on failure.
>> + */
>> +struct irq_domain *irq_domain_add_hierarchy(struct irq_domain *parent,
>> + unsigned int flags,
>> + unsigned int size,
>> + struct device_node *node,
>> + const struct irq_domain_ops *ops,
>> + void *host_data)
>> +{
>> + struct irq_domain *domain;
>> +
>> + if (size)
>> + domain = irq_domain_add_linear(node, size, ops, host_data);
>> + else
>> + domain = irq_domain_add_tree(node, ops, host_data);
>> + if (domain) {
>> + domain->parent = parent;
>> + domain->flags |= flags;
>> + }
>
> Ok, I'm going to reply to this patch because it is on the splat path but
> fixing this would need a bit more reorganizing.
>
> So I'm seeing the lockdep splat below really early on an IVB laptop.
>
> Basically we're not supposed to do __GFP_FS allocations with IRQs off:
>
> 2737 /* We're only interested __GFP_FS allocations for now */
> 2738 if (!(gfp_mask & __GFP_FS))
> 2739 return;
> 2740
> 2741 /*
> 2742 * Oi! Can't be having __GFP_FS allocations with IRQs disabled.
> 2743 */
> 2744 if (DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))) <--- HERE!
> 2745 return;
> 2746
> 2747 mark_held_locks(curr, RECLAIM_FS);
> 2748 }
>
> Now, AFAICT, enable_IR_x2apic() disables interrupts and the whole init
> is done with IRQs off but down that path intel_setup_irq_remapping()
> calls irq_domain_add_hierarchy() and it does by default GFP_KERNEL
> allocations.
>
> The obvious fix is this and the machine boots fine with it. I'm not sure
> it is kosher though so I rather run it by people first:
>
> ---
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 7fac311057b8..c21a003b996a 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -46,14 +46,18 @@ struct irq_domain *__irq_domain_add(struct device_node *of_node, int size,
> void *host_data)
> {
> struct irq_domain *domain;
> + gfp_t gfp_flags = GFP_KERNEL;
> +
> + if (irqs_disabled())
> + gfp_flags = GFP_NOFS;
Hi Borislav,
Thanks for reporting and fixing this. How about using
GFP_ATOMIC here?
Regards!
Gerry

>
> domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
> - GFP_KERNEL, of_node_to_nid(of_node));
> + gfp_flags, of_node_to_nid(of_node));
> if (WARN_ON(!domain))
> return NULL;
>
> /* Fill structure */
> - INIT_RADIX_TREE(&domain->revmap_tree, GFP_KERNEL);
> + INIT_RADIX_TREE(&domain->revmap_tree, gfp_flags);
> domain->ops = ops;
> domain->host_data = host_data;
> domain->of_node = of_node_get(of_node);
> --
>
>
> [ 0.085221] dmar: Host address width 36
> [ 0.085298] dmar: DRHD base: 0x000000fed90000 flags: 0x0
> [ 0.085414] dmar: IOMMU 0: reg_base_addr fed90000 ver 1:0 cap c0000020e60262 ecap f0101a
> [ 0.085513] dmar: DRHD base: 0x000000fed91000 flags: 0x1
> [ 0.085598] dmar: IOMMU 1: reg_base_addr fed91000 ver 1:0 cap c9008020660262 ecap f0105a
> [ 0.085693] dmar: RMRR base: 0x000000da2ba000 end: 0x000000da2d0fff
> [ 0.085789] dmar: RMRR base: 0x000000db800000 end: 0x000000df9fffff
> [ 0.085955] IOAPIC id 2 under DRHD base 0xfed91000 IOMMU 1
> [ 0.086034] HPET id 0 under DRHD base 0xfed91000
> [ 0.086109] Queued invalidation will be enabled to support x2apic and Intr-remapping.
> [ 0.086351] ------------[ cut here ]------------
> [ 0.086438] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2744 lockdep_trace_alloc+0xd4/0xe0()
> [ 0.086537] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
> [ 0.086598] Modules linked in:
> [ 0.086783] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.18.0-rc6+ #1
> [ 0.086863] Hardware name: LENOVO 2320CTO/2320CTO, BIOS G2ET86WW (2.06 ) 11/13/2012
> [ 0.086959] 0000000000000009 ffff880213d07b58 ffffffff816502d7 0000000000000000
> [ 0.087240] ffff880213d07ba8 ffff880213d07b98 ffffffff81059790 0000000000000001
> [ 0.087521] 0000000000000092 0000000000000000 00000000000080d0 0000000000000000
> [ 0.087803] Call Trace:
> [ 0.087878] [<ffffffff816502d7>] dump_stack+0x4f/0x7b
> [ 0.087960] [<ffffffff81059790>] warn_slowpath_common+0x80/0xa0
> [ 0.088041] [<ffffffff810597f6>] warn_slowpath_fmt+0x46/0x50
> [ 0.088124] [<ffffffff810a5514>] lockdep_trace_alloc+0xd4/0xe0
> [ 0.088208] [<ffffffff8114877f>] __alloc_pages_nodemask+0x7f/0x970
> [ 0.088291] [<ffffffff81148158>] ? get_page_from_freelist+0x5b8/0x980
> [ 0.088375] [<ffffffff8109f72f>] ? trace_hardirqs_off_caller+0x1f/0xc0
> [ 0.088457] [<ffffffff811490af>] alloc_kmem_pages_node+0x3f/0x60
> [ 0.088542] [<ffffffff8119280e>] kmalloc_large_node+0x2e/0x70
> [ 0.088622] [<ffffffff811989a5>] __kmalloc_node+0x215/0x2b0
> [ 0.088707] [<ffffffff81539c10>] ? __dmar_enable_qi+0x40/0x130
> [ 0.088789] [<ffffffff810bafe1>] __irq_domain_add+0x41/0x110
> [ 0.088871] [<ffffffff810bb553>] irq_domain_add_hierarchy+0x23/0x50
> [ 0.088952] [<ffffffff81651f33>] intel_setup_irq_remapping.part.4+0xdc/0x169
> [ 0.089039] [<ffffffff81d61123>] intel_enable_irq_remapping+0x240/0x2f9
> [ 0.089123] [<ffffffff81d61315>] irq_remapping_enable+0x20/0x36
> [ 0.089208] [<ffffffff81d2cbaf>] enable_IR+0x3c/0x3e
> [ 0.089288] [<ffffffff81d2cd92>] enable_IR_x2apic+0x82/0x120
> [ 0.089369] [<ffffffff81d2e614>] default_setup_apic_routing+0x12/0x6b
> [ 0.089453] [<ffffffff81d2aadc>] native_smp_prepare_cpus+0x29b/0x39f
> [ 0.089538] [<ffffffff81d1df5d>] kernel_init_freeable+0x5f/0x1a2
> [ 0.089622] [<ffffffff8165933f>] ? ret_from_fork+0xf/0xb0
> [ 0.089705] [<ffffffff8164a640>] ? rest_init+0xd0/0xd0
> [ 0.089787] [<ffffffff8164a64e>] kernel_init+0xe/0xf0
> [ 0.089867] [<ffffffff816593ac>] ret_from_fork+0x7c/0xb0
> [ 0.089948] [<ffffffff8164a640>] ? rest_init+0xd0/0xd0
> [ 0.090032] ---[ end trace fac50e785fc22942 ]---
> [ 0.090387] Enabled IRQ remapping in x2apic mode
> [ 0.090466] Enabling x2apic
> [ 0.090537] Enabled x2apic
> [ 0.090630] Switched APIC routing to cluster x2apic.
> [ 0.091287] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
> [ 0.101375] smpboot: CPU0: Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz (fam: 06, model: 3a, stepping: 09)
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: SF Markus Elfring: "[PATCH 1/1] HID: Wacom: Deletion of unnecessary checks before the function call "input_free_device""
Previous message: SF Markus Elfring: "[PATCH 1/1] net: cassini: Deletion of an unnecessary check before the function call "vfree""
In reply to: Borislav Petkov: "Re: [tip:irq/irqdomain] irqdomain: Introduce helper function irq_domain_add_hierarchy()"
Next in thread: Borislav Petkov: "Re: [tip:irq/irqdomain] irqdomain: Introduce helper function irq_domain_add_hierarchy()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]