Re: BUG_ON in rcu_sync_func triggered

[Oleg Nesterov <oleg@redhat.com>]

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);
 	}
 }