Re: BUG_ON in rcu_sync_func triggered

[Oleg Nesterov]

On 09/13, Nikolay Borisov wrote:
>
>
> I just re-run the test with kernel 4.4.14 and PROVE_RCU and DEBUG_RCU_OBJECTS
> enabled and here is what I got:

Thanks again!

Damn. This reminds me that I forgot to send the patch which reworks rcu/sync.c.
Will do this week. But we need to investigate anyway, and I see nothing wrong
in the current code.

What did you do to reproduce?

> The bug on is this one: BUG_ON(need_wait && need_sync);

OK... perhaps the unbalanced up_write... I'll try to look at freeze/thaw code,
could test the debugging patch below meanwhile?

Oleg.


--- a/kernel/rcu/sync.c
+++ b/kernel/rcu/sync.c
@@ -82,6 +82,12 @@ void rcu_sync_init(struct rcu_sync *rsp, enum rcu_sync_type 
type)
        rsp->gp_type = type;
 }
 
+static void xxx(struct rcu_sync *rsp)
+{
+       pr_crit("XXX: %p gp=%d cnt=%d cb=%d\n",
+               rsp, rsp->gp_state, rsp->gp_count, rsp->cb_state);
+}
+
 /**
  * rcu_sync_enter() - Force readers onto slowpath
  * @rsp: Pointer to rcu_sync structure to use for synchronization
@@ -102,13 +108,14 @@ void rcu_sync_enter(struct rcu_sync *rsp)
        bool need_wait, need_sync;
 
        spin_lock_irq(&rsp->rss_lock);
+       if (WARN_ON(rsp->gp_count < 0)) xxx(rsp);
        need_wait = rsp->gp_count++;
        need_sync = rsp->gp_state == GP_IDLE;
        if (need_sync)
                rsp->gp_state = GP_PENDING;
        spin_unlock_irq(&rsp->rss_lock);
 
-       BUG_ON(need_wait && need_sync);
+       if (WARN_ON(need_wait && need_sync)) xxx(rsp);
 
        if (need_sync) {
                gp_ops[rsp->gp_type].sync();
@@ -122,7 +129,7 @@ void rcu_sync_enter(struct rcu_sync *rsp)
                 * Nobody has yet been allowed the 'fast' path and thus we can
                 * avoid doing any sync(). The callback will get 'dropped'.
                 */
-               BUG_ON(rsp->gp_state != GP_PASSED);
+               if (WARN_ON(rsp->gp_state != GP_PASSED)) xxx(rsp);
        }
 }
 
@@ -149,8 +156,9 @@ static void rcu_sync_func(struct rcu_head *rcu)
        struct rcu_sync *rsp = container_of(rcu, struct rcu_sync, cb_head);
        unsigned long flags;
 
-       BUG_ON(rsp->gp_state != GP_PASSED);
-       BUG_ON(rsp->cb_state == CB_IDLE);
+       if (WARN_ON(rsp->gp_count < 0)) xxx(rsp);
+       if (WARN_ON(rsp->gp_state != GP_PASSED)) xxx(rsp);
+       if (WARN_ON(rsp->cb_state == CB_IDLE)) xxx(rsp);
 
        spin_lock_irqsave(&rsp->rss_lock, flags);
        if (rsp->gp_count) {
@@ -189,6 +197,7 @@ static void rcu_sync_func(struct rcu_head *rcu)
 void rcu_sync_exit(struct rcu_sync *rsp)
 {
        spin_lock_irq(&rsp->rss_lock);
+       if (WARN_ON(rsp->gp_count <= 0)) xxx(rsp);
        if (!--rsp->gp_count) {
                if (rsp->cb_state == CB_IDLE) {
                        rsp->cb_state = CB_PENDING;
@@ -208,7 +217,7 @@ void rcu_sync_dtor(struct rcu_sync *rsp)
 {
        int cb_state;
 
-       BUG_ON(rsp->gp_count);
+       if (WARN_ON(rsp->gp_count)) xxx(rsp);
 
        spin_lock_irq(&rsp->rss_lock);
        if (rsp->cb_state == CB_REPLAY)
@@ -218,6 +227,6 @@ void rcu_sync_dtor(struct rcu_sync *rsp)
 
        if (cb_state != CB_IDLE) {
                gp_ops[rsp->gp_type].wait();
-               BUG_ON(rsp->cb_state != CB_IDLE);
+               if (WARN_ON(rsp->cb_state != CB_IDLE)) xxx(rsp);
        }
 }