All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, linux-acpi@vger.kernel.org,
	rientjes@google.com, liuj97@gmail.com, len.brown@intel.com,
	benh@kernel.crashing.org, paulus@samba.org, cl@linux.com,
	minchan.kim@gmail.com, akpm@linux-foundation.org,
	kosaki.motohiro@jp.fujitsu.com, wency@cn.fujitsu.com
Subject: Re: [RFC PATCH v3 3/13] memory-hotplug : unify argument of firmware_map_add_early/hotplug
Date: Wed, 11 Jul 2012 08:30:16 -0700	[thread overview]
Message-ID: <4FFD9C08.2070502@linux.vnet.ibm.com> (raw)
In-Reply-To: <4FFAB17F.2090209@jp.fujitsu.com>

On 07/09/2012 03:25 AM, Yasuaki Ishimatsu wrote:
> @@ -642,7 +642,7 @@ int __ref add_memory(int nid, u64 start,
>  	}
> 
>  	/* create new memmap entry */
> -	firmware_map_add_hotplug(start, start + size, "System RAM");
> +	firmware_map_add_hotplug(start, start + size - 1, "System RAM");

I know the firmware_map_*() calls use inclusive end addresses
internally, but do we really need to expose them?  Both of the callers
you mentioned do:

	firmware_map_add_hotplug(start, start + size - 1, "System RAM");

or

                firmware_map_add_early(entry->addr,
                        entry->addr + entry->size - 1,
                        e820_type_to_string(entry->type));

So it seems a _bit_ silly to keep all of the callers doing this size-1
thing.  I also noted that the new caller that you added does the same
thing.  Could we just change the external calling convention to be
exclusive?

BTW, this patch should probably be first in your series.  It's a real
bugfix.


WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, linux-acpi@vger.kernel.org,
	rientjes@google.com, liuj97@gmail.com, len.brown@intel.com,
	benh@kernel.crashing.org, paulus@samba.org, cl@linux.com,
	minchan.kim@gmail.com, akpm@linux-foundation.org,
	kosaki.motohiro@jp.fujitsu.com, wency@cn.fujitsu.com
Subject: Re: [RFC PATCH v3 3/13] memory-hotplug : unify argument of firmware_map_add_early/hotplug
Date: Wed, 11 Jul 2012 08:30:16 -0700	[thread overview]
Message-ID: <4FFD9C08.2070502@linux.vnet.ibm.com> (raw)
In-Reply-To: <4FFAB17F.2090209@jp.fujitsu.com>

On 07/09/2012 03:25 AM, Yasuaki Ishimatsu wrote:
> @@ -642,7 +642,7 @@ int __ref add_memory(int nid, u64 start,
>  	}
> 
>  	/* create new memmap entry */
> -	firmware_map_add_hotplug(start, start + size, "System RAM");
> +	firmware_map_add_hotplug(start, start + size - 1, "System RAM");

I know the firmware_map_*() calls use inclusive end addresses
internally, but do we really need to expose them?  Both of the callers
you mentioned do:

	firmware_map_add_hotplug(start, start + size - 1, "System RAM");

or

                firmware_map_add_early(entry->addr,
                        entry->addr + entry->size - 1,
                        e820_type_to_string(entry->type));

So it seems a _bit_ silly to keep all of the callers doing this size-1
thing.  I also noted that the new caller that you added does the same
thing.  Could we just change the external calling convention to be
exclusive?

BTW, this patch should probably be first in your series.  It's a real
bugfix.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Cc: len.brown@intel.com, wency@cn.fujitsu.com,
	linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, paulus@samba.org, minchan.kim@gmail.com,
	kosaki.motohiro@jp.fujitsu.com, rientjes@google.com,
	cl@linux.com, linuxppc-dev@lists.ozlabs.org,
	akpm@linux-foundation.org, liuj97@gmail.com
Subject: Re: [RFC PATCH v3 3/13] memory-hotplug : unify argument of firmware_map_add_early/hotplug
Date: Wed, 11 Jul 2012 08:30:16 -0700	[thread overview]
Message-ID: <4FFD9C08.2070502@linux.vnet.ibm.com> (raw)
In-Reply-To: <4FFAB17F.2090209@jp.fujitsu.com>

On 07/09/2012 03:25 AM, Yasuaki Ishimatsu wrote:
> @@ -642,7 +642,7 @@ int __ref add_memory(int nid, u64 start,
>  	}
> 
>  	/* create new memmap entry */
> -	firmware_map_add_hotplug(start, start + size, "System RAM");
> +	firmware_map_add_hotplug(start, start + size - 1, "System RAM");

