On 10/30/25 07:46, Paul E. McKenney wrote:
On Thu, Oct 23, 2025 at 11:27:42AM +0800, Kunwu Chan wrote:
From: Kunwu Chan <[email protected]>

The original comments introduced in commit 05c5df31afd1
("rcu: Make RCU able to tolerate undefined CONFIG_RCU_FANOUT"),
contained confusing annotations.

Specifically, the #else and #endif comments did not clearly reflect
their corresponding condition blocks, hampering readability.

Fixes condition branch comments. And adds explicit explanations of
the overall purpose:
defining middle/leaf fan-out parameters, their relation to Kconfig,
and how they shape the RCU hierarchy based on CPU count.

Make the hierarchical configuration logic of the RCU easier to understand.

Signed-off-by: Kunwu Chan <[email protected]>
Thank you for posting this!  Please see below for some comments.

                                                        Thanx, Paul

---
  include/linux/rcu_node_tree.h | 16 ++++++++++++----
  1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/linux/rcu_node_tree.h b/include/linux/rcu_node_tree.h
index 78feb8ba7358..b03c0ce91dec 100644
--- a/include/linux/rcu_node_tree.h
+++ b/include/linux/rcu_node_tree.h
@@ -25,26 +25,34 @@
  /*
   * Define shape of hierarchy based on NR_CPUS, CONFIG_RCU_FANOUT, and
   * CONFIG_RCU_FANOUT_LEAF.
+ * - RCU_FANOUT: Controls fan-out of middle levels in the RCU hierarchy.
+ * - RCU_FANOUT_LEAF: Controls fan-out of the leaf level (directly managing 
CPUs).
+ *
+ * These parameters are determined by Kconfig options if configured; otherwise,
+ * they use sensible defaults based on system architecture (for RCU_FANOUT)
+ * or a fixed default (for RCU_FANOUT_LEAF).
I have no objections to this change if at least one of my fellow
maintainers is willing to speak up for it and none of the others object
to it.

   * In theory, it should be possible to add more levels straightforwardly.
   * In practice, this did work well going from three levels to four.
   * Of course, your mileage may vary.
   */
+/* Define RCU_FANOUT: middle-level fan-out parameter */
  #ifdef CONFIG_RCU_FANOUT
  #define RCU_FANOUT CONFIG_RCU_FANOUT
-#else /* #ifdef CONFIG_RCU_FANOUT */
+#else /* #ifndef CONFIG_RCU_FANOUT */
  # ifdef CONFIG_64BIT
  # define RCU_FANOUT 64
  # else
  # define RCU_FANOUT 32
  # endif
-#endif /* #else #ifdef CONFIG_RCU_FANOUT */
+#endif
+/* Define RCU_FANOUT_LEAF: leaf-level fan-out parameter (manages CPUs directly) */
  #ifdef CONFIG_RCU_FANOUT_LEAF
  #define RCU_FANOUT_LEAF CONFIG_RCU_FANOUT_LEAF
-#else /* #ifdef CONFIG_RCU_FANOUT_LEAF */
+#else /* #ifndef CONFIG_RCU_FANOUT_LEAF */
  #define RCU_FANOUT_LEAF 16
-#endif /* #else #ifdef CONFIG_RCU_FANOUT_LEAF */
+#endif
But these much stay as they are.  The #else echos the "#if" condition, and
the #endif contains "#else" followed by the "#if" condition.  This means
that you can tell where you are without having to find the matching "#if"
and without having to figure out whether there is an intervening "#else".

Hi Paul,

Thank you for the feedback! I reviewed Documentation/process/coding-style.rst and found the guidance on #endif comments (section 19), but I didn't find explicit guidance on the #else comment format. I wasn't aware of the specific convention used in the RCU codebase for #else and #endif directives. I understand now that this format helps readers quickly identify which conditional branch they're in
without having to search backwards for the matching #if.

I'll prepare a V2 patch that restores the original #else and #endif comment format while keeping the new explanatory comments about RCU_FANOUT and RCU_FANOUT_LEAF.


  #define RCU_FANOUT_1        (RCU_FANOUT_LEAF)
  #define RCU_FANOUT_2        (RCU_FANOUT_1 * RCU_FANOUT)
--
2.25.1

--
Thanks,
       Kunwu Chan.


Reply via email to