2017-12-07 05:15:47

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] cpufreq: schedutil: update CFS util only if used

On 30-11-17, 15:57, Patrick Bellasi wrote:
> Yes, that's a pretty trivial update with a confusing changelog.
>
> If we think it's worth to keep (and correct as well) I'll update the
> commit message.

We also need to update the commit log based on feedback from Vikram on
V2. Which said that the utilization can't change around the lock here
as we are within rq lock section, though max can change (maybe). So
this patch only takes care of locking before reading max.

--
viresh


2017-12-07 14:19:33

by Patrick Bellasi

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] cpufreq: schedutil: update CFS util only if used

On 07-Dec 10:45, Viresh Kumar wrote:
> On 30-11-17, 15:57, Patrick Bellasi wrote:
> > Yes, that's a pretty trivial update with a confusing changelog.
> >
> > If we think it's worth to keep (and correct as well) I'll update the
> > commit message.
>
> We also need to update the commit log based on feedback from Vikram on
> V2. Which said that the utilization can't change around the lock here
> as we are within rq lock section, though max can change (maybe). So
> this patch only takes care of locking before reading max.

Ok, right... will do.

Thus you are still of the opinion to keep this patch in the series?

--
#include <best/regards.h>

Patrick Bellasi

2017-12-14 04:45:28

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] cpufreq: schedutil: update CFS util only if used

On 07-12-17, 14:19, Patrick Bellasi wrote:
> On 07-Dec 10:45, Viresh Kumar wrote:
> > On 30-11-17, 15:57, Patrick Bellasi wrote:
> > > Yes, that's a pretty trivial update with a confusing changelog.
> > >
> > > If we think it's worth to keep (and correct as well) I'll update the
> > > commit message.
> >
> > We also need to update the commit log based on feedback from Vikram on
> > V2. Which said that the utilization can't change around the lock here
> > as we are within rq lock section, though max can change (maybe). So
> > this patch only takes care of locking before reading max.

I have more doubts on the max reason as well. Max isn't protected by the
sg_policy lock of schedutil and it can change any time. So even after moving
code around with this patch, we wouldn't fix any race and so I am not sure this
patch helps at all. But, I have sent the same diff for another reason now in my
series. Maybe I should have kept you as the author of that patch, but I forgot.
Will do that if I need to send a V2.

> Ok, right... will do.
>
> Thus you are still of the opinion to keep this patch in the series?

Yes, but we need a good reason for that :)

--
viresh