linux-kernel - [PATCH 0/3] Remove volatile from arch_raw_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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <20220324173927.2230447-1-bigeasy@linutronix.de>
Date:   Thu, 24 Mar 2022 18:39:24 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     torvalds@...ux-foundation.org
Cc:     akpm@...ux-foundation.org, bigeasy@...utronix.de,
        boqun.feng@...il.com, bp@...en8.de, linux-kernel@...r.kernel.org,
        longman@...hat.com, mingo@...nel.org, peterz@...radead.org,
        tglx@...utronix.de, will@...nel.org,
        Dennis Zhou <dennis@...nel.org>, Tejun Heo <tj@...nel.org>,
        Christoph Lameter <cl@...ux.com>
Subject: [PATCH 0/3] Remove volatile from arch_raw_cpu_ptr() and revert the hacks.

On 2022-03-23 10:17:08 [-0700], Linus Torvalds wrote:
> I get the feeling that the real problem is that on x86, we have this:
> 
> #define arch_raw_cpu_ptr(ptr)                           \
> ({                                                      \
>         unsigned long tcp_ptr__;                        \
>         asm volatile("add " __percpu_arg(1) ", %0"      \
>                      : "=r" (tcp_ptr__)                 \
>                      : "m" (this_cpu_off), "0" (ptr));  \
>         (typeof(*(ptr)) __kernel __force *)tcp_ptr__;   \
> })
> 
> and that "volatile" is just *WRONG*.
> 
> That volatile is what literally tells the compiler "you can't remove
> this if it isn't used".

It is indeed just x86. After double checking arm/mips removes that code
properly.

> But there's no point to that.
> 
> So how about we
> 
>  (a) just revert commit 9983a9d577db4
> 
>  (b) remove that bogus 'volatile'
> 
> Doesn't that fix the problem?

The following series does that. The assembly code looks okay. In a few
simple test cases the this_cpu_ptr() usage is always created and is not
moved passed preempt_enable() statement.
The resulting vmlinux shrunk a bit. The test config lost ~2KiB:
     text      data       bss       dec      hex filename
 22533901  10722831  13963496  47220228  2d08604 vmlinux.volatile
 22531589  10722831  13971688  47226108  2d09cfc vmlinux.patched

after looking at it it was sometimes due avoiding this_cpu_ptr(),
sometimes it looked that the compiler made other decisions at the
earlier resulting to be more beneficial later on.

Sebastian