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]
Date:	Mon, 28 Jan 2013 09:53:46 +0800
From:	Tang Chen <tangchen@...fujitsu.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	jiang.liu@...wei.com, wujianguo@...wei.com, hpa@...or.com,
	wency@...fujitsu.com, laijs@...fujitsu.com, linfeng@...fujitsu.com,
	yinghai@...nel.org, isimatu.yasuaki@...fujitsu.com,
	rob@...dley.net, kosaki.motohiro@...fujitsu.com,
	minchan.kim@...il.com, mgorman@...e.de, rientjes@...gle.com,
	guz.fnst@...fujitsu.com, rusty@...tcorp.com.au, lliubbo@...il.com,
	jaegeuk.hanse@...il.com, tony.luck@...el.com,
	glommer@...allels.com, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org
Subject: Re: [PATCH 2/3] acpi, memory-hotplug: Extend movablemem_map ranges
 to the end of node.

Hi Andrew,

On 01/26/2013 08:36 AM, Andrew Morton wrote:
>
> The patch generates a bunch of rejects, partly due to linux-next
> changes but I think I fixed everything up OK.

Thank you for your fixing. :)

>
>> index 4ddf497..f841d0e 100644
>> --- a/arch/x86/mm/srat.c
>> +++ b/arch/x86/mm/srat.c
>> @@ -141,11 +141,16 @@ static inline int save_add_info(void) {return 1;}
>>   static inline int save_add_info(void) {return 0;}
>>   #endif
>>
>> +#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
>> +extern struct movablemem_map movablemem_map;
>> +#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
>
> Well.
>
> a) we shouldn't put extern declarations in C files - put them in
>     headers so we can be assured that all compilation units agree on the
>     type.

OK, got it. :)

>
> b) the ifdefs are unneeded - a unused extern declaration is OK (as
>     long as the type itself is always defined!)

I think struct movablemem_map is defined in mm.h, protected by 
CONFIG_HAVE_MEMBLOCK_NODE_MAP.
So the declaration needs to be protected by CONFIG_HAVE_MEMBLOCK_NODE_MAP.

>
> c) movablemem_map is already declared in memblock.h.

Hum, yes, it is defined in mm.h, included by memblock.h :)

>
> So I zapped the above three lines.
>
>> @@ -178,9 +185,57 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
>>
>>   	node_set(node, numa_nodes_parsed);
>>
>> -	printk(KERN_INFO "SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx]\n",
>> +	printk(KERN_INFO "SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx] %s\n",
>>   	       node, pxm,
>> -	       (unsigned long long) start, (unsigned long long) end - 1);
>> +	       (unsigned long long) start, (unsigned long long) end - 1,
>> +	       hotpluggable ? "Hot Pluggable": "");
>> +
>> +#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
>> +	int overlap;
>> +	unsigned long start_pfn, end_pfn;
>
> no, we don't put declarations of locals in the middle of C statements
> like this:
>
> arch/x86/mm/srat.c: In function 'acpi_numa_memory_affinity_init':
> arch/x86/mm/srat.c:185: warning: ISO C90 forbids mixed declarations and code
>
> Did your compiler not emit this warning?

Sorry, I think it did, but I missed it.
THanks for fixing that. :)

