linux-kernel - Re: [RFC Part2 v1 01/21] irqdomain: Introduce new interfaces to support hierarchy irqdomains

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: <alpine.DEB.2.10.1409160859080.4247@nanos>
Date:	Tue, 16 Sep 2014 10:43:50 -0700 (PDT)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Jiang Liu <jiang.liu@...ux.intel.com>
cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Randy Dunlap <rdunlap@...radead.org>,
	Yinghai Lu <yinghai@...nel.org>,
	Borislav Petkov <bp@...en8.de>,
	Grant Likely <grant.likely@...aro.org>,
	Marc Zyngier <marc.zyngier@....com>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tony Luck <tony.luck@...el.com>,
	Joerg Roedel <joro@...tes.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	x86@...nel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, linux-acpi@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [RFC Part2 v1 01/21] irqdomain: Introduce new interfaces to
 support hierarchy irqdomains

Jiang,

On Thu, 11 Sep 2014, Jiang Liu wrote:
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>  	/* Create mapping */
> -	virq = irq_create_mapping(domain, hwirq);
> +#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
> +	if (domain->ops->alloc)
> +		virq = irq_domain_alloc_irqs(domain, -1, 1, NUMA_NO_NODE,
> +					     irq_data);
> +	else
> +#endif
> +		virq = irq_create_mapping(domain, hwirq);

I'd prefer to get rid of the #ifdef CONFIG...s in the code. So this
can be written:

        if (irq_domain_has_hierarchy(domain))
		virq = irq_domain_alloc_irqs(domain, -1, 1, NUMA_NO_NODE,
					     irq_data);
	else
		virq = irq_create_mapping(domain, hwirq);
	   
	   

>  	if (!virq)
>  		return virq;
>  
> @@ -540,7 +542,11 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
>  		return 0;
>  
>  	if (hwirq < domain->revmap_direct_max_irq) {
> +#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
> +		data = irq_domain_get_irq_data(domain, hwirq);
> +#else
>  		data = irq_get_irq_data(hwirq);
> +#endif

Similar here. Make irq_domain_get_irq_data() map to irq_get_irq_data() for
the non hierarchy mode so you end up with a single line:

-  		data = irq_get_irq_data(hwirq);
+		data = irq_domain_get_irq_data(domain, hwirq);


> +#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
> +/**
> + * irq_domain_alloc_irqs - Allocate IRQs from domain
> + * @domain: domain to allocate from
> + * @irq_base: allocate specified IRQ nubmer if irq_base >= 0
> + * @nr_irqs: number of IRQs to allocate
> + * @node: NUMA node id for memory allocation
> + * @arg: domain specific argument
> + * @realloc: IRQ descriptors have already been allocated if true
> + *
> + * Allocate IRQ numbers and initialized all data structures to support
> + * hiearchy IRQ domains.
> + * Parameter @realloc is mainly to support legacy IRQs.

What's the issue with the legacy irqs? So this has the interrupt
descriptors allocated already. Are they already wired up for serving
interrupts and what's the state of those lines?

> + * Returns error code or allocated IRQ number

Can you please add some documentation how the hierarchical allocation
is supposed to work and how the domains are connected. That should
probably go to Documentation/IRQ-domains.txt.

Other than that this looks pretty good! Nice work!

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/