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