linux-kernel - Re: [patch 06/10] cpu topology support for s390.

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: <20080312161137.63bdfa67.akpm@linux-foundation.org>
Date:	Wed, 12 Mar 2008 16:11:37 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Martin Schwidefsky <schwidefsky@...ibm.com>
Cc:	linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
	heiko.carstens@...ibm.com, schwidefsky@...ibm.com
Subject: Re: [patch 06/10] cpu topology support for s390.

On Wed, 12 Mar 2008 18:32:01 +0100
Martin Schwidefsky <schwidefsky@...ibm.com> wrote:

> From: Heiko Carstens <heiko.carstens@...ibm.com>
> 
> Add s390 backend so we can give the scheduler some hints about the
> cpu topology.
> 
> ===================================================================
> --- /dev/null
> +++ quilt-2.6/arch/s390/kernel/topology.c
> @@ -0,0 +1,271 @@
> +/*
> + *  arch/s390/kernel/topology.c
> + *
> + *    Copyright IBM Corp. 2007
> + *    Author(s): Heiko Carstens <heiko.carstens@...ibm.com>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/bootmem.h>
> +#include <linux/sched.h>
> +#include <linux/workqueue.h>
> +#include <linux/cpu.h>
> +#include <linux/smp.h>
> +#include <asm/delay.h>
> +#include <asm/s390_ext.h>
> +
> +#define CPU_BITS 64
> +
> +struct tl_cpu {
> +	unsigned char reserved[6];
> +	unsigned short origin;
> +	unsigned long mask[CPU_BITS / BITS_PER_LONG];
> +};

mask[] will be too small for CPU_BITS=65 ;)

> ...
>
> +static union tl_entry *next_tle(union tl_entry *tle)
> +{
> +	if (tle->nl)
> +		return (union tl_entry *)((struct tl_container *)tle + 1);
> +	else
> +		return (union tl_entry *)((struct tl_cpu *)tle + 1);
> +}

omg.

> +static void tl_to_cores(struct tl_info *info)
> +{
> +	union tl_entry *tle, *end;
> +	struct core_info *core = &core_info;
> +
> +	mutex_lock(&smp_cpu_state_mutex);
> +	clear_cores();
> +	tle = (union tl_entry *)&info->tle;

and this cast was unneeded!

> +	end = (union tl_entry *)((unsigned long)info + info->length);

I'd suggest that you take a look at all the pointer arith games which are
being played in this code and see if it can be done better with a more
appropriate use of the C type system.  Before someone dies.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/