linux-kernel - Re: [PATCH - resend ] memory-hotplug: create /sys/firmware/memmap entry for new memory(v3)

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: <20100108110810.GA6153@localhost>
Date:	Fri, 8 Jan 2010 19:08:10 +0800
From:	Wu Fengguang <fengguang.wu@...el.com>
To:	"Zheng, Shaohui" <shaohui.zheng@...el.com>
Cc:	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"ak@...ux.intel.com" <ak@...ux.intel.com>,
	"y-goto@...fujitsu.com" <y-goto@...fujitsu.com>,
	Dave Hansen <haveblue@...ibm.com>,
	"x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH - resend ] memory-hotplug: create /sys/firmware/memmap
	entry for new memory(v3)

On Fri, Jan 08, 2010 at 11:16:13AM +0800, Zheng, Shaohui wrote:
> Resend the patch to the mailing-list, the original patch URL is at 
> http://patchwork.kernel.org/patch/69071/. It is already reviewed, but It is still not 
> accepted and no comments, I guess that it should be ignored since we have so many 
> patches each day, send it again.  
> 
> memory-hotplug: create /sys/firmware/memmap entry for hot-added memory
> 
> Interface firmware_map_add was not called in explicit, Remove it and add function
> firmware_map_add_hotplug as hotplug interface of memmap.
> 
> When we hot-add new memory, sysfs does not export memmap entry for it. we add
>  a call in function add_memory to function firmware_map_add_hotplug.
> 
> Add a new function add_sysfs_fw_map_entry to create memmap entry, it can avoid 
> duplicated codes.
> 
> Thanks for the careful review from Fengguang Wu and Dave Hansen.
> 
> Signed-off-by: Shaohui Zheng <shaohui.zheng@...el.com>
> Acked-by: Andi Kleen <ak@...ux.intel.com>
> Acked-by: Yasunori Goto <y-goto@...fujitsu.com>
> Acked-by: Dave Hansen <dave@...ux.vnet.ibm.com>
> Reviewed-by: Wu Fengguang <fengguang.wu@...el.com>
> ---
> diff --git a/drivers/firmware/memmap.c b/drivers/firmware/memmap.c
> index 56f9234..ec8c3d4 100644
> --- a/drivers/firmware/memmap.c
> +++ b/drivers/firmware/memmap.c
> @@ -123,52 +123,75 @@ static int firmware_map_add_entry(u64 start, u64 end,
>  }
>  
>  /**
> - * firmware_map_add() - Adds a firmware mapping entry.
> + * Add memmap entry on sysfs
> + */
> +static int add_sysfs_fw_map_entry(struct firmware_map_entry *entry) {

Minor style issue:

ERROR: open brace '{' following function declarations go on the next line
#31: FILE: drivers/firmware/memmap.c:128:
+static int add_sysfs_fw_map_entry(struct firmware_map_entry *entry) {

total: 1 errors, 0 warnings, 145 lines checked

patches/memory-hotplug-create-sys-firmware-memmap-entry-for-new-memory-v3.patch
has style problems, please review.  If any of these e rrors
are false positives report them to the maintainer, see CHECKPATCH in
MAINTAINERS.

> +	static int map_entries_nr;
> +	static struct kset *mmap_kset;
> +
> +	if (!mmap_kset) {
> +		mmap_kset = kset_create_and_add("memmap", NULL, firmware_kobj);
> +		if (WARN_ON(!mmap_kset))

This WARN_ON() may never trigger, or when things go terribly wrong it
repeatedly produce a dozen stack dumps, which don't really help
diagnose the root cause.  Better to just remove it.

> +			return -ENOMEM;
> +	}
> +
> +	entry->kobj.kset = mmap_kset;
> +	if (kobject_add(&entry->kobj, NULL, "%d", map_entries_nr++))
> +		kobject_put(&entry->kobj);
> +
> +	return 0;
> +}
> +
> +/**
> + * firmware_map_add_early() - Adds a firmware mapping entry.
>   * @start: Start of the memory range.
>   * @end:   End of the memory range (inclusive).
>   * @type:  Type of the memory range.
>   *
> - * This function uses kmalloc() for memory
> - * allocation. Use firmware_map_add_early() if you want to use the bootmem
> - * allocator.
> + * Adds a firmware mapping entry. This function uses the bootmem allocator
> + * for memory allocation.
>   *
>   * That function must be called before late_initcall.
>   *
>   * Returns 0 on success, or -ENOMEM if no memory could be allocated.
>   **/
> -int firmware_map_add(u64 start, u64 end, const char *type)
> +int __init firmware_map_add_early(u64 start, u64 end, const char *type)
>  {
>  	struct firmware_map_entry *entry;
>  
> -	entry = kmalloc(sizeof(struct firmware_map_entry), GFP_ATOMIC);
> -	if (!entry)
> +	entry = alloc_bootmem(sizeof(struct firmware_map_entry));
> +	if (WARN_ON(!entry))

Ditto.

Thanks,
Fengguang

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