[PATCH] drivers/perf: riscv: Remove redundant macro check

[PATCH] drivers/perf: riscv: Remove redundant macro check

[Xiao Wang]

The macro CONFIG_RISCV_PMU must have been defined when riscv_pmu.c gets
compiled, so this patch removes the redundant check.

Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
---
 drivers/perf/riscv_pmu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
index 0a02e85a8951..7644147d50b4 100644
--- a/drivers/perf/riscv_pmu.c
+++ b/drivers/perf/riscv_pmu.c
@@ -39,7 +39,6 @@ void arch_perf_update_userpage(struct perf_event *event,
 	userpg->cap_user_time_short = 0;
 	userpg->cap_user_rdpmc = riscv_perf_user_access(event);
 
-#ifdef CONFIG_RISCV_PMU
 	/*
 	 * The counters are 64-bit but the priv spec doesn't mandate all the
 	 * bits to be implemented: that's why, counter width can vary based on
@@ -47,7 +46,6 @@ void arch_perf_update_userpage(struct perf_event *event,
 	 */
 	if (userpg->cap_user_rdpmc)
 		userpg->pmc_width = to_riscv_pmu(event->pmu)->ctr_get_width(event->hw.idx) + 1;
-#endif
 
 	do {
 		rd = sched_clock_read_begin(&seq);
-- 
2.25.1

Re: [PATCH] drivers/perf: riscv: Remove redundant macro check

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

On Mon, Jul 08, 2024 at 08:12:24PM +0800, Xiao Wang wrote:
> The macro CONFIG_RISCV_PMU must have been defined when riscv_pmu.c gets
> compiled, so this patch removes the redundant check.

Did you investigate why this define was added? Why do you think that it
is redundant, rather than checking the incorrect config option?

Cheers,
Conor.

> 
> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> ---
>  drivers/perf/riscv_pmu.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> index 0a02e85a8951..7644147d50b4 100644
> --- a/drivers/perf/riscv_pmu.c
> +++ b/drivers/perf/riscv_pmu.c
> @@ -39,7 +39,6 @@ void arch_perf_update_userpage(struct perf_event *event,
>  	userpg->cap_user_time_short = 0;
>  	userpg->cap_user_rdpmc = riscv_perf_user_access(event);
>  
> -#ifdef CONFIG_RISCV_PMU
>  	/*
>  	 * The counters are 64-bit but the priv spec doesn't mandate all the
>  	 * bits to be implemented: that's why, counter width can vary based on
> @@ -47,7 +46,6 @@ void arch_perf_update_userpage(struct perf_event *event,
>  	 */
>  	if (userpg->cap_user_rdpmc)
>  		userpg->pmc_width = to_riscv_pmu(event->pmu)->ctr_get_width(event->hw.idx) + 1;
> -#endif
>  
>  	do {
>  		rd = sched_clock_read_begin(&seq);
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] drivers/perf: riscv: Remove redundant macro check

[Charlie Jenkins]

On Mon, Jul 08, 2024 at 01:22:11PM +0100, Conor Dooley wrote:
> On Mon, Jul 08, 2024 at 08:12:24PM +0800, Xiao Wang wrote:
> > The macro CONFIG_RISCV_PMU must have been defined when riscv_pmu.c gets
> > compiled, so this patch removes the redundant check.
> 
> Did you investigate why this define was added? Why do you think that it
> is redundant, rather than checking the incorrect config option?

This file is only compiled with CONFIG_RISCV_PMU:

# drivers/perf/Makefile
obj-$(CONFIG_RISCV_PMU) += riscv_pmu.o

So having this check does seem redundant. I am copying Alex as it looks
like he wrote this.

- Charlie

> 
> Cheers,
> Conor.
> 
> > 
> > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > ---
> >  drivers/perf/riscv_pmu.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> > index 0a02e85a8951..7644147d50b4 100644
> > --- a/drivers/perf/riscv_pmu.c
> > +++ b/drivers/perf/riscv_pmu.c
> > @@ -39,7 +39,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> >  	userpg->cap_user_time_short = 0;
> >  	userpg->cap_user_rdpmc = riscv_perf_user_access(event);
> >  
> > -#ifdef CONFIG_RISCV_PMU
> >  	/*
> >  	 * The counters are 64-bit but the priv spec doesn't mandate all the
> >  	 * bits to be implemented: that's why, counter width can vary based on
> > @@ -47,7 +46,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> >  	 */
> >  	if (userpg->cap_user_rdpmc)
> >  		userpg->pmc_width = to_riscv_pmu(event->pmu)->ctr_get_width(event->hw.idx) + 1;
> > -#endif
> >  
> >  	do {
> >  		rd = sched_clock_read_begin(&seq);
> > -- 
> > 2.25.1
> > 
> > 
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] drivers/perf: riscv: Remove redundant macro check

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2362 bytes --]

On Tue, Jul 09, 2024 at 01:29:42PM -0700, Charlie Jenkins wrote:
> On Mon, Jul 08, 2024 at 01:22:11PM +0100, Conor Dooley wrote:
> > On Mon, Jul 08, 2024 at 08:12:24PM +0800, Xiao Wang wrote:
> > > The macro CONFIG_RISCV_PMU must have been defined when riscv_pmu.c gets
> > > compiled, so this patch removes the redundant check.
> > 
> > Did you investigate why this define was added? Why do you think that it
> > is redundant, rather than checking the incorrect config option?
> 
> This file is only compiled with CONFIG_RISCV_PMU:

I might be ill, but I can still read. I was not disagreeing with Xiao
that the condition is redundant as written - I want to know whether they
made sure that this check was intentionally using CONFIG_RISCV_PMU in the
first place, or if another option should have been here instead.

> 
> # drivers/perf/Makefile
> obj-$(CONFIG_RISCV_PMU) += riscv_pmu.o
> 
> So having this check does seem redundant. I am copying Alex as it looks
> like he wrote this.
> 
> - Charlie
> 
> > 
> > Cheers,
> > Conor.
> > 
> > > 
> > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > > ---
> > >  drivers/perf/riscv_pmu.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> > > index 0a02e85a8951..7644147d50b4 100644
> > > --- a/drivers/perf/riscv_pmu.c
> > > +++ b/drivers/perf/riscv_pmu.c
> > > @@ -39,7 +39,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> > >  	userpg->cap_user_time_short = 0;
> > >  	userpg->cap_user_rdpmc = riscv_perf_user_access(event);
> > >  
> > > -#ifdef CONFIG_RISCV_PMU
> > >  	/*
> > >  	 * The counters are 64-bit but the priv spec doesn't mandate all the
> > >  	 * bits to be implemented: that's why, counter width can vary based on
> > > @@ -47,7 +46,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> > >  	 */
> > >  	if (userpg->cap_user_rdpmc)
> > >  		userpg->pmc_width = to_riscv_pmu(event->pmu)->ctr_get_width(event->hw.idx) + 1;
> > > -#endif
> > >  
> > >  	do {
> > >  		rd = sched_clock_read_begin(&seq);
> > > -- 
> > > 2.25.1
> > > 
> > > 
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] drivers/perf: riscv: Remove redundant macro check

[Charlie Jenkins]

On Tue, Jul 09, 2024 at 09:44:17PM +0100, Conor Dooley wrote:
> On Tue, Jul 09, 2024 at 01:29:42PM -0700, Charlie Jenkins wrote:
> > On Mon, Jul 08, 2024 at 01:22:11PM +0100, Conor Dooley wrote:
> > > On Mon, Jul 08, 2024 at 08:12:24PM +0800, Xiao Wang wrote:
> > > > The macro CONFIG_RISCV_PMU must have been defined when riscv_pmu.c gets
> > > > compiled, so this patch removes the redundant check.
> > > 
> > > Did you investigate why this define was added? Why do you think that it
> > > is redundant, rather than checking the incorrect config option?
> > 
> > This file is only compiled with CONFIG_RISCV_PMU:
> 
> I might be ill, but I can still read. I was not disagreeing with Xiao
> that the condition is redundant as written - I want to know whether they
> made sure that this check was intentionally using CONFIG_RISCV_PMU in the
> first place, or if another option should have been here instead.

Makes sense! Looking through the lists I see this RFC from Atish where
he introduced a different config option for this
"CONFIG_RISCV_PMU_COMMON"[1]. I wonder if something got confused in the
development of these two patches.

Link:
https://lore.kernel.org/lkml/20240217005738.3744121-12-atishp@rivosinc.com/
[1]

- Charlie

> 
> > 
> > # drivers/perf/Makefile
> > obj-$(CONFIG_RISCV_PMU) += riscv_pmu.o
> > 
> > So having this check does seem redundant. I am copying Alex as it looks
> > like he wrote this.
> > 
> > - Charlie
> > 
> > > 
> > > Cheers,
> > > Conor.
> > > 
> > > > 
> > > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > > > ---
> > > >  drivers/perf/riscv_pmu.c | 2 --
> > > >  1 file changed, 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> > > > index 0a02e85a8951..7644147d50b4 100644
> > > > --- a/drivers/perf/riscv_pmu.c
> > > > +++ b/drivers/perf/riscv_pmu.c
> > > > @@ -39,7 +39,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> > > >  	userpg->cap_user_time_short = 0;
> > > >  	userpg->cap_user_rdpmc = riscv_perf_user_access(event);
> > > >  
> > > > -#ifdef CONFIG_RISCV_PMU
> > > >  	/*
> > > >  	 * The counters are 64-bit but the priv spec doesn't mandate all the
> > > >  	 * bits to be implemented: that's why, counter width can vary based on
> > > > @@ -47,7 +46,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> > > >  	 */
> > > >  	if (userpg->cap_user_rdpmc)
> > > >  		userpg->pmc_width = to_riscv_pmu(event->pmu)->ctr_get_width(event->hw.idx) + 1;
> > > > -#endif
> > > >  
> > > >  	do {
> > > >  		rd = sched_clock_read_begin(&seq);
> > > > -- 
> > > > 2.25.1
> > > > 
> > > > 
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > 

Re: [PATCH] drivers/perf: riscv: Remove redundant macro check

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3406 bytes --]

On Tue, Jul 09, 2024 at 01:57:47PM -0700, Charlie Jenkins wrote:
> On Tue, Jul 09, 2024 at 09:44:17PM +0100, Conor Dooley wrote:
> > On Tue, Jul 09, 2024 at 01:29:42PM -0700, Charlie Jenkins wrote:
> > > On Mon, Jul 08, 2024 at 01:22:11PM +0100, Conor Dooley wrote:
> > > > On Mon, Jul 08, 2024 at 08:12:24PM +0800, Xiao Wang wrote:
> > > > > The macro CONFIG_RISCV_PMU must have been defined when riscv_pmu.c gets
> > > > > compiled, so this patch removes the redundant check.
> > > > 
> > > > Did you investigate why this define was added? Why do you think that it
> > > > is redundant, rather than checking the incorrect config option?
> > > 
> > > This file is only compiled with CONFIG_RISCV_PMU:
> > 
> > I might be ill, but I can still read. I was not disagreeing with Xiao
> > that the condition is redundant as written - I want to know whether they
> > made sure that this check was intentionally using CONFIG_RISCV_PMU in the
> > first place, or if another option should have been here instead.
> 
> Makes sense! Looking through the lists I see this RFC from Atish where
> he introduced a different config option for this
> "CONFIG_RISCV_PMU_COMMON"[1]. I wonder if something got confused in the
> development of these two patches.

Perhaps.. What I was worried about was the wrong option being here
(maybe that it should have been RISCV_PMU_SBI or similar) and depending
on how the kernel is configured, userspace would get the wrong info
here. But maybe it is innocuous your theory would suggest, and there's
nothing to worry about. But that's for someone with a functioning brain
to figure out ;)

Cheers,
Conor.

> 
> Link:
> https://lore.kernel.org/lkml/20240217005738.3744121-12-atishp@rivosinc.com/
> [1]
> 
> > > # drivers/perf/Makefile
> > > obj-$(CONFIG_RISCV_PMU) += riscv_pmu.o
> > > 
> > > So having this check does seem redundant. I am copying Alex as it looks
> > > like he wrote this.
> > > 
> > > > > 
> > > > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > > > > ---
> > > > >  drivers/perf/riscv_pmu.c | 2 --
> > > > >  1 file changed, 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> > > > > index 0a02e85a8951..7644147d50b4 100644
> > > > > --- a/drivers/perf/riscv_pmu.c
> > > > > +++ b/drivers/perf/riscv_pmu.c
> > > > > @@ -39,7 +39,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> > > > >  	userpg->cap_user_time_short = 0;
> > > > >  	userpg->cap_user_rdpmc = riscv_perf_user_access(event);
> > > > >  
> > > > > -#ifdef CONFIG_RISCV_PMU
> > > > >  	/*
> > > > >  	 * The counters are 64-bit but the priv spec doesn't mandate all the
> > > > >  	 * bits to be implemented: that's why, counter width can vary based on
> > > > > @@ -47,7 +46,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> > > > >  	 */
> > > > >  	if (userpg->cap_user_rdpmc)
> > > > >  		userpg->pmc_width = to_riscv_pmu(event->pmu)->ctr_get_width(event->hw.idx) + 1;
> > > > > -#endif
> > > > >  
> > > > >  	do {
> > > > >  		rd = sched_clock_read_begin(&seq);
> > > > > -- 
> > > > > 2.25.1
> > > > > 
> > > > > 
> > > > > _______________________________________________
> > > > > linux-riscv mailing list
> > > > > linux-riscv@lists.infradead.org
> > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] drivers/perf: riscv: Remove redundant macro check

[Atish Patra]

On Tue, Jul 9, 2024 at 2:05 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Jul 09, 2024 at 01:57:47PM -0700, Charlie Jenkins wrote:
> > On Tue, Jul 09, 2024 at 09:44:17PM +0100, Conor Dooley wrote:
> > > On Tue, Jul 09, 2024 at 01:29:42PM -0700, Charlie Jenkins wrote:
> > > > On Mon, Jul 08, 2024 at 01:22:11PM +0100, Conor Dooley wrote:
> > > > > On Mon, Jul 08, 2024 at 08:12:24PM +0800, Xiao Wang wrote:
> > > > > > The macro CONFIG_RISCV_PMU must have been defined when riscv_pmu.c gets
> > > > > > compiled, so this patch removes the redundant check.
> > > > >
> > > > > Did you investigate why this define was added? Why do you think that it
> > > > > is redundant, rather than checking the incorrect config option?
> > > >
> > > > This file is only compiled with CONFIG_RISCV_PMU:
> > >
> > > I might be ill, but I can still read. I was not disagreeing with Xiao
> > > that the condition is redundant as written - I want to know whether they
> > > made sure that this check was intentionally using CONFIG_RISCV_PMU in the
> > > first place, or if another option should have been here instead.
> >
> > Makes sense! Looking through the lists I see this RFC from Atish where
> > he introduced a different config option for this
> > "CONFIG_RISCV_PMU_COMMON"[1]. I wonder if something got confused in the
> > development of these two patches.
>

Nope. That's not related as the above series is a complete revamp of
PMU infrastructure
and far from being merged.

> Perhaps.. What I was worried about was the wrong option being here
> (maybe that it should have been RISCV_PMU_SBI or similar) and depending
> on how the kernel is configured, userspace would get the wrong info
> here. But maybe it is innocuous your theory would suggest, and there's
> nothing to worry about. But that's for someone with a functioning brain
> to figure out ;)
>
This macro has been there since v3 of the original series.
https://lore.kernel.org/lkml/20230802080328.1213905-8-alexghiti@rivosinc.com/

