Re: [tip: locking/urgent] locking/ww_mutex: Simplify use_ww_ctx & ww_ctx handling

Waiman Long
2021-03-17T13:44:09Z

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@redhat.com>
>> AuthorDate: Tue, 16 Mar 2021 11:31:16 -04:00
>> Committer: Ingo Molnar <mingo@kernel.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