linux-kernel - Re: [PATCH 7/9] irq: remove needless lock in takedown

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: <87y1zys9f7.ffs@tglx>
Date:   Thu, 21 Apr 2022 18:11:56 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Pingfan Liu <kernelfans@...il.com>, linux-kernel@...r.kernel.org
Cc:     Pingfan Liu <kernelfans@...il.com>,
        Valentin Schneider <valentin.schneider@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Frederic Weisbecker <frederic@...nel.org>,
        Baokun Li <libaokun1@...wei.com>,
        Mark Rutland <mark.rutland@....com>,
        Steven Price <steven.price@....com>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Yuan ZhaoXiong <yuanzhaoxiong@...du.com>,
        YueHaibing <yuehaibing@...wei.com>,
        Randy Dunlap <rdunlap@...radead.org>
Subject: Re: [PATCH 7/9] irq: remove needless lock in takedown_cpu()

On Wed, Apr 20 2022 at 22:05, Pingfan Liu wrote:

First of all, the subject prefix for the core interrupt subsystem is
'genirq' and the sentence after the colon starts with an uppercase
letter. See:

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html

> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index d0a9aa0b42e8..94a6b512c26d 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1033,18 +1033,16 @@ static int takedown_cpu(unsigned int cpu)
>  	kthread_park(st->thread);
>  
>  	/*
> -	 * Prevent irq alloc/free while the dying cpu reorganizes the
> -	 * interrupt affinities.
> +	 * RCU keeps watching 'cpu' until do_idle()->rcu_report_dead().
> +	 * And cpu_stopper's fn is dispatched with preemption disabled.
> +	 * So it can not occur to release a irq_desc.
>  	 */
> -	irq_lock_sparse();

Not everything is about RCU here. You really need to look at all moving
parts:

irq_migrate_all_off_this_cpu() relies on the allocated_irqs bitmap and
the sparse tree to be in consistent state, which is only guaranteed when
the sparse lock is held.

I'm not sure what you are trying to solve here. Not taking sparse_irq_lock
here is not gaining anything.

Thanks,

        tglx