>
> I fixed this by moving the code into a new function
> "handle_movablemem".  Feel free to suggest a more appropriate name!
>
>
> From: Andrew Morton<akpm@...ux-foundation.org>
> Subject: acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix
>
> clean up code, fix build warning
>
> Cc: "Brown, Len"<len.brown@...el.com>
> Cc: "H. Peter Anvin"<hpa@...or.com>
> Cc: Ingo Molnar<mingo@...e.hu>
> Cc: Jiang Liu<jiang.liu@...wei.com>
> Cc: Jianguo Wu<wujianguo@...wei.com>
> Cc: KOSAKI Motohiro<kosaki.motohiro@...fujitsu.com>
> Cc: Kamezawa Hiroyuki<kamezawa.hiroyu@...fujitsu.com>
> Cc: Lai Jiangshan<laijs@...fujitsu.com>
> Cc: Len Brown<lenb@...nel.org>
> Cc: Tang Chen<tangchen@...fujitsu.com>
> Cc: Thomas Gleixner<tglx@...utronix.de>
> Cc: Wu Jianguo<wujianguo@...wei.com>
> Cc: Yasuaki Ishimatsu<isimatu.yasuaki@...fujitsu.com>
> Signed-off-by: Andrew Morton<akpm@...ux-foundation.org>
> ---
>
>   arch/x86/mm/srat.c |   93 ++++++++++++++++++++++---------------------
>   1 file changed, 49 insertions(+), 44 deletions(-)
>
> diff -puN arch/x86/mm/srat.c~acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix arch/x86/mm/srat.c
> --- a/arch/x86/mm/srat.c~acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix
> +++ a/arch/x86/mm/srat.c
> @@ -142,50 +142,8 @@ static inline int save_add_info(void) {r
>   #endif
>
>   #ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> -extern struct movablemem_map movablemem_map;
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> -
> -/* Callback for parsing of the Proximity Domain<->  Memory Area mappings */
> -int __init
> -acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
> +static void __init handle_movablemem(int node, u64 start, u64 end)
>   {
> -	u64 start, end;
> -	u32 hotpluggable;
> -	int node, pxm;
> -
> -	if (srat_disabled())
> -		goto out_err;
> -	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity))
> -		goto out_err_bad_srat;
> -	if ((ma->flags&  ACPI_SRAT_MEM_ENABLED) == 0)
> -		goto out_err;
> -	hotpluggable = ma->flags&  ACPI_SRAT_MEM_HOT_PLUGGABLE;
> -	if (hotpluggable&&  !save_add_info())
> -		goto out_err;
> -
> -	start = ma->base_address;
> -	end = start + ma->length;
> -	pxm = ma->proximity_domain;
> -	if (acpi_srat_revision<= 1)
> -		pxm&= 0xff;
> -
> -	node = setup_node(pxm);
> -	if (node<  0) {
> -		printk(KERN_ERR "SRAT: Too many proximity domains.\n");
> -		goto out_err_bad_srat;
> -	}
> -
> -	if (numa_add_memblk(node, start, end)<  0)
> -		goto out_err_bad_srat;
> -
> -	node_set(node, numa_nodes_parsed);
> -
> -	printk(KERN_INFO "SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx] %s\n",
> -	       node, pxm,
> -	       (unsigned long long) start, (unsigned long long) end - 1,
> -	       hotpluggable ? "Hot Pluggable": "");
> -
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
>   	int overlap;
>   	unsigned long start_pfn, end_pfn;
>
> @@ -229,7 +187,54 @@ acpi_numa_memory_affinity_init(struct ac
>   			 */
>   			insert_movablemem_map(start_pfn, end_pfn);
>   	}
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> +}
> +#else		/* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> +static inline void handle_movablemem(int node, u64 start, u64 end)
> +{
> +}
> +#endif		/* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> +
> +/* Callback for parsing of the Proximity Domain<->  Memory Area mappings */
> +int __init
> +acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
> +{
> +	u64 start, end;
> +	u32 hotpluggable;
> +	int node, pxm;
> +
> +	if (srat_disabled())
> +		goto out_err;
> +	if (ma->header.length != sizeof(struct acpi_srat_mem_affinity))
> +		goto out_err_bad_srat;
> +	if ((ma->flags&  ACPI_SRAT_MEM_ENABLED) == 0)
> +		goto out_err;
> +	hotpluggable = ma->flags&  ACPI_SRAT_MEM_HOT_PLUGGABLE;
> +	if (hotpluggable&&  !save_add_info())
> +		goto out_err;
> +
> +	start = ma->base_address;
> +	end = start + ma->length;
> +	pxm = ma->proximity_domain;
> +	if (acpi_srat_revision<= 1)
> +		pxm&= 0xff;
> +
> +	node = setup_node(pxm);
> +	if (node<  0) {
> +		printk(KERN_ERR "SRAT: Too many proximity domains.\n");
> +		goto out_err_bad_srat;
> +	}
> +
> +	if (numa_add_memblk(node, start, end)<  0)
> +		goto out_err_bad_srat;
> +
> +	node_set(node, numa_nodes_parsed);
> +
> +	printk(KERN_INFO "SRAT: Node %u PXM %u [mem %#010Lx-%#010Lx] %s\n",
> +	       node, pxm,
> +	       (unsigned long long) start, (unsigned long long) end - 1,
> +	       hotpluggable ? "Hot Pluggable": "");
> +
> +	handle_movablemem(node, start, end);
>
>   	return 0;
>   out_err_bad_srat:
> diff -puN include/linux/mm.h~acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix include/linux/mm.h
> diff -puN mm/page_alloc.c~acpi-memory-hotplug-extend-movablemem_map-ranges-to-the-end-of-node-fix mm/page_alloc.c
> _
>
>
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