Messages in this thread | | | Date | Tue, 14 Oct 2008 10:09:53 -0700 | From | "Yinghai Lu" <> | Subject | Re: [patch]x86: arch_add_memory round up address |
| |
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
| |