Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1580738 > unrolled thread
| Started by | Uladzislau Rezki <urezki@gmail.com> |
|---|---|
| First post | 2017-02-14 19:30 +0100 |
| Last post | 2017-02-16 12:30 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [RFC,v2 3/3] sched: ignore task_h_load for CPU_NEWLY_IDLE Uladzislau Rezki <urezki@gmail.com> - 2017-02-14 19:30 +0100
Re: [RFC,v2 3/3] sched: ignore task_h_load for CPU_NEWLY_IDLE Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-02-15 20:00 +0100
Re: [RFC,v2 3/3] sched: ignore task_h_load for CPU_NEWLY_IDLE Uladzislau Rezki <urezki@gmail.com> - 2017-02-16 12:30 +0100
| From | Uladzislau Rezki <urezki@gmail.com> |
|---|---|
| Date | 2017-02-14 19:30 +0100 |
| Subject | Re: [RFC,v2 3/3] sched: ignore task_h_load for CPU_NEWLY_IDLE |
| Message-ID | <taQ1s-5hv-9@gated-at.bofh.it> |
>> >> So that is useful information that should have been in the Changelog. >> >> OK, can you respin this patch with adjusted Changelog and taking Mike's >> feedback? >> > Yes, i will prepare a patch accordingly, no problem. > >> >> Also, I worry about the effects of this on !PREEMPT kernels, the first >> hunk (which explicitly states is about latency) should be under >> CONFIG_PREEMPT to match the similar case we already have in >> detach_tasks(). >> >> But your second hunk, which ignores the actual load of tasks in favour >> of just moving _something_ already, is utterly dangerous if not coupled >> with these two other conditions, so arguably that too should be under >> CONFIG_PREEMPT. >> > I see your point. Will round both with CONFIG_PREEMPT. > I have upload a new patch, please find it here: https://lkml.org/lkml/2017/2/14/334 -- Uladzislau Rezki
[toc] | [next] | [standalone]
| From | Dietmar Eggemann <dietmar.eggemann@arm.com> |
|---|---|
| Date | 2017-02-15 20:00 +0100 |
| Message-ID | <tbcY2-3Ds-19@gated-at.bofh.it> |
| In reply to | #1580738 |
On 02/14/2017 06:28 PM, Uladzislau Rezki wrote: >>> >>> So that is useful information that should have been in the Changelog. >>> >>> OK, can you respin this patch with adjusted Changelog and taking Mike's >>> feedback? >>> >> Yes, i will prepare a patch accordingly, no problem. >> >>> >>> Also, I worry about the effects of this on !PREEMPT kernels, the first >>> hunk (which explicitly states is about latency) should be under >>> CONFIG_PREEMPT to match the similar case we already have in >>> detach_tasks(). This one uses #ifdef CONFIG_PREEMPT whereas you use IS_ENABLED(CONFIG_PREEMPT). Is there a particular reason for this? >>> But your second hunk, which ignores the actual load of tasks in favour >>> of just moving _something_ already, is utterly dangerous if not coupled >>> with these two other conditions, so arguably that too should be under >>> CONFIG_PREEMPT. >>> >> I see your point. Will round both with CONFIG_PREEMPT. >> > I have upload a new patch, please find it here: > https://lkml.org/lkml/2017/2/14/334 >
[toc] | [prev] | [next] | [standalone]
| From | Uladzislau Rezki <urezki@gmail.com> |
|---|---|
| Date | 2017-02-16 12:30 +0100 |
| Message-ID | <tbsq5-5wK-5@gated-at.bofh.it> |
| In reply to | #1581571 |
On Wed, Feb 15, 2017 at 7:58 PM, Dietmar Eggemann
<dietmar.eggemann@arm.com> wrote:
> On 02/14/2017 06:28 PM, Uladzislau Rezki wrote:
>>>>
>>>>
>>>> So that is useful information that should have been in the Changelog.
>>>>
>>>> OK, can you respin this patch with adjusted Changelog and taking Mike's
>>>> feedback?
>>>>
>>> Yes, i will prepare a patch accordingly, no problem.
>>>
>>>>
>>>> Also, I worry about the effects of this on !PREEMPT kernels, the first
>>>> hunk (which explicitly states is about latency) should be under
>>>> CONFIG_PREEMPT to match the similar case we already have in
>>>> detach_tasks().
>
>
> This one uses #ifdef CONFIG_PREEMPT whereas you use
> IS_ENABLED(CONFIG_PREEMPT). Is there a particular reason for this?
>
I just wanted to put it under one line instead of using #ifdefs in my
second hunk,
so that is a matter of taste. Also, please find below different
variants of how it can be
rewriten:
<variant 1>
#ifdef CONFIG_PREEMPT
if (env->idle != CPU_NEWLY_IDLE)
#endif
if ((load / 2) > env->imbalance)
goto next;
<variant 1>
<variant 2>
#ifdef CONFIG_PREEMPT
if (env->idle != CPU_NEWLY_IDLE &&
(load / 2) > env->imbalance)
goto next;
#else
if ((load / 2) > env->imbalance)
goto next;
#endif
<variant 2>
If somebody has any preferences or concerns, please comment, i will
re-spin the patch.
--
Uladzislau Rezki
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web