[GIT PULL] x86: more header untangling

[GIT PULL] x86: more header untangling

[Jeremy Fitzhardinge]

Hi Ingo,

I updated my x86/untangle branch with a merge of tip/x86/headers, which 
needed a couple of little conflict resolutions.  Basically this series 
splits out all the simple definitions into -defs.h headers, which in 
turn only have very simple dependencies (either on other -defs.h 
headers, or core generic headers like linux/types.h).  It should go some 
way to making it easier to simplify other header's dependencies.

BTW, I've been trying to work out what the real distinction between 
asm/page*.h and asm/pgtable*.h is.  pgtable*.h obviously has stuff that 
specifically relates to the details of the page table structures, entry 
formats, bits, and so on, whereas page*.h has more general things like 
page sizes, the layout of the kernel's physical and virtual address 
spaces, etc.  So it seems like an anomaly that its page*.h which ends up 
defining all the pte_t/pteval_t/etc types and their associated 
functions.  We could move all those into pgtable*.h, but pgtable*.h 
would still depend on page*.h for things like PTE_PFN_MASK, etc, so I 
don't think it would make any practical difference.

Thanks,
    J

The following changes since commit c47c1b1f3a9d6973108020df1dcab7604f7774dd:
  Ingo Molnar (1):
        x86, pgtable.h: fix 2-level 32-bit build

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git x86/untangle

Jeremy Fitzhardinge (12):
      Include linux/mmzone to avoid cyclic dependency
      Revert "Include linux/mmzone to avoid cyclic dependency"
      Split pgtable.h into pgtable-defs.h and pgtable.h
      Split pgtable_32.h into pgtable_32.h and pgtable_32-defs.h
      Split pgtable_64.h into pgtable_64-defs.h and pgtable_64.h
      Include pgtable_32|64-defs.h in pgtable-defs.h
      create -defs.h counterparts for page*.h
      x86: move 2 and 3 level asm-generic defs into page-defs
      x86: move defs around to allow paravirt.h to just include page-defs.h
      define pud_flags and pud_large properly to allow non-PAE builds
      Merge remote branch 'tip/x86/headers' into x86/untangle
      x86: reinstate lost pud_large()

 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(-)
 create mode 100644 arch/x86/include/asm/page-defs.h
 create mode 100644 arch/x86/include/asm/page_32-defs.h
 create mode 100644 arch/x86/include/asm/page_64-defs.h
 create mode 100644 arch/x86/include/asm/pgtable-defs.h
 create mode 100644 arch/x86/include/asm/pgtable_32-defs.h
 create mode 100644 arch/x86/include/asm/pgtable_64-defs.h

Re: [GIT PULL] x86: more header untangling

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Hi Ingo,
>
> I updated my x86/untangle branch with a merge of tip/x86/headers, which  
> needed a couple of little conflict resolutions.  Basically this series  
> splits out all the simple definitions into -defs.h headers, which in  
> turn only have very simple dependencies (either on other -defs.h  
> headers, or core generic headers like linux/types.h).  It should go some  
> way to making it easier to simplify other header's dependencies.
>
> BTW, I've been trying to work out what the real distinction between  
> asm/page*.h and asm/pgtable*.h is.  pgtable*.h obviously has stuff that  
> specifically relates to the details of the page table structures, entry  
> formats, bits, and so on, whereas page*.h has more general things like  
> page sizes, the layout of the kernel's physical and virtual address  
> spaces, etc.  So it seems like an anomaly that its page*.h which ends up  
> defining all the pte_t/pteval_t/etc types and their associated  
> functions.  We could move all those into pgtable*.h, but pgtable*.h  
> would still depend on page*.h for things like PTE_PFN_MASK, etc, so I  
> don't think it would make any practical difference.

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.

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.

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?

	Ingo

Re: [GIT PULL] x86: more header untangling

[Jeremy Fitzhardinge]

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, and 2) the define constants and small 
inlines as well as types.

    J

Re: [GIT PULL] x86: more header untangling

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> 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

Re: [GIT PULL] x86: more header untangling

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> constants can be considered data types too.

Huh, that's a pretty broad definition of "type", to the degree that's 
its fairly counter-intuitive and misleading.  But I don't care that much.

>  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.
>   

In this case the inlines are the accessor functions to do the pte_t <-> 
pteval_t (un)wrapping.  They're trivial and have no dependencies apart 
from the types they're right next to.

Anyway, check out 
git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git 
x86/untangle now.  I had to merge in tip/x86/paravirt to get the 
pte_flags changes I made there, and unfortunately it didn't merge 
completely cleanly, so there's probably some spurious changes in there.  
I guess I can respin it into a clean branch.

    J

Re: [GIT PULL] x86: more header untangling

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Ingo Molnar wrote:
>> constants can be considered data types too.
>
> Huh, that's a pretty broad definition of "type", to the degree that's  
> its fairly counter-intuitive and misleading.  But I don't care that much.

Well, it's a stretch, but constants of a specific type are a lot closer to
the notion of 'type' than to the notion of 'function/method/code'.

The problem we are trying to solve here is dependency hell.

Dependencies get generated by methods, which are functions/operators defined
over multiple type-spaces. For example:

   static inline void mm_init_owner(struct mm_struct *mm, struct task_struct *p)

Is a (mathematical) function defined over the:

   (struct mm_struct x struct task_struct) 

two-dimensional type space.

The problem that this inline causes is that it couples two, otherwise
largely independent type spaces: 'struct mm_struct' and 'struct task_struct'.

Given a high enough count of such random couplings, and given the fact that any
two subsystems will have some method that connects them, it can be seen that
to be able to define arbitrary inline methods, we need to include pretty much all 
headers into 'super-headers' like sched.h or mm.h.

Pure types are simple constructs: they only depend on the particular types they
embedd. They dont depend on the types that happen to embedd them.

Same goes for constants: they are of a specific type, and hence are more similar
to the 'type' notion than the 'method' notion.

So regardless of how we call them, we want constants to be near the types they are
related to. They do not cause dependency hell, hence they can be in the _types.h
headers.

Another possibility would be to make a further distinction between 'local methods' 
and 'compound methods'. Local methods are the ones that only relate to a given
data type. Compound methods combine multiple types. We could allow local methods in 
type headers, and forbid compound methods.

	Ingo

Re: [GIT PULL] x86: more header untangling

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> Another possibility would be to make a further distinction between 'local methods' 
> and 'compound methods'. Local methods are the ones that only relate to a given
> data type. Compound methods combine multiple types. We could allow local methods in 
> type headers, and forbid compound methods.
>   

Yes, that's pretty much the approach I've been taking.  Particularly 
since these little helper functions are smoothing over a given type 
being defined in different ways in different environments (the types 
themselves are different, but the types of the functions operating on 
them are the same).

    J

Re: [GIT PULL] x86: more header untangling

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Ingo Molnar wrote:
>> Another possibility would be to make a further distinction between 
>> 'local methods' and 'compound methods'. Local methods are the ones that 
>> only relate to a given
>> data type. Compound methods combine multiple types. We could allow 
>> local methods in type headers, and forbid compound methods.
>>   
>
> Yes, that's pretty much the approach I've been taking.  Particularly  
> since these little helper functions are smoothing over a given type  
> being defined in different ways in different environments (the types  
> themselves are different, but the types of the functions operating on  
> them are the same).

ok, cool, no disagreements then. Your point about the type being
standardized by the helpers is important, i missed that aspect.

	Ingo