This test is used in a couple of places. Add a version to vmalloc.h
and replace the other checks.
Signed-off-by: Christoph Lameter <[email protected]>
---
drivers/net/cxgb3/cxgb3_offload.c | 4 +---
fs/ntfs/malloc.h | 3 +--
fs/proc/kcore.c | 2 +-
fs/xfs/linux-2.6/kmem.c | 3 +--
fs/xfs/linux-2.6/xfs_buf.c | 3 +--
include/linux/mm.h | 8 ++++++++
mm/sparse.c | 10 +---------
7 files changed, 14 insertions(+), 19 deletions(-)
Index: linux-2.6/include/linux/mm.h
===================================================================
--- linux-2.6.orig/include/linux/mm.h 2007-09-17 21:46:06.000000000 -0700
+++ linux-2.6/include/linux/mm.h 2007-09-17 23:56:54.000000000 -0700
@@ -1158,6 +1158,14 @@ static inline unsigned long vma_pages(st
return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
}
+/* Determine if an address is within the vmalloc range */
+static inline int is_vmalloc_addr(const void *x)
+{
+ unsigned long addr = (unsigned long)x;
+
+ return addr >= VMALLOC_START && addr < VMALLOC_END;
+}
+
pgprot_t vm_get_page_prot(unsigned long vm_flags);
struct vm_area_struct *find_extend_vma(struct mm_struct *, unsigned long addr);
int remap_pfn_range(struct vm_area_struct *, unsigned long addr,
Index: linux-2.6/mm/sparse.c
===================================================================
--- linux-2.6.orig/mm/sparse.c 2007-09-17 21:45:24.000000000 -0700
+++ linux-2.6/mm/sparse.c 2007-09-17 23:56:26.000000000 -0700
@@ -289,17 +289,9 @@ got_map_ptr:
return ret;
}
-static int vaddr_in_vmalloc_area(void *addr)
-{
- if (addr >= (void *)VMALLOC_START &&
- addr < (void *)VMALLOC_END)
- return 1;
- return 0;
-}
-
static void __kfree_section_memmap(struct page *memmap, unsigned long nr_pages)
{
- if (vaddr_in_vmalloc_area(memmap))
+ if (is_vmalloc_addr(memmap))
vfree(memmap);
else
free_pages((unsigned long)memmap,
Index: linux-2.6/drivers/net/cxgb3/cxgb3_offload.c
===================================================================
--- linux-2.6.orig/drivers/net/cxgb3/cxgb3_offload.c 2007-09-17 21:45:24.000000000 -0700
+++ linux-2.6/drivers/net/cxgb3/cxgb3_offload.c 2007-09-17 21:46:06.000000000 -0700
@@ -1035,9 +1035,7 @@ void *cxgb_alloc_mem(unsigned long size)
*/
void cxgb_free_mem(void *addr)
{
- unsigned long p = (unsigned long)addr;
-
- if (p >= VMALLOC_START && p < VMALLOC_END)
+ if (is_vmalloc_addr(addr))
vfree(addr);
else
kfree(addr);
Index: linux-2.6/fs/ntfs/malloc.h
===================================================================
--- linux-2.6.orig/fs/ntfs/malloc.h 2007-09-17 21:45:24.000000000 -0700
+++ linux-2.6/fs/ntfs/malloc.h 2007-09-17 21:46:06.000000000 -0700
@@ -85,8 +85,7 @@ static inline void *ntfs_malloc_nofs_nof
static inline void ntfs_free(void *addr)
{
- if (likely(((unsigned long)addr < VMALLOC_START) ||
- ((unsigned long)addr >= VMALLOC_END ))) {
+ if (!is_vmalloc_addr(addr)) {
kfree(addr);
/* free_page((unsigned long)addr); */
return;
Index: linux-2.6/fs/proc/kcore.c
===================================================================
--- linux-2.6.orig/fs/proc/kcore.c 2007-09-17 21:45:24.000000000 -0700
+++ linux-2.6/fs/proc/kcore.c 2007-09-17 21:46:06.000000000 -0700
@@ -325,7 +325,7 @@ read_kcore(struct file *file, char __use
if (m == NULL) {
if (clear_user(buffer, tsz))
return -EFAULT;
- } else if ((start >= VMALLOC_START) && (start < VMALLOC_END)) {
+ } else if (is_vmalloc_addr((void *)start)) {
char * elf_buf;
struct vm_struct *m;
unsigned long curstart = start;
Index: linux-2.6/fs/xfs/linux-2.6/kmem.c
===================================================================
--- linux-2.6.orig/fs/xfs/linux-2.6/kmem.c 2007-09-17 21:45:24.000000000 -0700
+++ linux-2.6/fs/xfs/linux-2.6/kmem.c 2007-09-17 21:46:06.000000000 -0700
@@ -92,8 +92,7 @@ kmem_zalloc_greedy(size_t *size, size_t
void
kmem_free(void *ptr, size_t size)
{
- if (((unsigned long)ptr < VMALLOC_START) ||
- ((unsigned long)ptr >= VMALLOC_END)) {
+ if (!is_vmalloc_addr(ptr)) {
kfree(ptr);
} else {
vfree(ptr);
Index: linux-2.6/fs/xfs/linux-2.6/xfs_buf.c
===================================================================
--- linux-2.6.orig/fs/xfs/linux-2.6/xfs_buf.c 2007-09-17 21:45:24.000000000 -0700
+++ linux-2.6/fs/xfs/linux-2.6/xfs_buf.c 2007-09-17 21:46:06.000000000 -0700
@@ -696,8 +696,7 @@ static inline struct page *
mem_to_page(
void *addr)
{
- if (((unsigned long)addr < VMALLOC_START) ||
- ((unsigned long)addr >= VMALLOC_END)) {
+ if ((!is_vmalloc_addr(addr))) {
return virt_to_page(addr);
} else {
return vmalloc_to_page(addr);
--
On Tue, 18 Sep 2007, Christoph Lameter wrote:
> Index: linux-2.6/include/linux/mm.h
> ===================================================================
> --- linux-2.6.orig/include/linux/mm.h 2007-09-17 21:46:06.000000000 -0700
> +++ linux-2.6/include/linux/mm.h 2007-09-17 23:56:54.000000000 -0700
> @@ -1158,6 +1158,14 @@ static inline unsigned long vma_pages(st
> return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> }
>
> +/* Determine if an address is within the vmalloc range */
> +static inline int is_vmalloc_addr(const void *x)
> +{
> + unsigned long addr = (unsigned long)x;
> +
> + return addr >= VMALLOC_START && addr < VMALLOC_END;
> +}
This breaks on i386 because VMALLOC_END is defined in terms of PKMAP_BASE
in the CONFIG_HIGHMEM case.
This function should probably be in include/linux/vmalloc.h instead since
all callers already include it anyway.
On 19 Sep 2007, at 07:32, David Rientjes wrote:
> On Tue, 18 Sep 2007, Christoph Lameter wrote:
>> Index: linux-2.6/include/linux/mm.h
>> ===================================================================
>> --- linux-2.6.orig/include/linux/mm.h 2007-09-17
>> 21:46:06.000000000 -0700
>> +++ linux-2.6/include/linux/mm.h 2007-09-17 23:56:54.000000000 -0700
>> @@ -1158,6 +1158,14 @@ static inline unsigned long vma_pages(st
>> return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>> }
>>
>> +/* Determine if an address is within the vmalloc range */
>> +static inline int is_vmalloc_addr(const void *x)
>> +{
>> + unsigned long addr = (unsigned long)x;
>> +
>> + return addr >= VMALLOC_START && addr < VMALLOC_END;
>> +}
>
> This breaks on i386 because VMALLOC_END is defined in terms of
> PKMAP_BASE
> in the CONFIG_HIGHMEM case.
That is incorrect. This works perfectly on i386 and on ALL
architectures supported by Linux. A lot of places in the kernel
already do this today (mostly hand coded though, eg XFS and NTFS)...
There even is such a function already in mm/
sparse.c::vaddr_in_vmalloc_area() with pretty identical content. I
would suggest either this new inline should go away completely and
use the existing one and export it or the existing one should go away
and the inline should be used. It seems silly to have two functions
with different names doing exactly the same thing!
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/
On Wed, 19 Sep 2007, Anton Altaparmakov wrote:
> > > Index: linux-2.6/include/linux/mm.h
> > > ===================================================================
> > > --- linux-2.6.orig/include/linux/mm.h 2007-09-17 21:46:06.000000000
> > > -0700
> > > +++ linux-2.6/include/linux/mm.h 2007-09-17 23:56:54.000000000 -0700
> > > @@ -1158,6 +1158,14 @@ static inline unsigned long vma_pages(st
> > > return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> > > }
> > >
> > > +/* Determine if an address is within the vmalloc range */
> > > +static inline int is_vmalloc_addr(const void *x)
> > > +{
> > > + unsigned long addr = (unsigned long)x;
> > > +
> > > + return addr >= VMALLOC_START && addr < VMALLOC_END;
> > > +}
> >
> > This breaks on i386 because VMALLOC_END is defined in terms of PKMAP_BASE
> > in the CONFIG_HIGHMEM case.
>
> That is incorrect. This works perfectly on i386 and on ALL architectures
> supported by Linux. A lot of places in the kernel already do this today
> (mostly hand coded though, eg XFS and NTFS)...
>
Hmm, really?
After applying patches 1-3 in this series and compiling on my i386 with
defconfig, I get this:
In file included from include/linux/suspend.h:11,
from arch/i386/kernel/asm-offsets.c:11:
include/linux/mm.h: In function 'is_vmalloc_addr':
include/linux/mm.h:1166: error: 'PKMAP_BASE' undeclared (first use in this function)
include/linux/mm.h:1166: error: (Each undeclared identifier is reported only once
include/linux/mm.h:1166: error: for each function it appears in.)
so I don't know what you're talking about.
On 19 Sep 2007, at 09:09, David Rientjes wrote:
> On Wed, 19 Sep 2007, Anton Altaparmakov wrote:
>>>> Index: linux-2.6/include/linux/mm.h
>>>> ===================================================================
>>>> --- linux-2.6.orig/include/linux/mm.h 2007-09-17 21:46:06.000000000
>>>> -0700
>>>> +++ linux-2.6/include/linux/mm.h 2007-09-17 23:56:54.000000000
>>>> -0700
>>>> @@ -1158,6 +1158,14 @@ static inline unsigned long vma_pages(st
>>>> return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>>>> }
>>>>
>>>> +/* Determine if an address is within the vmalloc range */
>>>> +static inline int is_vmalloc_addr(const void *x)
>>>> +{
>>>> + unsigned long addr = (unsigned long)x;
>>>> +
>>>> + return addr >= VMALLOC_START && addr < VMALLOC_END;
>>>> +}
>>>
>>> This breaks on i386 because VMALLOC_END is defined in terms of
>>> PKMAP_BASE
>>> in the CONFIG_HIGHMEM case.
>>
>> That is incorrect. This works perfectly on i386 and on ALL
>> architectures
>> supported by Linux. A lot of places in the kernel already do this
>> today
>> (mostly hand coded though, eg XFS and NTFS)...
>
> Hmm, really?
>
> After applying patches 1-3 in this series and compiling on my i386
> with
> defconfig, I get this:
>
> In file included from include/linux/suspend.h:11,
> from arch/i386/kernel/asm-offsets.c:11:
> include/linux/mm.h: In function 'is_vmalloc_addr':
> include/linux/mm.h:1166: error: 'PKMAP_BASE' undeclared (first use
> in this function)
> include/linux/mm.h:1166: error: (Each undeclared identifier is
> reported only once
> include/linux/mm.h:1166: error: for each function it appears in.)
>
> so I don't know what you're talking about.
Just a compile failure not inherently broken!
Add:
#include <linux/highmem.h> to the top of linux/mm.h and it should
compile just fine.
Although it may cause a problem as highmem.h also includes mm.h so a
bit of trickery may be needed to get it to compile...
I suspect that is_vmalloc_addr() should not be in linux/mm.h at all
and should be in linux/vmalloc.h instead and vmalloc.h should include
linux/highmem.h. That would be more sensible than sticking a vmalloc
related function into linux/mm.h where it does not belong...
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/
On Wed, 19 Sep 2007, Anton Altaparmakov wrote:
> Although it may cause a problem as highmem.h also includes mm.h so a bit of
> trickery may be needed to get it to compile...
>
> I suspect that is_vmalloc_addr() should not be in linux/mm.h at all and should
> be in linux/vmalloc.h instead and vmalloc.h should include linux/highmem.h.
> That would be more sensible than sticking a vmalloc related function into
> linux/mm.h where it does not belong...
>
That is why I suggested include/linux/vmalloc.h as its home in the first
place. And no, adding an include for linux/highmem.h (and asm/pgtable.h)
to linux/vmalloc.h does not work.
On 19 Sep 2007, at 10:19, David Rientjes wrote:
> On Wed, 19 Sep 2007, Anton Altaparmakov wrote:
>> Although it may cause a problem as highmem.h also includes mm.h so
>> a bit of
>> trickery may be needed to get it to compile...
>>
>> I suspect that is_vmalloc_addr() should not be in linux/mm.h at
>> all and should
>> be in linux/vmalloc.h instead and vmalloc.h should include linux/
>> highmem.h.
>> That would be more sensible than sticking a vmalloc related
>> function into
>> linux/mm.h where it does not belong...
>
> That is why I suggested include/linux/vmalloc.h as its home in the
> first
> place. And no, adding an include for linux/highmem.h (and asm/
> pgtable.h)
> to linux/vmalloc.h does not work.
I am sure Christoph can figure out somewhere that it will work.
After all, the code in that function already exists both as another
function and open coded in several places and compiles fine there...
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/
On Wed, 19 Sep 2007, Anton Altaparmakov wrote:
> There even is such a function already in mm/sparse.c::vaddr_in_vmalloc_area()
> with pretty identical content. I would suggest either this new inline should
> go away completely and use the existing one and export it or the existing one
> should go away and the inline should be used. It seems silly to have two
> functions with different names doing exactly the same thing!
Just in case you have not noticed: This patchset removes the
vaddr_in_vmalloc_area() in sparse.c
On Wed, 19 Sep 2007, Anton Altaparmakov wrote:
> I suspect that is_vmalloc_addr() should not be in linux/mm.h at all and should
> be in linux/vmalloc.h instead and vmalloc.h should include linux/highmem.h.
> That would be more sensible than sticking a vmalloc related function into
> linux/mm.h where it does not belong...
Tried it and that leads to all sort of other failures.
On 19 Sep 2007, at 18:29, Christoph Lameter wrote:
> On Wed, 19 Sep 2007, Anton Altaparmakov wrote:
>> There even is such a function already in mm/
>> sparse.c::vaddr_in_vmalloc_area()
>> with pretty identical content. I would suggest either this new
>> inline should
>> go away completely and use the existing one and export it or the
>> existing one
>> should go away and the inline should be used. It seems silly to
>> have two
>> functions with different names doing exactly the same thing!
>
> Just in case you have not noticed: This patchset removes the
> vaddr_in_vmalloc_area() in sparse.c
That's great! And sorry I did not notice that bit...
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/
On 19 Sep 2007, at 18:29, Christoph Lameter wrote:
> On Wed, 19 Sep 2007, Anton Altaparmakov wrote:
>> I suspect that is_vmalloc_addr() should not be in linux/mm.h at
>> all and should
>> be in linux/vmalloc.h instead and vmalloc.h should include linux/
>> highmem.h.
>> That would be more sensible than sticking a vmalloc related
>> function into
>> linux/mm.h where it does not belong...
>
> Tried it and that leads to all sort of other failures.
Ah, a new header file "vaddr.h" or something then?
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/