linux-kernel - Re: [PATCH] x86: UV BAU fix for non-consecutive nasids

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] [day] [month] [year] [list]

Message-ID: <20110510063445.GB11595@elte.hu>
Date:	Tue, 10 May 2011 08:34:45 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Cliff Wickman <cpw@....com>
Cc:	linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] x86: UV BAU fix for non-consecutive nasids


* Cliff Wickman <cpw@....com> wrote:

> @@ -1365,12 +1373,14 @@ uv_activation_descriptor_init(int node,
>  		memset(bd2, 0, sizeof(struct bau_desc));
>  		bd2->header.sw_ack_flag = 1;
>  		/*
> -		 * base_dest_nodeid is the nasid of the first uvhub
> -		 * in the partition. The bit map will indicate uvhub numbers,
> -		 * which are 0-N in a partition. Pnodes are unique system-wide.
> +		 * The base_dest_nasid set in the message header is the nasid
> +		 * of the first uvhub in the partition. The bit map will
> +		 * indicate destination pnode numbers relative to that base.
> +		 * They may not be consecutive if nasid striding is being used.
>  		 */
> -		bd2->header.base_dest_nodeid = UV_PNODE_TO_NASID(uv_partition_base_pnode);
> -		bd2->header.dest_subnodeid = 0x10; /* the LB */
> +		bd2->header.base_dest_nasid =
> +					UV_PNODE_TO_NASID(part_base_pnode);
> +		bd2->header.dest_subnodeid = UV_LB_SUBNODEID;
>  		bd2->header.command = UV_NET_ENDPOINT_INTD;
>  		bd2->header.int_both = 1;

This code is sick. Illness: too long lines.

checkpatch warned you but you applied the wrong cure and
broke the lines in an ugly way.

Can you	think of another cure? One of the many options:

  - eliminate many repetitive strings
  - reduce indentation by the introduction of helper inlines
  - sensible shortening of variable/field/definition names

... applies to this particular case and will solve the problem.

> @@ -1528,6 +1539,15 @@ static int __init uv_init_per_cpu(int nu
>  		bcp = &per_cpu(bau_control, cpu);
>  		memset(bcp, 0, sizeof(struct bau_control));
>  		pnode = uv_cpu_hub_info(cpu)->pnode;
> +		if ((pnode - base_part_pnode) >= UV_DISTRIBUTION_SIZE) {
> +			printk(KERN_EMERG
> +				"cpu %d pnode %d-%d beyond %d; BAU disabled\n",
> +				cpu, pnode, base_part_pnode,
> +				UV_DISTRIBUTION_SIZE);

See my points above.

> @@ -1585,6 +1605,20 @@ static int __init uv_init_per_cpu(int nu
>  nextsocket:
>  			socket++;
>  			socket_mask = (socket_mask >> 1);
> +			/* each socket gets a local array of pnodes/hubs */
> +			bcp = smaster;
> +			bcp->target_hub_and_pnode = kmalloc_node(
> +				sizeof(struct hub_and_pnode) *
> +				num_possible_cpus(), GFP_KERNEL, bcp->osnode);
> +			memset(bcp->target_hub_and_pnode, 0,
> +				sizeof(struct hub_and_pnode) *
> +				num_possible_cpus());
> +			for_each_present_cpu(tcpu) {
> +				bcp->target_hub_and_pnode[tcpu].pnode =
> +					uv_cpu_hub_info(tcpu)->pnode;
> +				bcp->target_hub_and_pnode[tcpu].uvhub =
> +					uv_cpu_hub_info(tcpu)->numa_blade_id;
> +			}

See my points above.

Thanks,

	Ingo
--
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/