| Message ID | 1348242975-19184-9-git-send-email-cyril@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 21, 2012 at 04:56:06PM +0100, Cyril Chemparathy wrote: > From: Vitaly Andrianov <vitalya@ti.com> > > The free_memmap() was mistakenly using unsigned long type to represent > physical addresses. This breaks on PAE systems where memory could be placed > above the 32-bit addressible limit. > > This patch fixes this function to properly use phys_addr_t instead. > > Signed-off-by: Vitaly Andrianov <vitalya@ti.com> > Signed-off-by: Cyril Chemparathy <cyril@ti.com> > Acked-by: Nicolas Pitre <nico@linaro.org> > --- > arch/arm/mm/init.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c > index 9aec41f..19ba70b 100644 > --- a/arch/arm/mm/init.c > +++ b/arch/arm/mm/init.c > @@ -457,7 +457,7 @@ static inline void > free_memmap(unsigned long start_pfn, unsigned long end_pfn) > { > struct page *start_pg, *end_pg; > - unsigned long pg, pgend; > + phys_addr_t pg, pgend; > > /* > * Convert start_pfn/end_pfn to a struct page pointer. > @@ -469,8 +469,8 @@ free_memmap(unsigned long start_pfn, unsigned long end_pfn) > * Convert to physical addresses, and > * round start upwards and end downwards. > */ > - pg = (unsigned long)PAGE_ALIGN(__pa(start_pg)); > - pgend = (unsigned long)__pa(end_pg) & PAGE_MASK; > + pg = PAGE_ALIGN(__pa(start_pg)); > + pgend = __pa(end_pg) & PAGE_MASK; > > /* > * If there are free pages between these, This function also calls free_bootmem() which takes unsigned long. Are patches sent separately for this or we just ignore holes in memmap? There are other calls to free_bootmem() or reserve_bootmem(), do they just work with the high phys addresses?
On Mon, Sep 24, 2012 at 02:29:42PM +0100, Catalin Marinas wrote: > This function also calls free_bootmem() which takes unsigned long. Are > patches sent separately for this or we just ignore holes in memmap? > There are other calls to free_bootmem() or reserve_bootmem(), do they > just work with the high phys addresses? Bootmem only deals with physical addresses which fit within the size of an 'unsigned long'. Unfortunately, the bootmem API is a mess of 'unsigned long' physical addresses and PFNs. Years ago there was a patch to make it use only PFNs but other changes resulted in that patch being thrown away.
On 9/24/2012 9:41 AM, Russell King - ARM Linux wrote: > On Mon, Sep 24, 2012 at 02:29:42PM +0100, Catalin Marinas wrote: >> This function also calls free_bootmem() which takes unsigned long. Are >> patches sent separately for this or we just ignore holes in memmap? >> There are other calls to free_bootmem() or reserve_bootmem(), do they >> just work with the high phys addresses? > > Bootmem only deals with physical addresses which fit within the size > of an 'unsigned long'. Unfortunately, the bootmem API is a mess of > 'unsigned long' physical addresses and PFNs. > > Years ago there was a patch to make it use only PFNs but other changes > resulted in that patch being thrown away. > A separate patch has been posted for bootmem (see [1]). Tejun suggested that we'd be better off moving entirely to memblock instead (see [2]). I'd be happy to take this up, but I'm not very familiar with the reasoning behind the mixed bootmem + memblock usage that we have today. Some background and pointers on this topic would greatly help. [1] https://lkml.org/lkml/2012/9/12/435 [2] https://lkml.org/lkml/2012/9/13/511
On Mon, Sep 24, 2012 at 11:09:50AM -0400, Cyril Chemparathy wrote: > On 9/24/2012 9:41 AM, Russell King - ARM Linux wrote: >> On Mon, Sep 24, 2012 at 02:29:42PM +0100, Catalin Marinas wrote: >>> This function also calls free_bootmem() which takes unsigned long. Are >>> patches sent separately for this or we just ignore holes in memmap? >>> There are other calls to free_bootmem() or reserve_bootmem(), do they >>> just work with the high phys addresses? >> >> Bootmem only deals with physical addresses which fit within the size >> of an 'unsigned long'. Unfortunately, the bootmem API is a mess of >> 'unsigned long' physical addresses and PFNs. >> >> Years ago there was a patch to make it use only PFNs but other changes >> resulted in that patch being thrown away. >> > > A separate patch has been posted for bootmem (see [1]). > > Tejun suggested that we'd be better off moving entirely to memblock > instead (see [2]). Yes we should, but that's not as easy as typing a few words in an email. When I tried it when I integrated memblock into our boot sequence, I found that sparsemem wouldn't work without bootmem being in place. It may be that things have now moved on, and we can just eliminate bootmem once and for all, but that's something I've not looked at for quite some time.
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c index 9aec41f..19ba70b 100644 --- a/arch/arm/mm/init.c +++ b/arch/arm/mm/init.c @@ -457,7 +457,7 @@ static inline void free_memmap(unsigned long start_pfn, unsigned long end_pfn) { struct page *start_pg, *end_pg; - unsigned long pg, pgend; + phys_addr_t pg, pgend; /* * Convert start_pfn/end_pfn to a struct page pointer. @@ -469,8 +469,8 @@ free_memmap(unsigned long start_pfn, unsigned long end_pfn) * Convert to physical addresses, and * round start upwards and end downwards. */ - pg = (unsigned long)PAGE_ALIGN(__pa(start_pg)); - pgend = (unsigned long)__pa(end_pg) & PAGE_MASK; + pg = PAGE_ALIGN(__pa(start_pg)); + pgend = __pa(end_pg) & PAGE_MASK; /* * If there are free pages between these,