linux-kernel - Re: [PATCH V2] arch_topology: Clear LLC sibling when cacheinfo teardown

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230320072029.j6wsdpmmq7gmlvhg@bogus>
Date:   Mon, 20 Mar 2023 07:20:29 +0000
From:   Sudeep Holla <sudeep.holla@....com>
To:     Song Shuai <suagrfillet@...il.com>
Cc:     gregkh@...uxfoundation.org, rafael@...nel.org,
        conor.dooley@...rochip.com, ionela.voinescu@....com,
        Sudeep Holla <sudeep.holla@....com>, Pierre.Gondois@....com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] arch_topology: Clear LLC sibling when cacheinfo
 teardown

On Mon, Mar 20, 2023 at 06:20:34AM +0000, Song Shuai wrote:
>
> My original point is to clear the llc_sibling right after clearing of
> share_cpu_map like what you did in 3fcbf1c77d08.
>

Yes I understood that. There were other issues that were fixed later
and hence the state of current code.

> And the ~~issue~~ I described above was found when I manually tested
> the 'base/cacheinfo:online' hpstate,  which can be triggered by the
> following commands:
>
> ```
> hpid=$(sed -n '/cacheinfo/s/:.*//p' /sys/devices/system/cpu/hotplug/states)
> echo $((hpid-1)) > /sys/devices/system/cpu/cpu2/hotplug/target
>
> ```
>

Thanks for the detailed steps. I had guessed something very similar.

> Anyway, the short inconsistency window you explained seems acceptable to me.
>

Yes just inconsistency but technically the CPU topology is still correct
including LLC information. I don't see a point in clearing the cacheinfo
at this point and just couple of hotplug state later reset all the topology
as the CPU is being removed. I feel it to be redundant and we can add if
we absolutely need it(i.e. if there are new users of that information and
need it to be aligned to cacheinfo which I think is highly unlikely).

-- 
Regards,
Sudeep