Re: [PATCH 01 of 31] define MADV_HUGEPAGE

[Pekka Enberg <penberg@cs.helsinki.fi>]

On Wed, Jan 27, 2010 at 6:37 PM, Hugh Dickins
<hugh.dickins@tiscali.co.uk> wrote:
> On Tue, 26 Jan 2010, Andrea Arcangeli wrote:
>
>> From: Andrea Arcangeli <aarcange@redhat.com>
>>
>> Define MADV_HUGEPAGE.
>>
>> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
>> ---
>>
>> diff --git a/include/asm-generic/mman-common.h b/include/asm-generic/mman-common.h
>> --- a/include/asm-generic/mman-common.h
>> +++ b/include/asm-generic/mman-common.h
>> @@ -45,6 +45,8 @@
>>  #define MADV_MERGEABLE   12          /* KSM may merge identical pages */
>>  #define MADV_UNMERGEABLE 13          /* KSM may not merge identical pages */
>>
>> +#define MADV_HUGEPAGE        15              /* Worth backing with hugepages */
>> +
>>  /* compatibility flags */
>>  #define MAP_FILE     0
>
> It embarrasses me to find the time to comment on so trivial a patch,
> and none more interesting; but I have to say that I don't think this
> patch can be right - in two ways.
>
> You moved MADV_HUGEPAGE from 14 to 15 because someone noticed
> #define MADV_16K_PAGES 14 in arch/parisc/include/asm/mman.h?
>
> Well, if we need to respect that, then we ought to respect its
> /* The range 12-64 is reserved for page size specification. */:
> 15 would be intended for 32K pages.
>
> I don't know why parisc (even as far back as 2.4.0) wants those
> definitions: I guess to allow some peculiar-to-parisc program
> to build on Linux, yet fail with EINVAL when it runs?  I rather
> think they should never have been added (and perhaps could even
> be removed).
>
> But, whether 14 or 15 or something else, I expect you're preventing
> mm/madvise.c from building on alpha, mips, parisc and xtensa.
> Those arches don't include asm-generic/mman-common.h, because of
> various divergencies, of which MADV_16K_PAGES would be just one.
>
> So I think you should follow what we did with MADV_MERGEABLE:
> define it in asm-generic/mman-common.h and the four arches,
> use the expected number 14 wherever you can, and 67 for parisc.
>
> Or if you feel there's virtue in using the same number on all
> arches (it would be less confusing, yes) and want to pave that way
> (as we'd have better done with MADV_MERGEABLE), add a comment into
> four of those files to point to parisc's peculiar group, and use
> the same number 67 on all (perhaps via an asm-generic/madv-common.h).
>
> I'd take the lazy way out and follow what we did with MADV_MERGEABLE,
> unless Arnd (Mr Asm-Generic) would prefer something else.

Hey, lets bring some PARISC people to the party! Maybe MADV_16K_PAGES
can be fixed in a nice way?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>