Re: [PATCH v3] sched/topology: Enable topology_span_sane check only for debug builds

[Naman Jain <namjain@linux.microsoft.com>]

On 2/6/2025 3:49 PM, K Prateek Nayak wrote:
> Hello Naman,
> 
> On 2/6/2025 3:17 PM, Naman Jain wrote:
>> [..snip..]
>>>
>>> This is why I think that the topology_span_sane() check is redundant
>>> when the x86 bits have already ensured masks cannot overlap in all
>>> cases except for potentially in the (*) case.
>>>
>>> So circling back to my original question around "SDTL_ARCH_VERIFIED",
>>> would folks be okay to an early bailout from topology_span_sane() on:
>>>
>>>      if (!sched_debug() && (tl->flags & SDTL_ARCH_VERIFIED))
>>>          return;
>>>
>>> and more importantly, do folks care enough about topology_span_sane()
>>> to have it run on other architectures and not just have it guarded
>>> behind just "sched_debug()" which starts off as false by default?
>>>
>>> (Sorry for the long answer explaining my thought process.)
>>>
>>
>>
>> Thanks for sharing your valuable insights.
>> I am sorry, I could not find SDTL_ARCH_VERIFIED in linux-next tip. Am I
>> missing something?
> 
> It does not exits yet. I was proposing on defining this new flag
> "SDTL_ARCH_VERIFIED" which a particular arch can set if the topology
> parsing code has taken care of making sure that the cpumasks cannot
> overlap. The original motivation for topology_span_sane() discussed in
> [1] came from an ARM processor where the functions that returns the
> cpumask is not based on ID checks and can possibly allow overlapping
> masks.
> 
> With the exception of AMD Fam 0x15 processors which populates cu_id
> (and that too it is theoretical case), I believe all x86 processors can
> set this new flag "SDTL_ARCH_VERIFIED" and can safely skip the
> topology_span_sane() since it checks for a condition that cannot
> possibly be false as result of how these masks are built on x86.
> 
> [1] https://lore.kernel.org/lkml/ 
> f6bf04e8-3007-4a44-86d8-2cc671c85247@amd.com/

I think the check for sched_debug() should suffice here, without making
it more complicated. This way, we give the control to the user to have
it or not. I'll wait for a few more days to get any additional feedback
and post v4 with your initial review comments addressed.

Regards,
Naman