Re: Additional compiler barrier required in sched_preempt_enable_no_resched?

[Thomas Gleixner]

On Fri, 13 May 2016, Vikram Mulukutla wrote:
> On 5/13/2016 7:58 AM, Peter Zijlstra wrote:
> > diff --git a/include/asm-generic/preempt.h b/include/asm-generic/preempt.h
> > index 5d8ffa3e6f8c..c1cde3577551 100644
> > --- a/include/asm-generic/preempt.h
> > +++ b/include/asm-generic/preempt.h
> > @@ -7,10 +7,10 @@
> > 
> >   static __always_inline int preempt_count(void)
> >   {
> > -	return current_thread_info()->preempt_count;
> > +	return READ_ONCE(current_thread_info()->preempt_count);
> >   }
> > 
> > -static __always_inline int *preempt_count_ptr(void)
> > +static __always_inline volatile int *preempt_count_ptr(void)
> >   {
> >   	return &current_thread_info()->preempt_count;
> >   }
> > 
> 
> Thanks Peter, this patch worked for me. The compiler no longer optimizes out
> the increment/decrement of the preempt_count.

I have a hard time to understand why the compiler optimizes out stuff w/o that
patch.

We already have:

#define preempt_disable() \
do { \
        preempt_count_inc(); \
        barrier(); \
} while (0)

#define sched_preempt_enable_no_resched() \
do { \
        barrier(); \
        preempt_count_dec(); \
} while (0)

#define preempt_enable() \
do { \
        barrier(); \
        if (unlikely(preempt_count_dec_and_test())) \
                __preempt_schedule(); \
} while (0)

So the barriers already forbid that the compiler reorders code. How on earth
is the compiler supposed to optimize the dec/inc out?

There is more code than the preempt stuff depending on barrier() and expecting
that the compiler does not optimize out things at will. Are we supposed to
hunt all occurences and amend them with READ_ONCE or whatever one by one?

Thanks,

	tglx