Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Pingfan Liu <kernelfans@gmail.com>, linux-kernel@vger.kernel.org
Cc:     Pingfan Liu <kernelfans@gmail.com>,
        Valentin Schneider <valentin.schneider@arm.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Frederic Weisbecker <frederic@kernel.org>,
        Baokun Li <libaokun1@huawei.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Steven Price <steven.price@arm.com>,
        "Jason A. Donenfeld" <Jason@zx2c4.com>,
        Yuan ZhaoXiong <yuanzhaoxiong@baidu.com>,
        YueHaibing <yuehaibing@huawei.com>,
        Randy Dunlap <rdunlap@infradead.org>
Subject: Re: [PATCH 7/9] irq: remove needless lock in takedown_cpu()
In-Reply-To: <20220420140521.45361-8-kernelfans@gmail.com>
References: <20220420140521.45361-1-kernelfans@gmail.com>
 <20220420140521.45361-8-kernelfans@gmail.com>
Date:   Thu, 21 Apr 2022 18:11:56 +0200
Message-ID: <87y1zys9f7.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

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