srcu: Fix error handling in init_srcu_struct_fields()
authorJoel Fernandes (Google) <joel@joelfernandes.org>
Sat, 29 Jul 2023 14:27:32 +0000 (14:27 +0000)
committerFrederic Weisbecker <frederic@kernel.org>
Wed, 13 Sep 2023 20:27:41 +0000 (22:27 +0200)
The current error handling in init_srcu_struct_fields() is a bit
inconsistent.  If init_srcu_struct_nodes() fails, the function either
returns -ENOMEM or 0 depending on whether ssp->sda_is_static is true or
false. This can make init_srcu_struct_fields() return 0 even if memory
allocation failed!

Simplify the error handling by always returning -ENOMEM if either
init_srcu_struct_nodes() or the per-CPU allocation fails. This makes the
control flow easier to follow and avoids the inconsistent return values.

Add goto labels to avoid duplicating the error cleanup code.

Link: https://lore.kernel.org/r/20230404003508.GA254019@google.com
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
kernel/rcu/srcutree.c

index 20d7a23..f1a9052 100644 (file)
@@ -255,29 +255,31 @@ static int init_srcu_struct_fields(struct srcu_struct *ssp, bool is_static)
        ssp->srcu_sup->sda_is_static = is_static;
        if (!is_static)
                ssp->sda = alloc_percpu(struct srcu_data);
-       if (!ssp->sda) {
-               if (!is_static)
-                       kfree(ssp->srcu_sup);
-               return -ENOMEM;
-       }
+       if (!ssp->sda)
+               goto err_free_sup;
        init_srcu_struct_data(ssp);
        ssp->srcu_sup->srcu_gp_seq_needed_exp = 0;
        ssp->srcu_sup->srcu_last_gp_end = ktime_get_mono_fast_ns();
        if (READ_ONCE(ssp->srcu_sup->srcu_size_state) == SRCU_SIZE_SMALL && SRCU_SIZING_IS_INIT()) {
-               if (!init_srcu_struct_nodes(ssp, GFP_ATOMIC)) {
-                       if (!ssp->srcu_sup->sda_is_static) {
-                               free_percpu(ssp->sda);
-                               ssp->sda = NULL;
-                               kfree(ssp->srcu_sup);
-                               return -ENOMEM;
-                       }
-               } else {
-                       WRITE_ONCE(ssp->srcu_sup->srcu_size_state, SRCU_SIZE_BIG);
-               }
+               if (!init_srcu_struct_nodes(ssp, GFP_ATOMIC))
+                       goto err_free_sda;
+               WRITE_ONCE(ssp->srcu_sup->srcu_size_state, SRCU_SIZE_BIG);
        }
        ssp->srcu_sup->srcu_ssp = ssp;
        smp_store_release(&ssp->srcu_sup->srcu_gp_seq_needed, 0); /* Init done. */
        return 0;
+
+err_free_sda:
+       if (!is_static) {
+               free_percpu(ssp->sda);
+               ssp->sda = NULL;
+       }
+err_free_sup:
+       if (!is_static) {
+               kfree(ssp->srcu_sup);
+               ssp->srcu_sup = NULL;
+       }
+       return -ENOMEM;
 }
 
 #ifdef CONFIG_DEBUG_LOCK_ALLOC