All of lore.kernel.org
 help / color / mirror / Atom feed
* memblock_is_region_memory() vs. memblock_is_region_reserved()
@ 2012-05-15  1:11 Stephen Boyd
  2012-05-15 15:15 ` Tejun Heo
  0 siblings, 1 reply; 5+ messages in thread
From: Stephen Boyd @ 2012-05-15  1:11 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Linux Kernel Mailing List, Yinghai Lu, Benjamin Herrenschmidt

I was looking at memblock and noticed that memblock_is_region_memory()
is implemented differently than memblock_is_region_reserved().

memblock_is_region_memory() returns true only if the region you are
testing is fully contained within the bounds of a memory block. If the
region is half in and half out of a memory region (i.e. overlaps memory
and a hole) it is not memory and returns false.

memblock_is_region_reserved() is the opposite. If the region are you are
testing is half in and half out of a reserved region (i.e. overlaps
reserved memory) it returns true, otherwise it returns false.

These functions sound like they do the same thing by testing to see if
what you specify is either memory or reserved memory, but the semantics
are a bit different in that the former doesn't allow overlap, and the
latter accepts overlap. Should we rename memblock_is_region_reserved()
to memblock_overlaps_reserved() to make it clearer what the intention
is? Or perhaps rename memblock_is_region_memory() to
memblock_is_region_within_memory()?

If anything, perhaps this patch will help clarify things?

diff --git a/mm/memblock.c b/mm/memblock.c
index a44eab3..f097e79 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -855,6 +855,16 @@ int __init_memblock memblock_is_memory(phys_addr_t addr)
 	return memblock_search(&memblock.memory, addr) != -1;
 }
 
+/**
+ * memblock_is_region_memory - check if a region is a subset of memory
+ * @base: base of region to check
+ * @size: size of region to check
+ *
+ * Check if the region [@base, @base+@size) is a subset of a memory block.
+ *
+ * RETURNS:
+ * 0 if false, non-zero if true
+ */
 int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size)
 {
 	int idx = memblock_search(&memblock.memory, base);
@@ -867,6 +877,16 @@ int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size
 		 memblock.memory.regions[idx].size) >= end;
 }
 
+/**
+ * memblock_is_region_reserved - check if a region intersects reserved memory
+ * @base: base of region to check
+ * @size: size of region to check
+ *
+ * Check if the region [@base, @base+@size) intersects a reserved memory block.
+ *
+ * RETURNS:
+ * 0 if false, non-zero if true
+ */
 int __init_memblock memblock_is_region_reserved(phys_addr_t base, phys_addr_t size)
 {
 	memblock_cap_size(base, &size);


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: memblock_is_region_memory() vs. memblock_is_region_reserved()
  2012-05-15  1:11 memblock_is_region_memory() vs. memblock_is_region_reserved() Stephen Boyd
@ 2012-05-15 15:15 ` Tejun Heo
  2012-05-24  7:45   ` [PATCH] memblock: Document memblock_is_region_{memory,reserved}() Stephen Boyd
  0 siblings, 1 reply; 5+ messages in thread
From: Tejun Heo @ 2012-05-15 15:15 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Linux Kernel Mailing List, Yinghai Lu, Benjamin Herrenschmidt

On Mon, May 14, 2012 at 06:11:20PM -0700, Stephen Boyd wrote:
> I was looking at memblock and noticed that memblock_is_region_memory()
> is implemented differently than memblock_is_region_reserved().
> 
> memblock_is_region_memory() returns true only if the region you are
> testing is fully contained within the bounds of a memory block. If the
> region is half in and half out of a memory region (i.e. overlaps memory
> and a hole) it is not memory and returns false.
> 
> memblock_is_region_reserved() is the opposite. If the region are you are
> testing is half in and half out of a reserved region (i.e. overlaps
> reserved memory) it returns true, otherwise it returns false.
> 
> These functions sound like they do the same thing by testing to see if
> what you specify is either memory or reserved memory, but the semantics
> are a bit different in that the former doesn't allow overlap, and the
> latter accepts overlap. Should we rename memblock_is_region_reserved()
> to memblock_overlaps_reserved() to make it clearer what the intention
> is? Or perhaps rename memblock_is_region_memory() to
> memblock_is_region_within_memory()?

The difference is there because for a memory region to be available,
all of it should be while a memory region partially reserved should
still be treated as reserved.

> If anything, perhaps this patch will help clarify things?

Yeah, looks good to me.  Can you please add patch description,
sign-off and send it to Ingo?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] memblock: Document memblock_is_region_{memory,reserved}()
  2012-05-15 15:15 ` Tejun Heo
