lkml.org 
[lkml]   [2010]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC patch 2/3] tree/tiny rcu: Add debug RCU head objects (v3)
Very nice you do not touch struct rcu_head.

Mathieu Desnoyers wrote:

> +
> +#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
> +# define STATE_RCU_HEAD_READY 0
> +# define STATE_RCU_HEAD_QUEUED 1
> +
> +extern struct debug_obj_descr rcuhead_debug_descr;
> +
> +static inline void debug_rcu_head_queue(struct rcu_head *head)
> +{
> + debug_object_activate(head, &rcuhead_debug_descr);
> + debug_object_active_state(head, &rcuhead_debug_descr,
> + STATE_RCU_HEAD_READY,
> + STATE_RCU_HEAD_QUEUED);
> +}
> +
> +static inline void debug_rcu_head_unqueue(struct rcu_head *head)
> +{
> + debug_object_active_state(head, &rcuhead_debug_descr,
> + STATE_RCU_HEAD_QUEUED,
> + STATE_RCU_HEAD_READY);
> + debug_object_deactivate(head, &rcuhead_debug_descr);
> +}

I'm a little foolish. I don't know why we need
STATE_RCU_HEAD_READY/QUEUED.

1) obj->astate does not be set to STATE_RCU_HEAD_READY in
debug_object_activate/init().

Maybe you need to add a function:
debug_object_activate_with_astate(head, descr, init_astate);
(the same as debug_object_activate(), but init obj->astate also)

Or you need:
@debugobjects.h
# define DEBUG_OBJECT_ASTATE_INIT 0
@rcupdate.c
# define STATE_RCU_HEAD_READY DEBUG_OBJECT_ASTATE_INIT
# define STATE_RCU_HEAD_QUEUED (STATE_RCU_HEAD_READY + 1)

2) In debug_rcu_head_queue(), debug_object_active_state()
is always success when debug_object_activate() is success.

In debug_rcu_head_unqueue(), debug_object_active_state()
is always success if RCU subsystem is running correctly.

(Correct me if I'm wrong) So we don't need debug_object_active_state()
here if we just find racy users of call_rcu().

> + */
> +static int rcuhead_fixup_init(void *addr, enum debug_obj_state state)
> +{
> + struct rcu_head *head = addr;
> +
> + switch (state) {
> + case ODEBUG_STATE_ACTIVE:
> + /*
> + * Ensure that queued callbacks are all executed.
> + * If we detect that we are nested in a RCU read-side critical
> + * section, we should simply fail, otherwise we would deadlock.
> + */
> + if (rcu_preempt_depth() != 0 || preempt_count() != 0 ||
> + irqs_disabled()) {
> + WARN_ON(1);
> + return 0;
> + }

preempt_count() != 0 ??? What happen when !CONFIG_PREEMPT?

> + rcu_barrier();

We may need call rcu_barrier(), rcu_barrier_sched(), rcu_barrier_bh() together?
rcu_barrier() does not ensure to flush callbacks of RCU_BH, RCU_SCHED.

> + debug_object_init(head, &rcuhead_debug_descr);
> + return 1;
> + default:
> + return 0;
> + }
> +}
> +



\
 
 \ /
  Last update: 2010-03-22 04:35    [W:0.950 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site