Re: Memory barrier question.

[Eric Dumazet <eric.dumazet@gmail.com>]

Le samedi 30 octobre 2010 à 14:48 +0900, Tetsuo Handa a écrit :
> Paul E. McKenney wrote:
> > For the CPU, let's take it by type of architecture...
> > 
> > First, let's get the UP-only architectures out of the way.  These would
> > always see their changes in order, so woiuld always see "hello".
> 
> Of course.
> 
> > Second, let's consider the TSO architectures, including x86, SPARC,
> > PA-RISC, and IBM Mainframe.  On these architectures, reads are not
> > reordered by the CPU, so if they see the new pointer, they will also
> > see the new characters -- hence "hello".
> 
> Yes.
> 
> > Next, let's consider weakly ordered systems that respect dependency
> > ordering (ARM, PowerPC, Itanium).  The load of the pointer would
> > always be ordered with respect to any dereference of the pointer,
> > so they would always see "hello".
> 
> I'm relieved to hear that.
> 
> > This leave DEC Alpha.  In this architecture, smp_read_barrier_depends()
> > expands to smp_rmb(), which forces the ordering as required.  So
> > Alpha also sees "hello."
> 
> Yes.
> 
> > I believe that this covers all of the cases.
> > 
> > Am I missing anything?
> 
> You are right. Regarding list elements, they are appropriately protected.
> 
> 
> 
> Then, what about list heads that are dynamically allocated/initialized?
> 
> sruct hashed_word {
> 	struct list_head list;
> 	void *key;
> 	char *buf;
> };
> static struct list_head tables[1u << TABLEBIT];
> static void (*some_callback) (void *);
> 

typedef void (*some_callback_t) (void *);

static some_callback_t __rcu *some_callback;

> static void init(void)
> {
> 	int i;
> 	/* Initialize the table. */
> 	for (i = 0; i < (1u << TABLEBIT); i++)
> 		INIT_LIST_HEAD(&tables[i]);
> 	smp_wmb();
> 	/* Allow other CPUs to access the table. */
> 	some_callback = func;

rcu_assign_pointer(some_callback, func);

> }
> 
> /*
>  * This function is called by
>  *
>  *	if (some_callback)
>  *		some_callback(some_ptr);
>  *
>  * rather than
>  *
>  *	func(some_ptr);

well no, see later.

>  *
>  * .
>  */
> static void func(void *some_ptr)
> {
> 	struct hashed_word *ptr;
> 	struct list_head *table = &tables[hash_ptr(some_ptr, TABLEBIT)];
> 	/* We must make sure that readers see table->next != NULL. */


> 	smp_rmb();
Nope.. delete this

> 	rcu_read_lock();
> 	list_for_each_entry_rcu(ptr, table, list) {
> 		if (ptr->key != some_ptr)
> 			continue;
> 		printk("%s\n", ptr->buf);
> 		break;
> 	}
> 	rcu_read_unlock();
> }
> 
> How can I make sure INIT_LIST_HEAD() in init() takes effect?
> Is smp_rmb() appropriate?

Please avoid smp_rmb()/smb_wmb() and use RCU api only.

some_callback_t *ptr;

rcu_read_lock();
ptr = rcu_dereference(some_callback);
if (ptr) {
	list_for_each_entry_rcu(aux, table, list) {
 		if (aux->key != ptr)
 			continue;
 		printk("%s\n", aux->buf);
 		break;
 	}
 }
rcu_read_unlock();