Re: [GIT PULL] x86: more header untangling

From: Ingo Molnar
Date: Wed Feb 11 2009 - 12:15:09 EST



* Jeremy Fitzhardinge <jeremy@xxxxxxxx> wrote:

> Ingo Molnar wrote:
>> The difference is this:
>>
>> - page*.h is for page frame definitions and general
>> address space layout details that derive from the page frame.
>>
>> - pgtable*.h is the pagetable hw format and all things related to it.
>>
>
> Right.
>
>> Sure, pgtable.h still has to inherit page.h (we cannot talk about a
>> page table without knowing about a page), but not the other way around.
>>
>> I.e. the practical difference is not to pgtable.h, but to page.h: we stop
>> polluting those places with pte_t/pteval_t/etc details that only need
>> page.h.
>>
>> I'm sure there will be .c code fallout from moving definitions like this,
>> but we'll fix those. The previous batch of changes from you stabilized
>> quickly (we needed only 3 build fixes), so this approach seems to scale
>> well so far.
>>
>> So please move those definitions to their logically consistent place and
>> dont worry about the build fallout.
>>
>
> OK.
>
>> Another thing:
>>
>>
>>> arch/x86/include/asm/page-defs.h | 155 ++++++++++++++++++++++
>>> arch/x86/include/asm/page.h | 146 +--------------------
>>> arch/x86/include/asm/page_32-defs.h | 92 +++++++++++++
>>> arch/x86/include/asm/page_32.h | 89 +------------
>>> arch/x86/include/asm/page_64-defs.h | 105 +++++++++++++++
>>> arch/x86/include/asm/page_64.h | 101 +--------------
>>> arch/x86/include/asm/paravirt.h | 2 +-
>>> arch/x86/include/asm/pgtable-defs.h | 227 +++++++++++++++++++++++++++++++
>>> arch/x86/include/asm/pgtable.h | 228 +-------------------------------
>>> arch/x86/include/asm/pgtable_32-defs.h | 46 +++++++
>>> arch/x86/include/asm/pgtable_32.h | 42 +------
>>> arch/x86/include/asm/pgtable_64-defs.h | 46 +++++++
>>> arch/x86/include/asm/pgtable_64.h | 48 +-------
>>> 13 files changed, 688 insertions(+), 639 deletions(-)
>>>
>>
>> The splitup looks good (sans the comment above), but could you please name
>> them page_types.h, pgtable_types.h, like we did it for other, cleaned up
>> headers like spinlock_types.h?
>>
>
> I considered it, but I went with -defs because 1) there are the existing
> -defs.h headers in this area, [...]

well, there's just 2 defs in this area, while there are 96 *types*.h headers in all
of Linux. So we should just move pgtable-2level-defs.h and pgtable-3level-defs.h to
the _types notation.

> [...] and 2) the define constants and small inlines as well as types.

constants can be considered data types too. Small inlines are borderlines,
they should generally not be in _types.h headers. Really, _types.h headers
are only there to instantiate a type, to enable dependent inline methods
to use them.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/