Re: [patch] mspec driver for 2.6.12-rc2-mm3

[Christoph Hellwig <hch@infradead.org>]

> >> int count; /* Number of pages allocated. */ + int type; /* Type of
> >> pages allocated. */ + unsigned long maddr[1]; /* Array of MSPEC
> >> addresses. */
> 
> Christoph> dito
> 
> The code use the size to calculate, it could be changed either way,
> don't think it's worth making the change.

The current code is obsufcated, see the pages-1 stuff and co.
Please change it.

> >> + /* + * The kernel requires a page structure to be returned upon +
> >> * success, but there are no page structures for low granule pages.
> >> + * remap_page_range() creates the pte for us and we return a + *
> >> bogus page back to the kernel fault handler to keep it happy + *
> >> (the page is freed immediately there).  + */
> 
> Christoph> Please don't use the ->nopage approach thenm but do
> Christoph> remap_pfn_range beforehand.  ->nopage is very nice if the
> Christoph> region is actually backed by normal RAM, but what you're
> Christoph> doing doesn't make much sense.
> 
> Thats what I used to think, however you want the node-local setup for
> performance reasons. Otherwise I would have switched to remap_pfn_range.

Then fixup remap_pfn_range (or rather add a new _node variant).  The
current code relies on deep magic to work and could be broken by a new
kernel release easily.

> >> +/* + * Walk the EFI memory map to pull out leftover pages in the
> >> lower + * memory regions which do not end up in the regular memory
> >> map and + * stick them into the uncached allocator + */ +static
> >> void __init +mspec_walk_efi_memmap_uc (void)
> 
> Christoph> I'm pretty sure this was NACKed on the ia64 list, and SGI
> Christoph> was told to do a more generic efi memmap walk.
> 
> No the issue back then was that the driver just took the memory and
> kept it to itself. The new approach exports it for other users.

That comment doesn't make sense at all to me.  exports what to what other
users.  And through what way.  Please bring this issue up on the ia64
list again.  (also please post this patch to linux-ia64, too)


Jes, is it just me or are you trying to chicken out on all the real
problems? :-)