Re: [PATCH V2 2/4] posix-timers: Initialise timer->it_id in posix_timer_add()

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, Feb 20 2025 at 09:48, Eric Dumazet wrote:
> On Thu, Feb 20, 2025 at 9:12 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>> On Wed, Feb 19 2025 at 12:55, Eric Dumazet wrote:
>> > A timer is visible only when both timer->signal and timer->i_id
>> > are set to their final values.
>> >
>> > We can initialize timer->it_id sooner.
>>
>> This changelog tells nothing. What is this fixing, why is it required to
>> initialize this sooner?
>
> Just  to make the series more readable and bisectable.

That's fine. But this changelog does not explain what the patch is about.

>> We can... We also can not ... Aside of that please write changelogs in
>> imperative mood as Documentation asks for.
>
> I do not understand.

What's so hard to understand?

What does "We can initialize timer->it_id sooner" tell me?

Yes, we can initialize it sooner, but what's the point? What's the
problem this is solving?

Also if you can't find the relevant documentation about imperative mood,
let me cite it to you:

 "Describe your changes in imperative mood, e.g. “make xyzzy do frotz”
  instead of “[This patch] makes xyzzy do frotz” or “[I] changed xyzzy to
  do frotz”, as if you are giving orders to the codebase to change its
  behaviour."

> I wrote more than 5000 upstream linux patches, you  are trying to
> educate me in 2025 ?

No. I'm asking you to provide proper change logs which match what's
requested in documentation. I'm asking that from anyone independent of
patch count.

>> >       /*
>> >        * Add the timer to the hash table. The timer is not yet valid
>> > -      * because new_timer::it_signal is still NULL. The timer id is also
>> > -      * not yet visible to user space.
>>
>> Even with this change the timer ID is not yet visible to user space
>> because it has not yet been copied back ....
>>
> It is seen in concurrent lookups.

But still user space cannot observe it, right?

Thanks,

        tglx