lkml.org 
[lkml]   [2020]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] sched/core: Fix reset-on-fork from RT with uclamp
On 15-Apr 09:31, Doug Anderson wrote:
> Hi,
>
> On Wed, Apr 15, 2020 at 1:20 AM Quentin Perret <qperret@google.com> wrote:
> >
> > Hi Doug,
> > On Tuesday 14 Apr 2020 at 13:45:03 (-0700), Doug Anderson wrote:
> > > Hi,
> > > On Tue, Apr 14, 2020 at 9:13 AM Quentin Perret <qperret@google.com> wrote:

[...]

> > > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > > > index 3a61a3b8eaa9..9ea3e484eea2 100644
> > > > --- a/kernel/sched/core.c
> > > > +++ b/kernel/sched/core.c
> > > > @@ -1234,10 +1234,6 @@ static void uclamp_fork(struct task_struct *p)
> > > > for_each_clamp_id(clamp_id) {
> > > > unsigned int clamp_value = uclamp_none(clamp_id);
> > > >
> > > > - /* By default, RT tasks always get 100% boost */
> > > > - if (unlikely(rt_task(p) && clamp_id == UCLAMP_MIN))
> > > > - clamp_value = uclamp_none(UCLAMP_MAX);
> > > > -
> > > > uclamp_se_set(&p->uclamp_req[clamp_id], clamp_value, false);
> > >
> > > The local variable "clamp_value" doesn't have a lot of value anymore,
> > > does it? (Pun intended).
> >
> > :)
> >
> > > Remove it?
> >
> > Right, but I figured the generated code should be similar, and
> > 'uclamp_se_set(&p->uclamp_req[clamp_id], uclamp_none(clamp_id), false);'
> > doesn't fit in 80 cols at this identation level, so I kept the local
> > var. No strong opinion, though.
>
> OK. It's definitely a bikeshed color issue and since you'll spend
> more time in this bikeshed than I will I'll leave it to you to pick
> the color.
>
> I'm not at all an expert on this code but it sure looks sane to me.
> If you think my review tag is worth anything in this context feel free
> to add it, but since you already have Patrick's mine probably adds
> very little value.

Honestly I was almost there to ask for the same cleanup. :)

If you get rid of the useless variable we can go back to the exact
same code modified by the commit we are fixing, i.e. 1a00d999971c.

So, not strong opinions here too, but if it's not a big problem I
would post a cleaned up v2.

Best,
Patrick

--
#include <best/regards.h>

Patrick Bellasi

\
 
 \ /
  Last update: 2020-04-15 19:47    [W:0.096 / U:1.944 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site