linux-kernel - Re: [RFC PATCH] introduce sys_membarrier(): process-wide memory barrier (v6)

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] [day] [month] [year] [list]

Date:	Fri, 15 Jan 2010 21:40:28 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
Cc:	linux-kernel@...r.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Nicholas Miell <nmiell@...cast.net>, mingo@...e.hu,
	laijs@...fujitsu.com, dipankar@...ibm.com,
	akpm@...ux-foundation.org, josh@...htriplett.org,
	dvhltc@...ibm.com, niv@...ibm.com, tglx@...utronix.de,
	peterz@...radead.org, Valdis.Kletnieks@...edu, dhowells@...hat.com
Subject: Re: [RFC PATCH] introduce sys_membarrier(): process-wide memory
 barrier (v6)

On Fri, 2010-01-15 at 16:13 -0500, Mathieu Desnoyers wrote:
> * Steven Rostedt (rostedt@...dmis.org) wrote:

> > For the check of MEMBARRIER_QUERY and thread_group_empty(), sure that
> > could be 99.9% true. But for num_online_cpus() == 1, I can see that
> > always being true. How may boxes do you have that are uniprocessor that
> > run without CONFIG_SMP=y?  Every distro I know enables this, only
> > embedded boards seem not to. So you just made the 100% true condition
> > unlikely on those boards. Grant it, this may not be such a big deal, but
> > I like to shed light on the overuse of unlikely. I don't think this
> > unlikely is worth it.
> 
> Good point. So I'll remove the (num_online_cpus() == 1) from unlikely
> then, it will become:
> 
>         if (unlikely(flags & MEMBARRIER_QUERY
>                      || thread_group_empty(current))
>                      || num_online_cpus() == 1)
> 
> (removed extra () around (num_online_cpus() == 1) too)

I wonder if that unlikely even matters. It's not really that critical of
a path. If anything it just documents what would be unlikely.

Honestly, I'd recommend getting rid of that unlikely altogether. But I'm
fine with this version with or without it.

> > >  int rcu_expedited_torture_stats(char *page)
> > > Index: linux-2.6-lttng/arch/x86/include/asm/mmu_context.h
> > > ===================================================================
> > > --- linux-2.6-lttng.orig/arch/x86/include/asm/mmu_context.h	2010-01-12 10:59:31.000000000 -0500
> > > +++ linux-2.6-lttng/arch/x86/include/asm/mmu_context.h	2010-01-12 11:59:49.000000000 -0500
> > > @@ -36,6 +36,11 @@ static inline void switch_mm(struct mm_s
> > >  	unsigned cpu = smp_processor_id();
> > >  
> > >  	if (likely(prev != next)) {
> > > +		/*
> > > +		 * smp_mb() between user-space thread execution and
> > > +		 * mm_cpumask clear is required by sys_membarrier().
> > 
> > Of course on x86, this just turns into a barrier();, perhaps the comment
> > could state, that the use of smp_mb__before_clear_bit() is more of an
> > example for other archs since x86 only requires a barrier().
> 
> Does something like this work ?
> 
>                /*
>                  * smp_mb() between user-space thread execution and
>                  * mm_cpumask clear is required by sys_membarrier().
>                  * smp_mb__before_clear_bit() turns into a barrier() on x86. It
>                  * is left here to document that this barrier is needed, as an
>                  * example for other architectures.
>                  */
>                 smp_mb__before_clear_bit();
> 

That's fine with me.

-- Steve



--
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/