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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230323124711.GB2512024@hirez.programming.kicks-ass.net>
Date:   Thu, 23 Mar 2023 13:47:11 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Anna-Maria Behnsen <anna-maria@...utronix.de>
Cc:     linux-kernel@...r.kernel.org, John Stultz <jstultz@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Eric Dumazet <edumazet@...gle.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        Arjan van de Ven <arjan@...radead.org>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Frederic Weisbecker <fweisbec@...il.com>,
        Rik van Riel <riel@...riel.com>
Subject: Re: [PATCH v5 16/18] timer: Implement the hierarchical pull model

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