linux-kernel - Re: [PATCH] x86: sparse_irq need spin

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: <20080820210336.3e6ffd6d.akpm@linux-foundation.org>
Date:	Wed, 20 Aug 2008 21:03:36 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Yinghai Lu <yhlu.kernel@...il.com>
Cc:	Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86: sparse_irq need spin_lock in alloc

On Wed, 20 Aug 2008 20:46:25 -0700 Yinghai Lu <yhlu.kernel@...il.com> wrote:

> acording to Suresh Siddha, we should have spin_lock around it
> 
> Signed-off-by: Yinghai Lu <yhlu.kernel@...il.com>
> 
> ---
>  arch/x86/kernel/io_apic.c |    6 ++++++
>  kernel/irq/handle.c       |    7 +++++++
>  2 files changed, 13 insertions(+)
> 
> Index: linux-2.6/kernel/irq/handle.c
> ===================================================================
> --- linux-2.6.orig/kernel/irq/handle.c
> +++ linux-2.6/kernel/irq/handle.c
> @@ -166,6 +166,9 @@ struct irq_desc *irq_to_desc(unsigned in
>  	}
>  	return NULL;
>  }
> +
> +static DEFINE_SPINLOCK(sparse_irq_lock);
> +
>  struct irq_desc *irq_to_desc_alloc(unsigned int irq)
>  {
>  	struct irq_desc *desc, *desc_pri;
> @@ -182,6 +185,7 @@ struct irq_desc *irq_to_desc_alloc(unsig
>  		count++;
>  	}
>  
> +	spin_lock(&sparse_irq_lock);
>  	/*
>  	 *  we run out of pre-allocate ones, allocate more
>  	 */
> @@ -223,6 +227,9 @@ struct irq_desc *irq_to_desc_alloc(unsig
>  	else
>  		sparse_irqs = desc;
>  	desc->irq = irq;
> +
> +	spin_unlock(&sparse_irq_lock);
> +
>  	printk(KERN_DEBUG "found new irq_desc for irq %d\n", desc->irq);
>  #ifdef CONFIG_HAVE_SPARSE_IRQ_DEBUG
>  	{
> Index: linux-2.6/arch/x86/kernel/io_apic.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/io_apic.c
> +++ linux-2.6/arch/x86/kernel/io_apic.c
> @@ -210,6 +210,8 @@ static struct irq_cfg *irq_cfg(unsigned
>  	return NULL;
>  }
>  
> +static DEFINE_SPINLOCK(irq_cfg_lock);
> +
>  static struct irq_cfg *irq_cfg_alloc(unsigned int irq)
>  {
>  	struct irq_cfg *cfg, *cfg_pri;
> @@ -226,6 +228,7 @@ static struct irq_cfg *irq_cfg_alloc(uns
>  		count++;
>  	}
>  
> +	spin_lock(&irq_cfg_lock);
>  	if (!irq_cfgx_free) {
>  		unsigned long phys;
>  		unsigned long total_bytes;
> @@ -263,6 +266,9 @@ static struct irq_cfg *irq_cfg_alloc(uns
>  	else
>  		irq_cfgx = cfg;
>  	cfg->irq = irq;
> +
> +	spin_unlock(&irq_cfg_lock);
> +
>  	printk(KERN_DEBUG "found new irq_cfg for irq %d\n", cfg->irq);
>  #ifdef CONFIG_HAVE_SPARSE_IRQ_DEBUG
>  	{

Each of these locks can be made local to the function in which they are
used (and hence they should be made local).

It would be nice to add a comment explaining what they are protecting,
unless that is obvious (I didn't look).

--
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/