linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Andi Kleen <ak@suse.de>, Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org, jbeulich@novell.com,
	venkatesh.pallipadi@intel.com, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: CPA boot crash (was: [PATCH] [0/36] Great change_page_attr patch series v3)
Date: Tue, 22 Jan 2008 15:21:47 +0100	[thread overview]
Message-ID: <20080122142147.GD19936@wotan.suse.de> (raw)
In-Reply-To: <alpine.LFD.1.00.0801221439520.3199@apollo.tec.linutronix.de>

On Tue, Jan 22, 2008 at 03:06:00PM +0100, Thomas Gleixner wrote:
> On Tue, 22 Jan 2008, Andi Kleen wrote:
> 
> > > First priority is getting CPA and PAT consolidated before we put new
> > 
> > PAT seems to be still quite unstable and frankly for me it is 
> > unclear how long it will take to it become stable. It would
> > not surprise me if it takes longer than the .26 merge window.
> 
> Definitely, if we change the code further without doing anything to
> consolidate it in the first place. 
> 
> Have you even cared to look, why PAT is so ugly and fragile ? Simply

Well I was second generation hacker on the patchkit (after Eric B.) and
wrote quite some code in it, so yes I'm a little familiar with how it works... 

> because it interferes/interacts with CPA and the page table code. So

No that is not its main problem I believe. Main problem are 
all the driver and other subsystem interactions (it is a little
bit similar to power management where you have lots of little
bits all over right instead of a single big one). The actual
page table handling is the smallest issue and well understood
anyways.

gbpages on the other hand does not change the driver interaction
problem at all.

> adding further stuff to that area without considering the requirements
> of PAT will make it worse.

I don't think gbpages has much to do with how well PAT works or not.

It is just a different way to map the large areas of the direct mapping
that do not contain any mmio or aperture mappings. These areas
are not affected by PAT. By definition (in Linux) if PAT is active
for something there are no gbpages anymore.

PAT essentially only works on areas which are already split into
4K and the gbpages code does not come into play on those at all.

> > That seems to me like against your own principles -- simple stuff
> > first -- that you two harped on so extensively on earlier this thread.
> 
> Not at all. If the simple stuff makes it harder to do something else,

I don't think that's the case here.

> 
> If your patches are so simple, then they can be done on top of a
> consolidated CPA/PAT easily.

Sure they can -- i did that in fact with PAT only -- my worry is just that
there is no time frame when someone will actually produce 
working PAT and then consolidated CPA. So basically my relatively
simple (and imho not very intrusive) feature is queued behind two very 
complicated projects with unclear time frame and might
be delayed forever for those.

And the rationales I so far heard for this particular prioritization
were not very convincing to say the least. Frankly I suspect Ingo 
hadn't actually looked at the gbpage code really before coming up with
it and from your comments it doesn't sound like you did either.

> > But now that it is gone again anyways delaying the gbpages for it again
> > would be quite unfortunate from my perspective.
> 
> I can understand that, because it is in the way of your particular
> interests, but we have to look at the global picture and not at the
> personal preferences of you or anyone else.

According to you and Ingo "the global perspective" is to get
simple stuff first in. But in this case you're doing the complicated
(and worse the unfinished) stuff first which seems to be against
your own principles.

