linux-kernel - Re: [PATCH 3/4] sched: Pull preemption disablement to _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 2 Feb 2015 18:53:45 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Ingo Molnar <mingo@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 3/4] sched: Pull preemption disablement to __schedule()
 caller

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/