Re: [PATCH 1/8] mm: introduce for_each_process_rcu and for_each_thread_rcu

[Michal Hocko]

You want to CC Oleg at least. Now added

Also prefixing this patch with mm is confusing as this has nothing to do
with mm.

On Fri 04-09-26 16:29:53, Ye Liu wrote:
> From: Ye Liu <liuye@xxxxxxxxxx>
> 
> Introduce for_each_process_rcu(), for_each_thread_rcu() and
> for_each_process_thread_rcu() macros that acquire the RCU read lock
> before the iteration starts and release it when the loop is left, so
> that the RCU read-side critical section is scoped to the loop body
> instead of an externally managed rcu_read_lock()/rcu_read_unlock()
> pair.
> 
> Signed-off-by: Ye Liu <liuye@xxxxxxxxxx>

Apparat from potential naming changes this makes a lot of sense to me.
Many users really need RCU in the scope of iteration and this simplifies
the code and it makes less prone to bugs.

I do not really care much about the naming TBH
Acked-by: Michal Hocko <mhocko@xxxxxxxx>
> ---
>  include/linux/sched/signal.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
> index 584ae88b435e..fe0c41dc99a1 100644
> --- a/include/linux/sched/signal.h
> +++ b/include/linux/sched/signal.h
> @@ -2,6 +2,7 @@
>  #ifndef _LINUX_SCHED_SIGNAL_H
>  #define _LINUX_SCHED_SIGNAL_H
>  
> +#include <linux/cleanup.h>
>  #include <linux/rculist.h>
>  #include <linux/signal.h>
>  #include <linux/sched.h>
> @@ -663,6 +664,24 @@ extern bool current_is_single_threaded(void);
>  #define for_each_process_thread(p, t)	\
>  	for_each_process(p) for_each_thread(p, t)
>  
> +/*
> + * RCU-internal variants: automatically acquire and release the RCU read
> + * lock around the iteration.  Equivalent to scoped_guard(rcu) combined
> + * with the respective non-_rcu variant.
> + */
> +#define for_each_process_rcu(p) \
> +	scoped_guard(rcu) \
> +	for (p = &init_task ; (p = next_task(p)) != &init_task ; )
> +
> +#define for_each_thread_rcu(p, t) \
> +	scoped_guard(rcu) \
> +	__for_each_thread((p)->signal, t)
> +
> +/* Careful: this is a double loop, 'break' won't work as expected. */
> +#define for_each_process_thread_rcu(p, t) \
> +	scoped_guard(rcu) \
> +	for_each_process(p) for_each_thread(p, t)
> +
>  typedef int (*proc_visitor)(struct task_struct *p, void *data);
>  void walk_process_tree(struct task_struct *top, proc_visitor, void *);
>  
> -- 
> 2.25.1
> 

-- 
Michal Hocko
SUSE Labs