* [patch]x86: arch_add_memory round up address
@ 2008-10-09 5:31 Shaohua Li
2008-10-09 6:04 ` Yinghai Lu
0 siblings, 1 reply; 10+ messages in thread
From: Shaohua Li @ 2008-10-09 5:31 UTC (permalink / raw)
To: lkml; +Cc: Andrew Morton, Ingo Molnar
Round up address to a page, otherwise the last page isn't mapped.
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
arch/x86/mm/init_64.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux/arch/x86/mm/init_64.c
===================================================================
--- linux.orig/arch/x86/mm/init_64.c 2008-10-09 11:42:33.000000000 +0800
+++ linux/arch/x86/mm/init_64.c 2008-10-09 11:43:22.000000000 +0800
@@ -721,7 +721,8 @@ int arch_add_memory(int nid, u64 start,
unsigned long nr_pages = size >> PAGE_SHIFT;
int ret;
- last_mapped_pfn = init_memory_mapping(start, start + size-1);
+ last_mapped_pfn = init_memory_mapping(start,
+ round_up(start + size-1, PAGE_SIZE));
if (last_mapped_pfn > max_pfn_mapped)
max_pfn_mapped = last_mapped_pfn;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]x86: arch_add_memory round up address
2008-10-09 5:31 [patch]x86: arch_add_memory round up address Shaohua Li
@ 2008-10-09 6:04 ` Yinghai Lu
2008-10-09 6:08 ` Shaohua Li
0 siblings, 1 reply; 10+ messages in thread
From: Yinghai Lu @ 2008-10-09 6:04 UTC (permalink / raw)
To: Shaohua Li; +Cc: lkml, Andrew Morton, Ingo Molnar
On Wed, Oct 8, 2008 at 10:31 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> Round up address to a page, otherwise the last page isn't mapped.
>
> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> ---
> arch/x86/mm/init_64.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux/arch/x86/mm/init_64.c
> ===================================================================
> --- linux.orig/arch/x86/mm/init_64.c 2008-10-09 11:42:33.000000000 +0800
> +++ linux/arch/x86/mm/init_64.c 2008-10-09 11:43:22.000000000 +0800
> @@ -721,7 +721,8 @@ int arch_add_memory(int nid, u64 start,
> unsigned long nr_pages = size >> PAGE_SHIFT;
> int ret;
>
> - last_mapped_pfn = init_memory_mapping(start, start + size-1);
> + last_mapped_pfn = init_memory_mapping(start,
> + round_up(start + size-1, PAGE_SIZE));
> if (last_mapped_pfn > max_pfn_mapped)
> max_pfn_mapped = last_mapped_pfn;
should use
last_mapped_pfn = init_memory_mapping(start, start + size);
YH
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]x86: arch_add_memory round up address
2008-10-09 6:04 ` Yinghai Lu
@ 2008-10-09 6:08 ` Shaohua Li
2008-10-09 6:22 ` Yinghai Lu
0 siblings, 1 reply; 10+ messages in thread
From: Shaohua Li @ 2008-10-09 6:08 UTC (permalink / raw)
To: Yinghai Lu; +Cc: lkml, Andrew Morton, Ingo Molnar
On Thu, 2008-10-09 at 14:04 +0800, Yinghai Lu wrote:
> On Wed, Oct 8, 2008 at 10:31 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> > Round up address to a page, otherwise the last page isn't mapped.
> >
> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > ---
> > arch/x86/mm/init_64.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Index: linux/arch/x86/mm/init_64.c
> > ===================================================================
> > --- linux.orig/arch/x86/mm/init_64.c 2008-10-09 11:42:33.000000000 +0800
> > +++ linux/arch/x86/mm/init_64.c 2008-10-09 11:43:22.000000000 +0800
> > @@ -721,7 +721,8 @@ int arch_add_memory(int nid, u64 start,
> > unsigned long nr_pages = size >> PAGE_SHIFT;
> > int ret;
> >
> > - last_mapped_pfn = init_memory_mapping(start, start + size-1);
> > + last_mapped_pfn = init_memory_mapping(start,
> > + round_up(start + size-1, PAGE_SIZE));
> > if (last_mapped_pfn > max_pfn_mapped)
> > max_pfn_mapped = last_mapped_pfn;
>
> should use
>
> last_mapped_pfn = init_memory_mapping(start, start + size);
No, this still can't guarantee page aligned, though this works in my
test
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]x86: arch_add_memory round up address
2008-10-09 6:08 ` Shaohua Li
@ 2008-10-09 6:22 ` Yinghai Lu
2008-10-09 6:28 ` Shaohua Li
0 siblings, 1 reply; 10+ messages in thread
From: Yinghai Lu @ 2008-10-09 6:22 UTC (permalink / raw)
To: Shaohua Li; +Cc: lkml, Andrew Morton, Ingo Molnar
On Wed, Oct 8, 2008 at 11:08 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> On Thu, 2008-10-09 at 14:04 +0800, Yinghai Lu wrote:
>> On Wed, Oct 8, 2008 at 10:31 PM, Shaohua Li <shaohua.li@intel.com> wrote:
>> > Round up address to a page, otherwise the last page isn't mapped.
>> >
>> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
>> > ---
>> > arch/x86/mm/init_64.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > Index: linux/arch/x86/mm/init_64.c
>> > ===================================================================
>> > --- linux.orig/arch/x86/mm/init_64.c 2008-10-09 11:42:33.000000000 +0800
>> > +++ linux/arch/x86/mm/init_64.c 2008-10-09 11:43:22.000000000 +0800
>> > @@ -721,7 +721,8 @@ int arch_add_memory(int nid, u64 start,
>> > unsigned long nr_pages = size >> PAGE_SHIFT;
>> > int ret;
>> >
>> > - last_mapped_pfn = init_memory_mapping(start, start + size-1);
>> > + last_mapped_pfn = init_memory_mapping(start,
>> > + round_up(start + size-1, PAGE_SIZE));
>> > if (last_mapped_pfn > max_pfn_mapped)
>> > max_pfn_mapped = last_mapped_pfn;
>>
>> should use
>>
>> last_mapped_pfn = init_memory_mapping(start, start + size);
> No, this still can't guarantee page aligned, though this works in my
> test
who will call arch_add_memory? that should be start and size already
be page aligned.
YH
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]x86: arch_add_memory round up address
2008-10-09 6:22 ` Yinghai Lu
@ 2008-10-09 6:28 ` Shaohua Li
2008-10-09 6:40 ` Yinghai Lu
0 siblings, 1 reply; 10+ messages in thread
From: Shaohua Li @ 2008-10-09 6:28 UTC (permalink / raw)
To: Yinghai Lu; +Cc: lkml, Andrew Morton, Ingo Molnar
On Thu, Oct 09, 2008 at 02:22:50PM +0800, Yinghai Lu wrote:
> On Wed, Oct 8, 2008 at 11:08 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> > On Thu, 2008-10-09 at 14:04 +0800, Yinghai Lu wrote:
> >> On Wed, Oct 8, 2008 at 10:31 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> >> > Round up address to a page, otherwise the last page isn't mapped.
> >> >
> >> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> >> > ---
> >> > arch/x86/mm/init_64.c | 3 ++-
> >> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >> >
> >> > Index: linux/arch/x86/mm/init_64.c
> >> > ===================================================================
> >> > --- linux.orig/arch/x86/mm/init_64.c 2008-10-09 11:42:33.000000000 +0800
> >> > +++ linux/arch/x86/mm/init_64.c 2008-10-09 11:43:22.000000000 +0800
> >> > @@ -721,7 +721,8 @@ int arch_add_memory(int nid, u64 start,
> >> > unsigned long nr_pages = size >> PAGE_SHIFT;
> >> > int ret;
> >> >
> >> > - last_mapped_pfn = init_memory_mapping(start, start + size-1);
> >> > + last_mapped_pfn = init_memory_mapping(start,
> >> > + round_up(start + size-1, PAGE_SIZE));
> >> > if (last_mapped_pfn > max_pfn_mapped)
> >> > max_pfn_mapped = last_mapped_pfn;
> >>
> >> should use
> >>
> >> last_mapped_pfn = init_memory_mapping(start, start + size);
> > No, this still can't guarantee page aligned, though this works in my
> > test
>
> who will call arch_add_memory? that should be start and size already
> be page aligned.
It's memory hotplug. Doing a round up is always ok and safe even it might be already aligned.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]x86: arch_add_memory round up address
2008-10-09 6:28 ` Shaohua Li
@ 2008-10-09 6:40 ` Yinghai Lu
2008-10-09 7:06 ` Shaohua Li
0 siblings, 1 reply; 10+ messages in thread
From: Yinghai Lu @ 2008-10-09 6:40 UTC (permalink / raw)
To: Shaohua Li; +Cc: lkml, Andrew Morton, Ingo Molnar
On Wed, Oct 8, 2008 at 11:28 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> On Thu, Oct 09, 2008 at 02:22:50PM +0800, Yinghai Lu wrote:
>> On Wed, Oct 8, 2008 at 11:08 PM, Shaohua Li <shaohua.li@intel.com> wrote:
>> > On Thu, 2008-10-09 at 14:04 +0800, Yinghai Lu wrote:
>> >> On Wed, Oct 8, 2008 at 10:31 PM, Shaohua Li <shaohua.li@intel.com> wrote:
>> >> > Round up address to a page, otherwise the last page isn't mapped.
>> >> >
>> >> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
>> >> > ---
>> >> > arch/x86/mm/init_64.c | 3 ++-
>> >> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >> >
>> >> > Index: linux/arch/x86/mm/init_64.c
>> >> > ===================================================================
>> >> > --- linux.orig/arch/x86/mm/init_64.c 2008-10-09 11:42:33.000000000 +0800
>> >> > +++ linux/arch/x86/mm/init_64.c 2008-10-09 11:43:22.000000000 +0800
>> >> > @@ -721,7 +721,8 @@ int arch_add_memory(int nid, u64 start,
>> >> > unsigned long nr_pages = size >> PAGE_SHIFT;
>> >> > int ret;
>> >> >
>> >> > - last_mapped_pfn = init_memory_mapping(start, start + size-1);
>> >> > + last_mapped_pfn = init_memory_mapping(start,
>> >> > + round_up(start + size-1, PAGE_SIZE));
>> >> > if (last_mapped_pfn > max_pfn_mapped)
>> >> > max_pfn_mapped = last_mapped_pfn;
>> >>
>> >> should use
>> >>
>> >> last_mapped_pfn = init_memory_mapping(start, start + size);
>> > No, this still can't guarantee page aligned, though this works in my
>> > test
>>
>> who will call arch_add_memory? that should be start and size already
>> be page aligned.
> It's memory hotplug. Doing a round up is always ok and safe even it might be already aligned.
>
it seems rounding up in that case is wrong...
if the caller call that funtion with extra half page, you don't need
map that half page, because you can not use it.
YH
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]x86: arch_add_memory round up address
2008-10-09 6:40 ` Yinghai Lu
@ 2008-10-09 7:06 ` Shaohua Li
2008-10-14 5:16 ` Yasunori Goto
0 siblings, 1 reply; 10+ messages in thread
From: Shaohua Li @ 2008-10-09 7:06 UTC (permalink / raw)
To: Yinghai Lu; +Cc: lkml, Andrew Morton, Ingo Molnar
On Thu, Oct 09, 2008 at 02:40:34PM +0800, Yinghai Lu wrote:
> On Wed, Oct 8, 2008 at 11:28 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> > On Thu, Oct 09, 2008 at 02:22:50PM +0800, Yinghai Lu wrote:
> >> On Wed, Oct 8, 2008 at 11:08 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> >> > On Thu, 2008-10-09 at 14:04 +0800, Yinghai Lu wrote:
> >> >> On Wed, Oct 8, 2008 at 10:31 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> >> >> > Round up address to a page, otherwise the last page isn't mapped.
> >> >> >
> >> >> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> >> >> > ---
> >> >> > arch/x86/mm/init_64.c | 3 ++-
> >> >> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > Index: linux/arch/x86/mm/init_64.c
> >> >> > ===================================================================
> >> >> > --- linux.orig/arch/x86/mm/init_64.c 2008-10-09 11:42:33.000000000 +0800
> >> >> > +++ linux/arch/x86/mm/init_64.c 2008-10-09 11:43:22.000000000 +0800
> >> >> > @@ -721,7 +721,8 @@ int arch_add_memory(int nid, u64 start,
> >> >> > unsigned long nr_pages = size >> PAGE_SHIFT;
> >> >> > int ret;
> >> >> >
> >> >> > - last_mapped_pfn = init_memory_mapping(start, start + size-1);
> >> >> > + last_mapped_pfn = init_memory_mapping(start,
> >> >> > + round_up(start + size-1, PAGE_SIZE));
> >> >> > if (last_mapped_pfn > max_pfn_mapped)
> >> >> > max_pfn_mapped = last_mapped_pfn;
> >> >>
> >> >> should use
> >> >>
> >> >> last_mapped_pfn = init_memory_mapping(start, start + size);
> >> > No, this still can't guarantee page aligned, though this works in my
> >> > test
> >>
> >> who will call arch_add_memory? that should be start and size already
> >> be page aligned.
> > It's memory hotplug. Doing a round up is always ok and safe even it might be already aligned.
> >
>
> it seems rounding up in that case is wrong...
>
> if the caller call that funtion with extra half page, you don't need
> map that half page, because you can not use it.
shouldn't we mark such page as reserved and so it will not be used? This is the way we handle hole.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]x86: arch_add_memory round up address
2008-10-09 7:06 ` Shaohua Li
@ 2008-10-14 5:16 ` Yasunori Goto
2008-10-14 7:30 ` Shaohua Li
0 siblings, 1 reply; 10+ messages in thread
From: Yasunori Goto @ 2008-10-14 5:16 UTC (permalink / raw)
To: Shaohua Li; +Cc: Yinghai Lu, lkml, Andrew Morton, Ingo Molnar
> On Thu, Oct 09, 2008 at 02:40:34PM +0800, Yinghai Lu wrote:
> > On Wed, Oct 8, 2008 at 11:28 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> > > On Thu, Oct 09, 2008 at 02:22:50PM +0800, Yinghai Lu wrote:
> > >> On Wed, Oct 8, 2008 at 11:08 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> > >> > On Thu, 2008-10-09 at 14:04 +0800, Yinghai Lu wrote:
> > >> >> On Wed, Oct 8, 2008 at 10:31 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> > >> >> > Round up address to a page, otherwise the last page isn't mapped.
> > >> >> >
> > >> >> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > >> >> > ---
> > >> >> > arch/x86/mm/init_64.c | 3 ++-
> > >> >> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >> >> >
> > >> >> > Index: linux/arch/x86/mm/init_64.c
> > >> >> > ===================================================================
> > >> >> > --- linux.orig/arch/x86/mm/init_64.c 2008-10-09 11:42:33.000000000 +0800
> > >> >> > +++ linux/arch/x86/mm/init_64.c 2008-10-09 11:43:22.000000000 +0800
> > >> >> > @@ -721,7 +721,8 @@ int arch_add_memory(int nid, u64 start,
> > >> >> > unsigned long nr_pages = size >> PAGE_SHIFT;
> > >> >> > int ret;
> > >> >> >
> > >> >> > - last_mapped_pfn = init_memory_mapping(start, start + size-1);
> > >> >> > + last_mapped_pfn = init_memory_mapping(start,
> > >> >> > + round_up(start + size-1, PAGE_SIZE));
> > >> >> > if (last_mapped_pfn > max_pfn_mapped)
> > >> >> > max_pfn_mapped = last_mapped_pfn;
> > >> >>
> > >> >> should use
> > >> >>
> > >> >> last_mapped_pfn = init_memory_mapping(start, start + size);
> > >> > No, this still can't guarantee page aligned, though this works in my
> > >> > test
> > >>
> > >> who will call arch_add_memory? that should be start and size already
> > >> be page aligned.
> > > It's memory hotplug. Doing a round up is always ok and safe even it might be already aligned.
> > >
> >
> > it seems rounding up in that case is wrong...
> >
> > if the caller call that funtion with extra half page, you don't need
> > map that half page, because you can not use it.
> shouldn't we mark such page as reserved and so it will not be used? This is the way we handle hole.
Just curious,
have you ever seen a real machine which needs this patch?
If this patch is needed, there is the firmware which specifies
half page size for new memory's size...
Bye.
--
Yasunori Goto
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]x86: arch_add_memory round up address
2008-10-14 5:16 ` Yasunori Goto
@ 2008-10-14 7:30 ` Shaohua Li
2008-10-14 17:09 ` Yinghai Lu
0 siblings, 1 reply; 10+ messages in thread
From: Shaohua Li @ 2008-10-14 7:30 UTC (permalink / raw)
To: Yasunori Goto; +Cc: Yinghai Lu, lkml, Andrew Morton, Ingo Molnar
On Tue, Oct 14, 2008 at 01:16:46PM +0800, Yasunori Goto wrote:
> > On Thu, Oct 09, 2008 at 02:40:34PM +0800, Yinghai Lu wrote:
> > > On Wed, Oct 8, 2008 at 11:28 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> > > > On Thu, Oct 09, 2008 at 02:22:50PM +0800, Yinghai Lu wrote:
> > > >> On Wed, Oct 8, 2008 at 11:08 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> > > >> > On Thu, 2008-10-09 at 14:04 +0800, Yinghai Lu wrote:
> > > >> >> On Wed, Oct 8, 2008 at 10:31 PM, Shaohua Li <shaohua.li@intel.com> wrote:
> > > >> >> > Round up address to a page, otherwise the last page isn't mapped.
> > > >> >> >
> > > >> >> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > > >> >> > ---
> > > >> >> > arch/x86/mm/init_64.c | 3 ++-
> > > >> >> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >> >> >
> > > >> >> > Index: linux/arch/x86/mm/init_64.c
> > > >> >> > ===================================================================
> > > >> >> > --- linux.orig/arch/x86/mm/init_64.c 2008-10-09 11:42:33.000000000 +0800
> > > >> >> > +++ linux/arch/x86/mm/init_64.c 2008-10-09 11:43:22.000000000 +0800
> > > >> >> > @@ -721,7 +721,8 @@ int arch_add_memory(int nid, u64 start,
> > > >> >> > unsigned long nr_pages = size >> PAGE_SHIFT;
> > > >> >> > int ret;
> > > >> >> >
> > > >> >> > - last_mapped_pfn = init_memory_mapping(start, start + size-1);
> > > >> >> > + last_mapped_pfn = init_memory_mapping(start,
> > > >> >> > + round_up(start + size-1, PAGE_SIZE));
> > > >> >> > if (last_mapped_pfn > max_pfn_mapped)
> > > >> >> > max_pfn_mapped = last_mapped_pfn;
> > > >> >>
> > > >> >> should use
> > > >> >>
> > > >> >> last_mapped_pfn = init_memory_mapping(start, start + size);
> > > >> > No, this still can't guarantee page aligned, though this works in my
> > > >> > test
> > > >>
> > > >> who will call arch_add_memory? that should be start and size already
> > > >> be page aligned.
> > > > It's memory hotplug. Doing a round up is always ok and safe even it might be already aligned.
> > > >
> > >
> > > it seems rounding up in that case is wrong...
> > >
> > > if the caller call that funtion with extra half page, you don't need
> > > map that half page, because you can not use it.
> > shouldn't we mark such page as reserved and so it will not be used? This is the way we handle hole.
>
> Just curious,
> have you ever seen a real machine which needs this patch?
No, I just did some experiments on a desktop for memory hotplug and this bug
triggered a crash in my test.
Yinghai's suggestion also fixed the bug. I just want to have safer method. Anyway, either approach is ok to me.
Thanks,
Shaohua
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]x86: arch_add_memory round up address
2008-10-14 7:30 ` Shaohua Li
@ 2008-10-14 17:09 ` Yinghai Lu
0 siblings, 0 replies; 10+ messages in thread
From: Yinghai Lu @ 2008-10-14 17:09 UTC (permalink / raw)
To: Shaohua Li; +Cc: Yasunori Goto, lkml, Andrew Morton, Ingo Molnar
On Tue, Oct 14, 2008 at 12:30 AM, Shaohua Li <shaohua.li@intel.com> wrote:
> On Tue, Oct 14, 2008 at 01:16:46PM +0800, Yasunori Goto wrote:
>> > On Thu, Oct 09, 2008 at 02:40:34PM +0800, Yinghai Lu wrote:
>> > > On Wed, Oct 8, 2008 at 11:28 PM, Shaohua Li <shaohua.li@intel.com> wrote:
>> > > > On Thu, Oct 09, 2008 at 02:22:50PM +0800, Yinghai Lu wrote:
>> > > >> On Wed, Oct 8, 2008 at 11:08 PM, Shaohua Li <shaohua.li@intel.com> wrote:
>> > > >> > On Thu, 2008-10-09 at 14:04 +0800, Yinghai Lu wrote:
>> > > >> >> On Wed, Oct 8, 2008 at 10:31 PM, Shaohua Li <shaohua.li@intel.com> wrote:
>> > > >> >> > Round up address to a page, otherwise the last page isn't mapped.
>> > > >> >> >
>> > > >> >> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
>> > > >> >> > ---
>> > > >> >> > arch/x86/mm/init_64.c | 3 ++-
>> > > >> >> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> > > >> >> >
>> > > >> >> > Index: linux/arch/x86/mm/init_64.c
>> > > >> >> > ===================================================================
>> > > >> >> > --- linux.orig/arch/x86/mm/init_64.c 2008-10-09 11:42:33.000000000 +0800
>> > > >> >> > +++ linux/arch/x86/mm/init_64.c 2008-10-09 11:43:22.000000000 +0800
>> > > >> >> > @@ -721,7 +721,8 @@ int arch_add_memory(int nid, u64 start,
>> > > >> >> > unsigned long nr_pages = size >> PAGE_SHIFT;
>> > > >> >> > int ret;
>> > > >> >> >
>> > > >> >> > - last_mapped_pfn = init_memory_mapping(start, start + size-1);
>> > > >> >> > + last_mapped_pfn = init_memory_mapping(start,
>> > > >> >> > + round_up(start + size-1, PAGE_SIZE));
>> > > >> >> > if (last_mapped_pfn > max_pfn_mapped)
>> > > >> >> > max_pfn_mapped = last_mapped_pfn;
>> > > >> >>
>> > > >> >> should use
>> > > >> >>
>> > > >> >> last_mapped_pfn = init_memory_mapping(start, start + size);
>> > > >> > No, this still can't guarantee page aligned, though this works in my
>> > > >> > test
>> > > >>
>> > > >> who will call arch_add_memory? that should be start and size already
>> > > >> be page aligned.
>> > > > It's memory hotplug. Doing a round up is always ok and safe even it might be already aligned.
>> > > >
>> > >
>> > > it seems rounding up in that case is wrong...
>> > >
>> > > if the caller call that funtion with extra half page, you don't need
>> > > map that half page, because you can not use it.
>> > shouldn't we mark such page as reserved and so it will not be used? This is the way we handle hole.
>>
>> Just curious,
>> have you ever seen a real machine which needs this patch?
> No, I just did some experiments on a desktop for memory hotplug and this bug
> triggered a crash in my test.
> Yinghai's suggestion also fixed the bug. I just want to have safer method. Anyway, either approach is ok to me.
good, please resubmit updated one with
last_mapped_pfn = init_memory_mapping(start, start + size);
Thanks
YH
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2008-10-14 17:10 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-09 5:31 [patch]x86: arch_add_memory round up address Shaohua Li
2008-10-09 6:04 ` Yinghai Lu
2008-10-09 6:08 ` Shaohua Li
2008-10-09 6:22 ` Yinghai Lu
2008-10-09 6:28 ` Shaohua Li
2008-10-09 6:40 ` Yinghai Lu
2008-10-09 7:06 ` Shaohua Li
2008-10-14 5:16 ` Yasunori Goto
2008-10-14 7:30 ` Shaohua Li
2008-10-14 17:09 ` Yinghai Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®