Re: [PATCH v6 08/11] platform/x86/intel/ifs: Add scan test support

[Thomas Gleixner <tglx@linutronix.de>]

On Fri, May 06 2022 at 11:49, Luck, Tony wrote:
> On Fri, May 06, 2022 at 03:30:30PM +0200, Thomas Gleixner wrote:
>> 1) How is that supposed to work on a system which has HT enabled in BIOS,
>>    but disabled on the kernel command line or via /sys/..../smt/control or
>>    when a HT sibling is offlined temporarily?
>> 
>>    I assume it cannot work, but I can't see anything which handles those
>>    cases.
>
> Correct. If HT is disabled in BIOS, then there is no other thread, so
> core tests just use a single thread.
>
> If a logical CPU is "offline" due to Linux actions, then core test will
> fail. In an earlier version we did attempt to detect this before trying
> to run the test. But we didn't find a simple way to determine that a
> core has one thread online, and another offline. Rather than a bunch of
> code to detect an operator error it seemed better to let it run &
> fail.

Fair enough.

> GregKH wasn't a fan of this itty bitty driver cluttering up
> Documentation/x86. He said:
>
>    I don't know which is better, it's just that creating a whole new
>    documentation file for a single tiny driver feels very odd as it will
>    get out of date and is totally removed from the driver itself.
>
>    I'd prefer that drivers be self-contained, including the documentation,
>    as it is much more obvious what is happening with that.  Spreading stuff
>    around the tree only causes stuff to get out of sync easier.

Well, I agree to some extent, but the documentation which I want to see
is documentation for admins. I'm not sure whether we want them to search
the code. Those are consumers of Documentation/ AFAICT.

> So the documentation patch was dropped after v3. Last version here:
>
> https://lore.kernel.org/r/20220419163859.2228874-3-tony.luck@intel.com
>
> That doc would need pathnames updated to match the move from a platform
> device to a virtual misc device. But otherwise seems still accurate.
>
> Does that cover what you want from documentation for this driver
> (wherever it gets located in the tree)? Are you looking for more?

It's pretty detailed on the inner workings, but lacks a big fat warning
for the admin vs. the impact, i.e. that it makes the core go out for
lunch for a while, which has consequences on workloads and interrupts
directed at that core. Plus some explanation vs. the HT (SMT=off, soft
offline) case above. Similar to what we have e.g. for buslocks.

Thanks,

        tglx