LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Arjan van de Ven <arjan@linux.intel.com>
Cc: "Siddha, Suresh B" <suresh.b.siddha@intel.com>,
	tglx@linutronix.de, linux-kernel@vger.kernel.org
Subject: Re: issue with patch "x86: no CPA on iounmap"
Date: Tue, 5 Feb 2008 08:05:35 +0100	[thread overview]
Message-ID: <20080205070535.GA10695@elte.hu> (raw)
In-Reply-To: <47A7EA88.3020404@linux.intel.com>


* Arjan van de Ven <arjan@linux.intel.com> wrote:

> Siddha, Suresh B wrote:
>> This is wrt to x86 git commit f56d005d30342a45d8af2b75ecccc82200f09600
>> 	"x86: no CPA on iounmap"
>>
>> This can use performance issue. When a GART driver unmaps a RAM page,
>
> thinking about this some more...
>
> afaik the gart driver doesn't use ioremap....
>
> (and it does caching control explicitly, and sets its pages back to 
> cached)

there are many GART drivers, and the method used depends on the GART 
driver. The following GART drivers still use ioremap in one way or 
another:

 drivers/char/agp/amd-k7-agp.c
 drivers/char/agp/ati-agp.c
 drivers/char/agp/generic.c
 drivers/char/agp/sworks-agp.c
 drivers/char/drm/radeon_cp.c

the method use is in all cases the same: they use __get_free_page() to 
pick up a general RAM page, they do SetPageReserved() and then they use 
ioremap_nocache() to map it non-cached, and then they also program the 
GART to access those pages.

when the GART code deinits, it does an iounmap() on those pages, unmaps 
it from the GART hardware itself, does a ClearPageReserved() and does 
__free_page() to put the page into the general page pool again. So 
Suresh is right: these pages are currently marked UC at this point and 
we need to mark them cacheable.

we could do this automatically in iounmap() upon seeing a page_is_ram() 
that has PageReserved set. Or we could stick in a set_memory_wb() into 
the deinit [and ioremap_nocache()-failure] sequence.

Since we treat PageReserved pages specially in ioremap() already [we 
allow them, despite them being listed in the e820 map], i think the more 
robust solution is to recognize them in iounmap() as well - this way it 
cannot be forgotten accidentally. (and UC pages in the buddy are _hard_ 
to notice after the fact) There is no aliasing danger i believe: IO bars 
should never be marked as general RAM in the e820.

	Ingo

  reply	other threads:[~2008-02-05  7:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-05  1:13 Siddha, Suresh B
2008-02-05  4:41 ` Arjan van de Ven
2008-02-05  4:48 ` Arjan van de Ven
2008-02-05  7:05   ` Ingo Molnar [this message]
2008-02-05  7:14     ` Arjan van de Ven
2008-02-07 20:02     ` Siddha, Suresh B

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=20080205070535.GA10695@elte.hu \
    --to=mingo@elte.hu \
    --cc=arjan@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=suresh.b.siddha@intel.com \
    --cc=tglx@linutronix.de \
    --subject='Re: issue with patch "x86: no CPA on iounmap"' \
    /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

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