Ideally, this should have been RISCV_PMU_SBI as ctr_get_width was not
defined earlier for legacy.
However, that was fixed here
https://lore.kernel.org/lkml/20240227170002.188671-3-vadim.shakirov@syntacore.com/

So we can remove the macro check here.

Reviewed-by: Atish Patra <atishp@rivosinc.com>

> Cheers,
> Conor.
>
> >
> > Link:
> > https://lore.kernel.org/lkml/20240217005738.3744121-12-atishp@rivosinc.com/
> > [1]
> >
> > > > # drivers/perf/Makefile
> > > > obj-$(CONFIG_RISCV_PMU) += riscv_pmu.o
> > > >
> > > > So having this check does seem redundant. I am copying Alex as it looks
> > > > like he wrote this.
> > > >
> > > > > >
> > > > > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > > > > > ---
> > > > > >  drivers/perf/riscv_pmu.c | 2 --
> > > > > >  1 file changed, 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> > > > > > index 0a02e85a8951..7644147d50b4 100644
> > > > > > --- a/drivers/perf/riscv_pmu.c
> > > > > > +++ b/drivers/perf/riscv_pmu.c
> > > > > > @@ -39,7 +39,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> > > > > >       userpg->cap_user_time_short = 0;
> > > > > >       userpg->cap_user_rdpmc = riscv_perf_user_access(event);
> > > > > >
> > > > > > -#ifdef CONFIG_RISCV_PMU
> > > > > >       /*
> > > > > >        * The counters are 64-bit but the priv spec doesn't mandate all the
> > > > > >        * bits to be implemented: that's why, counter width can vary based on
> > > > > > @@ -47,7 +46,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> > > > > >        */
> > > > > >       if (userpg->cap_user_rdpmc)
> > > > > >               userpg->pmc_width = to_riscv_pmu(event->pmu)->ctr_get_width(event->hw.idx) + 1;
> > > > > > -#endif
> > > > > >
> > > > > >       do {
> > > > > >               rd = sched_clock_read_begin(&seq);
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > > > >
> > > > > > _______________________________________________
> > > > > > linux-riscv mailing list
> > > > > > linux-riscv@lists.infradead.org
> > > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > >
> >
> >



-- 
Regards,
Atish

RE: [PATCH] drivers/perf: riscv: Remove redundant macro check

[Wang, Xiao W]

> -----Original Message-----
> From: Atish Patra <atishp@atishpatra.org>
> Sent: Wednesday, July 10, 2024 5:44 AM
> To: Conor Dooley <conor@kernel.org>
> Cc: Charlie Jenkins <charlie@rivosinc.com>; Wang, Xiao W
> <xiao.w.wang@intel.com>; Alexandre Ghiti <alexghiti@rivosinc.com>;
> paul.walmsley@sifive.com; palmer@dabbelt.com; aou@eecs.berkeley.edu;
> anup@brainfault.org; linux-riscv@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] drivers/perf: riscv: Remove redundant macro check
> 
> On Tue, Jul 9, 2024 at 2:05 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Tue, Jul 09, 2024 at 01:57:47PM -0700, Charlie Jenkins wrote:
> > > On Tue, Jul 09, 2024 at 09:44:17PM +0100, Conor Dooley wrote:
> > > > On Tue, Jul 09, 2024 at 01:29:42PM -0700, Charlie Jenkins wrote:
> > > > > On Mon, Jul 08, 2024 at 01:22:11PM +0100, Conor Dooley wrote:
> > > > > > On Mon, Jul 08, 2024 at 08:12:24PM +0800, Xiao Wang wrote:
> > > > > > > The macro CONFIG_RISCV_PMU must have been defined when
> riscv_pmu.c gets
> > > > > > > compiled, so this patch removes the redundant check.
> > > > > >
> > > > > > Did you investigate why this define was added? Why do you think
> that it
> > > > > > is redundant, rather than checking the incorrect config option?
> > > > >
> > > > > This file is only compiled with CONFIG_RISCV_PMU:
> > > >
> > > > I might be ill, but I can still read. I was not disagreeing with Xiao
> > > > that the condition is redundant as written - I want to know whether they
> > > > made sure that this check was intentionally using CONFIG_RISCV_PMU in
> the
> > > > first place, or if another option should have been here instead.
> > >
> > > Makes sense! Looking through the lists I see this RFC from Atish where
> > > he introduced a different config option for this
> > > "CONFIG_RISCV_PMU_COMMON"[1]. I wonder if something got
> confused in the
> > > development of these two patches.
> >
> 
> Nope. That's not related as the above series is a complete revamp of
> PMU infrastructure
> and far from being merged.
> 
> > Perhaps.. What I was worried about was the wrong option being here
> > (maybe that it should have been RISCV_PMU_SBI or similar) and depending
> > on how the kernel is configured, userspace would get the wrong info
> > here. But maybe it is innocuous your theory would suggest, and there's
> > nothing to worry about. But that's for someone with a functioning brain
> > to figure out ;)
> >
> This macro has been there since v3 of the original series.
> https://lore.kernel.org/lkml/20230802080328.1213905-8-
> alexghiti@rivosinc.com/
> 
> Ideally, this should have been RISCV_PMU_SBI as ctr_get_width was not
> defined earlier for legacy.
> However, that was fixed here
> https://lore.kernel.org/lkml/20240227170002.188671-3-
> vadim.shakirov@syntacore.com/
> 
> So we can remove the macro check here.
> 
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
> 

