Re: [PATCH v2 4/5] timer_migration: Fix possible race in tmigr_active_up() in setup path

[Frederic Weisbecker]

Le Tue, Jun 25, 2024 at 12:01:27AM +0200, Frederic Weisbecker a écrit :
> > +static void tmigr_setup_active_up(struct tmigr_group *group, struct tmigr_group *child)
> > +{
> > +	union tmigr_state curstate, childstate;
> > +	bool walk_done;
> > +
> > +	/*
> > +	 * Memory barrier is paired with the cmpxchg in tmigr_inactive_up() to
> > +	 * make sure updates of child and group states are ordered. The ordering
> > +	 * is mandatory, as the update of the group state is only valid for when
> > +	 * child state is active.
> > +	 */
> > +	curstate.state = atomic_read_acquire(&group->migr_state);
> > +
> > +	for (;;) {
> 
> /*
>  * If the child hasn't yet propagated anything to the top level, the above
>  * acquire has no effect. However thanks to child locking (see comment in
>  * tmigr_connect_child_parent()), either the latest child->migr_state is
>  * observed or the remote CPU now observes the new parent and is about to
>  * propagate to the new parent. In the latter case it will either beat the
>  * current trial update or overwrite it.
>  */
> 
> And another comment later:
> 
> > +		childstate.state = atomic_read(&child->migr_state);
> > +		if (!childstate.active)
> > +			return;

Hmm, on the other hand the remote group doesn't use locking if some tmc activates
after that and so there is no guarantee it will see the new parent. Does it
matter?

Thanks.