@ 2012-05-24  7:45   ` Stephen Boyd
  2012-06-07 17:23     ` Stephen Boyd
  2012-06-08 14:54     ` [tip:core/urgent] memblock: Document memblock_is_region_{memory, reserved}() tip-bot for Stephen Boyd
  0 siblings, 2 replies; 5+ messages in thread
From: Stephen Boyd @ 2012-05-24  7:45 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Tejun Heo

At first glance one would think that memblock_is_region_memory()
and memblock_is_region_reserved() would be implemented in the
same way. Unfortunately they aren't and the former returns
whether the region specified is a subset of a memory bank while
the latter returns whether the region specified intersects with
reserved memory.

Document the two functions so that users aren't tempted to
make the implementation the same between them and to clarify the
purpose of the functions.

Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 mm/memblock.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/mm/memblock.c b/mm/memblock.c
index a44eab3..e35e39b 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -855,6 +855,16 @@ int __init_memblock memblock_is_memory(phys_addr_t addr)
 	return memblock_search(&memblock.memory, addr) != -1;
 }
 
+/**
+ * memblock_is_region_memory - check if a region is a subset of memory
+ * @base: base of region to check
+ * @size: size of region to check
+ *
+ * Check if the region [@base, @base+@size) is a subset of a memory block.
+ *
+ * RETURNS:
+ * 0 if false, non-zero if true
+ */
 int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size)
 {
 	int idx = memblock_search(&memblock.memory, base);
@@ -867,6 +877,16 @@ int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size
 		 memblock.memory.regions[idx].size) >= end;
 }
 
+/**
+ * memblock_is_region_reserved - check if a region intersects reserved memory
+ * @base: base of region to check
+ * @size: size of region to check
+ *
+ * Check if the region [@base, @base+@size) intersects a reserved memory block.
+ *
+ * RETURNS:
+ * 0 if false, non-zero if true
+ */
 int __init_memblock memblock_is_region_reserved(phys_addr_t base, phys_addr_t size)
 {
 	memblock_cap_size(base, &size);
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] memblock: Document memblock_is_region_{memory,reserved}()
  2012-05-24  7:45   ` [PATCH] memblock: Document memblock_is_region_{memory,reserved}() Stephen Boyd
@ 2012-06-07 17:23     ` Stephen Boyd
  2012-06-08 14:54     ` [tip:core/urgent] memblock: Document memblock_is_region_{memory, reserved}() tip-bot for Stephen Boyd
  1 sibling, 0 replies; 5+ messages in thread
From: Stephen Boyd @ 2012-06-07 17:23 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Tejun Heo

On 05/24/12 00:45, Stephen Boyd wrote:
> At first glance one would think that memblock_is_region_memory()
> and memblock_is_region_reserved() would be implemented in the
> same way. Unfortunately they aren't and the former returns
> whether the region specified is a subset of a memory bank while
> the latter returns whether the region specified intersects with
> reserved memory.
>
> Document the two functions so that users aren't tempted to
> make the implementation the same between them and to clarify the
> purpose of the functions.
>
> Cc: Tejun Heo <tj@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---

Hi Ingo, can you pick up or review this patch (I realize it was sent
during the merge window)?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip:core/urgent] memblock: Document memblock_is_region_{memory, reserved}()
  2012-05-24  7:45   ` [PATCH] memblock: Document memblock_is_region_{memory,reserved}() Stephen Boyd
  2012-06-07 17:23     ` Stephen Boyd
@ 2012-06-08 14:54     ` tip-bot for Stephen Boyd
  1 sibling, 0 replies; 5+ messages in thread
From: tip-bot for Stephen Boyd @ 2012-06-08 14:54 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, tj, tglx, sboyd

Commit-ID:  eab309494ae2b9e15f85520f00de3893162c2e43
Gitweb:     http://git.kernel.org/tip/eab309494ae2b9e15f85520f00de3893162c2e43
Author:     Stephen Boyd <sboyd@codeaurora.org>
AuthorDate: Thu, 24 May 2012 00:45:21 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 8 Jun 2012 11:59:46 +0200

memblock: Document memblock_is_region_{memory,reserved}()

At first glance one would think that memblock_is_region_memory()
and memblock_is_region_reserved() would be implemented in the
same way. Unfortunately they aren't and the former returns
whether the region specified is a subset of a memory bank while
the latter returns whether the region specified intersects with
reserved memory.

Document the two functions so that users aren't tempted to
make the implementation the same between them and to clarify the
purpose of the functions.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Tejun Heo <tj@kernel.org>
Link: http://lkml.kernel.org/r/1337845521-32755-1-git-send-email-sboyd@codeaurora.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 mm/memblock.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 952123e..32a0a5e 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -867,6 +867,16 @@ int __init_memblock memblock_is_memory(phys_addr_t addr)
 	return memblock_search(&memblock.memory, addr) != -1;
 }
 
+/**
+ * memblock_is_region_memory - check if a region is a subset of memory
+ * @base: base of region to check
+ * @size: size of region to check
+ *
+ * Check if the region [@base, @base+@size) is a subset of a memory block.
+ *
+ * RETURNS:
+ * 0 if false, non-zero if true
+ */
 int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size)
 {
 	int idx = memblock_search(&memblock.memory, base);
@@ -879,6 +889,16 @@ int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size
 		 memblock.memory.regions[idx].size) >= end;
 }
 
+/**
+ * memblock_is_region_reserved - check if a region intersects reserved memory
+ * @base: base of region to check
+ * @size: size of region to check
+ *
+ * Check if the region [@base, @base+@size) intersects a reserved memory block.
+ *
+ * RETURNS:
+ * 0 if false, non-zero if true
+ */
 int __init_memblock memblock_is_region_reserved(phys_addr_t base, phys_addr_t size)
 {
 	memblock_cap_size(base, &size);

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-06-08 14:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-15  1:11 memblock_is_region_memory() vs. memblock_is_region_reserved() Stephen Boyd
2012-05-15 15:15 ` Tejun Heo
2012-05-24  7:45   ` [PATCH] memblock: Document memblock_is_region_{memory,reserved}() Stephen Boyd
2012-06-07 17:23     ` Stephen Boyd
2012-06-08 14:54     ` [tip:core/urgent] memblock: Document memblock_is_region_{memory, reserved}() tip-bot for Stephen Boyd

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.