LKML Archive on
help / color / mirror / Atom feed
From: Nick Piggin <>
To: Hugh Dickins <>
Cc: Mike Stroyan <>,
	Andrew Morton <>,
	Rohit Seth <>,
	"Luck, Tony" <>,,
Subject: Re: Fw: [PATCH] ia64: race flushing icache in do_no_page path
Date: Sat, 28 Apr 2007 12:16:04 +1000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hugh Dickins wrote:
> On Fri, 27 Apr 2007, Nick Piggin wrote:
>>But that's because of ia64's cache coherency implementation. I don't really
>>follow the documentation to know whether it should be one way or the other,
>>but surely it should be done either before or after the set_pte_at, not both.
>>Anyway, how about fremap or mprotect, for example?
>>OK, I'm still not sure that I understand why lazy_mmu_prot_update should be
>>used rather than flush_icache_page (in concept, not ia64 implementation).
>>Sure, flush_icache_page isn't given the pte, but let's assume we can change
> You're asking lots of good questions.  I wish the ia64 people would
> know the answers, but from the length of time the "lazy_mmu_prot_update"
> stuff took to get into the tree, and the length of time it's taken to be
> found defective, I suspect they don't, and we'll have to guess for them.
> Some guesses I'm working with...
> I presume Mike and Anil are correct, that it needs to be done before
> putting pte into page table, not left until after: but as you've
> guessed, that needs to be done everywhere, not just in the two
> places so far identified.
> When it was discussed last year (in connection with Peter's page
> cleaning patches) it was thought to be a variant of update_mmu_cache()
> (after setting pte), and we added the fremap one to accompany it;
> but now it looks to be a variant of flush_icache_page() (before
> setting pte).

Right. I think.

> I believe lazy_mmu_prot_update(pteval) came into existence primarily
> for mprotect's change_pte_range() case.  If ia64 filled in its
> flush_icache_page(vma, page), that could have been used there
> (checking 'vm_flags & VM_EXEC' instead of pte_exec): but that would
> involve a relatively expensive(?) pte_page() in a place which doesn't
> need to know the struct page for other cases.

Well, I think we could always add a pte argument to flush_icache_page...
Then, there might be logic to have a flush_lazy_icache_page when
changing protections, but that operation (currently called
lazy_mmu_prot_update) really doesn't seem like it should be called in
all the other places that it is, flush_icache_page should be used for

But AFAIKS, if we really want correctness, flush_icache_page should go
away and be implemented in flush_dcache_page.

> Well, not pte_page(), it needs to be vm_normal_page() doesn't it?
> and ia64's current lazy_mmu_prot_update is unsafe when !pfn_valid.
> Some flush_icache_pages are already in place, others are not: do
> we need to add some?  But those architectures which have a non-empty
> flush_icache_page seem to have survived without the additional calls
> - so they might be unnecessarily slowed down by additional calls.

Well flush_icache seems to be intended solely to bring icache in sync
with dcache modifications, but they try to skimp out on most of the
flushes required to handle dcache aliases... but really, I don't think
that is possible to do 100% correctly.

SUSE Labs, Novell Inc.

  parent reply	other threads:[~2007-04-28  2:16 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <>
2007-04-26  7:53 ` Nick Piggin
2007-04-26 17:35   ` Mike Stroyan
2007-04-27 11:55     ` Nick Piggin
2007-04-27 14:18       ` Hugh Dickins
2007-04-27 17:02         ` David Mosberger-Tang
2007-04-28  1:31         ` Rohit Seth
2007-04-28  5:34           ` Hugh Dickins
2007-04-28 18:17             ` Rohit Seth
2007-05-01 11:52               ` Nick Piggin
2007-05-02  0:36                 ` Rohit Seth
2007-05-02  2:05                   ` Nick Piggin
2007-04-28  2:16         ` Nick Piggin [this message]
2007-04-28  1:24       ` Rohit Seth
2007-04-28  2:00         ` Nick Piggin
2007-04-28  3:04           ` Nick Piggin
2007-04-28  5:20             ` Hugh Dickins
2007-04-28  6:03               ` Nick Piggin
2007-04-28 18:30                 ` Rohit Seth
2007-05-01 11:47                   ` Nick Piggin
2007-05-02  0:36                     ` Rohit Seth
2007-04-28 18:05               ` Rohit Seth
2007-05-01 11:43                 ` Nick Piggin
2007-05-04 21:32                   ` Mike Stroyan
2007-04-28  4:11           ` Nick Piggin
2007-04-28 17:57           ` Rohit Seth
2007-05-01 11:39             ` Nick Piggin
2007-05-02  0:36               ` Rohit Seth
2007-05-02  1:57                 ` Nick Piggin
2007-07-04 14:24 Zoltan Menyhart
2007-07-04 16:58 ` KAMEZAWA Hiroyuki
2007-07-05  8:57   ` Zoltan Menyhart
2007-07-05 17:36     ` Mike Stroyan

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \
    --subject='Re: Fw: [PATCH] ia64: race flushing icache in do_no_page path' \

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