Re: [PATCH v4 14/16] timer: Implement the hierarchical pull model

[Frederic Weisbecker]

On Fri, Nov 04, 2022 at 03:57:35PM +0100, Anna-Maria Behnsen wrote:
> +static bool tmigr_handle_remote_up(struct tmigr_group *group,
> +				   struct tmigr_group *child,
> +				   void *ptr)
> +{
> +	struct tmigr_remote_data *data = ptr;
> +	u64 now, next = KTIME_MAX;
> +	unsigned long flags, jif;
> +	struct tmigr_event *evt;
> +	u32 childmask;
> +
> +	jif = data->basej;
> +	now = data->now;
> +
> +	childmask = data->childmask;
> +
> +again:
> +	/*
> +	 * Handle the group only if @childmask is the migrator or if the
> +	 * group has no migrator. Otherwise the group is active and is
> +	 * handled by its own migrator.
> +	 */
> +	if (!tmigr_check_migrator(group, childmask))
> +		return true;
> +
> +	raw_spin_lock_irqsave(&group->lock, flags);
> +
> +	evt = tmigr_next_expired_groupevt(group, now);
> +
> +	if (evt) {
> +		unsigned int remote_cpu;
> +
> +		remote_cpu = READ_ONCE(evt->cpu);

Is that READ_ONCE() really necessary?

Thanks.

> +
> +		raw_spin_unlock_irqrestore(&group->lock, flags);
> +
> +		next = tmigr_handle_remote_cpu(remote_cpu, now, jif);
> +
> +		/* check if there is another event, that needs to be handled */
> +		goto again;
> +	} else {
> +		raw_spin_unlock_irqrestore(&group->lock, flags);
> +	}
> +
> +	/* Update of childmask for next level */
> +	data->childmask = group->childmask;
> +	data->wakeup = next;
> +
> +	return false;
> +}