Re: [XenPPC] [PATCH] [RFC] Xencomm patch to fix modules

[Hollis Blanchard <hollisb@xxxxxxxxxx>]

On Thu, 2007-01-18 at 16:18 -0500, Jimi Xenidis wrote:
> On Jan 18, 2007, at 1:55 PM, Hollis Blanchard wrote:
> 
> > On Thu, 2007-01-18 at 12:17 -0500, Jimi Xenidis wrote:
> >> I agree with most of Hollis's comments, but have some of my own.
> >>
> >> First, I do not think that the implementation of
> is_phys_contiguous()
> >> answers the question in its name and IMNSHO is bogus.  Perhaps
> >> something like:
> >>    mm/sparse.c vaddr_in_vmalloc_area 232 static int
> >> vaddr_in_vmalloc_area(void *addr)
> >>
> >> And use if (!vaddr_in_vmalloc_area)
> >
> > The name was my suggestion. It should be commented, but think about
> > it:
> > we don't care if something is vmalloc or not. We care if it's
> > physically
> > contiguous or not, so I strongly believe that should be the name of
> > the
> > test.
> 
> I'm not big on functions that do not implement what the name says it
> does.

It does exactly what the name says it does: it returns 0 if the area is
known to be physically discontiguous, and 1 otherwise.

It's called "malloc", not "allocate_from_buddy" or
"allocate_first_from_bitmap". That's because you can provide any
implementation of the interface, and it can change at any time, and when
it does change, you don't need to change the callers.

> However, the worst that can happen is a false-negative, (unless it is
> an ioremap() address which would be other problems).
> 
> Hey, wouldn't virt_addr_valid() do?

I can pass a perfectly "valid" virtual address that is within a
physically discontiguous area of memory, and this function would return
0.

-- 
Hollis Blanchard
IBM Linux Technology Center


_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel