Messages in this thread Patch in this message | | | Date | Thu, 2 Jul 2009 11:40:27 +0100 | From | Ralf Baechle <> | Subject | Re: [PATCH] Fix virt_to_phys() warnings |
| |
On Wed, Jul 01, 2009 at 10:46:27PM -0700, Andrew Morton wrote:
> > mm/page_alloc.c: In function 'alloc_pages_exact': > > mm/page_alloc.c:1986: warning: passing argument 1 of 'virt_to_phys' makes pointer from integer without a cast > > > > drivers/usb/mon/mon_bin.c: In function 'mon_alloc_buff': > > drivers/usb/mon/mon_bin.c:1264: warning: passing argument 1 of 'virt_to_phys' makes pointer from integer without a cast > > > > Signed-off-by: Kevin Cernekee <cernekee@gmail.com> > > --- > > diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c > > index f8d9045..0f7a30b 100644 > > --- a/drivers/usb/mon/mon_bin.c > > +++ b/drivers/usb/mon/mon_bin.c > > @@ -1261,7 +1261,7 @@ static int mon_alloc_buff(struct mon_pgmap *map, int npages) > > return -ENOMEM; > > } > > map[n].ptr = (unsigned char *) vaddr; > > - map[n].pg = virt_to_page(vaddr); > > + map[n].pg = virt_to_page((void *) vaddr); > > } > > return 0; > > } > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index 5d714f8..f6180db 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -1983,7 +1983,7 @@ void *alloc_pages_exact(size_t size, gfp_t gfp_mask) > > unsigned long alloc_end = addr + (PAGE_SIZE << order); > > unsigned long used = addr + PAGE_ALIGN(size); > > > > - split_page(virt_to_page(addr), order); > > + split_page(virt_to_page((void *)addr), order); > > while (used < alloc_end) { > > free_page(used); > > used += PAGE_SIZE; > > The virt_to_foo() functions were written back in the Linux dark ages and > they're horrid. A byzantine mixture of macros and typecasts which make > it really hard to work out what type their argument actually should be.
No disagreement there.
> virt_to_page() takes an argument of type `unsigned long'. (except for > the include/asm-generic/page.h version which takes any damn type at > all).
You meant it takes a pointer argument, for example:
mm/page_alloc.c: __free_pages(virt_to_page((void *)addr), order); mm/slob.c: static inline struct slob_page *slob_page(const void *addr) { return (struct slob_page *)virt_to_page(addr); }
> virt_to_phys() takes an argument of `const void *' (weird, huh?)
volatile void * seems to be the most common type. MIPS is the only arch that accepts volatile const void *; Alpha uses a plain void *.
I don't recall why MIPS uses volatile const - probably to silence warnings about discarding qualifiers from pointer target type.
> But arch/mips/include/asm/page.h does > > #define virt_to_page(kaddr) pfn_to_page(PFN_DOWN(virt_to_phys(kaddr))) > > thereby passing an `unsigned long' where a void* was expected. > > > So perhaps something along these lines: > > > --- a/arch/mips/include/asm/page.h~a > +++ a/arch/mips/include/asm/page.h > @@ -184,7 +184,11 @@ typedef struct { unsigned long pgprot; } > > #endif > > -#define virt_to_page(kaddr) pfn_to_page(PFN_DOWN(virt_to_phys(kaddr))) > +static inline struct page *virt_to_page(unsigned long kaddr) > +{ > + return pfn_to_page(PFN_DOWN(virt_to_phys((void *)kaddr))); > +} > +
Doesn't compile because pfn_to_page is defined in <asm-generic/memory_model.h> which will only be included further down and moving the order of the order of things around to get this to build is getting ugly, so I gave up on it.
Anyway, since virt_to_page and virt_to_phys (imho :-) take the same argument times I'm in favor of Kevin's patch.
Ralf
| |