From: Ingo Molnar <mingo@elte.hu>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: peterz@infradead.org, roland@redhat.com, efault@gmx.de,
rjw@sisk.pl, jdike@addtoit.com,
user-mode-linux-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, torvalds@linux-foundation.org,
akpm@linux-foundation.org
Subject: Re: [patch] don't preempt not TASK_RUNNING tasks
Date: Fri, 20 Mar 2009 11:53:06 +0100 [thread overview]
Message-ID: <20090320105306.GG6256@elte.hu> (raw)
In-Reply-To: <E1Lkc6U-0003II-W1@pomaz-ex.szeredi.hu>
* Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, 20 Mar 2009, Peter Zijlstra wrote:
> > On Fri, 2009-03-20 at 10:43 +0100, Miklos Szeredi wrote:
> > > Ingo,
> > >
> > > I tested this one, and I think it makes sense in any case as an
> > > optimization. It should also be good for -stable kernels.
> > >
> > > Does it look OK?
> >
> > The idea is good, but there is a risk of preemption latencies here. Some
> > code paths aren't real quick between setting ->state != TASK_RUNNING and
> > calling schedule.
> >
> > [ Both quick: as in O(1) and few instructions ]
> >
> > So if we're going to do this, we'd need to audit all such code paths --
> > and there be lots.
>
> Oh, yes.
>
> In a random sample the most common pattern is something like this:
>
> spin_lock(&some_lock);
> /* do something */
> set_task_state(TASK_SOMESLEEP);
> /* do something more */
> spin_unlock(&some_lock);
> schedule();
> ...
>
> Which should only positively be impacted by the change. But I can
> imagine rare cases where it's more complex.
I'd suggest spin_unlock_no_resched() and task_unlock_no_resched()
instead of open-coding preempt-disable sequences.
> > The first line of attack for this problem is making
> > wait_task_inactive() sucks less, which shouldn't be too hard,
> > that unconditional 1 jiffy sleep is simply retarded.
>
> I completely agree. However, I'd like to have a non-invasive
> solution that can go into current and stable kernels so UML users
> don't need to suffer any more.
Agreed. task_unlock_no_resched() should do that i think.
Ingo
next prev parent reply other threads:[~2009-03-20 10:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-03-20 9:43 [patch] don't preempt not TASK_RUNNING tasks Miklos Szeredi
2009-03-20 10:03 ` Peter Zijlstra
2009-03-20 10:37 ` Miklos Szeredi
2009-03-20 10:53 ` Ingo Molnar [this message]
2009-03-20 11:25 ` Miklos Szeredi
2009-03-20 11:39 ` Ingo Molnar
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=20090320105306.GG6256@elte.hu \
--to=mingo@elte.hu \
--cc=akpm@linux-foundation.org \
--cc=efault@gmx.de \
--cc=jdike@addtoit.com \
--cc=linux-kernel@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=peterz@infradead.org \
--cc=rjw@sisk.pl \
--cc=roland@redhat.com \
--cc=torvalds@linux-foundation.org \
--cc=user-mode-linux-devel@lists.sourceforge.net \
/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