Linux Power Management development
 help / color / mirror / Atom feed
* [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n
@ 2025-06-14  2:20 Calvin Owens
  2025-06-16  1:30 ` Zhang, Rui
  0 siblings, 1 reply; 6+ messages in thread
From: Calvin Owens @ 2025-06-14  2:20 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-pm, Len Brown

Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
turbostat can display temperatures when running on kernels compiled
without multiuser support.

Signed-off-by: Calvin Owens <calvin@wbinvd.org>
---
 tools/power/x86/turbostat/turbostat.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 925556b90770..f7d665913a52 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
 	int ret = 0;
 
 	caps = cap_get_proc();
-	if (caps == NULL)
+	if (caps == NULL) {
+		/* Support CONFIG_MULTIUSER=n */
+		if (errno == ENOSYS)
+			return 0;
+
 		return 1;
+	}
 
 	if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE, &cap_flag_value)) {
 		ret = 1;
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n
  2025-06-14  2:20 [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n Calvin Owens
@ 2025-06-16  1:30 ` Zhang, Rui
  2025-06-16  4:38   ` Calvin Owens
  0 siblings, 1 reply; 6+ messages in thread
From: Zhang, Rui @ 2025-06-16  1:30 UTC (permalink / raw)
  To: calvin@wbinvd.org, linux-kernel@vger.kernel.org
  Cc: linux-pm@vger.kernel.org, lenb@kernel.org

On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> turbostat can display temperatures when running on kernels compiled
> without multiuser support.
> 
> Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> ---
>  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/power/x86/turbostat/turbostat.c
> b/tools/power/x86/turbostat/turbostat.c
> index 925556b90770..f7d665913a52 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
>  	int ret = 0;
>  
>  	caps = cap_get_proc();
> -	if (caps == NULL)
> +	if (caps == NULL) {
> +		/* Support CONFIG_MULTIUSER=n */
> +		if (errno == ENOSYS)

Can you point me where this knowledge comes from?

I downloaded the libcap source and didn't see how ENOSYS is set.

thanks,
rui
> +			return 0;
> +
>  		return 1;
> +	}
>  
>  	if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> &cap_flag_value)) {
>  		ret = 1;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n
  2025-06-16  1:30 ` Zhang, Rui
@ 2025-06-16  4:38   ` Calvin Owens
  2025-06-16  4:44     ` Calvin Owens
  2025-06-18  3:28     ` Zhang, Rui
  0 siblings, 2 replies; 6+ messages in thread
From: Calvin Owens @ 2025-06-16  4:38 UTC (permalink / raw)
  To: Zhang, Rui
  Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	lenb@kernel.org

On Monday 06/16 at 01:30 +0000, Zhang, Rui wrote:
> On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> > Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> > turbostat can display temperatures when running on kernels compiled
> > without multiuser support.
> > 
> > Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> > ---
> >  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/power/x86/turbostat/turbostat.c
> > b/tools/power/x86/turbostat/turbostat.c
> > index 925556b90770..f7d665913a52 100644
> > --- a/tools/power/x86/turbostat/turbostat.c
> > +++ b/tools/power/x86/turbostat/turbostat.c
> > @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
> >  	int ret = 0;
> >  
> >  	caps = cap_get_proc();
> > -	if (caps == NULL)
> > +	if (caps == NULL) {
> > +		/* Support CONFIG_MULTIUSER=n */
> > +		if (errno == ENOSYS)
> 
> Can you point me where this knowledge comes from?
> 
> I downloaded the libcap source and didn't see how ENOSYS is set.

Hi Rui,

When the kernel is built without multiuser support, the capget() et al.
syscalls are #ifdef'd out:

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/capability.c#n32

...so userspace will get -ENOSYS when it tries to call them, and that
ends up being propagated to errno in userspace.

Admittedly it is sort of implicit. Maybe a better way to "fix" this
would be to warn the user if the capability check fails, but still
attempt to access the MSR devices? I can do that if you prefer.

That is my only problem here: when check_for_cap_sys_rawio() fails, the
current code doesn't attempt to access the MSR devices at all, even
though in my case it would actually work.

I realize this is very weird: it came up when I was recently including
turbostat as part of an extremely tiny bootable utility image.

Thanks,
Calvin

> thanks,
> rui
> > +			return 0;
> > +
> >  		return 1;
> > +	}
> >  
> >  	if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> > &cap_flag_value)) {
> >  		ret = 1;
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n
  2025-06-16  4:38   ` Calvin Owens
@ 2025-06-16  4:44     ` Calvin Owens
  2025-06-18  3:28     ` Zhang, Rui
  1 sibling, 0 replies; 6+ messages in thread
From: Calvin Owens @ 2025-06-16  4:44 UTC (permalink / raw)
  To: Zhang, Rui
  Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	lenb@kernel.org

On Sunday 06/15 at 21:38 -0700, Calvin Owens wrote:
> On Monday 06/16 at 01:30 +0000, Zhang, Rui wrote:
> > On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> > > Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> > > turbostat can display temperatures when running on kernels compiled
> > > without multiuser support.
> > > 
> > > Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> > > ---
> > >  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/power/x86/turbostat/turbostat.c
> > > b/tools/power/x86/turbostat/turbostat.c
> > > index 925556b90770..f7d665913a52 100644
> > > --- a/tools/power/x86/turbostat/turbostat.c
> > > +++ b/tools/power/x86/turbostat/turbostat.c
> > > @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
> > >  	int ret = 0;
> > >  
> > >  	caps = cap_get_proc();
> > > -	if (caps == NULL)
> > > +	if (caps == NULL) {
> > > +		/* Support CONFIG_MULTIUSER=n */
> > > +		if (errno == ENOSYS)
> > 
> > Can you point me where this knowledge comes from?
> > 
> > I downloaded the libcap source and didn't see how ENOSYS is set.
> 
> Hi Rui,
> 
> When the kernel is built without multiuser support, the capget() et al.
> syscalls are #ifdef'd out:
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/capability.c#n32
> 
> ...so userspace will get -ENOSYS when it tries to call them, and that
> ends up being propagated to errno in userspace.

Sorry, what I meant was: the kernel syscall returns -ENOSYS to
userspace, and the syscall() wrapper (in userspace) assigns the positive
error value to errno, where it remains after cap_get_proc() returns.

> Admittedly it is sort of implicit. Maybe a better way to "fix" this
> would be to warn the user if the capability check fails, but still
> attempt to access the MSR devices? I can do that if you prefer.
> 
> That is my only problem here: when check_for_cap_sys_rawio() fails, the
> current code doesn't attempt to access the MSR devices at all, even
> though in my case it would actually work.
> 
> I realize this is very weird: it came up when I was recently including
> turbostat as part of an extremely tiny bootable utility image.
> 
> Thanks,
> Calvin
> 
> > thanks,
> > rui
> > > +			return 0;
> > > +
> > >  		return 1;
> > > +	}
> > >  
> > >  	if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> > > &cap_flag_value)) {
> > >  		ret = 1;
> > 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n
  2025-06-16  4:38   ` Calvin Owens
  2025-06-16  4:44     ` Calvin Owens
@ 2025-06-18  3:28     ` Zhang, Rui
  2025-06-18 14:32       ` Len Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Zhang, Rui @ 2025-06-18  3:28 UTC (permalink / raw)
  To: calvin@wbinvd.org
  Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	lenb@kernel.org

On Sun, 2025-06-15 at 21:38 -0700, Calvin Owens wrote:
> On Monday 06/16 at 01:30 +0000, Zhang, Rui wrote:
> > On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> > > Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> > > turbostat can display temperatures when running on kernels compiled
> > > without multiuser support.
> > > 
> > > Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> > > ---
> > >  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/power/x86/turbostat/turbostat.c
> > > b/tools/power/x86/turbostat/turbostat.c
> > > index 925556b90770..f7d665913a52 100644
> > > --- a/tools/power/x86/turbostat/turbostat.c
> > > +++ b/tools/power/x86/turbostat/turbostat.c
> > > @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
> > >  	int ret = 0;
> > >  
> > >  	caps = cap_get_proc();
> > > -	if (caps == NULL)
> > > +	if (caps == NULL) {
> > > +		/* Support CONFIG_MULTIUSER=n */
> > > +		if (errno == ENOSYS)
> > 
> > Can you point me where this knowledge comes from?
> > 
> > I downloaded the libcap source and didn't see how ENOSYS is set.
> 
> Hi Rui,
> 
> When the kernel is built without multiuser support, the capget() et al.
> syscalls are #ifdef'd out:
> 
>    
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/capability.c#n32
> 
> ...so userspace will get -ENOSYS when it tries to call them, and that
> ends up being propagated to errno in userspace.
> 
> Admittedly it is sort of implicit. Maybe a better way to "fix" this
> would be to warn the user if the capability check fails, but still
> attempt to access the MSR devices? I can do that if you prefer.

if cap_get_proc returns -ENOSYS only when CONFIG_MULTIUSER=n, then I
think the patch is good, maybe we just need some more detailed comment.

and I don't think we should continue when the capability check fails in
general.

> 
> That is my only problem here: when check_for_cap_sys_rawio() fails, the
> current code doesn't attempt to access the MSR devices at all, even
> though in my case it would actually work.

Or maybe we can cover this with the "--force" parameter?
say, does it make sense to address the problem by running "turbostat --
force"?
I'll leave this question to Len.

thanks,
rui
> 
> I realize this is very weird: it came up when I was recently including
> turbostat as part of an extremely tiny bootable utility image.
> 
> Thanks,
> Calvin
> 
> > thanks,
> > rui
> > > +			return 0;
> > > +
> > >  		return 1;
> > > +	}
> > >  
> > >  	if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> > > &cap_flag_value)) {
> > >  		ret = 1;
> > 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n
  2025-06-18  3:28     ` Zhang, Rui
@ 2025-06-18 14:32       ` Len Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Len Brown @ 2025-06-18 14:32 UTC (permalink / raw)
  To: Zhang, Rui
  Cc: calvin@wbinvd.org, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org

Thanks for the patch, Calvin, I think it is correct.

Rui,
I'll tweak the comment to explain that the system call doesn't always exist.

I think it is correct to continue on -- the rawio check is basically a
graceful way
to prevent an error in accessing MSRs later.  If we don't have that
graceful mechanism,
then we will or will not hit the access error later, and that's okay.

Interesting configuration you have there Calvin.

thanks,
-Len


On Tue, Jun 17, 2025 at 11:28 PM Zhang, Rui <rui.zhang@intel.com> wrote:
>
> On Sun, 2025-06-15 at 21:38 -0700, Calvin Owens wrote:
> > On Monday 06/16 at 01:30 +0000, Zhang, Rui wrote:
> > > On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> > > > Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> > > > turbostat can display temperatures when running on kernels compiled
> > > > without multiuser support.
> > > >
> > > > Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> > > > ---
> > > >  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/power/x86/turbostat/turbostat.c
> > > > b/tools/power/x86/turbostat/turbostat.c
> > > > index 925556b90770..f7d665913a52 100644
> > > > --- a/tools/power/x86/turbostat/turbostat.c
> > > > +++ b/tools/power/x86/turbostat/turbostat.c
> > > > @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
> > > >   int ret = 0;
> > > >
> > > >   caps = cap_get_proc();
> > > > - if (caps == NULL)
> > > > + if (caps == NULL) {
> > > > +         /* Support CONFIG_MULTIUSER=n */
> > > > +         if (errno == ENOSYS)
> > >
> > > Can you point me where this knowledge comes from?
> > >
> > > I downloaded the libcap source and didn't see how ENOSYS is set.
> >
> > Hi Rui,
> >
> > When the kernel is built without multiuser support, the capget() et al.
> > syscalls are #ifdef'd out:
> >
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/capability.c#n32
> >
> > ...so userspace will get -ENOSYS when it tries to call them, and that
> > ends up being propagated to errno in userspace.
> >
> > Admittedly it is sort of implicit. Maybe a better way to "fix" this
> > would be to warn the user if the capability check fails, but still
> > attempt to access the MSR devices? I can do that if you prefer.
>
> if cap_get_proc returns -ENOSYS only when CONFIG_MULTIUSER=n, then I
> think the patch is good, maybe we just need some more detailed comment.
>
> and I don't think we should continue when the capability check fails in
> general.
>
> >
> > That is my only problem here: when check_for_cap_sys_rawio() fails, the
> > current code doesn't attempt to access the MSR devices at all, even
> > though in my case it would actually work.
>
> Or maybe we can cover this with the "--force" parameter?
> say, does it make sense to address the problem by running "turbostat --
> force"?
> I'll leave this question to Len.
>
> thanks,
> rui
> >
> > I realize this is very weird: it came up when I was recently including
> > turbostat as part of an extremely tiny bootable utility image.
> >
> > Thanks,
> > Calvin
> >
> > > thanks,
> > > rui
> > > > +                 return 0;
> > > > +
> > > >           return 1;
> > > > + }
> > > >
> > > >   if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> > > > &cap_flag_value)) {
> > > >           ret = 1;
> > >
>


-- 
Len Brown, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-06-18 14:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-14  2:20 [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n Calvin Owens
2025-06-16  1:30 ` Zhang, Rui
2025-06-16  4:38   ` Calvin Owens
2025-06-16  4:44     ` Calvin Owens
2025-06-18  3:28     ` Zhang, Rui
2025-06-18 14:32       ` Len Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox