Re: [RFC][Patch] RCU documentation

[Nikita Danilov]

Paul E. McKenney writes:
 > Hello!

Hello Paul,

[...]

 > 
 > +	static inline int audit_upd_rule(struct audit_rule *rule,
 > +					 struct list_head *list,
 > +					 __u32 newaction,
 > +					 __u32 newfield_count)
 > +	{
 > +		struct audit_entry  *e;
 > +		struct audit_newentry *ne;
 > +
 > +		list_for_each_entry(e, list, list) {
 > +			if (!audit_compare_rule(rule, &e->rule)) {
 > +				ne = kmalloc(sizeof(*entry), GFP_ATOMIC);
 > +				if (ne == NULL)
 > +					return _ENOMEM;

-ENOMEM;

 > +				audit_copy_rule(&ne->rule, &e->rule);
 > +				ne->rule.action = newaction;

[...]

 > +	static enum audit_state audit_filter_task(struct task_struct *tsk)
 > +	{
 > +		struct audit_entry *e;
 > +		enum audit_state   state;
 > +
 > +		rcu_read_lock();
 > +		list_for_each_entry_rcu(e, &audit_tsklist, list) {
 > +			if (audit_filter_rules(tsk, &e->rule, NULL, &state)) {
 > +				spin_lock(&e->lock);
 > +				if (e->deleted) {
 > +					spin_unlock(&e->lock);
 > +					rcu_read_unlock();
 > +					return AUDIT_BUILD_CONTEXT;

Shouldn't this be "continue", to work correctly in the face of mutators
similar to audit_upd_rule(), that at some point leave both old (marked
->deleted) and new versions on the list?

Also, RCU used instead of existential lock is so typical, that it
probably deserves dedicated example.

 > +				}

[...]

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