linux-kernel - Re: [RFC v6 1/5] sched: Introduce switch to enable TurboSched for task packing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0fb8fa5c-0edd-913d-912f-df383a3d4007@linux.intel.com>
Date:   Wed, 22 Jan 2020 13:37:06 -0800
From:   Tim Chen <tim.c.chen@...ux.intel.com>
To:     Parth Shah <parth@...ux.ibm.com>, linux-kernel@...r.kernel.org,
        linux-pm@...r.kernel.org
Cc:     peterz@...radead.org, mingo@...hat.com, vincent.guittot@...aro.org,
        dietmar.eggemann@....com, patrick.bellasi@...bug.net,
        valentin.schneider@....com, pavel@....cz, dsmythies@...us.net,
        qperret@...gle.com
Subject: Re: [RFC v6 1/5] sched: Introduce switch to enable TurboSched for
 task packing

On 1/20/20 10:33 PM, Parth Shah wrote:
> Create a static key which allows to enable or disable TurboSched feature at
> runtime.
> 
> This key is added in order to enable the TurboSched feature only when
> required. This helps in optimizing the scheduler fast-path when the
> TurboSched feature is disabled.
> 
> Also provide get/put methods to keep track of the tasks using the
> TurboSched feature and also refcount classified background tasks. This
> allows to enable the feature on setting first task classified as background
> noise, similarly disable the feature on unsetting of such last task.
> 
> Signed-off-by: Parth Shah <parth@...ux.ibm.com>
> ---
>  kernel/sched/core.c  | 25 +++++++++++++++++++++++++
>  kernel/sched/sched.h | 12 ++++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index a9e5d157b1a5..dfbb52d66b29 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -73,6 +73,31 @@ __read_mostly int scheduler_running;
>   */
>  int sysctl_sched_rt_runtime = 950000;
>  
> +#ifdef CONFIG_SCHED_SMT
> +DEFINE_STATIC_KEY_FALSE(__turbo_sched_enabled);
> +static DEFINE_MUTEX(turbo_sched_lock);
> +static int turbo_sched_count;
> +
> +void turbo_sched_get(void)
> +{
> +	mutex_lock(&turbo_sched_lock);
> +	if (!turbo_sched_count++)
> +		static_branch_enable(&__turbo_sched_enabled);

If you use static_branch_inc(&__turbo_sched_enabled) and
static_branch_dec(&__turbo_sched_enabled),  you don't have
to define turbo_sched_count. And turbo_sched_lock is
also unnecessary as static_branch_inc/dec are atomic.

> +	mutex_unlock(&turbo_sched_lock);
> +}
> +
> +void turbo_sched_put(void)
> +{
> +	mutex_lock(&turbo_sched_lock);
> +	if (!--turbo_sched_count)
> +		static_branch_disable(&__turbo_sched_enabled);
> +	mutex_unlock(&turbo_sched_lock);
> +}
> +#else
> +void turbo_sched_get(void) { return ; }
> +void turbo_sched_get(void) { return ; }

Double definition of turbo_sched_get.
You probably meant turbo_sched_put in the second definition.

Tim