[PATCH] rcu: Improve comments for RCU_FANOUT and RCU_FANOUT_LEAF

Kunwu Chan posted 1 patch 3 months, 2 weeks ago
There is a newer version of this series
include/linux/rcu_node_tree.h | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
[PATCH] rcu: Improve comments for RCU_FANOUT and RCU_FANOUT_LEAF
Posted by Kunwu Chan 3 months, 2 weeks ago
From: Kunwu Chan <chentao@kylinos.cn>

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 <chentao@kylinos.cn>
---
 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).
  * 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
 
 #define RCU_FANOUT_1	      (RCU_FANOUT_LEAF)
 #define RCU_FANOUT_2	      (RCU_FANOUT_1 * RCU_FANOUT)
-- 
2.25.1
Re: [PATCH] rcu: Improve comments for RCU_FANOUT and RCU_FANOUT_LEAF
Posted by Paul E. McKenney 3 months, 1 week ago
On Thu, Oct 23, 2025 at 11:27:42AM +0800, Kunwu Chan wrote:
> From: Kunwu Chan <chentao@kylinos.cn>
> 
> 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 <chentao@kylinos.cn>

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".

>  #define RCU_FANOUT_1	      (RCU_FANOUT_LEAF)
>  #define RCU_FANOUT_2	      (RCU_FANOUT_1 * RCU_FANOUT)
> -- 
> 2.25.1
>
Re: [PATCH] rcu: Improve comments for RCU_FANOUT and RCU_FANOUT_LEAF
Posted by Kunwu Chan 2 months ago
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 <chentao@kylinos.cn>
>>
>> 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 <chentao@kylinos.cn>
> 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.
[PATCH v2] rcu: Improve comments for RCU_FANOUT and RCU_FANOUT_LEAF
Posted by Kunwu Chan 2 months ago
From: Kunwu Chan <chentao@kylinos.cn>

Add explicit explanations of the overall purpose of RCU_FANOUT and
RCU_FANOUT_LEAF parameters: defining middle/leaf fan-out parameters,
their relation to Kconfig, and how they shape the RCU hierarchy based
on CPU count.

This makes the hierarchical configuration logic of the RCU easier to
understand.

Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
---
Changes in v2:
- Restores the original #else #endif comment format
- Reword commit msg
---
 include/linux/rcu_node_tree.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/rcu_node_tree.h b/include/linux/rcu_node_tree.h
index 78feb8ba7358..a87ca215de4b 100644
--- a/include/linux/rcu_node_tree.h
+++ b/include/linux/rcu_node_tree.h
@@ -25,11 +25,18 @@
 /*
  * 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).
  * 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 */
@@ -40,6 +47,7 @@
 # endif
 #endif /* #else #ifdef CONFIG_RCU_FANOUT */
 
+/* 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 */
-- 
2.25.1