Re: [V2 PATCH 1/3] module: use rcu to protect module list read

[Eric Dumazet]

Le dimanche 11 mars 2012 Ã 18:54 +0800, Cong Wang a Ãcrit :
> V2:
...
>  
> @@ -1639,7 +1639,7 @@ static void mod_sysfs_teardown(struct module *mod)
>  static int __unlink_module(void *_mod)
>  {
>  	struct module *mod = _mod;
> -	list_del(&mod->list);
> +	list_del_rcu(&mod->list);
>  	module_bug_cleanup(mod);
>  	return 0;
>  }
> @@

You mix too many different things in a single patch. 

For example, lets review this __unlink_module() change...

If this was really needed, it should be a single patch so that it can be
a stable submission.

As it is not needed (since its called from stop_machine()), this makes
your whole patch looking suspicious.

I suggest you make a 100% cleanup patch, changing the title as well,
because module code _already_ uses RCU.

"module: use rcu to protect module list read" makes no sense at all.

To meet current RCU API best pratices, you change preempt_enable() by
rcu_read_unlock_sched() and preempt_disable() by rcu_read_lock_sched()

Then, if you believe other changes are needed, submit them with an
explicit changelog explaining the change, not hiding them in a big
cleanup patch.

Ref: Documentation/SubmittingPatches section 3)



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