linux-kernel - Re: [PATCH] mm/mlock: Fix WARN_ON

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] [day] [month] [year] [list]

Message-ID: <57fa3641-4166-4a0d-8851-2bddb4a19f1c@lucifer.local>
Date: Fri, 7 Feb 2025 09:28:10 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Liu Ye <liuye@...inos.cn>
Cc: akpm@...ux-foundation.org, Liam.Howlett@...cle.com, vbabka@...e.cz,
        jannh@...gle.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/mlock: Fix WARN_ON_ONCE condition.

On Fri, Feb 07, 2025 at 02:04:48PM +0800, Liu Ye wrote:
> The condition in WARN_ON_ONCE(cpu_online(cpu)) should be !cpu_online(cpu)
> to ensure the warning is triggered when the CPU is offline.
>
> Signed-off-by: Liu Ye <liuye@...inos.cn>

NACK.

This is a really odd patch to send, if you were right we'd be seeing
constant warnings on this no? And as per Dev, this seems completely wrong?

You also MUST provide more justification if you're going to do something as
crazy as this. An 'it's wrong because I say it is' isn't acceptable on any
level whatsoever.

> ---
>  mm/mlock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/mlock.c b/mm/mlock.c
> index cde076fa7d5e..40acf7897917 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -224,7 +224,7 @@ void mlock_drain_remote(int cpu)
>  {
>  	struct folio_batch *fbatch;
>
> -	WARN_ON_ONCE(cpu_online(cpu));
> +	WARN_ON_ONCE(!cpu_online(cpu));
>  	fbatch = &per_cpu(mlock_fbatch.fbatch, cpu);
>  	if (folio_batch_count(fbatch))
>  		mlock_folio_batch(fbatch);
> --
> 2.25.1
>