RISC-V: cacheinfo: add init_cache_level()

Message ID 20240122013510.55788-1-cuiyunhui@bytedance.com
State New
Headers
Series RISC-V: cacheinfo: add init_cache_level() |

Commit Message

yunhui cui Jan. 22, 2024, 1:35 a.m. UTC
  When cacheinfo_sysfs_init() is executed, the general weak function
init_cache_level() returns -ENOENT, causing failure to add the "cache"
node to /sys/devices/system/cpu/cpux/. Implement the init_cache_level()
function on RISC-V to fix it.

Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 arch/riscv/kernel/cacheinfo.c | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Conor Dooley Jan. 22, 2024, 8:09 a.m. UTC | #1
On Mon, Jan 22, 2024 at 09:35:10AM +0800, Yunhui Cui wrote:
> When cacheinfo_sysfs_init() is executed, the general weak function
> init_cache_level() returns -ENOENT, causing failure to add the "cache"
> node to /sys/devices/system/cpu/cpux/. Implement the init_cache_level()
> function on RISC-V to fix it.

If you recall correctly, I asked you to explain how to reproduce this
when you sent the patch.

Thanks,
Conor.

> 
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  arch/riscv/kernel/cacheinfo.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
> index 09e9b88110d1..be9169a38bac 100644
> --- a/arch/riscv/kernel/cacheinfo.c
> +++ b/arch/riscv/kernel/cacheinfo.c
> @@ -71,6 +71,12 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
>  	this_leaf->type = type;
>  }
>  
> +int init_cache_level(unsigned int cpu)
> +{
> +	/* The topology has been parsed by acpi or dt, return true. */
> +	return 0;
> +}
> +
>  int populate_cache_leaves(unsigned int cpu)
>  {
>  	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
> -- 
> 2.20.1
> 
>
  
yunhui cui Jan. 22, 2024, 8:32 a.m. UTC | #2
Hi Conor,

On Mon, Jan 22, 2024 at 4:09 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Jan 22, 2024 at 09:35:10AM +0800, Yunhui Cui wrote:
> > When cacheinfo_sysfs_init() is executed, the general weak function
> > init_cache_level() returns -ENOENT, causing failure to add the "cache"
> > node to /sys/devices/system/cpu/cpux/. Implement the init_cache_level()
> > function on RISC-V to fix it.
>
> If you recall correctly, I asked you to explain how to reproduce this
> when you sent the patch.

In fact, the reason has been explained in the commit log. As for how
to reproduce it, you can check whether there is a "cache" node in
/sys/devices/system/cpu/cpux/ on the riscv platform.

Thanks,
Yunhui
  
Conor Dooley Jan. 22, 2024, 8:55 a.m. UTC | #3
On Mon, Jan 22, 2024 at 04:32:15PM +0800, yunhui cui wrote:
> Hi Conor,
> 
> On Mon, Jan 22, 2024 at 4:09 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Mon, Jan 22, 2024 at 09:35:10AM +0800, Yunhui Cui wrote:
> > > When cacheinfo_sysfs_init() is executed, the general weak function
> > > init_cache_level() returns -ENOENT, causing failure to add the "cache"
> > > node to /sys/devices/system/cpu/cpux/. Implement the init_cache_level()
> > > function on RISC-V to fix it.
> >
> > If you recall correctly, I asked you to explain how to reproduce this
> > when you sent the patch.
> 
> In fact, the reason has been explained in the commit log. As for how
> to reproduce it, you can check whether there is a "cache" node in
> /sys/devices/system/cpu/cpux/ on the riscv platform.

That's the thing - I tried to reproduce this several times and either:
a) The system had cache information in DT and the directory was
   created. If I hot unplugged and re-plugged the directory was
   re-created.
b) The system had no cache information in DT and the directory was never
   created.

You said in your original report that you came across this problem in
qemu - can you share the qemu command required to reproduce please?

Thanks,
Conor.
  
yunhui cui Jan. 22, 2024, 10:57 a.m. UTC | #4
Hi Conor,

On Mon, Jan 22, 2024 at 4:55 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Jan 22, 2024 at 04:32:15PM +0800, yunhui cui wrote:
> > Hi Conor,
> >
> > On Mon, Jan 22, 2024 at 4:09 PM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Mon, Jan 22, 2024 at 09:35:10AM +0800, Yunhui Cui wrote:
> > > > When cacheinfo_sysfs_init() is executed, the general weak function
> > > > init_cache_level() returns -ENOENT, causing failure to add the "cache"
> > > > node to /sys/devices/system/cpu/cpux/. Implement the init_cache_level()
> > > > function on RISC-V to fix it.
> > >
> > > If you recall correctly, I asked you to explain how to reproduce this
> > > when you sent the patch.
> >
> > In fact, the reason has been explained in the commit log. As for how
> > to reproduce it, you can check whether there is a "cache" node in
> > /sys/devices/system/cpu/cpux/ on the riscv platform.
>
> That's the thing - I tried to reproduce this several times and either:
> a) The system had cache information in DT and the directory was
>    created. If I hot unplugged and re-plugged the directory was
>    re-created.
> b) The system had no cache information in DT and the directory was never
>    created.

Indeed, I verified it again, it’s because there is no cache node in
dts, thank you for reminding me.

Thanks,
Yunhui
  
Sudeep Holla Jan. 22, 2024, 5:02 p.m. UTC | #5
On Mon, Jan 22, 2024 at 06:57:49PM +0800, yunhui cui wrote:
> Hi Conor,
> 
> On Mon, Jan 22, 2024 at 4:55 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Mon, Jan 22, 2024 at 04:32:15PM +0800, yunhui cui wrote:
> > > Hi Conor,
> > >
> > > On Mon, Jan 22, 2024 at 4:09 PM Conor Dooley <conor@kernel.org> wrote:
> > > >
> > > > On Mon, Jan 22, 2024 at 09:35:10AM +0800, Yunhui Cui wrote:
> > > > > When cacheinfo_sysfs_init() is executed, the general weak function
> > > > > init_cache_level() returns -ENOENT, causing failure to add the "cache"
> > > > > node to /sys/devices/system/cpu/cpux/. Implement the init_cache_level()
> > > > > function on RISC-V to fix it.
> > > >
> > > > If you recall correctly, I asked you to explain how to reproduce this
> > > > when you sent the patch.
> > >
> > > In fact, the reason has been explained in the commit log. As for how
> > > to reproduce it, you can check whether there is a "cache" node in
> > > /sys/devices/system/cpu/cpux/ on the riscv platform.
> >
> > That's the thing - I tried to reproduce this several times and either:
> > a) The system had cache information in DT and the directory was
> >    created. If I hot unplugged and re-plugged the directory was
> >    re-created.
> > b) The system had no cache information in DT and the directory was never
> >    created.
> 
> Indeed, I verified it again, it’s because there is no cache node in
> dts, thank you for reminding me.
>

That's good. I wasn't able to figure out why the issue could occur without
issues in DT/ACPI.
  

Patch

diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
index 09e9b88110d1..be9169a38bac 100644
--- a/arch/riscv/kernel/cacheinfo.c
+++ b/arch/riscv/kernel/cacheinfo.c
@@ -71,6 +71,12 @@  static void ci_leaf_init(struct cacheinfo *this_leaf,
 	this_leaf->type = type;
 }
 
+int init_cache_level(unsigned int cpu)
+{
+	/* The topology has been parsed by acpi or dt, return true. */
+	return 0;
+}
+
 int populate_cache_leaves(unsigned int cpu)
 {
 	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);