linux-kernel - Re: [PATCH v3 1/1] sched/uclamp: add SCHED_FLAG_UTIL_CLAMP

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]

Date:   Wed, 28 Oct 2020 11:39:43 +0000
From:   Qais Yousef <qais.yousef@....com>
To:     Patrick Bellasi <patrick.bellasi@...bug.net>
Cc:     Yun Hsiang <hsiang023167@...il.com>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        peterz@...radead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/1] sched/uclamp: add SCHED_FLAG_UTIL_CLAMP_RESET
 flag to reset uclamp

On 10/28/20 11:11, Patrick Bellasi wrote:
> >>  
> >>  		/*
> >>  		 * RT by default have a 100% boost value that could be modified
> >>  		 * at runtime.
> >>  		 */
> >>  		if (unlikely(rt_task(p) && clamp_id == UCLAMP_MIN))
> >> -			__uclamp_update_util_min_rt_default(p);
> >> +			value = sysctl_sched_uclamp_util_min_rt_default;
> 
> By removing this usage of __uclamp_updadate_util_min_rt_default(p),
> the only other usage remaining is the call from:
>    uclamp_udpate_util_min_rt_default().
> 
> What about an additional cleanup by in-lining the only surviving usage?

This is not a cleanup IMO. There is special rule about updating that are
encoded and documented in this helper function. Namely:

	* p->pi_lock must be held.
	* p->uclamp_req[].user_defined must be false.

I don't see open coding helps but rather makes the code harder to read and
prone to introduce bugs if anything gets reshuffled in the future.

Thanks

--
Qais Yousef