public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arjan van de Ven <arjan@linux.intel.com>
To: "Thomas Hellström" <thomas@tungstengraphics.com>
Cc: Andi Kleen <andi@firstfloor.org>,
	Dave Airlie <airlied@redhat.com>,
	linux-kernel@vger.kernel.org, tglx@linutronix.de,
	mingo@redhat.com
Subject: Re: [PATCH] x86: create array based interface to change page attribute
Date: Wed, 02 Apr 2008 07:01:33 -0700	[thread overview]
Message-ID: <47F391BD.1060306@linux.intel.com> (raw)
In-Reply-To: <47F32EDD.1080306@tungstengraphics.com>

Thomas Hellström wrote:
> Arjan van de Ven wrote:
>> Thomas Hellström wrote:
>>
>>> But what mappings are there,  immediately after alloc_page(), that 
>>> set_memory_np won't catch? 
>>
>> For example on x86 64bit, the kernel text is mapped (to allow 
>> relocatable) in another
>> space as well.. and to allow 2Mb tlbs for this, that second mapping is 
>> bigger than it strictly needs to be.
>> So your alloc_page() could get a page from the free pool that comes 
>> from the pages that have a second mapping
>> due to this rounding.
>> (and more fun, since it's close to frequently accessed memory, the hw 
>> prefetchers may actually just decide to pull
>> such pages into the cache preemptively)
>>
>>
>>> Drivers relying on set_memory_uc touching all mappings the driver 
>>> hasn't set up itself must then have the same problem and needs to be 
>>> fixed; referring in particular to agpgart for which driver the old 
>>> CPA functionality was once created, IIRC.
>>
>> "uc" is different than "np"; for "uc" the implementation, if your cpu 
>> needs this, will fix
>> up the shadow text mappings already today.
>>
>> for "present" this doesn't make sense, since no cpu needs this.
> If this is the checkalias() thingy in x86/pageattr.c, it looks like it 
> doesn't handle np different from the uc case. It does obviously skip NX 
> bit manipulations, though.
> 
> Anyway, a more direct question: If we were to fix whatever's missing for 
> the code to fixup the x86-64 shadow text mapping, would you be opposing 
> this way 

I do really oppose set_memory_np/p to work on anything but the mapping it is handled.
that _uc and co touch other mappings is a cpu specific property and an implementation
detail to the API. If/When CPUs exist that don't have issues with aliases, those CPUs
will not change the other mappings.

 > to fix the long standing uc/wc aliasing issue, provided we

I'm not opposed to a real fix. I am opposed to a bad hack.

> don't hit any other problems, like clflush() refusing to run on an NP page?

yes clflush doesn't work on not present pages ;)

  reply	other threads:[~2008-04-02 14:03 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-31  5:19 [PATCH] x86: create array based interface to change page attribute Dave Airlie
2008-03-31  6:54 ` Thomas Hellström
2008-03-31  9:33   ` Arjan van de Ven
2008-03-31 11:04     ` Thomas Hellström
2008-03-31  7:25 ` Andi Kleen
2008-03-31  7:55   ` Thomas Hellström
2008-03-31  8:38     ` Andi Kleen
2008-03-31  9:06       ` Thomas Hellström
2008-03-31  9:18         ` Andi Kleen
2008-03-31 11:10           ` Thomas Hellström
2008-03-31 16:08             ` Arjan van de Ven
2008-03-31 16:41               ` Thomas Hellström
2008-03-31 16:49                 ` Arjan van de Ven
2008-03-31 17:26                   ` Thomas Hellström
2008-04-01 20:58                     ` Arjan van de Ven
2008-04-01 21:29                       ` Thomas Hellström
2008-04-01 22:30                         ` Arjan van de Ven
2008-04-02  6:30                           ` Thomas Hellström
2008-04-02  6:35                             ` Arjan van de Ven
2008-04-02  6:59                               ` Thomas Hellström
2008-04-02 14:01                                 ` Arjan van de Ven [this message]
2008-04-02 17:57                                   ` Thomas Hellström
2008-04-07 18:23                                     ` Jesse Barnes
2008-04-07 19:51                                       ` Thomas Hellström
2008-04-07 19:59                                         ` Jesse Barnes
2008-04-07 20:46                                           ` Thomas Hellström
2008-04-07 20:57                                             ` Arjan van de Ven
2008-04-08  6:12                                               ` Thomas Hellström
2008-04-07 21:04                                             ` Jesse Barnes
2008-04-08  6:21                                               ` Thomas Hellström
2008-04-08 14:27                                                 ` Jesse Barnes
2008-04-07 20:56                                           ` Arjan van de Ven
2008-04-07 21:02                                             ` Jesse Barnes
2008-04-07 21:09                                               ` Jesse Barnes
2008-03-31  9:56   ` Arjan van de Ven
2008-03-31 11:21   ` Dave Airlie
2008-03-31 11:46     ` Andi Kleen
2008-04-02  1:35       ` Dave Airlie
2008-04-01 18:20 ` Arjan van de Ven

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=47F391BD.1060306@linux.intel.com \
    --to=arjan@linux.intel.com \
    --cc=airlied@redhat.com \
    --cc=andi@firstfloor.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=thomas@tungstengraphics.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