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
mm/sparse.c vaddr_in_vmalloc_area 232 static int
And use if (!vaddr_in_vmalloc_area)
The name was my suggestion. It should be commented, but think about
we don't care if something is vmalloc or not. We care if it's
contiguous or not, so I strongly believe that should be the name of
I'm not big on functions that do not implement what the name says it
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?
oops this last line slipped by.. I wrote the email over multiple
More importantly... (and this needs to be addressed before the patch
I like the "map" name change, but not sure about "early".
What is your request? Just renaming it?
My point is the "early" is not the only reason mini was used.
There are 2 reasons why we had mini/inline/early:
1) because the allocator was not ready, I think this applies to a
small number of hcalls
2) we cannot "sleep" (in_interrupt()) and the allocator sleeps,
Mainly evtchn related and console.
So early covers (1) but (2) will be problematic, I noted the ones
below that may reflect (2), I for one, have not been diligent in
commenting why mini/inline is actually used and I think we need to do
What, add a comment describing a code path that may change in the
future? I would object to that.
Was that sarcasm?
My request is that ,at a minimum, everything that was inline/mini
must be _early because we cannot alloc anything while in_interrupt().
Ultimately, we call the right allocator at the right time.
And since we have interfaces to detect both early and in_interrupt we
can probably bring this down to a single inteligent xencomm_map() call.
So giving this some more thought, and jerking you around even more
(sorry), aside from using inline to optimize this:
- We can detect (1) with slab_is_available() and use mini
- We can detect (2) with in_interrupt() and try GFP_ATOMIC then
Not a request, but something to think about.
What *is* your request then, the one that needs to be addressed?
Mini, has always bugged me, it seems to me that this could be
satisfied by a per_cpu static, or preallocated buffer to for the xen
descriptor. This may not be viable because it assumes no interrupts,
and though for asynchronous interrupts this may be a safe assumption,
if one were to suffer a series of page faults (from a wild pointer in
this path) we would have a silent hang, which is never good.
So mini has always bugged you, but you agree there's no better way
it and are just thinking out loud?
Xen-ppc-devel mailing list