Re: [PATCH v2] futex: Rewrite get_inode_sequence_number() to make code simpler

[Thomas Gleixner]

On Wed, Oct 09 2024 at 16:43, André Almeida wrote:
> Em 04/10/2024 05:52, Uros Bizjak escreveu:
>> Rewrite get_inode_sequence_number() to make code simpler:
>> 
>> a) Rewrite FOR loop to a DO-WHILE loop with returns moved
>> out of the loop.
>> 
>> b) Use atomic64_inc_return() instead of atomic64_add_return().
>> 
>> c) Use !atomic64_try_cmpxchg_relaxed(*ptr, &old, new) instead of
>> atomic64_cmpxchg_relaxed (*ptr, old, new) != old.  x86 CMPXCHG
>> instruction returns success in ZF flag, so this change also saves
>> a compare instruction after CMPXCHG.
>
> Remember, it's easy to see in the diff that you replace the function, 
> but might be not so clear why you did so. I think it would be better to 
> understand if you write like:
>
> We are trying to set a value for the i_sequence, that we expect that is 
> zero, but if we fail to do so, we are happy to use the current non-zero 
> i_sequence value that we found. Instead of using 
> atomic64_cmpxchg_relaxed(), use atomic64_try_cmpxchg_relaxed() which 
> provides a better semantic for this situation.

We are not expecting, trying, happy.. That's non-technical babbling.

See Documentation/process/* for futher explanation.

I agree with you that the change log should be more informative about
the why instead of listing the what, but not for the price of a
non-technical 'we' novel.

Thanks,

        tglx