Re: [PATCH v5 16/18] timer: Implement the hierarchical pull model

[Peter Zijlstra]

On Wed, Mar 01, 2023 at 03:17:42PM +0100, Anna-Maria Behnsen wrote:

> +static void tmigr_init_group(struct tmigr_group *group, unsigned int lvl,
> +			     unsigned int node, atomic_t *migr_state)
> +{
> +	union tmigr_state s;
> +
> +	raw_spin_lock_init(&group->lock);
> +
> +	group->level = lvl;
> +	group->numa_node = lvl < tmigr_crossnode_level ? node : NUMA_NO_NODE;
> +
> +	group->num_childs = 0;
> +
> +	/*
> +	 * num_cores is required for level=0 groups only during setup and
> +	 * when siblings exists but it doesn't matter if this value is set
> +	 * in other groups as well
> +	 */
> +	group->num_cores = 1;
> +
> +	s.migrator = TMIGR_NONE;
> +	s.active = 0;
> +	s.seq = 0;
> +	atomic_set(migr_state, s.state);
> +
> +	group->migr_state = migr_state;
> +
> +	timerqueue_init_head(&group->events);
> +	timerqueue_init(&group->groupevt.nextevt);
> +	group->groupevt.nextevt.expires = KTIME_MAX;
> +	group->next_expiry = KTIME_MAX;
> +	group->groupevt.ignore = 1;
> +}

> +static struct tmigr_group *tmigr_get_group(unsigned int cpu, unsigned int node,
> +					   unsigned int lvl)
> +{
> +	struct tmigr_group *tmp, *group = NULL;
> +	bool first_loop = true;
> +	atomic_t *migr_state;
> +

...

> +
> +	/* Allocate and	set up a new group with corresponding migr_state */
> +	group = kzalloc_node(sizeof(*group), GFP_KERNEL, node);
> +	if (!group)
> +		return ERR_PTR(-ENOMEM);
> +
> +	migr_state = kzalloc_node(sizeof(atomic_t), GFP_KERNEL, node);
> +	if (!migr_state) {
> +		kfree(group);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	tmigr_init_group(group, lvl, node, migr_state);

I'm confused by migr_state.. why is that allocated seperately, if it is
*always* 1:1 related to the group. Why isn't it a direct member?

> +	/* Setup successful. Add it to the hierarchy */
> +	list_add(&group->list, &tmigr_level_list[lvl]);
> +	return group;
> +}