public inbox for linux-kernel@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox