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

Peter Zijlstra
2023-03-23T12:47:31Z

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;
> +}