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

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2881 bytes --]

On Tue, 2023-02-07 at 15:39 +0100, Thomas Gleixner wrote:
> 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.
> 

That's not the client; that's the stupid mail system doing it in
transit. Sorry, I'd already filed a ticket about that idiocy last week
when I noticed they'd started adding HTML parts to a previously sane
mail. But obviously they haven't managed to fix it yet.

The correct thing to do in the meantime is use a non-broken mail
account, and I just forgot this morning until half way through the
thread, when you'll note the coffee kicked in and I switched.


> > 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?

Ack.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]