From: Eliezer Tamir <eliezer.tamir@linux.intel.com>
To: Peter Zijlstra <peterz@infradead.org>,
Arjan van de Ven <arjan@linux.intel.com>,
lenb@kernel.org, rjw@rjwysocki.net,
Chris Leech <christopher.leech@intel.com>,
David Miller <davem@davemloft.net>,
rui.zhang@intel.com, jacob.jun.pan@linux.intel.com,
Mike Galbraith <bitbucket@online.de>,
Ingo Molnar <mingo@kernel.org>,
hpa@zytor.com, Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org
Subject: Re: [PATCH 6/7] sched: Clean up preempt_enable_no_resched() abuse
Date: Wed, 20 Nov 2013 20:02:54 +0200 [thread overview]
Message-ID: <528CF94E.8020300@linux.intel.com> (raw)
In-Reply-To: <20131120162736.691879744@infradead.org>
On 20/11/2013 18:04, Peter Zijlstra wrote:
> The only valid use of preempt_enable_no_resched() is if the very next
> line is schedule() or if we know preemption cannot actually be enabled
> by that statement due to known more preempt_count 'refs'.
The reason I used the no resched version is that busy_poll_end_time()
is almost always called with rcu read lock held, so it seemed the more
correct option.
I have no issue with you changing this.
> As to the busy_poll mess; that looks to be completely and utterly
> broken, sched_clock() can return utter garbage with interrupts enabled
> (rare but still), it can drift unbounded between CPUs, so if you get
> preempted/migrated and your new CPU is years behind on the previous
> CPU we get to busy spin for a _very_ long time. There is a _REASON_
> sched_clock() warns about preemptability - papering over it with a
> preempt_disable()/preempt_enable_no_resched() is just terminal brain
> damage on so many levels.
IMHO This has been reviewed thoroughly.
When Ben Hutchings voiced concerns I rewrote the code to use time_after,
so even if you do get switched over to a CPU where the time is random
you will at most poll another full interval.
Linus asked me to remove this since it makes us use two time values
instead of one. see https://lkml.org/lkml/2013/7/8/345.
Cheers,
Eliezer
next prev parent reply other threads:[~2013-11-20 18:03 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-20 16:04 [PATCH 0/7] Cure some vaux idle wrackage Peter Zijlstra
2013-11-20 16:04 ` [PATCH 1/7] x86, acpi, idle: Restructure the mwait idle routines Peter Zijlstra
2013-11-20 16:04 ` [PATCH 2/7] sched, preempt: Fixup missed PREEMPT_NEED_RESCHED folding Peter Zijlstra
2013-11-21 8:25 ` Peter Zijlstra
2013-11-20 16:04 ` [PATCH 3/7] idle, thermal, acpi: Remove home grown idle implementations Peter Zijlstra
2013-11-20 16:40 ` Arjan van de Ven
2013-11-20 16:59 ` Peter Zijlstra
2013-11-20 17:23 ` Thomas Gleixner
2013-11-20 17:23 ` Arjan van de Ven
2013-11-20 17:55 ` Thomas Gleixner
2013-11-20 18:21 ` Arjan van de Ven
2013-11-20 19:38 ` Thomas Gleixner
2013-11-20 22:08 ` Jacob Pan
2013-11-21 0:54 ` Jacob Pan
2013-11-21 8:21 ` Peter Zijlstra
2013-11-21 16:07 ` Paul E. McKenney
2013-11-21 16:21 ` Arjan van de Ven
2013-11-21 19:19 ` Paul E. McKenney
2013-11-21 19:45 ` Arjan van de Ven
2013-11-21 20:07 ` Paul E. McKenney
2013-11-22 0:10 ` Jacob Pan
2013-11-22 4:20 ` Paul E. McKenney
2013-11-22 11:33 ` Peter Zijlstra
2013-11-22 17:17 ` Paul E. McKenney
2013-11-21 16:29 ` Peter Zijlstra
2013-11-21 17:27 ` Paul E. McKenney
2013-11-20 16:04 ` [PATCH 4/7] preempt, locking: Rework local_bh_{dis,en}able() Peter Zijlstra
2013-11-20 16:04 ` [PATCH 5/7] locking: Optimize lock_bh functions Peter Zijlstra
2013-11-20 16:04 ` [PATCH 6/7] sched: Clean up preempt_enable_no_resched() abuse Peter Zijlstra
2013-11-20 18:02 ` Eliezer Tamir [this message]
2013-11-20 18:15 ` Peter Zijlstra
2013-11-20 20:14 ` Eliezer Tamir
2013-11-21 10:10 ` Peter Zijlstra
2013-11-21 13:26 ` Eliezer Tamir
2013-11-21 13:39 ` Peter Zijlstra
2013-11-22 6:56 ` Eliezer Tamir
2013-11-22 11:30 ` Peter Zijlstra
2013-11-26 7:15 ` Eliezer Tamir
2013-11-26 10:51 ` Thomas Gleixner
2013-11-20 16:04 ` [PATCH 7/7] preempt: Take away preempt_enable_no_resched() from modules Peter Zijlstra
2013-11-20 18:54 ` Jacob Pan
2013-11-20 19:00 ` Peter Zijlstra
2013-11-20 19:18 ` Peter Zijlstra
2013-11-20 19:29 ` Jacob Pan
2013-11-20 16:34 ` [PATCH 0/7] Cure some vaux idle wrackage Peter Zijlstra
2013-11-20 17:19 ` Jacob Pan
2013-11-20 17:24 ` Peter Zijlstra
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=528CF94E.8020300@linux.intel.com \
--to=eliezer.tamir@linux.intel.com \
--cc=arjan@linux.intel.com \
--cc=bitbucket@online.de \
--cc=christopher.leech@intel.com \
--cc=davem@davemloft.net \
--cc=hpa@zytor.com \
--cc=jacob.jun.pan@linux.intel.com \
--cc=lenb@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=rui.zhang@intel.com \
--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;
as well as URLs for NNTP newsgroup(s).