linux-kernel - Re: [PATCH v2 1/2] irq/matrix: Split out the CPU finding code into a helper

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 for Android: free password hash cracker in your pocket

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

Message-ID: <alpine.DEB.2.21.1809081155280.1402@nanos.tec.linutronix.de>
Date:   Sat, 8 Sep 2018 11:59:19 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Dou Liyang <dou_liyang@....com>
cc:     linux-kernel@...r.kernel.org, x86@...nel.org, mingo@...hat.com,
        hpa@...or.com, douly.fnst@...fujitsu.com
Subject: Re: [PATCH v2 1/2] irq/matrix: Split out the CPU finding code into
 a helper

On Sat, 8 Sep 2018, Dou Liyang wrote:
> +/* Find the best CPU which has the lowest vector allocation count */
> +static int matrix_find_best_cpu(struct irq_matrix *m,
> +				const struct cpumask *msk, int *best_cpu)
> +{
> +	unsigned int cpu, maxavl = 0;
> +	struct cpumap *cm;
> +
> +	for_each_cpu(cpu, msk) {
> +		cm = per_cpu_ptr(m->maps, cpu);
> +
> +		if (!cm->online || cm->available <= maxavl)
> +			continue;
> +
> +		*best_cpu = cpu;
> +		maxavl = cm->available;
> +	}
> +
> +	return maxavl;

This return value makes no sense whatsoever. Why not doing the obvious

static unsigned int matrix_find_best_cpu(struct irq_matrix *m,
	                             	 const struct cpumask *msk)
{
	unsigned int cpu, best_cpu, maxavl = 0;

	best_cpu = UINT_MAX;
	.....
	return best_cpu;
}

and at the call site:

       cpu = matrix_find_best_cpu();
       if (cpu == UINT_MAX)
       		return -ENOSPC;

Hmm?

> +	cm = per_cpu_ptr(m->maps, best_cpu);
> +	bit = matrix_alloc_area(m, cm, 1, false);
> +	if (bit < m->alloc_end) {

So if you're removing one indentation level, then it'd be consequent to
remove this one as well while at it :)

Thanks,

	tglx