Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 31 Jul 2021 15:48:31 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Alexander Motin <mav@freebsd.org>
Cc:        src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org
Subject:   Re: git: 74f80bc1af2f - main - coretemp(4): Switch to smp_rendezvous_cpus().
Message-ID:  <YQVGn7VhKstcGndr@kib.kiev.ua>
In-Reply-To: <202107300326.16U3QGHR070204@gitrepo.freebsd.org>
References:  <202107300326.16U3QGHR070204@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jul 30, 2021 at 03:26:16AM +0000, Alexander Motin wrote:
> The branch main has been updated by mav:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=74f80bc1af2ffd56ec290f610c80e46f768731a0
> 
> commit 74f80bc1af2ffd56ec290f610c80e46f768731a0
> Author:     Alexander Motin <mav@FreeBSD.org>
> AuthorDate: 2021-07-30 03:16:22 +0000
> Commit:     Alexander Motin <mav@FreeBSD.org>
> CommitDate: 2021-07-30 03:26:10 +0000
> 
>     coretemp(4): Switch to smp_rendezvous_cpus().
>     
>     Use of smp_rendezvous_cpus() instead of sched_bind() allows to not
>     block indefinitely if target CPU is running some thread with higher
>     priority, while all we need is single rdmsr/wrmsr instruction call.
>     I guess it should also be much cheaper than full thread migration.
>     
>     MFC after:      2 weeks
>     Sponsored by:   iXsystems, Inc.
> ---
>  sys/dev/coretemp/coretemp.c | 59 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 37 insertions(+), 22 deletions(-)
> 
> diff --git a/sys/dev/coretemp/coretemp.c b/sys/dev/coretemp/coretemp.c
> index 884ed6309f0e..53a2434254f6 100644
> --- a/sys/dev/coretemp/coretemp.c
> +++ b/sys/dev/coretemp/coretemp.c
> @@ -42,7 +42,7 @@ __FBSDID("$FreeBSD$");
>  #include <sys/module.h>
>  #include <sys/mutex.h>
>  #include <sys/proc.h>	/* for curthread */
> -#include <sys/sched.h>
> +#include <sys/smp.h>
>  #include <sys/sysctl.h>
>  #include <sys/systm.h>
>  
> @@ -310,14 +310,32 @@ coretemp_detach(device_t dev)
>  	return (0);
>  }
>  
> +struct coretemp_args {
> +	u_int		msr;
> +	uint64_t	val;
> +};
> +
> +static void
> +coretemp_rdmsr(void *arg)
> +{
> +	struct coretemp_args *args = arg;
> +
> +	args->val = rdmsr(args->msr);
> +}
> +
> +static void
> +coretemp_wrmsr(void *arg)
> +{
> +	struct coretemp_args *args = arg;
> +
> +	wrmsr(args->msr, args->val);
> +}
We have x86_msr_op().  It covers coretemp_wrmsr(), and with slight
change of the interface would also handle the functionality of
coretemp_rdmsr().

> +
>  static uint64_t
>  coretemp_get_thermal_msr(int cpu)
>  {
> -	uint64_t msr;
> -
> -	thread_lock(curthread);
> -	sched_bind(curthread, cpu);
> -	thread_unlock(curthread);
> +	struct coretemp_args args;
> +	cpuset_t cpus;
>  
>  	/*
>  	 * The digital temperature reading is located at bit 16
> @@ -329,27 +347,24 @@ coretemp_get_thermal_msr(int cpu)
>  	 * The temperature is computed by subtracting the temperature
>  	 * reading by Tj(max).
>  	 */
> -	msr = rdmsr(MSR_THERM_STATUS);
> -
> -	thread_lock(curthread);
> -	sched_unbind(curthread);
> -	thread_unlock(curthread);
> -
> -	return (msr);
> +	args.msr = MSR_THERM_STATUS;
> +	CPU_SETOF(cpu, &cpus);
> +	smp_rendezvous_cpus(cpus, smp_no_rendezvous_barrier, coretemp_rdmsr,
> +	    smp_no_rendezvous_barrier, &args);
> +	return (args.val);
>  }
>  
>  static void
>  coretemp_clear_thermal_msr(int cpu)
>  {
> -	thread_lock(curthread);
> -	sched_bind(curthread, cpu);
> -	thread_unlock(curthread);
> -
> -	wrmsr(MSR_THERM_STATUS, 0);
> -
> -	thread_lock(curthread);
> -	sched_unbind(curthread);
> -	thread_unlock(curthread);
> +	struct coretemp_args args;
> +	cpuset_t cpus;
> +
> +	args.msr = MSR_THERM_STATUS;
> +	args.val = 0;
> +	CPU_SETOF(cpu, &cpus);
> +	smp_rendezvous_cpus(cpus, smp_no_rendezvous_barrier, coretemp_wrmsr,
> +	    smp_no_rendezvous_barrier, &args);
>  }
>  
>  static int



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YQVGn7VhKstcGndr>