public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH] sched/kthread: Complain loudly when others violate our flags
Date: Thu, 6 Oct 2011 18:40:14 -0700	[thread overview]
Message-ID: <20111007014014.GB5458@google.com> (raw)
In-Reply-To: <1317637238.20367.6.camel@twins>

Hello, Peter.

Sorry about the delays.  I just moved and things are still pretty
hectic.

On Mon, Oct 03, 2011 at 12:20:38PM +0200, Peter Zijlstra wrote:
> On Sun, 2011-10-02 at 18:15 -0700, Tejun Heo wrote:
> > Anyways, I don't think I'm gonna take this one.  There are some
> > attractions to the approach - ie. making the users determine whether
> > they need strict affinity or not and mandating those users to shut
> > down properly from cpu down callbacks and if we're doing this from the
> > scratch, this probably would be a sane choice.  But we already have
> > tons of users and relatively well tested code.  I don't see compelling
> > reasons to perturb that at this point.
>
> So wtf am I going to do with people who want PF_THREAD_BOUND to actually
> do what it means? Put a warning in the scheduler code to flag all
> violations and let you sort out the workqueue fallout?

The only thing workqueue requires is that set_cpus_allowed_ptr() and
cpuset don't diddle with workers.  We can either add another flag or
just add check for PF_WQ_WORKER there.

> I didn't write this change for fun, I actually need to get
> PF_THREAD_BOUND back to sanity, this change alone isn't enough, but it
> gets rid of the worst abuse. This isn't frivolous perturbation.

One thing I was curious about and Steven didn't answer was that
whether the RT's requirement prohibits the bound threads from changing
its own affinity because that's currently explicitly allowed whether
the new affinity is single CPU or any set of CPUs.

> > Also, on a quick glance, the change is breaking non-reentrance
> > guarantee.
>
> How so? Afaict it does exactly what the trustee thread used to do, or is
> it is related to the NON_AFFINE moving the worklets around?

Yeap, that was my bad.  I thought you were pushing out works to other
cpus before flushing local.  Can you please re-post rolled-up patch?
Hopefully at least boot tested?

One thing that's not clear to me is how the new code is gonna handle
long-running work items.  It seems like the code assumes that all
long-running work items should be queued on NON_AFFINE workqueue.  Is
that correct?  The trustee thing is pretty complex but the
requirements there are pretty weird for historical reasons.  If it can
simplify the code while following the requirements (or audit and
update all users), great but I still can't see how it can.

Thanks.

-- 
tejun

  reply	other threads:[~2011-10-07  1:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-27 21:17 [PATCH] sched/kthread: Complain loudly when others violate our flags Steven Rostedt
2011-09-28  0:22 ` Thomas Gleixner
2011-09-30  3:48 ` Tejun Heo
2011-09-30  4:05   ` Steven Rostedt
2011-09-30  4:14     ` Tejun Heo
2011-09-30  8:34       ` Peter Zijlstra
2011-09-30  8:55         ` Tejun Heo
2011-09-30  9:04           ` Peter Zijlstra
2011-10-03  1:15             ` Tejun Heo
2011-10-03 10:20               ` Peter Zijlstra
2011-10-07  1:40                 ` Tejun Heo [this message]
2011-10-03 10:33               ` Peter Zijlstra
2011-10-03 13:43                 ` Thomas Gleixner
2011-10-03 10:50               ` Peter Zijlstra
2011-09-30  9:23       ` Peter Zijlstra
2011-09-30  9:29       ` Peter Zijlstra
2011-09-30  9:27   ` Peter Zijlstra
2011-10-03  1:22     ` Tejun Heo
2011-09-30  3:55 ` Tejun Heo
2011-09-30  4:08   ` Steven Rostedt
2011-09-30  4:28     ` Tejun Heo

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=20111007014014.GB5458@google.com \
    --to=htejun@gmail.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /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