From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org,
rafael@kernel.org, sudeep.holla@arm.com,
yongxuan.wang@sifive.com, pierre.gondois@arm.com,
vincent.chen@sifive.com, greentime.hu@sifive.com,
yangyicong@huawei.com, prime.zeng@hisilicon.com,
palmer@rivosinc.com, puwen@hygon.cn
Subject: Re: [PATCH 0/2] drivers: base: cacheinfo: Fix shared_cpu_list inconsistency in event of CPU hotplug
Date: Wed, 17 May 2023 18:02:54 -0700 [thread overview]
Message-ID: <20230518010254.GA19915@ranerica-svr.sc.intel.com> (raw)
In-Reply-To: <20230508084115.1157-1-kprateek.nayak@amd.com>
On Mon, May 08, 2023 at 02:11:13PM +0530, K Prateek Nayak wrote:
> Since v6.3-rc1, the shared_cpu_list in per-cpu cacheinfo breaks in case
> of hotplug activity on x86. This can be tracked back to two commits:
>
> o commit 198102c9103f ("cacheinfo: Fix shared_cpu_map to handle shared
> caches at different levels") that matches cache instance IDs without
> considering if the instance IDs belong to same cache level or not.
>
> o commit 5c2712387d48 ("cacheinfo: Fix LLC is not exported through
> sysfs") which skips calling populate_cache_leaves() if
> last_level_cache_is_valid(cpu) returns true. populate_cache_leaves()
> on x86 would have populated the shared_cpu_map when CPU comes online,
> which is now skipped, and the alternate path has an early bailout
> before setting the CPU in the shared_cpu_map is even attempted.
>
> On x86, populate_cache_leaves() also sets the
> cpu_cacheinfo->cpu_map_populated flag when the cacheinfo is first
> populated, the cache_shared_cpu_map_setup() in the driver is bypassed
> when a thread comes back online during the hotplug activity. This leads
> to the shared_cpu_list displaying abnormal values for the CPU that was
> offlined and then onlined since the shared_cpu_maps are never
> revaluated.
>
> Following is the output from a dual socket 3rd Generation AMD EPYC
> processor (2 x 64C/128T) for cachinfo when offlining and then onlining
> CPU8:
>
> o v6.3-rc5 with no changes:
>
> # for i in /sys/devices/system/cpu/cpu8/cache/index*/shared_cpu_list; do echo -n "$i: "; cat $i; done
> /sys/devices/system/cpu/cpu8/cache/index0/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index1/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index2/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index3/shared_cpu_list: 8-15,136-143
>
> # echo 0 > /sys/devices/system/cpu/cpu8/online
> # echo 1 > /sys/devices/system/cpu/cpu8/online
>
> # for i in /sys/devices/system/cpu/cpu8/cache/index*/shared_cpu_list; do echo -n "$i: "; cat $i; done
> /sys/devices/system/cpu/cpu8/cache/index0/shared_cpu_list: 8
> /sys/devices/system/cpu/cpu8/cache/index1/shared_cpu_list: 8
> /sys/devices/system/cpu/cpu8/cache/index2/shared_cpu_list: 8
> /sys/devices/system/cpu/cpu8/cache/index3/shared_cpu_list: 8
>
> # cat /sys/devices/system/cpu/cpu136/cache/index0/shared_cpu_list
> 136
>
> # cat /sys/devices/system/cpu/cpu136/cache/index3/shared_cpu_list
> 9-15,136-143
>
> o v6.3-rc5 with commit 5c2712387d48 ("cacheinfo: Fix LLC is not exported
> through sysfs") reverted (Behavior consistent with v6.2):
>
> # for i in /sys/devices/system/cpu/cpu8/cache/index*/shared_cpu_list; do echo -n "$i: "; cat $i; done
> /sys/devices/system/cpu/cpu8/cache/index0/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index1/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index2/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index3/shared_cpu_list: 8-15,136-143
>
> # echo 0 > /sys/devices/system/cpu/cpu8/online
> # echo 1 > /sys/devices/system/cpu/cpu8/online
>
> # for i in /sys/devices/system/cpu/cpu8/cache/index*/shared_cpu_list; do echo -n "$i: "; cat $i; done
> /sys/devices/system/cpu/cpu8/cache/index0/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index1/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index2/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index3/shared_cpu_list: 8-15,136-143
>
> # cat /sys/devices/system/cpu/cpu136/cache/index0/shared_cpu_list
> 8,136
>
> # cat /sys/devices/system/cpu/cpu136/cache/index3/shared_cpu_list
> 8-15,136-143
>
> This is not only limited to AMD processors but affects Intel processors
> too. Following is the output from same experiment on a dual socket Intel
> Ice Lake server (2 x 32C/64T) running kernel v6.3-rc5:
>
> # for i in /sys/devices/system/cpu/cpu8/cache/index*/shared_cpu_list; do echo -n "$i: "; cat $i; done
> /sys/devices/system/cpu/cpu8/cache/index0/shared_cpu_list: 8,72
> /sys/devices/system/cpu/cpu8/cache/index1/shared_cpu_list: 8,72
> /sys/devices/system/cpu/cpu8/cache/index2/shared_cpu_list: 8,72
> /sys/devices/system/cpu/cpu8/cache/index3/shared_cpu_list: 0,2,4,6,8,10,12,14,16,18,20,22,24,
> 26,28,30,32,34,36,38,40,42,44,46,48,50,52,54,56,58,60,62,64,66,68,70,72,74,76,78,80,82,84,86,
> 88,90,92,94,96,98,100,102,104,106,108,110,112,114,116,118,120,122,124,126
>
> # echo 0 > /sys/devices/system/cpu/cpu8/online
> # echo 1 > /sys/devices/system/cpu/cpu8/online
>
> # for i in /sys/devices/system/cpu/cpu8/cache/index*/shared_cpu_list; do echo -n "$i: "; cat $i; done
> /sys/devices/system/cpu/cpu8/cache/index0/shared_cpu_list: 8
> /sys/devices/system/cpu/cpu8/cache/index1/shared_cpu_list: 8
> /sys/devices/system/cpu/cpu8/cache/index2/shared_cpu_list: 8
> /sys/devices/system/cpu/cpu8/cache/index3/shared_cpu_list: 8
>
> # cat /sys/devices/system/cpu/cpu72/cache/index0/shared_cpu_list
> 72
>
> # cat /sys/devices/system/cpu/cpu72/cache/index3/shared_cpu_list
> 0,2,4,6,10,12,14,16,18,20,22,24,26,28,30,32,34,36,38,40,42,44,46,48,50,52,54,56,58,60,62,64,
> 66,68,70,72,74,76,78,80,82,84,86,88,90,92,94,96,98,100,102,104,106,108,110,112,114,116,118,
> 120,122,124,126
>
> This patch addresses two issues associated with building
> shared_cpu_list:
>
> o Patch 1 fixes an ID matching issue that can lead to cacheinfo
> associating CPUs from different cache levels in case IDs are not
> unique across all the different cache levels.
>
> o Patch 2 clears the cpu_cacheinfo->cpu_map_populated flag when CPU goes
> offline and is removed from the shared_cpu_map.
>
> Following are the results after applying the series on v6.3-rc5 on
> respective x86 platforms:
>
> o 3rd Generation AMD EPYC Processor (2 x 64C/128T)
>
> # for i in /sys/devices/system/cpu/cpu8/cache/index*/shared_cpu_list; do echo -n "$i: "; cat $i; done
> /sys/devices/system/cpu/cpu8/cache/index0/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index1/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index2/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index3/shared_cpu_list: 8-15,136-143
>
> # echo 0 > /sys/devices/system/cpu/cpu8/online
> # echo 1 > /sys/devices/system/cpu/cpu8/online
>
> # for i in /sys/devices/system/cpu/cpu8/cache/index*/shared_cpu_list; do echo -n "$i: "; cat $i; done
> /sys/devices/system/cpu/cpu8/cache/index0/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index1/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index2/shared_cpu_list: 8,136
> /sys/devices/system/cpu/cpu8/cache/index3/shared_cpu_list: 8-15,136-143
>
> # cat /sys/devices/system/cpu/cpu136/cache/index0/shared_cpu_list
> 8,136
>
> # cat /sys/devices/system/cpu/cpu136/cache/index3/shared_cpu_list
> 8-15,136-143
>
> o Intel Ice Lake Xeon (2 x 32C/128T)
>
> # for i in /sys/devices/system/cpu/cpu8/cache/index*/shared_cpu_list; do echo -n "$i: "; cat $i; done
> /sys/devices/system/cpu/cpu8/cache/index0/shared_cpu_list: 8,72
> /sys/devices/system/cpu/cpu8/cache/index1/shared_cpu_list: 8,72
> /sys/devices/system/cpu/cpu8/cache/index2/shared_cpu_list: 8,72
> /sys/devices/system/cpu/cpu8/cache/index3/shared_cpu_list: 0,2,4,6,8,10,12,14,16,18,20,22,24,26,
> 28,30,32,34,36,38,40,42,44,46,48,50,52,54,56,58,60,62,64,66,68,70,72,74,76,78,80,82,84,86,88,90,
> 92,94,96,98,100,102,104,106,108,110,112,114,116,118,120,122,124,126
>
> # echo 0 > /sys/devices/system/cpu/cpu8/online
> # echo 1 > /sys/devices/system/cpu/cpu8/online
>
> # for i in /sys/devices/system/cpu/cpu8/cache/index*/shared_cpu_list; do echo -n "$i: "; cat $i; done
> /sys/devices/system/cpu/cpu8/cache/index0/shared_cpu_list: 8,72
> /sys/devices/system/cpu/cpu8/cache/index1/shared_cpu_list: 8,72
> /sys/devices/system/cpu/cpu8/cache/index2/shared_cpu_list: 8,72
> /sys/devices/system/cpu/cpu8/cache/index3/shared_cpu_list: 0,2,4,6,8,10,12,14,16,18,20,22,24,26,
> 28,30,32,34,36,38,40,42,44,46,48,50,52,54,56,58,60,62,64,66,68,70,72,74,76,78,80,82,84,86,88,90,
> 92,94,96,98,100,102,104,106,108,110,112,114,116,118,120,122,124,126
>
> # cat /sys/devices/system/cpu/cpu72/cache/index0/shared_cpu_list
> 8,72
>
> # cat /sys/devices/system/cpu/cpu72/cache/index3/shared_cpu_list
> 0,2,4,6,8,10,12,14,16,18,20,22,24,26,28,30,32,34,36,38,40,42,44,46,48,50,52,54,56,58,60,62,64,66,
> 68,70,72,74,76,78,80,82,84,86,88,90,92,94,96,98,100,102,104,106,108,110,112,114,116,118,120,122,
> 124,126
>
> Running "grep -r 'cpu_map_populated' arch/" shows MIPS and loongarch too
> set the cpu_cacheinfo->cpu_map_populated who might also be affected by
> the changes in commit 5c2712387d48 ("cacheinfo: Fix LLC is not exported
> through sysfs") and this series. Changes from Patch 1 might also affect
> RISC-V since Yong-Xuan Wang <yongxuan.wang@sifive.com> from SiFive last
> made changes to cache_shared_cpu_map_setup() and
> cache_shared_cpu_map_remove() in commit 198102c9103f ("cacheinfo: Fix
> shared_cpu_map to handle shared caches at different levels").
I was able to reproduce the issue on an Alder Lake system (single socket).
I also was able to verify that the this patchset fixes the issue.
FWIW, Tested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
next prev parent reply other threads:[~2023-05-18 1:00 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-08 8:41 K Prateek Nayak
2023-05-08 8:41 ` [PATCH 1/2] drivers: base: cacheinfo: Fix shared_cpu_map changes " K Prateek Nayak
2023-05-08 8:41 ` [PATCH 2/2] drivers: base: cacheinfo: Update cpu_map_populated during CPU Hotplug K Prateek Nayak
2023-05-20 6:56 ` Yicong Yang
2023-05-24 3:43 ` K Prateek Nayak
2023-05-09 15:01 ` [PATCH 0/2] drivers: base: cacheinfo: Fix shared_cpu_list inconsistency in event of CPU hotplug Sudeep Holla
2023-05-09 15:10 ` K Prateek Nayak
2023-05-18 1:02 ` Ricardo Neri [this message]
2023-05-18 2:08 ` K Prateek Nayak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230518010254.GA19915@ranerica-svr.sc.intel.com \
--to=ricardo.neri-calderon@linux.intel.com \
--cc=greentime.hu@sifive.com \
--cc=gregkh@linuxfoundation.org \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=palmer@rivosinc.com \
--cc=pierre.gondois@arm.com \
--cc=prime.zeng@hisilicon.com \
--cc=puwen@hygon.cn \
--cc=rafael@kernel.org \
--cc=sudeep.holla@arm.com \
--cc=vincent.chen@sifive.com \
--cc=yangyicong@huawei.com \
--cc=yongxuan.wang@sifive.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®