I know the firmware_map_*() calls use inclusive end addresses
internally, but do we really need to expose them?  Both of the callers
you mentioned do:

	firmware_map_add_hotplug(start, start + size - 1, "System RAM");

or

                firmware_map_add_early(entry->addr,
                        entry->addr + entry->size - 1,
                        e820_type_to_string(entry->type));

So it seems a _bit_ silly to keep all of the callers doing this size-1
thing.  I also noted that the new caller that you added does the same
thing.  Could we just change the external calling convention to be
exclusive?

BTW, this patch should probably be first in your series.  It's a real
bugfix.

  reply	other threads:[~2012-07-11 15:31 UTC|newest]

Thread overview: 172+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-09 10:21 [RFC PATCH v3 0/13] memory-hotplug : hot-remove physical memory Yasuaki Ishimatsu
2012-07-09 10:21 ` Yasuaki Ishimatsu
2012-07-09 10:21 ` Yasuaki Ishimatsu
2012-07-09 10:21 ` Yasuaki Ishimatsu
2012-07-09 10:23 ` [RFC PATCH v3 1/13] memory-hotplug : rename remove_memory to offline_memory Yasuaki Ishimatsu
2012-07-09 10:23   ` Yasuaki Ishimatsu
2012-07-09 10:23   ` Yasuaki Ishimatsu
2012-07-09 10:23   ` Yasuaki Ishimatsu
2012-07-09 10:24 ` [RFC PATCH v3 2/13] memory-hotplug : add physical memory hotplug code to acpi_memory_device_remove Yasuaki Ishimatsu
2012-07-09 10:24   ` Yasuaki Ishimatsu
2012-07-09 10:24   ` Yasuaki Ishimatsu
2012-07-13  3:26   ` Wen Congyang
2012-07-13  3:26     ` Wen Congyang
2012-07-13  3:26     ` Wen Congyang
2012-07-17  0:46     ` Yasuaki Ishimatsu
2012-07-17  0:46       ` Yasuaki Ishimatsu
2012-07-17  0:46       ` Yasuaki Ishimatsu
2012-07-17  0:46       ` Yasuaki Ishimatsu
2012-07-13  3:35   ` Wen Congyang
2012-07-13  3:35     ` Wen Congyang
2012-07-13  3:35     ` Wen Congyang
2012-07-17  1:44     ` Yasuaki Ishimatsu
2012-07-17  1:44       ` Yasuaki Ishimatsu
2012-07-17  1:44       ` Yasuaki Ishimatsu
2012-07-17  1:54       ` Yasuaki Ishimatsu
2012-07-17  1:54         ` Yasuaki Ishimatsu
2012-07-17  1:54         ` Yasuaki Ishimatsu
2012-07-17  2:32         ` Wen Congyang
2012-07-17  2:32           ` Wen Congyang
2012-07-17  2:32           ` Wen Congyang
2012-07-17  3:08           ` Yasuaki Ishimatsu
2012-07-17  3:08             ` Yasuaki Ishimatsu
2012-07-17  3:08             ` Yasuaki Ishimatsu
2012-07-17  3:08             ` Yasuaki Ishimatsu
2012-07-17  3:32             ` Wen Congyang
2012-07-17  3:32               ` Wen Congyang
2012-07-17  3:32               ` Wen Congyang
2012-07-17  4:51               ` Yasuaki Ishimatsu
2012-07-17  4:51                 ` Yasuaki Ishimatsu
2012-07-17  4:51                 ` Yasuaki Ishimatsu
2012-07-17  5:17                 ` Wen Congyang
2012-07-17  5:17                   ` Wen Congyang
2012-07-17  5:17                   ` Wen Congyang
2012-07-17  5:19                   ` Yasuaki Ishimatsu
2012-07-17  5:19                     ` Yasuaki Ishimatsu
2012-07-17  5:19                     ` Yasuaki Ishimatsu
2012-07-17  5:19                     ` Yasuaki Ishimatsu
2012-07-13 10:40   ` Wen Congyang
2012-07-13 10:40     ` Wen Congyang
2012-07-13 10:40     ` Wen Congyang
2012-07-17  1:10     ` Yasuaki Ishimatsu
2012-07-17  1:10       ` Yasuaki Ishimatsu
2012-07-17  1:10       ` Yasuaki Ishimatsu
2012-07-17  1:10       ` Yasuaki Ishimatsu
2012-07-09 10:25 ` [RFC PATCH v3 3/13] memory-hotplug : unify argument of firmware_map_add_early/hotplug Yasuaki Ishimatsu
2012-07-09 10:25   ` Yasuaki Ishimatsu
2012-07-09 10:25   ` Yasuaki Ishimatsu
2012-07-09 10:25   ` Yasuaki Ishimatsu
2012-07-11 15:30   ` Dave Hansen [this message]
2012-07-11 15:30     ` Dave Hansen
2012-07-11 15:30     ` Dave Hansen
2012-07-12  4:52     ` Yasuaki Ishimatsu
2012-07-12  4:52       ` Yasuaki Ishimatsu
2012-07-12  4:52       ` Yasuaki Ishimatsu
2012-07-12  4:52       ` Yasuaki Ishimatsu
2012-07-12 13:40       ` Dave Hansen
2012-07-12 13:40         ` Dave Hansen
2012-07-12 13:40         ` Dave Hansen
2012-07-13  4:34         ` Yasuaki Ishimatsu
2012-07-13  4:34           ` Yasuaki Ishimatsu
2012-07-13  4:34           ` Yasuaki Ishimatsu
2012-07-13  5:11         ` Yasuaki Ishimatsu
2012-07-13  5:11           ` Yasuaki Ishimatsu
2012-07-13  5:11           ` Yasuaki Ishimatsu
2012-07-09 10:26 ` [RFC PATCH v3 4/13] memory-hotplug : remove /sys/firmware/memmap/X sysfs Yasuaki Ishimatsu
2012-07-09 10:26   ` Yasuaki Ishimatsu
2012-07-09 10:26   ` Yasuaki Ishimatsu
2012-07-09 10:26   ` Yasuaki Ishimatsu
2012-07-13  9:10   ` Wen Congyang
2012-07-13  9:10     ` Wen Congyang
2012-07-13  9:10     ` Wen Congyang
2012-07-17  0:28     ` Yasuaki Ishimatsu
2012-07-17  0:28       ` Yasuaki Ishimatsu
2012-07-17  0:28       ` Yasuaki Ishimatsu
2012-07-16  2:32   ` Wen Congyang
2012-07-16  2:32     ` Wen Congyang
2012-07-16  2:32     ` Wen Congyang
2012-07-17  0:30     ` Yasuaki Ishimatsu
2012-07-17  0:30       ` Yasuaki Ishimatsu
2012-07-17  0:30       ` Yasuaki Ishimatsu
2012-07-17  0:30       ` Yasuaki Ishimatsu
2012-07-09 10:26 ` [RFC PATCH v3 5/13] memory-hotplug : does not release memory region in PAGES_PER_SECTION chunks Yasuaki Ishimatsu
2012-07-09 10:26   ` Yasuaki Ishimatsu
2012-07-09 10:26   ` Yasuaki Ishimatsu
2012-07-09 10:26   ` Yasuaki Ishimatsu
2012-07-13  3:42   ` Wen Congyang
2012-07-13  3:42     ` Wen Congyang
2012-07-13  3:42     ` Wen Congyang
2012-07-09 10:27 ` [RFC PATCH v3 6/13] memory-hotplug : add memory_block_release Yasuaki Ishimatsu
2012-07-09 10:27   ` Yasuaki Ishimatsu
2012-07-09 10:27   ` Yasuaki Ishimatsu
2012-07-09 10:27   ` Yasuaki Ishimatsu
2012-07-09 10:28 ` [RFC PATCH v3 7/13] memory-hotplug : remove_memory calls __remove_pages Yasuaki Ishimatsu
2012-07-09 10:28   ` Yasuaki Ishimatsu
2012-07-09 10:28   ` Yasuaki Ishimatsu
2012-07-09 10:28   ` Yasuaki Ishimatsu
2012-07-09 10:29 ` [RFC PATCH v3 8/13] memory-hotplug : check page type in get_page_bootmem Yasuaki Ishimatsu
2012-07-09 10:29   ` Yasuaki Ishimatsu
2012-07-09 10:29   ` Yasuaki Ishimatsu
2012-07-09 10:30 ` [RFC PATCH v3 9/13] memory-hotplug : move register_page_bootmem_info_node and put_page_bootmem for sparse-vmemmap Yasuaki Ishimatsu
2012-07-09 10:30   ` Yasuaki Ishimatsu
2012-07-09 10:30   ` Yasuaki Ishimatsu
2012-07-09 10:32 ` [RFC PATCH v3 10/13] memory-hotplug : implement register_page_bootmem_info_section of sparse-vmemmap Yasuaki Ishimatsu
2012-07-09 10:32   ` Yasuaki Ishimatsu
2012-07-09 10:32   ` Yasuaki Ishimatsu
2012-07-09 10:32   ` Yasuaki Ishimatsu
2012-07-09 10:33 ` [RFC PATCH v3 11/13] memory-hotplug : free memmap " Yasuaki Ishimatsu
2012-07-09 10:33   ` Yasuaki Ishimatsu
2012-07-09 10:33   ` Yasuaki Ishimatsu
2012-07-09 10:33   ` Yasuaki Ishimatsu
2012-07-11  5:06   ` Wen Congyang
2012-07-11  5:06     ` Wen Congyang
2012-07-11  5:06     ` Wen Congyang
2012-07-11  5:52     ` Yasuaki Ishimatsu
2012-07-11  5:52       ` Yasuaki Ishimatsu
2012-07-11  5:52       ` Yasuaki Ishimatsu
2012-07-11  6:25       ` Wen Congyang
2012-07-11  6:25         ` Wen Congyang
2012-07-11  6:25         ` Wen Congyang
2012-07-11  6:48         ` Yasuaki Ishimatsu
2012-07-11  6:48           ` Yasuaki Ishimatsu
2012-07-11  6:48           ` Yasuaki Ishimatsu
2012-07-11  7:27           ` Wen Congyang
2012-07-11  7:27             ` Wen Congyang
2012-07-11  7:27             ` Wen Congyang
2012-07-09 10:34 ` [RFC PATCH v3 12/13] memory-hotplug : add node_device_release Yasuaki Ishimatsu
2012-07-09 10:34   ` Yasuaki Ishimatsu
2012-07-09 10:34   ` Yasuaki Ishimatsu
2012-07-09 10:34   ` Yasuaki Ishimatsu
2012-07-09 10:35 ` [RFC PATCH v3 13/13] memory-hotplug : remove sysfs file of node Yasuaki Ishimatsu
2012-07-09 10:35   ` Yasuaki Ishimatsu
2012-07-09 10:35   ` Yasuaki Ishimatsu
2012-07-09 10:35   ` Yasuaki Ishimatsu
2012-07-09 15:18 ` [RFC PATCH v3 0/13] memory-hotplug : hot-remove physical memory Christoph Lameter
2012-07-09 15:18   ` Christoph Lameter
2012-07-09 15:18   ` Christoph Lameter
2012-07-10  9:58   ` Yasuaki Ishimatsu
2012-07-10  9:58     ` Yasuaki Ishimatsu
2012-07-10  9:58     ` Yasuaki Ishimatsu
2012-07-10  9:58     ` Yasuaki Ishimatsu
2012-07-10 16:50     ` Jiang Liu
2012-07-10 16:50       ` Jiang Liu
2012-07-10 16:50       ` Jiang Liu
2012-07-11  0:09       ` Yasuaki Ishimatsu
2012-07-11  0:09         ` Yasuaki Ishimatsu
2012-07-11  0:09         ` Yasuaki Ishimatsu
2012-07-11  0:09         ` Yasuaki Ishimatsu
2012-07-11  0:21         ` Jiang Liu
2012-07-11  0:21           ` Jiang Liu
2012-07-11  0:21           ` Jiang Liu
2012-07-11  0:54           ` Yasuaki Ishimatsu
2012-07-11  0:54             ` Yasuaki Ishimatsu
2012-07-11  0:54             ` Yasuaki Ishimatsu
2012-07-11 14:24             ` Jiang Liu
2012-07-11 14:24               ` Jiang Liu
2012-07-11 14:24               ` Jiang Liu
2012-07-11  1:52 ` Wen Congyang
2012-07-11  1:52   ` Wen Congyang
2012-07-11  1:52   ` Wen Congyang
2012-07-11  2:24   ` Wen Congyang
2012-07-11  2:24     ` Wen Congyang
2012-07-11  2:24     ` Wen Congyang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FFD9C08.2070502@linux.vnet.ibm.com \
    --to=dave@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=cl@linux.com \
    --cc=isimatu.yasuaki@jp.fujitsu.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=len.brown@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=liuj97@gmail.com \
    --cc=minchan.kim@gmail.com \
    --cc=paulus@samba.org \
    --cc=rientjes@google.com \
    --cc=wency@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.