Re: [PATCH v6 04/11] x86/smpboot: Reference count on smpboot_setup_warm_reset_vector()

[Thomas Gleixner <tglx@linutronix.de>]

David!

On Tue, Feb 07 2023 at 09:49, David Woodhouse wrote:

Can you please fix your mail client to _NOT_ send multipart/mixed mails?
Despite the CC list being insanely large, your replies are dropped by
vger and missing in the archives.

> On Tue, 2023-02-07 at 00:48 +0100, Thomas Gleixner wrote:
>> On Thu, Feb 02 2023 at 21:56, Usama Arif wrote:
>> > From: David Woodhouse <dwmw@amazon.co.uk>
>> > 
>> > If we want to do parallel CPU bringup, we're going to need to set this up
>> > and leave it until all CPUs are done. Might as well use the RTC spinlock
>> > to protect the refcount, as we need to take it anyway.
>> 
>> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog
>> 
>> Aside of the 'We' this does not explain anything at all.
>
> Er, doesn't it?
>
> We refcount the warm reset vector because when we do parallel bringup,
> we'll want to set it up once and then put it back to normal (for cold
> reset) once all the CPUs are up.
>
> I can rework the phrasing; I'm aware that the whole nonsense about
> pronouns and the imperative mood has grown legs in the last couple of
> years since I originally wrote it — but is there anything actually
> missing? 

We can settle the imperative mood debate over a beer at the next
conference, but stuff which goes through tip is required to follow those
rules. No exception for you :)

Vs. the content: This changelog lacks context. Changelogs have to be
self contained and self explanatory. Assuming that they are
understandable due to the context of the patch series is just wrong. I
fundamentally hate it when I have to dig out the context when I stare at
the changelog of a commit.

So something like this:

   The warm reset vector on X86 is setup through the RTC (CMOS) clock
   for each CPU bringup operation and cleared after the CPU came online.

   Parallel bringup of multiple CPUs requires that the warm reset vector
   is valid until all CPUs came online.

   To prepare for that add refcounting for the reset vector and protect
   it with the rtc_lock which has to be taken for the setup operation
   anyway.

gives the full context and is simply factual, no?

Thanks,

       tglx