linux-kernel - Re: [tip: locking/urgent] locking/ww_mutex: Simplify use_ww_ctx & ww

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: <85fbce04-c544-6041-6e7d-76f47b90e263@redhat.com>
Date:   Wed, 17 Mar 2021 09:43:20 -0400
From:   Waiman Long <longman@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org
Cc:     linux-tip-commits@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
        Davidlohr Bueso <dbueso@...e.de>, x86@...nel.org
Subject: Re: [tip: locking/urgent] locking/ww_mutex: Simplify use_ww_ctx &
 ww_ctx handling

On 3/17/21 8:59 AM, Peter Zijlstra wrote:
> On Wed, Mar 17, 2021 at 12:38:22PM -0000, tip-bot2 for Waiman Long wrote:
>> The following commit has been merged into the locking/urgent branch of tip:
>>
>> Commit-ID:     5de2055d31ea88fd9ae9709ac95c372a505a60fa
>> Gitweb:        https://git.kernel.org/tip/5de2055d31ea88fd9ae9709ac95c372a505a60fa
>> Author:        Waiman Long <longman@...hat.com>
>> AuthorDate:    Tue, 16 Mar 2021 11:31:16 -04:00
>> Committer:     Ingo Molnar <mingo@...nel.org>
>> CommitterDate: Wed, 17 Mar 2021 09:56:44 +01:00
>>
>> locking/ww_mutex: Simplify use_ww_ctx & ww_ctx handling
>>
>> The use_ww_ctx flag is passed to mutex_optimistic_spin(), but the
>> function doesn't use it. The frequent use of the (use_ww_ctx && ww_ctx)
>> combination is repetitive.
>>
>> In fact, ww_ctx should not be used at all if !use_ww_ctx.  Simplify
>> ww_mutex code by dropping use_ww_ctx from mutex_optimistic_spin() an
>> clear ww_ctx if !use_ww_ctx. In this way, we can replace (use_ww_ctx &&
>> ww_ctx) by just (ww_ctx).
> The reason this code was like this is because GCC could constant
> propagage use_ww_ctx but could not do the same for ww_ctx (since that's
> external).
>
> Please double check generated code to make sure you've not introduced a
> bunch of extra branches.
>
I see, but this patch just replaces (use_ww_ctx && ww_ctx) by (ww_ctx). 
Even if constant propagation isn't happening for ww_ctx, gcc shouldn't 
generate any worse code wrt ww_ctx. It could be that the generated 
machine code are more or less the same, but the source code will look 
cleaner with just one variable in the conditional clauses.

Using gcc 8.4.1, the generated __mutex_lock function has the same size 
(with last instruction at offset +5179) with or without this patch. 
Well, you can say that this patch is an no-op wrt generated code.

Cheers,
Longman