-Andi


  reply	other threads:[~2008-01-22 14:21 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-16 22:14 [PATCH] [0/36] Great change_page_attr patch series v3 Andi Kleen
2008-01-16 22:14 ` [PATCH] [1/36] Undo pat cpa patch Andi Kleen
2008-01-16 22:15 ` [PATCH] [2/36] Undo pageattr_32 portions of 11c9734cbcf4c5862260442a5d56dd4779799fcc Andi Kleen
2008-01-16 22:15 ` [PATCH] [3/36] Undo pageattr_64 parts of 4157e20af49a04d75a807e6d15b3e70c8e688ccc Andi Kleen
2008-01-16 22:15 ` [PATCH] [4/36] CPA: Undo white space changes Andi Kleen
2008-01-16 22:15 ` [PATCH] [5/36] CPA: Implement change_page_attr_addr entry point for i386 Andi Kleen
2008-01-16 22:15 ` [PATCH] [6/36] CPA Handle 4K split pages at boot on 64bit Andi Kleen
2008-01-16 22:15 ` [PATCH] [7/36] Shrink __PAGE_KERNEL/__PAGE_KERNEL_EXEC on non PAE kernels Andi Kleen
2008-01-16 22:15 ` [PATCH] [8/36] CPA: Do a simple self test at boot Andi Kleen
2008-01-16 22:15 ` [PATCH] [9/36] Add pte accessors for the global bit Andi Kleen
2008-01-16 22:15 ` [PATCH] [10/36] Add pte_pgprot on i386 Andi Kleen
2008-01-16 22:15 ` [PATCH] [11/36] Don't drop NX bit in pte modifier functions for 32bit Andi Kleen
2008-01-16 22:15 ` [PATCH] [12/36] Extract page table dumping code from i386 fault handler into dump_pagetable() Andi Kleen
2008-01-16 22:15 ` [PATCH] [13/36] CPA: Return the page table level in lookup_address() Andi Kleen
2008-01-16 22:15 ` [PATCH] [14/36] CPA: Add simple self test at boot Andi Kleen
2008-01-16 22:15 ` [PATCH] [15/36] CPA: Change kernel_map_pages to not use c_p_a() Andi Kleen
2008-01-16 22:15 ` [PATCH] [16/36] CPA: Change 32bit back to init_mm semaphore locking Andi Kleen
2008-01-16 22:15 ` [PATCH] [17/36] CPA: CLFLUSH support in change_page_attr() Andi Kleen
2008-01-16 22:15 ` [PATCH] [18/36] CPA: Use macros to modify the PG_arch_1 page flags in change_page_attr Andi Kleen
2008-01-16 22:15 ` [PATCH] [19/36] CPA: Use page granuality TLB flushing " Andi Kleen
2008-01-16 22:15 ` [PATCH] [20/36] CPA: Don't flush the caches when the CPU supports self-snoop Andi Kleen
2008-01-16 22:15 ` [PATCH] [21/36] CPA: Use wbinvd() macro instead of inline assembly in 64bit c_p_a() Andi Kleen
2008-01-16 22:15 ` [PATCH] [22/36] CPA: Reorder TLB / cache flushes to follow Intel recommendation Andi Kleen
2008-01-16 22:15 ` [PATCH] [23/36] CPA: Make change_page_attr() more robust against use of PAT bits Andi Kleen
2008-01-16 22:15 ` [PATCH] [24/36] CPA: Limit cache flushing to pages that really change caching Andi Kleen
2008-01-16 22:15 ` [PATCH] [25/36] CPA: Fix inaccurate comments in 64bit change_page_attr() Andi Kleen
2008-01-16 22:15 ` [PATCH] [26/36] CPA: Dump pagetable when inconsistency is detected Andi Kleen
2008-01-16 22:15 ` [PATCH] [27/36] CPA: Only queue actually unused page table pages for freeing Andi Kleen
2008-01-16 22:15 ` [PATCH] [28/36] CPA: Remove unnecessary masking of address Andi Kleen
2008-01-16 22:15 ` [PATCH] [29/36] CPA: Only unmap kernel init pages in text mapping when CONFIG_DEBUG_RODATA is set Andi Kleen
2008-01-16 22:15 ` [PATCH] [30/36] CPA: Always do full TLB flush when splitting large pages Andi Kleen
2008-01-16 22:15 ` [PATCH] [31/36] CPA: Fix reference counting when changing already changed pages Andi Kleen
2008-01-16 22:15 ` [PATCH] [32/36] CPA: Change comments of external interfaces to kerneldoc format Andi Kleen
2008-01-16 22:15 ` [PATCH] [33/36] CPA: Make kernel_text test match boot mapping initialization Andi Kleen
2008-01-16 22:15 ` [PATCH] [34/36] CPA: Add a BUG_ON checking for someone setting the kernel text NX Andi Kleen
2008-01-16 22:15 ` [PATCH] [35/36] Remove set_kernel_exec Andi Kleen
2008-01-16 22:15 ` [PATCH] [36/36] Clean up pte_exec Andi Kleen
2008-01-18  9:56 ` [PATCH] [0/36] Great change_page_attr patch series v3 Ingo Molnar
2008-01-18 15:33   ` CPA boot crash (was: [PATCH] [0/36] Great change_page_attr patch series v3) Ingo Molnar
2008-01-18 15:38     ` Ingo Molnar
2008-01-18 15:56     ` Ingo Molnar
2008-01-18 16:01       ` Andi Kleen
2008-01-18 16:05         ` CPA boot crash (was: [PATCH] [0/36] Great change_page_attr patch series v3) II Andi Kleen
2008-01-18 16:07         ` CPA boot crash (was: [PATCH] [0/36] Great change_page_attr patch series v3) Ingo Molnar
2008-01-18 16:16           ` Andi Kleen
2008-01-18 16:21             ` Ingo Molnar
2008-01-18 16:34               ` Andi Kleen
2008-01-18 16:48                 ` Ingo Molnar
2008-01-18 17:19                   ` Andi Kleen
2008-01-21 16:40                     ` Ingo Molnar
2008-01-21 17:13                       ` Andi Kleen
2008-01-22 13:12                         ` Thomas Gleixner
2008-01-22 13:23                           ` Andi Kleen
2008-01-22 14:06                             ` Thomas Gleixner
2008-01-22 14:21                               ` Andi Kleen [this message]
2008-01-23  0:00                                 ` Ingo Molnar
2008-01-23  9:05                                   ` Andi Kleen
2008-01-23  0:35                                 ` Thomas Gleixner
2008-01-23  8:09                                   ` Andi Kleen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080122142147.GD19936@wotan.suse.de \
    --to=ak@suse.de \
    --cc=hpa@zytor.com \
    --cc=jbeulich@novell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=venkatesh.pallipadi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).