LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH 3/4] sched: Pull preemption disablement to __schedule() caller
Date: Mon, 2 Feb 2015 18:53:45 +0100	[thread overview]
Message-ID: <20150202175343.GD11054@lerouge> (raw)
In-Reply-To: <20150128155044.GJ23038@twins.programming.kicks-ass.net>

On Wed, Jan 28, 2015 at 04:50:44PM +0100, Peter Zijlstra wrote:
> On Wed, Jan 28, 2015 at 01:24:11AM +0100, Frederic Weisbecker wrote:
> > +++ b/kernel/sched/core.c
> > @@ -2760,7 +2760,6 @@ static void __sched __schedule(void)
> >  	struct rq *rq;
> >  	int cpu;
> >  
> > -	preempt_disable();
> 
> Implies barrier();
> 
> >  	cpu = smp_processor_id();
> >  	rq = cpu_rq(cpu);
> >  	rcu_note_context_switch();
> > @@ -2822,8 +2821,6 @@ static void __sched __schedule(void)
> >  		raw_spin_unlock_irq(&rq->lock);
> >  
> >  	post_schedule(rq);
> 
> implies barrier();
> 
> > -
> > -	sched_preempt_enable_no_resched();
> >  }
> >  
> >  static inline void sched_submit_work(struct task_struct *tsk)
> 
> > @@ -2883,9 +2882,9 @@ void __sched schedule_preempt_disabled(void)
> >  static void preempt_schedule_common(void)
> >  {
> >  	do {
> > -		preempt_count_add(PREEMPT_ACTIVE);
> > +		preempt_count_add(PREEMPT_ACTIVE + PREEMPT_CHECK_OFFSET);
> 
> Does _NOT_ imply barrier();
> 
> >  		__schedule();
> > -		preempt_count_sub(PREEMPT_ACTIVE);
> 
> idem.

It looks like preempt_count_add/inc() mostly imply entering a context that we want
to be seen right away (thus want barrier() after) and preempt_count_sub/dec() mostly
want previous work to be visible before re-enabling interrupt, preemption, etc...
(thus want barrier() before).

So maybe these functions (the non-underscored ones) should imply a barrier() rather
than their callers (preempt_disable() and others). Inline functions instead of macros
would do the trick (if the headers hell let us do that).

Note the underscored implementations are all inline currently so this happens to
work by chance for direct calls to preempt_count_add/sub() outside preempt_disable().
If the non-underscored caller is turned into inline too I don't expect performance issues.

What do you think, does it make sense?


> 
> > +		preempt_count_sub(PREEMPT_ACTIVE + PREEMPT_CHECK_OFFSET);
> >  
> >  		/*
> >  		 * Check again in case we missed a preemption opportunity

  reply	other threads:[~2015-02-02 17:53 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-28  0:24 [PATCH 0/4] sched: schedule/preempt optimizations and cleanups Frederic Weisbecker
2015-01-28  0:24 ` [PATCH 1/4] sched: Pull resched loop to __schedule() callers Frederic Weisbecker
2015-02-04 14:36   ` [tip:sched/core] " tip-bot for Frederic Weisbecker
2015-01-28  0:24 ` [RFC PATCH 2/4] sched: Use traced preempt count operations to toggle PREEMPT_ACTIVE Frederic Weisbecker
2015-01-28  1:42   ` Steven Rostedt
2015-01-28 13:59     ` Frederic Weisbecker
2015-01-28 15:04       ` Steven Rostedt
2015-01-28 15:42   ` Peter Zijlstra
2015-02-02 17:22     ` Frederic Weisbecker
2015-01-28  0:24 ` [PATCH 3/4] sched: Pull preemption disablement to __schedule() caller Frederic Weisbecker
2015-01-28 15:50   ` Peter Zijlstra
2015-02-02 17:53     ` Frederic Weisbecker [this message]
2015-02-03 10:53       ` Peter Zijlstra
2015-02-04 17:31         ` Frederic Weisbecker
2015-02-04 17:48           ` Peter Zijlstra
2015-01-28  0:24 ` [RFC PATCH 4/4] sched: Account PREEMPT_ACTIVE context as atomic Frederic Weisbecker
2015-01-28 15:46   ` Peter Zijlstra
2015-02-02 17:29     ` Frederic Weisbecker

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=20150202175343.GD11054@lerouge \
    --to=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [PATCH 3/4] sched: Pull preemption disablement to __schedule() caller' \
    /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

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).