All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Luca Abeni <luca.abeni@santannapisa.it>
Cc: peterz@infradead.org, lizefan@huawei.com, mingo@redhat.com,
	rostedt@goodmis.org, claudio@evidence.eu.com, bristot@redhat.com,
	tommaso.cucinotta@santannapisa.it, juri.lelli@redhat.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2 0/7] sched/deadline: fix cpusets bandwidth accounting
Date: Mon, 5 Feb 2018 13:48:49 -0700	[thread overview]
Message-ID: <20180205204849.GA2621@xps15> (raw)
In-Reply-To: <20180202141750.5652d2b7@luca>

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

      reply	other threads:[~2018-02-05 20:49 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-01 16:51 [PATCH V2 0/7] sched/deadline: fix cpusets bandwidth accounting Mathieu Poirier
2018-02-01 16:51 ` [PATCH V2 1/7] sched/topology: Adding function partition_sched_domains_locked() Mathieu Poirier
2018-02-02 10:19   ` Juri Lelli
2018-02-05 18:11     ` Mathieu Poirier
2018-02-06  7:42       ` Juri Lelli
2018-02-01 16:51 ` [PATCH V2 2/7] cpuset: Rebuild root domain deadline accounting information Mathieu Poirier
2018-02-02 12:52   ` Juri Lelli
2018-02-05 18:59     ` Mathieu Poirier
2018-02-01 16:51 ` [PATCH V2 3/7] sched/deadline: Keep new DL task within root domain's boundary Mathieu Poirier
2018-02-02 14:35   ` Juri Lelli
2018-02-05 18:58     ` Mathieu Poirier
2018-02-01 16:51 ` [PATCH V2 4/7] cgroup: Constrain 'sched_load_balance' flag when DL tasks are present Mathieu Poirier
2018-02-01 16:51 ` [PATCH V2 5/7] cgroup: Constrain the addition of CPUs to a new CPUset Mathieu Poirier
2018-02-01 16:51 ` [PATCH V2 6/7] sched/core: Don't change the affinity of DL tasks Mathieu Poirier
2018-02-01 16:51 ` [PATCH V2 7/7] sched/deadline: Prevent CPU hotplug operation if DL task on CPU Mathieu Poirier
2018-02-02 13:17 ` [PATCH V2 0/7] sched/deadline: fix cpusets bandwidth accounting Luca Abeni
2018-02-05 20:48   ` Mathieu Poirier [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180205204849.GA2621@xps15 \
    --to=mathieu.poirier@linaro.org \
    --cc=bristot@redhat.com \
    --cc=claudio@evidence.eu.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=luca.abeni@santannapisa.it \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tommaso.cucinotta@santannapisa.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.