On Tue, Aug 13, 2024 at 08:47:12PM GMT, Kinsey Ho <kinseyho@xxxxxxxxxx> wrote:
> To obtain the pointer to the next memcg position, mem_cgroup_iter()
> currently holds css->refcnt during memcg traversal only to put
> css->refcnt at the end of the routine. This isn't necessary as an
> rcu_read_lock is already held throughout the function. The use of
> the RCU read lock with css_next_descendant_pre() guarantees that
> sibling linkage is safe without holding a ref on the passed-in @css.
>
> Remove css->refcnt usage during traversal by leveraging RCU.
>
> Signed-off-by: Kinsey Ho <kinseyho@xxxxxxxxxx>
> ---
> include/linux/memcontrol.h | 2 +-
> mm/memcontrol.c | 18 +-----------------
> 2 files changed, 2 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 90ecd2dbca06..1aaed2f1f6ae 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -75,7 +75,7 @@ struct lruvec_stats_percpu;
> struct lruvec_stats;
>
> struct mem_cgroup_reclaim_iter {
> - struct mem_cgroup *position;
> + struct mem_cgroup __rcu *position;
I'm not sure about this annotation.
This pointer could be modified concurrently with RCU read sections with
the cmpxchg which would assume that's equivalent with
rcu_assign_pointer(). (Which it might be but it's not idiomatic, so it
causes some head wrapping.)
Isn't this situation covered with a regular pointer and
READ_ONCE()+cmpxchg?
Regards,
Michal
Attachment:
signature.asc
Description: PGP signature
