linux-kernel - Re: [PATCH v2 1/8] RISC-V: Do not wait indefinitely in __cpu

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20190115135125.GA28946@infradead.org>
Date:   Tue, 15 Jan 2019 05:51:25 -0800
From:   Christoph Hellwig <hch@...radead.org>
To:     Atish Patra <atish.patra@....com>
Cc:     linux-riscv@...ts.infradead.org,
        Patrick Stählin <me@...ki.ch>,
        Albert Ou <aou@...s.berkeley.edu>,
        Jason Cooper <jason@...edaemon.net>,
        Alan Kao <alankao@...estech.com>,
        Dmitriy Cherkasov <dmitriy@...-tech.org>,
        Anup Patel <anup@...infault.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        linux-kernel@...r.kernel.org,
        Michael Clark <michaeljclark@....com>,
        Palmer Dabbelt <palmer@...ive.com>,
        Andreas Schwab <schwab@...e.de>,
        Marc Zyngier <marc.zyngier@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Zong Li <zongbox@...il.com>
Subject: Re: [PATCH v2 1/8] RISC-V: Do not wait indefinitely in __cpu_up

>  
>  void *__cpu_up_stack_pointer[NR_CPUS];
>  void *__cpu_up_task_pointer[NR_CPUS];
> +static DECLARE_COMPLETION(cpu_running);
>  
>  void __init smp_prepare_boot_cpu(void)
>  {
> @@ -81,6 +82,7 @@ void __init setup_smp(void)
>  
>  int __cpu_up(unsigned int cpu, struct task_struct *tidle)
>  {
> +	int ret = 0;
>  	int hartid = cpuid_to_hartid_map(cpu);
>  	tidle->thread_info.cpu = cpu;
>  
> @@ -96,10 +98,15 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
>  		  task_stack_page(tidle) + THREAD_SIZE);
>  	WRITE_ONCE(__cpu_up_task_pointer[hartid], tidle);
>  
> -	while (!cpu_online(cpu))
> -		cpu_relax();
> +	wait_for_completion_timeout(&cpu_running,
> +					    msecs_to_jiffies(1000));

Having a global completion here worries me.  I bet we have some higher
level serialization, but can we comment or even better lockdep assert on
that?

Also please use up your available lines (72 in commit logs, 80 in source
files) instead of adding spurious line wraps.