linux-kernel - Re: [tip:irq/irqdomain] irqdomain: Introduce helper function irq_domain_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <547BD086.1090903@linux.intel.com>
Date:	Mon, 01 Dec 2014 10:20:54 +0800
From:	Jiang Liu <jiang.liu@...ux.intel.com>
To:	Thomas Gleixner <tglx@...utronix.de>,
	Borislav Petkov <bp@...en8.de>
CC:	linux-tip-commits@...r.kernel.org, matthias.bgg@...il.com,
	tony.luck@...el.com, benh@...nel.crashing.org,
	linux-kernel@...r.kernel.org, hpa@...or.com,
	grant.likely@...aro.org, wangyijing@...wei.com,
	marc.zyngier@....com, bhelgaas@...gle.com,
	yingjoe.chen@...iatek.com, mingo@...nel.org
Subject: Re: [tip:irq/irqdomain] irqdomain: Introduce helper function irq_domain_add_hierarchy()



On 2014/11/30 4:42, Thomas Gleixner wrote:
> On Sat, 29 Nov 2014, Borislav Petkov wrote:
>> 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;
> 
> We want to use GFP_ATOMIC for that, but I really hate to do so. There
> is no reason except for the early boot stage to call into this code
> with interrupts disabled. And there we are covered by gfp_allowed_mask,
> so that a GFP_KERNEL allocation can succeed.
> 
> I have no idea, why enable_IR_x2apic() has been bolted into
> smp_prepare_cpus(). Probably just because.
> 
> There is no reason WHY this cannot be done in the early irq setup path
> (at least nowadays with the allocators being available early), but
> that is another area which needs some care and cleanup, but definitely
> too late before the 3.19 merge window opens.
Hi Thomas,
	I will have a look at this after 3.19 merge window:)

> 
> So we have to bite the bullet and apply something like this along with
> a big fat comment WHY we are doing so and I'm tempted to wrap this
> into a #ifdef CONFIG_X86 so that noone else thinks that calling this
> code with interrupts disabled - except for the early boot stage - is a
> brilliant idea.
> 
> Thanks,
> 
> 	tglx
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/