At least on ia64 vmalloc_end is a global variable that VMALLOC_END
expands to. Hence having a local variable named vmalloc_end and
initialized from VMALLOC_END won't work on such platforms. Rename
these variables, and for consistency also rename vmalloc_start.
Signed-off-by: Jan Beulich <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Tony Luck <[email protected]>
---
mm/vmalloc.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
--- linux-2.6.32/mm/vmalloc.c
+++ 2.6.32-dont-use-vmalloc_end/mm/vmalloc.c
@@ -2060,13 +2060,13 @@ static unsigned long pvm_determine_end(s
struct vmap_area **pprev,
unsigned long align)
{
- const unsigned long vmalloc_end = VMALLOC_END & ~(align - 1);
+ const unsigned long end = VMALLOC_END & ~(align - 1);
unsigned long addr;
if (*pnext)
- addr = min((*pnext)->va_start & ~(align - 1), vmalloc_end);
+ addr = min((*pnext)->va_start & ~(align - 1), end);
else
- addr = vmalloc_end;
+ addr = end;
while (*pprev && (*pprev)->va_end > addr) {
*pnext = *pprev;
@@ -2105,8 +2105,8 @@ struct vm_struct **pcpu_get_vm_areas(con
const size_t *sizes, int nr_vms,
size_t align, gfp_t gfp_mask)
{
- const unsigned long vmalloc_start = ALIGN(VMALLOC_START, align);
- const unsigned long vmalloc_end = VMALLOC_END & ~(align - 1);
+ const unsigned long vstart = ALIGN(VMALLOC_START, align);
+ const unsigned long vend = VMALLOC_END & ~(align - 1);
struct vmap_area **vas, *prev, *next;
struct vm_struct **vms;
int area, area2, last_area, term_area;
@@ -2142,7 +2142,7 @@ struct vm_struct **pcpu_get_vm_areas(con
}
last_end = offsets[last_area] + sizes[last_area];
- if (vmalloc_end - vmalloc_start < last_end) {
+ if (vend - vstart < last_end) {
WARN_ON(true);
return NULL;
}
@@ -2167,7 +2167,7 @@ retry:
end = start + sizes[area];
if (!pvm_find_next_prev(vmap_area_pcpu_hole, &next, &prev)) {
- base = vmalloc_end - last_end;
+ base = vend - last_end;
goto found;
}
base = pvm_determine_end(&next, &prev, align) - end;
@@ -2180,7 +2180,7 @@ retry:
* base might have underflowed, add last_end before
* comparing.
*/
- if (base + last_end < vmalloc_start + last_end) {
+ if (base + last_end < vstart + last_end) {
spin_unlock(&vmap_area_lock);
if (!purged) {
purge_vmap_area_lazy();
(cc linux-ia64)
On Mon, 07 Dec 2009 16:24:03 +0000
"Jan Beulich" <[email protected]> wrote:
> At least on ia64 vmalloc_end is a global variable that VMALLOC_END
> expands to. Hence having a local variable named vmalloc_end and
> initialized from VMALLOC_END won't work on such platforms. Rename
> these variables, and for consistency also rename vmalloc_start.
>
erk. So does 2.6.32's vmalloc() actually work correctly on ia64?
Perhaps vmalloc_end wasn't a well chosen name for an arch-specific
global variable.
arch/m68k/include/asm/pgtable_mm.h does the same thing. Did it break too?
> ---
> mm/vmalloc.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> --- linux-2.6.32/mm/vmalloc.c
> +++ 2.6.32-dont-use-vmalloc_end/mm/vmalloc.c
> @@ -2060,13 +2060,13 @@ static unsigned long pvm_determine_end(s
> struct vmap_area **pprev,
> unsigned long align)
> {
> - const unsigned long vmalloc_end = VMALLOC_END & ~(align - 1);
> + const unsigned long end = VMALLOC_END & ~(align - 1);
> unsigned long addr;
>
> if (*pnext)
> - addr = min((*pnext)->va_start & ~(align - 1), vmalloc_end);
> + addr = min((*pnext)->va_start & ~(align - 1), end);
> else
> - addr = vmalloc_end;
> + addr = end;
>
> while (*pprev && (*pprev)->va_end > addr) {
> *pnext = *pprev;
> @@ -2105,8 +2105,8 @@ struct vm_struct **pcpu_get_vm_areas(con
> const size_t *sizes, int nr_vms,
> size_t align, gfp_t gfp_mask)
> {
> - const unsigned long vmalloc_start = ALIGN(VMALLOC_START, align);
> - const unsigned long vmalloc_end = VMALLOC_END & ~(align - 1);
> + const unsigned long vstart = ALIGN(VMALLOC_START, align);
> + const unsigned long vend = VMALLOC_END & ~(align - 1);
> struct vmap_area **vas, *prev, *next;
> struct vm_struct **vms;
> int area, area2, last_area, term_area;
> @@ -2142,7 +2142,7 @@ struct vm_struct **pcpu_get_vm_areas(con
> }
> last_end = offsets[last_area] + sizes[last_area];
>
> - if (vmalloc_end - vmalloc_start < last_end) {
> + if (vend - vstart < last_end) {
> WARN_ON(true);
> return NULL;
> }
> @@ -2167,7 +2167,7 @@ retry:
> end = start + sizes[area];
>
> if (!pvm_find_next_prev(vmap_area_pcpu_hole, &next, &prev)) {
> - base = vmalloc_end - last_end;
> + base = vend - last_end;
> goto found;
> }
> base = pvm_determine_end(&next, &prev, align) - end;
> @@ -2180,7 +2180,7 @@ retry:
> * base might have underflowed, add last_end before
> * comparing.
> */
> - if (base + last_end < vmalloc_start + last_end) {
> + if (base + last_end < vstart + last_end) {
> spin_unlock(&vmap_area_lock);
> if (!purged) {
> purge_vmap_area_lazy();
>
On 12/08/2009 08:35 AM, Andrew Morton wrote:
> (cc linux-ia64)
>
> On Mon, 07 Dec 2009 16:24:03 +0000
> "Jan Beulich" <[email protected]> wrote:
>
>> At least on ia64 vmalloc_end is a global variable that VMALLOC_END
>> expands to. Hence having a local variable named vmalloc_end and
>> initialized from VMALLOC_END won't work on such platforms. Rename
>> these variables, and for consistency also rename vmalloc_start.
>>
>
> erk. So does 2.6.32's vmalloc() actually work correctly on ia64?
>
> Perhaps vmalloc_end wasn't a well chosen name for an arch-specific
> global variable.
>
> arch/m68k/include/asm/pgtable_mm.h does the same thing. Did it break too?
Hmmm... ISTR writing a patch updating ia64 so that it doesn't use that
macro. Looking it up.... Yeap, 126b3fcdecd350cad9700908d0ad845084e26a31
in percpu#for-next.
ia64: don't alias VMALLOC_END to vmalloc_end
If CONFIG_VIRTUAL_MEM_MAP is enabled, ia64 defines macro VMALLOC_END
as unsigned long variable vmalloc_end which is adjusted to prepare
room for vmemmap. This becomes probnlematic if a local variables
vmalloc_end is defined in some function (not very unlikely) and
VMALLOC_END is used in the function - the function thinks its
referencing the global VMALLOC_END value but would be referencing its
own local vmalloc_end variable.
There's no reason VMALLOC_END should be a macro. Just define it as an
unsigned long variable if CONFIG_VIRTUAL_MEM_MAP is set to avoid nasty
surprises.
Signed-off-by: Tejun Heo <[email protected]>
Acked-by: Tony Luck <[email protected]>
Cc: Fenghua Yu <[email protected]>
Cc: linux-ia64 <[email protected]>
Cc: Christoph Lameter <[email protected]>
2.6.32 doesn't use new allocator on ia64 yet and the above commit will
be sent to Linus soon which will also enable new allocator.
Thanks.
--
tejun
Hello,
On 12/08/2009 08:35 AM, Andrew Morton wrote:
> arch/m68k/include/asm/pgtable_mm.h does the same thing. Did it break too?
Oh... m64k does the same thing. I think the correct thing to do here
would be to convert arch code as in ia64. I think defining
VMALLOC_END to vmalloc_end is a bit error-prone. If it were defined
simply as vmalloc_end, it's unnoticeable by both the compiler and
developer.
Thanks.
--
tejun
On Mon, Dec 07, 2009 at 03:35:52PM -0800, Andrew Morton wrote:
> erk. So does 2.6.32's vmalloc() actually work correctly on ia64?
>
> Perhaps vmalloc_end wasn't a well chosen name for an arch-specific
> global variable.
Can we enable -Wshadow now? Please?
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On Mon, 7 Dec 2009 17:50:29 -0700
Matthew Wilcox <[email protected]> wrote:
> On Mon, Dec 07, 2009 at 03:35:52PM -0800, Andrew Morton wrote:
> > erk. So does 2.6.32's vmalloc() actually work correctly on ia64?
> >
> > Perhaps vmalloc_end wasn't a well chosen name for an arch-specific
> > global variable.
>
> Can we enable -Wshadow now? Please?
>
That would be good. How much mess would it make?
On SUN3, m68k defines macro VMALLOC_END as unsigned long variable
vmalloc_end which is adjusted from mmu_emu_init(). This becomes
problematic if a local variables vmalloc_end is defined in some
function (not very unlikely) and VMALLOC_END is used in the function -
the function thinks its referencing the global VMALLOC_END value but
would be referencing its own local vmalloc_end variable.
There's no reason VMALLOC_END should be a macro. Just define it as an
unsigned long variable to avoid nasty surprises.
Signed-off-by: Tejun Heo <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Roman Zippel <[email protected]>
---
Okay, here it is. Compile tested. Geert, Roman, if you guys don't
object, I'd like to push it with the rest of percpu changes to Linus.
What do you think?
Thanks.
arch/m68k/include/asm/pgtable_mm.h | 3 +--
arch/m68k/sun3/mmu_emu.c | 8 ++++----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/m68k/include/asm/pgtable_mm.h b/arch/m68k/include/asm/pgtable_mm.h
index fe60e1a..0ea9f09 100644
--- a/arch/m68k/include/asm/pgtable_mm.h
+++ b/arch/m68k/include/asm/pgtable_mm.h
@@ -83,9 +83,8 @@
#define VMALLOC_START (((unsigned long) high_memory + VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1))
#define VMALLOC_END KMAP_START
#else
-extern unsigned long vmalloc_end;
#define VMALLOC_START 0x0f800000
-#define VMALLOC_END vmalloc_end
+extern unsigned long VMALLOC_END;
#endif /* CONFIG_SUN3 */
/* zero page used for uninitialized stuff */
diff --git a/arch/m68k/sun3/mmu_emu.c b/arch/m68k/sun3/mmu_emu.c
index 3cd1939..25e2b14 100644
--- a/arch/m68k/sun3/mmu_emu.c
+++ b/arch/m68k/sun3/mmu_emu.c
@@ -45,8 +45,8 @@
** Globals
*/
-unsigned long vmalloc_end;
-EXPORT_SYMBOL(vmalloc_end);
+unsigned long VMALLOC_END;
+EXPORT_SYMBOL(VMALLOC_END);
unsigned long pmeg_vaddr[PMEGS_NUM];
unsigned char pmeg_alloc[PMEGS_NUM];
@@ -172,8 +172,8 @@ void mmu_emu_init(unsigned long bootmem_end)
#endif
// the lowest mapping here is the end of our
// vmalloc region
- if(!vmalloc_end)
- vmalloc_end = seg;
+ if (!VMALLOC_END)
+ VMALLOC_END = seg;
// mark the segmap alloc'd, and reserve any
// of the first 0xbff pages the hardware is
>>> Andrew Morton <[email protected]> 08.12.09 00:35 >>>
>(cc linux-ia64)
>
>On Mon, 07 Dec 2009 16:24:03 +0000
>"Jan Beulich" <[email protected]> wrote:
>
>> At least on ia64 vmalloc_end is a global variable that VMALLOC_END
>> expands to. Hence having a local variable named vmalloc_end and
>> initialized from VMALLOC_END won't work on such platforms. Rename
>> these variables, and for consistency also rename vmalloc_start.
>>
>
>erk. So does 2.6.32's vmalloc() actually work correctly on ia64?
According to Tejun the problem is just cosmetic (i.e. causes build
warnings), since the functions affected aren't being used (yet) on
ia64. So feel free to drop the patch again, given that he has a patch
queued to address the issue by renaming the arch variable.
I wonder though why that code is being built on ia64 at all if it's not
being used (i.e. why it doesn't depend on a CONFIG_*, HAVE_*, or
NEED_* manifest constant).
Jan
Hello,
On 12/08/2009 05:23 PM, Jan Beulich wrote:
> According to Tejun the problem is just cosmetic (i.e. causes build
> warnings), since the functions affected aren't being used (yet) on
> ia64. So feel free to drop the patch again, given that he has a patch
> queued to address the issue by renaming the arch variable.
>
> I wonder though why that code is being built on ia64 at all if it's not
> being used (i.e. why it doesn't depend on a CONFIG_*, HAVE_*, or
> NEED_* manifest constant).
Hmm... it shouldn't be building. Can you please attach .config?
Thanks.
--
tejun
>>> Tejun Heo <[email protected]> 08.12.09 09:29 >>>
>Hmm... it shouldn't be building.
How can it not be building? It's in vmalloc.c (which must be built) and not
inside any conditional.
>Can you please attach .config?
Attached anyway.
Jan
Hello,
On 12/08/2009 05:39 PM, Jan Beulich wrote:
>>>> Tejun Heo <[email protected]> 08.12.09 09:29 >>>
>> Hmm... it shouldn't be building.
>
> How can it not be building? It's in vmalloc.c (which must be built) and not
> inside any conditional.
Ah... yes, right. Somehow I was thinking it lived in percpu.c. Sorry
about that. Probably the right thing to do is to wrap the function
inside CONFIG ifdef's. I'll prep a patch.
Thanks.
--
tejun
On Tue, Dec 8, 2009 at 07:57, Tejun Heo <[email protected]> wrote:
> On SUN3, m68k defines macro VMALLOC_END as unsigned long variable
> vmalloc_end which is adjusted from mmu_emu_init(). This becomes
> problematic if a local variables vmalloc_end is defined in some
> function (not very unlikely) and VMALLOC_END is used in the function -
> the function thinks its referencing the global VMALLOC_END value but
> would be referencing its own local vmalloc_end variable.
>
> There's no reason VMALLOC_END should be a macro. Just define it as an
> unsigned long variable to avoid nasty surprises.
>
> Signed-off-by: Tejun Heo <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Roman Zippel <[email protected]>
> ---
> Okay, here it is. Compile tested. Geert, Roman, if you guys don't
> object, I'd like to push it with the rest of percpu changes to Linus.
> What do you think?
Fine for me, except that by convention allcaps is reserved for macros?
>
> Thanks.
>
> arch/m68k/include/asm/pgtable_mm.h | 3 +--
> arch/m68k/sun3/mmu_emu.c | 8 ++++----
> 2 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/m68k/include/asm/pgtable_mm.h b/arch/m68k/include/asm/pgtable_mm.h
> index fe60e1a..0ea9f09 100644
> --- a/arch/m68k/include/asm/pgtable_mm.h
> +++ b/arch/m68k/include/asm/pgtable_mm.h
> @@ -83,9 +83,8 @@
> #define VMALLOC_START (((unsigned long) high_memory + VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1))
> #define VMALLOC_END KMAP_START
> #else
> -extern unsigned long vmalloc_end;
> #define VMALLOC_START 0x0f800000
> -#define VMALLOC_END vmalloc_end
> +extern unsigned long VMALLOC_END;
> #endif /* CONFIG_SUN3 */
>
> /* zero page used for uninitialized stuff */
> diff --git a/arch/m68k/sun3/mmu_emu.c b/arch/m68k/sun3/mmu_emu.c
> index 3cd1939..25e2b14 100644
> --- a/arch/m68k/sun3/mmu_emu.c
> +++ b/arch/m68k/sun3/mmu_emu.c
> @@ -45,8 +45,8 @@
> ** Globals
> */
>
> -unsigned long vmalloc_end;
> -EXPORT_SYMBOL(vmalloc_end);
> +unsigned long VMALLOC_END;
> +EXPORT_SYMBOL(VMALLOC_END);
>
> unsigned long pmeg_vaddr[PMEGS_NUM];
> unsigned char pmeg_alloc[PMEGS_NUM];
> @@ -172,8 +172,8 @@ void mmu_emu_init(unsigned long bootmem_end)
> #endif
> // the lowest mapping here is the end of our
> // vmalloc region
> - if(!vmalloc_end)
> - vmalloc_end = seg;
> + if (!VMALLOC_END)
> + VMALLOC_END = seg;
>
> // mark the segmap alloc'd, and reserve any
> // of the first 0xbff pages the hardware is
>
--
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Tue, Dec 8, 2009 at 00:35, Andrew Morton <[email protected]> wrote:
> (cc linux-ia64)
>
> On Mon, 07 Dec 2009 16:24:03 +0000
> "Jan Beulich" <[email protected]> wrote:
>
>> At least on ia64 vmalloc_end is a global variable that VMALLOC_END
>> expands to. Hence having a local variable named vmalloc_end and
>> initialized from VMALLOC_END won't work on such platforms. Rename
>> these variables, and for consistency also rename vmalloc_start.
>>
>
> erk. So does 2.6.32's vmalloc() actually work correctly on ia64?
>
> Perhaps vmalloc_end wasn't a well chosen name for an arch-specific
> global variable.
>
> arch/m68k/include/asm/pgtable_mm.h does the same thing. Did it break too?
Rename to m68k_vmalloc_{end,start}?
Hmm, sounds better than introducing allcaps variables...
>
>> ---
>> mm/vmalloc.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> --- linux-2.6.32/mm/vmalloc.c
>> +++ 2.6.32-dont-use-vmalloc_end/mm/vmalloc.c
>> @@ -2060,13 +2060,13 @@ static unsigned long pvm_determine_end(s
>> struct vmap_area **pprev,
>> unsigned long align)
>> {
>> - const unsigned long vmalloc_end = VMALLOC_END & ~(align - 1);
>> + const unsigned long end = VMALLOC_END & ~(align - 1);
>> unsigned long addr;
>>
>> if (*pnext)
>> - addr = min((*pnext)->va_start & ~(align - 1), vmalloc_end);
>> + addr = min((*pnext)->va_start & ~(align - 1), end);
>> else
>> - addr = vmalloc_end;
>> + addr = end;
>>
>> while (*pprev && (*pprev)->va_end > addr) {
>> *pnext = *pprev;
>> @@ -2105,8 +2105,8 @@ struct vm_struct **pcpu_get_vm_areas(con
>> const size_t *sizes, int nr_vms,
>> size_t align, gfp_t gfp_mask)
>> {
>> - const unsigned long vmalloc_start = ALIGN(VMALLOC_START, align);
>> - const unsigned long vmalloc_end = VMALLOC_END & ~(align - 1);
>> + const unsigned long vstart = ALIGN(VMALLOC_START, align);
>> + const unsigned long vend = VMALLOC_END & ~(align - 1);
>> struct vmap_area **vas, *prev, *next;
>> struct vm_struct **vms;
>> int area, area2, last_area, term_area;
>> @@ -2142,7 +2142,7 @@ struct vm_struct **pcpu_get_vm_areas(con
>> }
>> last_end = offsets[last_area] + sizes[last_area];
>>
>> - if (vmalloc_end - vmalloc_start < last_end) {
>> + if (vend - vstart < last_end) {
>> WARN_ON(true);
>> return NULL;
>> }
>> @@ -2167,7 +2167,7 @@ retry:
>> end = start + sizes[area];
>>
>> if (!pvm_find_next_prev(vmap_area_pcpu_hole, &next, &prev)) {
>> - base = vmalloc_end - last_end;
>> + base = vend - last_end;
>> goto found;
>> }
>> base = pvm_determine_end(&next, &prev, align) - end;
>> @@ -2180,7 +2180,7 @@ retry:
>> * base might have underflowed, add last_end before
>> * comparing.
>> */
>> - if (base + last_end < vmalloc_start + last_end) {
>> + if (base + last_end < vstart + last_end) {
>> spin_unlock(&vmap_area_lock);
>> if (!purged) {
>> purge_vmap_area_lazy();
>>
>
--
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hello,
Geert Uytterhoeven wrote:
> Rename to m68k_vmalloc_{end,start}?
> Hmm, sounds better than introducing allcaps variables...
We definitely can do that too but I don't know. It still has the risk
of aliasing behind both the developer's and compiler's back. Aliasing
a macro directly to a macro just isn't a very good idea. We can
definitely make it more complex - make the prefix more unlikely,
prevent it from being used as lvalue and so on but given that it's a
pretty special case anyway, I think all caps variable isn't too bad
here.
But you're the maintainer, if you prefer the following version, I'll
go forward with it.
Thanks.
diff --git a/arch/m68k/include/asm/pgtable_mm.h b/arch/m68k/include/asm/pgtable_mm.h
index fe60e1a..2db057f 100644
--- a/arch/m68k/include/asm/pgtable_mm.h
+++ b/arch/m68k/include/asm/pgtable_mm.h
@@ -83,9 +83,9 @@
#define VMALLOC_START (((unsigned long) high_memory + VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1))
#define VMALLOC_END KMAP_START
#else
-extern unsigned long vmalloc_end;
#define VMALLOC_START 0x0f800000
-#define VMALLOC_END vmalloc_end
+extern unsigned long m68k_vmalloc_end;
+#define VMALLOC_END m68k_vmalloc_end
#endif /* CONFIG_SUN3 */
/* zero page used for uninitialized stuff */
diff --git a/arch/m68k/sun3/mmu_emu.c b/arch/m68k/sun3/mmu_emu.c
index 3cd1939..94f81ec 100644
--- a/arch/m68k/sun3/mmu_emu.c
+++ b/arch/m68k/sun3/mmu_emu.c
@@ -45,8 +45,8 @@
** Globals
*/
-unsigned long vmalloc_end;
-EXPORT_SYMBOL(vmalloc_end);
+unsigned long m68k_vmalloc_end;
+EXPORT_SYMBOL(m68k_vmalloc_end);
unsigned long pmeg_vaddr[PMEGS_NUM];
unsigned char pmeg_alloc[PMEGS_NUM];
@@ -172,8 +172,8 @@ void mmu_emu_init(unsigned long bootmem_end)
#endif
// the lowest mapping here is the end of our
// vmalloc region
- if(!vmalloc_end)
- vmalloc_end = seg;
+ if (!m68k_vmalloc_end)
+ m68k_vmalloc_end = seg;
// mark the segmap alloc'd, and reserve any
// of the first 0xbff pages the hardware is
--
tejun
On SUN3, m68k defines macro VMALLOC_END as unsigned long variable
vmalloc_end which is adjusted from mmu_emu_init(). This becomes
problematic if a local variables vmalloc_end is defined in some
function (not very unlikely) and VMALLOC_END is used in the function -
the function thinks its referencing the global VMALLOC_END value but
would be referencing its own local vmalloc_end variable.
Rename the global variable to m68k_vmlloc_end which is much less
likely to be used as local variable name.
Signed-off-by: Tejun Heo <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Roman Zippel <[email protected]>
---
This patch has been queued to percpu#for-linus. Thanks.
arch/m68k/include/asm/pgtable_mm.h | 4 ++--
arch/m68k/sun3/mmu_emu.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/m68k/include/asm/pgtable_mm.h b/arch/m68k/include/asm/pgtable_mm.h
index fe60e1a..aca0e28 100644
--- a/arch/m68k/include/asm/pgtable_mm.h
+++ b/arch/m68k/include/asm/pgtable_mm.h
@@ -83,9 +83,9 @@
#define VMALLOC_START (((unsigned long) high_memory + VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1))
#define VMALLOC_END KMAP_START
#else
-extern unsigned long vmalloc_end;
+extern unsigned long m68k_vmalloc_end;
#define VMALLOC_START 0x0f800000
-#define VMALLOC_END vmalloc_end
+#define VMALLOC_END m68k_vmalloc_end
#endif /* CONFIG_SUN3 */
/* zero page used for uninitialized stuff */
diff --git a/arch/m68k/sun3/mmu_emu.c b/arch/m68k/sun3/mmu_emu.c
index 3cd1939..94f81ec 100644
--- a/arch/m68k/sun3/mmu_emu.c
+++ b/arch/m68k/sun3/mmu_emu.c
@@ -45,8 +45,8 @@
** Globals
*/
-unsigned long vmalloc_end;
-EXPORT_SYMBOL(vmalloc_end);
+unsigned long m68k_vmalloc_end;
+EXPORT_SYMBOL(m68k_vmalloc_end);
unsigned long pmeg_vaddr[PMEGS_NUM];
unsigned char pmeg_alloc[PMEGS_NUM];
@@ -172,8 +172,8 @@ void mmu_emu_init(unsigned long bootmem_end)
#endif
// the lowest mapping here is the end of our
// vmalloc region
- if(!vmalloc_end)
- vmalloc_end = seg;
+ if (!m68k_vmalloc_end)
+ m68k_vmalloc_end = seg;
// mark the segmap alloc'd, and reserve any
// of the first 0xbff pages the hardware is
--
1.6.4.2
On Tue, 8 Dec 2009, Jan Beulich wrote:
> According to Tejun the problem is just cosmetic (i.e. causes build
> warnings), since the functions affected aren't being used (yet) on
> ia64. So feel free to drop the patch again, given that he has a patch
> queued to address the issue by renaming the arch variable.
I thought the new code must be used in order for the new percpu allocator
to work? Or is this referring to other code?
> I wonder though why that code is being built on ia64 at all if it's not
> being used (i.e. why it doesn't depend on a CONFIG_*, HAVE_*, or
> NEED_* manifest constant).
Tony: Can you confirm that the new percpu stuff works on IA64? (Or is
there nobody left to care?)
> Tony: Can you confirm that the new percpu stuff works on IA64.
If all the pieces are in (either Linus tree, or linux-next) then
it is working (well these both still build & boot on my test systems).
-Tony
Christoph Lameter wrote:
> On Tue, 8 Dec 2009, Jan Beulich wrote:
>
>> According to Tejun the problem is just cosmetic (i.e. causes build
>> warnings), since the functions affected aren't being used (yet) on
>> ia64. So feel free to drop the patch again, given that he has a patch
>> queued to address the issue by renaming the arch variable.
>
> I thought the new code must be used in order for the new percpu allocator
> to work? Or is this referring to other code?
>
>> I wonder though why that code is being built on ia64 at all if it's not
>> being used (i.e. why it doesn't depend on a CONFIG_*, HAVE_*, or
>> NEED_* manifest constant).
>
> Tony: Can you confirm that the new percpu stuff works on IA64? (Or is
> there nobody left to care?)
Christoph, I have access to a 640p system for a couple more weeks if
there's anything you'd like me to check out.
Thanks,
Mike
On Wed, 9 Dec 2009, Mike Travis wrote:
> > Tony: Can you confirm that the new percpu stuff works on IA64? (Or is
> > there nobody left to care?)
>
> Christoph, I have access to a 640p system for a couple more weeks if
> there's anything you'd like me to check out.
Boot with 2.6.32 and see if the per cpu allocator works. Check if there
are any changes to memory consumption. Create a few thousand virtual
ethernet devices and see if the system keels over.
It may also be good to run some scheduler test. Compare AIM9 of latest
SLES with 2.6.32. Concurrent page fault test? Then a performance test with
lots of concurrency but the usual stuff wont work since HPC apps usually
pin.
Run latencytest (available in the lldiag package) from
kernel.org/pub/linux/kernel/people/christoph/lldiag and see how the
disturbances by the OS are changed.
On Wed, 9 Dec 2009, Christoph Lameter wrote:
> Boot with 2.6.32 and see if the per cpu allocator works. Check if there
> are any changes to memory consumption. Create a few thousand virtual
> ethernet devices and see if the system keels over.
Argh. You have to wait till 2.6.33-rc1 I believe to get the conversion of
the SNMP MIBs for the new per cpu allocator.
Christoph Lameter wrote:
> On Wed, 9 Dec 2009, Mike Travis wrote:
>
>>> Tony: Can you confirm that the new percpu stuff works on IA64? (Or is
>>> there nobody left to care?)
>> Christoph, I have access to a 640p system for a couple more weeks if
>> there's anything you'd like me to check out.
>
> Boot with 2.6.32 and see if the per cpu allocator works. Check if there
> are any changes to memory consumption. Create a few thousand virtual
> ethernet devices and see if the system keels over.
Any advice on how to go about the above would be helpful... ;-)
>
> It may also be good to run some scheduler test. Compare AIM9 of latest
> SLES with 2.6.32. Concurrent page fault test? Then a performance test with
> lots of concurrency but the usual stuff wont work since HPC apps usually
> pin.
I'm doing some aim7/9 comparisons right now between SPARSE and DISCONTIG
memory configs using sles11 + 2.6.32. Which other benchmarks would you
recommend for the other tests?
>
> Run latencytest (available in the lldiag package) from
> kernel.org/pub/linux/kernel/people/christoph/lldiag and see how the
> disturbances by the OS are changed.
I'll put that on the list.
On Wed, 9 Dec 2009, Mike Travis wrote:
> > Boot with 2.6.32 and see if the per cpu allocator works. Check if there
> > are any changes to memory consumption. Create a few thousand virtual
> > ethernet devices and see if the system keels over.
>
> Any advice on how to go about the above would be helpful... ;-)
I believe you can create an additional alias device with
ifconfig eth0:<N>
or so.
> I'm doing some aim7/9 comparisons right now between SPARSE and DISCONTIG
> memory configs using sles11 + 2.6.32. Which other benchmarks would you
> recommend for the other tests?
See f.e. http://kernel-perf.sourceforge.net/about_tests.php
lmbench?
pcpu_get_vm_areas() is used only when dynamic percpu allocator is used
by the architecture. In 2.6.32, ia64 doesn't use dynamic percpu
allocator and has a macro which makes pcpu_get_vm_areas() buggy via
local/global variable aliasing and triggers compile warning.
The problem is fixed in upstream and ia64 uses dynamic percpu
allocators, so the only left issue is inclusion of unnecessary code
and compile warning on ia64 on 2.6.32.
Don't build pcpu_get_vm_areas() if legacy percpu allocator is in use.
Signed-off-by: Tejun Heo <[email protected]>
Reported-by: Jan Beulich <[email protected]>
Cc: [email protected]
---
Please note that this commit won't appear on upstream.
Thanks.
include/linux/vmalloc.h | 2 ++
mm/vmalloc.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 227c2a5..3c123c3 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -115,9 +115,11 @@ extern rwlock_t vmlist_lock;
extern struct vm_struct *vmlist;
extern __init void vm_area_register_early(struct vm_struct *vm, size_t align);
+#ifndef CONFIG_HAVE_LEGACY_PER_CPU_AREA
struct vm_struct **pcpu_get_vm_areas(const unsigned long *offsets,
const size_t *sizes, int nr_vms,
size_t align, gfp_t gfp_mask);
+#endif
void pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms);
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 0f551a4..7758726 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1993,6 +1993,7 @@ void free_vm_area(struct vm_struct *area)
}
EXPORT_SYMBOL_GPL(free_vm_area);
+#ifndef CONFIG_HAVE_LEGACY_PER_CPU_AREA
static struct vmap_area *node_to_va(struct rb_node *n)
{
return n ? rb_entry(n, struct vmap_area, rb_node) : NULL;
@@ -2257,6 +2258,7 @@ err_free:
kfree(vms);
return NULL;
}
+#endif
/**
* pcpu_free_vm_areas - free vmalloc areas for percpu allocator
On Thu, Dec 10, 2009 at 08:43:16AM +0900, Tejun Heo wrote:
> pcpu_get_vm_areas() is used only when dynamic percpu allocator is used
> by the architecture. In 2.6.32, ia64 doesn't use dynamic percpu
> allocator and has a macro which makes pcpu_get_vm_areas() buggy via
> local/global variable aliasing and triggers compile warning.
>
> The problem is fixed in upstream and ia64 uses dynamic percpu
> allocators, so the only left issue is inclusion of unnecessary code
> and compile warning on ia64 on 2.6.32.
>
> Don't build pcpu_get_vm_areas() if legacy percpu allocator is in use.
>
> Signed-off-by: Tejun Heo <[email protected]>
> Reported-by: Jan Beulich <[email protected]>
> Cc: [email protected]
> ---
> Please note that this commit won't appear on upstream.
So this is only needed for the .32 kernel stable tree? Not .31? And
it's not upstream as it was solved differently there?
thanks,
greg k-h
Hello, Greg.
On 12/17/2009 08:12 AM, Greg KH wrote:
>> Please note that this commit won't appear on upstream.
>
> So this is only needed for the .32 kernel stable tree? Not .31? And
> it's not upstream as it was solved differently there?
Yeap, .32 is the only affected one and in the upstream the problem is
solved way back and ia64 is already using the new dynamic allocator.
Thanks.
--
tejun
On Thu, Dec 17, 2009 at 09:01:52AM +0900, Tejun Heo wrote:
> Hello, Greg.
>
> On 12/17/2009 08:12 AM, Greg KH wrote:
> >> Please note that this commit won't appear on upstream.
> >
> > So this is only needed for the .32 kernel stable tree? Not .31? And
> > it's not upstream as it was solved differently there?
>
> Yeap, .32 is the only affected one and in the upstream the problem is
> solved way back and ia64 is already using the new dynamic allocator.
Great, I've queued this up now, thanks for letting me know.
greg k-h