linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Andi Kleen <ak@suse.de>
Cc: linux-kernel@vger.kernel.org, tglx@linutronix.de,
	jbeulich@novell.com, venkatesh.pallipadi@intel.com
Subject: Re: [PATCH] [0/36] Great change_page_attr patch series v3
Date: Fri, 18 Jan 2008 10:56:27 +0100	[thread overview]
Message-ID: <20080118095627.GB30637@elte.hu> (raw)
In-Reply-To: <200801161114.239449000@suse.de>


* Andi Kleen <ak@suse.de> wrote:

> Changes to previous versions: 
> - Ported to the latest git-x86 including the PAT patchkit
> This undoes some changes in the PAT patches and reimplements them
> in a different way. End result should be equivalent, but this
> made it easier for me to merge the patches.
> - Fix NX bit handling (I think even after Jeremy's fixes it was
> still not completely right) 
> - Minor fixes based on feedback

thanks Andi for porting your CPA queue ontop of PAT. Now that PAT 
support is getting into shape i've test-merged your CPA series to 
x86.git.

v2.6.25 merging of CPA is still somewhat in limbo but worst-case i think 
we can still get away with just doing wbinvd instead of clflush and get 
rid of most of the risks that way. Could you please add a boot option 
and Kconfig option that does that? Something like "noclflush" and a 
.config option to achieve the same - just like we do for PAT.

We've got way too much stuff going on at the moment - and the PAT bits 
are more fundamental and more important than nice but non-essential 
optimizations like CPA. There's still a lot of cruft all around this 
area.

One thing, you undid a cleanup patch:

|  Subject: CPA: Undo white space changes
|  From: Andi Kleen <ak@suse.de>
|
|  Undo random white space changes. This reverts
|  ddb53b5735793a19dc17bcd98b050f672

this is perfectly fine as we do not want to make your merging harder via 
cleanups, as long as you redo the cleanups after your series. Your new 
code is pretty ugly to look at, and this very much shows in the 
checkpatch metrics too:

                                       errors   lines of code   errors/KLOC
 arch/x86/mm/pageattr_32.c                 29             419          69.2
 arch/x86/mm/pageattr_64.c                 31             384          80.7

prior the undo it was:

                                       errors   lines of code   errors/KLOC
 arch/x86/mm/pageattr_32.c                  0             294             0
 arch/x86/mm/pageattr_64.c                  0             275             0

please restore that cleanliness state. Thanks,

	Ingo

  parent reply	other threads:[~2008-01-18  9:56 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 ` Ingo Molnar [this message]
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
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=20080118095627.GB30637@elte.hu \
    --to=mingo@elte.hu \
    --cc=ak@suse.de \
    --cc=jbeulich@novell.com \
    --cc=linux-kernel@vger.kernel.org \
    --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).