mbox series

[V2,0/7] sched/deadline: fix cpusets bandwidth accounting

Message ID 1517503869-3179-1-git-send-email-mathieu.poirier@linaro.org
Headers show
Series sched/deadline: fix cpusets bandwidth accounting | expand

Message

Mathieu Poirier Feb. 1, 2018, 4:51 p.m. UTC
This is the follow-up patchset to [1] that attempt to fix a problem
reported by Steve Rostedt [2] where DL bandwidth accounting is not
recomputed after CPUset and CPU hotplug operations.  When CPU hotplug and
some CUPset manipulation take place root domains are destroyed and new ones
created, loosing at the same time DL accounting information pertaining to
utilisation.  Please see [1] for a full description of the approach.

In this revision a shortcoming identified by Luca is addressed, with most
of the solution kept unchanged.

A notable addition is patch 7/7 - it addresses a problem seen when hot
plugging out a CPU where a DL task is running (see changelog for full
details).  The issue is unrelated to this patchset and will manifest
itself on a mainline kernel.

I will start working on that problem once done with this set but lumping
it in here to raise awareness and provide a stop-gap measure while a
better solution is designed.  This set is also available here [3] with the
instrumentation for patch 7/7 in this commit [4].

This set applies cleanly on top of v4.15.

Best regards,
Mathieu

------
Change for V2:
. Addressing a problem found by Luca Abeni where the mask of a DL task
  isn't modified when cpuset are collapsed.

[1]. https://groups.google.com/forum/#!topic/linux.kernel/uakbvOQE6rc
[2]. https://lkml.org/lkml/2016/2/3/966
[3]. https://git.linaro.org/people/mathieu.poirier/linux.git/log/?h=v4.15-bandwidth-accounting-v2
[4]. af68563a6c21 ("sched/debug: Add 'rq_debug' proc entry")

Mathieu Poirier (7):
  sched/topology: Adding function partition_sched_domains_locked()
  cpuset: Rebuild root domain deadline accounting information
  sched/deadline: Keep new DL task within root domain's boundary
  cgroup: Constrain 'sched_load_balance' flag when DL tasks are present
  cgroup: Constrain the addition of CPUs to a new CPUset
  sched/core: Don't change the affinity of DL tasks
  sched/deadline: Prevent CPU hotplug operation if DL task on CPU

 include/linux/cpuset.h         |   6 ++
 include/linux/sched.h          |   3 +
 include/linux/sched/deadline.h |   8 ++
 include/linux/sched/topology.h |   9 ++
 kernel/cgroup/cpuset.c         | 232 ++++++++++++++++++++++++++++++++++++++++-
 kernel/sched/core.c            |  32 +++++-
 kernel/sched/deadline.c        |  36 +++++++
 kernel/sched/sched.h           |   3 -
 kernel/sched/topology.c        |  31 +++++-
 9 files changed, 346 insertions(+), 14 deletions(-)

-- 
2.7.4

Comments

luca abeni Feb. 2, 2018, 1:17 p.m. UTC | #1
Hi Mathieu,

On Thu,  1 Feb 2018 09:51:02 -0700
Mathieu Poirier <mathieu.poirier@linaro.org> wrote:

> This is the follow-up patchset to [1] that attempt to fix a problem

> reported by Steve Rostedt [2] where DL bandwidth accounting is not

> recomputed after CPUset and CPU hotplug operations.  When CPU hotplug and

> some CUPset manipulation take place root domains are destroyed and new ones

> created, loosing at the same time DL accounting information pertaining to

> utilisation.  Please see [1] for a full description of the approach.


I do not know the cgroup / cpuset code too much, so I have no useful
comments on your patches... But I think this patchset is a nice
improvemnt respect to the current situation.

[...]
> A notable addition is patch 7/7 - it addresses a problem seen when hot

> plugging out a CPU where a DL task is running (see changelog for full

> details).  The issue is unrelated to this patchset and will manifest

> itself on a mainline kernel.


I think I introduced this bug with my reclaiming patches, so I am
interested.
When a cpu is hot-plugged out, which code in the kernel is responsible
for migrating the tasks that are executing on such CPU? I was sure I
was handling all the relevant codepaths, but this bug clearly shows
that I was wrong.


			Thanks,
				Luca
Mathieu Poirier Feb. 5, 2018, 8:48 p.m. UTC | #2
On Fri, Feb 02, 2018 at 02:17:50PM +0100, Luca Abeni wrote:
> Hi Mathieu,

> 

> On Thu,  1 Feb 2018 09:51:02 -0700

> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:

> 

> > This is the follow-up patchset to [1] that attempt to fix a problem

> > reported by Steve Rostedt [2] where DL bandwidth accounting is not

> > recomputed after CPUset and CPU hotplug operations.  When CPU hotplug and

> > some CUPset manipulation take place root domains are destroyed and new ones

> > created, loosing at the same time DL accounting information pertaining to

> > utilisation.  Please see [1] for a full description of the approach.

> 

> I do not know the cgroup / cpuset code too much, so I have no useful

> comments on your patches... But I think this patchset is a nice

> improvemnt respect to the current situation.

> 

> [...]

> > A notable addition is patch 7/7 - it addresses a problem seen when hot

> > plugging out a CPU where a DL task is running (see changelog for full

> > details).  The issue is unrelated to this patchset and will manifest

> > itself on a mainline kernel.

> 

> I think I introduced this bug with my reclaiming patches, so I am

> interested.

> When a cpu is hot-plugged out, which code in the kernel is responsible

> for migrating the tasks that are executing on such CPU?


sched_cpu_deactivate()
    cpuset_cpu_inactive()
        cpuset_update_active_cpus()
            cpuset_hotplug_workfn()
                hotplug_update_tasks_legacy()
                    hotplug_update_tasks()
                        set_cpus_allowed_ptr()
                            __set_cpus_allowed_ptr()


> I was sure I

> was handling all the relevant codepaths, but this bug clearly shows

> that I was wrong.


I remember reviewing your patchset and I too thought you had tackled all the
cases.  In function __set_cpus_allowed_ptr() you'll notice two cases are
handled, i.e the task is running or suspended.  I suspect the former to be the
culprit but haven't investigated fully.

Regards,
Mathieu
                                


> 

> 

> 			Thanks,

> 				Luca