linux-kernel - Re: Linux 6.15-rc3

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: <CAADnVQ+iFBxauKq99=-Xk+BdG+Lv=Xgvwi1dC4fpG0utmXJiiA@mail.gmail.com>
Date: Wed, 23 Apr 2025 08:41:30 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Michael Larabel <Michael@...ronix.com>, Shakeel Butt <shakeel.butt@...ux.dev>, 
	Dave Airlie <airlied@...il.com>, Sebastian Sewior <bigeasy@...utronix.de>, 
	Andrew Morton <akpm@...ux-foundation.org>, Linus Torvalds <torvalds@...ux-foundation.org>, 
	Alexei Starovoitov <ast@...nel.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Linux 6.15-rc3

On Wed, Apr 23, 2025 at 1:42 AM Vlastimil Babka <vbabka@...e.cz> wrote:
>
>
> ----8<----
> From: Vlastimil Babka <vbabka@...e.cz>
> Date: Wed, 23 Apr 2025 10:21:29 +0200
> Subject: [PATCH] locking/local_lock: fix _Generic() matching of
>  local_trylock_t
>
> Michael Larabel reported [1] a nginx performance regression in v6.15-rc3 and
> bisected it to commit 51339d99c013 ("locking/local_lock, mm: replace
> localtry_ helpers with local_trylock_t type")
>
> The problem is the _Generic() usage with a default association that
> masks the fact that "local_trylock_t *" association is not being
> selected as expected. Replacing the default with the only other expected
> type "local_lock_t *" reveals the underlying problem:
>
> ./include/linux/local_lock_internal.h:174:26: error: ‘_Generic’ selector of
> type ‘__seg_gs local_lock_t *’ is not compatible with any association
>
> The local_locki's are part of __percpu structures and thus the __percpu
> attribute is needed to associate the type properly. Add the attribute
> and keep the default replaced to turn any further mismatches into
> compile errors.
>
> The failure to recognize local_try_lock_t in __local_lock_release()
> means that a local_trylock[_irqsave]() operation will set tl->acquired
> to 1 (there's no _Generic() part in the trylock code), but then
> local_unlock[_irqrestore]() will not set tl->acquired back to 0, so
> further trylock operations will always fail on the same cpu+lock, while
> non-trylock operations continue to work - a lockdep_assert() is also
> not being executed in the _Generic() part of local_lock() code.
>
> This means consume_stock() and refill_stock() operations will fail
> deterministically, resulting in taking the slow paths and worse
> performance.
>
> Fixes: 51339d99c013 ("locking/local_lock, mm: replace localtry_ helpers with local_trylock_t type")
> Reported-by: Michael Larabel <Michael@...ronix.com>
> Closes: https://www.phoronix.com/review/linux-615-nginx-regression/2 [1]
> Signed-off-by: Vlastimil Babka <vbabka@...e.cz>

Acked-by: Alexei Starovoitov <ast@...nel.org>

Another lesson learned. I stayed on gcc-13 for too long :(

Linus,

maybe take the fix directly, since perf regression is severe
for network heavy workloads.

I can confirm netperf -t TCP_STREAM 3x regression and "perf report" shows:

12.90%  netperf          [kernel.kallsyms]           [k] page_counter_uncharge
        |
        ---page_counter_uncharge
           |
           |--11.68%--try_charge_memcg
           |          |
           |           --11.68%--mem_cgroup_charge_skmem
           |                     |
           |                      --11.50%--__sk_mem_raise_allocated
           |                                __sk_mem_schedule
           |                                |
           |                                |--9.42%--tcp_wmem_schedule
           |                                |          tcp_sendmsg_locked
           |                                |          tcp_sendmsg


With the fix memcg accounting takes < 1 %.