Thanks for all the comments from Conor, Charlie and Atish.
From userspace perspective (e.g. tools/lib/perf/mmap.c), if cap_user_rdpmc is enabled,
then pmc_width should be set. It's independent of arch and config options.

BRs,
Xiao

> > Cheers,
> > Conor.
> >
> > >
> > > Link:
> > > https://lore.kernel.org/lkml/20240217005738.3744121-12-
> atishp@rivosinc.com/
> > > [1]
> > >
> > > > > # drivers/perf/Makefile
> > > > > obj-$(CONFIG_RISCV_PMU) += riscv_pmu.o
> > > > >
> > > > > So having this check does seem redundant. I am copying Alex as it looks
> > > > > like he wrote this.
> > > > >
> > > > > > >
> > > > > > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > > > > > > ---
> > > > > > >  drivers/perf/riscv_pmu.c | 2 --
> > > > > > >  1 file changed, 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> > > > > > > index 0a02e85a8951..7644147d50b4 100644
> > > > > > > --- a/drivers/perf/riscv_pmu.c
> > > > > > > +++ b/drivers/perf/riscv_pmu.c
> > > > > > > @@ -39,7 +39,6 @@ void arch_perf_update_userpage(struct
> perf_event *event,
> > > > > > >       userpg->cap_user_time_short = 0;
> > > > > > >       userpg->cap_user_rdpmc = riscv_perf_user_access(event);
> > > > > > >
> > > > > > > -#ifdef CONFIG_RISCV_PMU
> > > > > > >       /*
> > > > > > >        * The counters are 64-bit but the priv spec doesn't mandate all
> the
> > > > > > >        * bits to be implemented: that's why, counter width can vary
> based on
> > > > > > > @@ -47,7 +46,6 @@ void arch_perf_update_userpage(struct
> perf_event *event,
> > > > > > >        */
> > > > > > >       if (userpg->cap_user_rdpmc)
> > > > > > >               userpg->pmc_width = to_riscv_pmu(event->pmu)-
> >ctr_get_width(event->hw.idx) + 1;
> > > > > > > -#endif
> > > > > > >
> > > > > > >       do {
> > > > > > >               rd = sched_clock_read_begin(&seq);
> > > > > > > --
> > > > > > > 2.25.1
> > > > > > >
> > > > > > >
> > > > > > > _______________________________________________
> > > > > > > linux-riscv mailing list
> > > > > > > linux-riscv@lists.infradead.org
> > > > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > > >
> > >
> > >
> 
> 
> 
> --
> Regards,
> Atish

Re: [PATCH] drivers/perf: riscv: Remove redundant macro check

[patchwork-bot+linux-riscv]

Hello:

This patch was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Mon,  8 Jul 2024 20:12:24 +0800 you wrote:
> The macro CONFIG_RISCV_PMU must have been defined when riscv_pmu.c gets
> compiled, so this patch removes the redundant check.
> 
> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> ---
>  drivers/perf/riscv_pmu.c | 2 --
>  1 file changed, 2 deletions(-)

Here is the summary with links:
  - drivers/perf: riscv: Remove redundant macro check
    https://git.kernel.org/riscv/c/1e206fad765b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html