All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zoltan Menyhart <Zoltan.Menyhart@bull.net>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: "tony.luck@intel.com" <tony.luck@intel.com>,
	linux-ia64@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	nickpiggin@yahoo.com.au
Subject: Re: [PATCH] flush icache before set_pte on ia64 take3
Date: Thu, 26 Jul 2007 15:13:52 +0000	[thread overview]
Message-ID: <46A8BA30.7000508@bull.net> (raw)
In-Reply-To: <20070726171323.a9c17eda.kamezawa.hiroyu@jp.fujitsu.com>

I do not think changing the semantics of flush_cache_page()
is a good idea. Please have a look at Documentation/cachetlb.txt.
This is for virtual address tagged caches.

The doc. does not say much about flush_icache_page().
It is used in:

install_page(): can release any previously existing mapping
		(similarly: for virtual address tagged I-cache)

On the other hand, I do not see why it is used in these two
cases, I think the virtual address tagged I-cache must have been
flushed when the old PTE of the page was removed:

do_no_page():
	if (pte_none(*page_table)) {
		flush_icache_page(vma, new_page);

do_swap_page():
	flush_icache_page(vma, page);

Anyway, it is a NO-OP on the machines with physical address
tagged caches.

Sure, the names could have been chosen more carefully.

Unless you have a very strong reason, do not change their
semantics (would require updates for the other architectures).

I can agree:
- lazy_mmu_prot_update() is not the right place for a flush
- to flush I-cache before inserting PTE (at least for the
  machines with physical address tagged caches)

Can we have a separate, specific exec_flush_phtag_icache_page()
that is to be called before inserting a PTE with the exec bit on
for physical address tagged I-caches) ?
It is defined in asm-generic/cacheflush.h as NO-OP.

Or exec_flush_icache_page_before_set_pte() ?

Thanks,

Zoltan


WARNING: multiple messages have this Message-ID (diff)
From: Zoltan Menyhart <Zoltan.Menyhart@bull.net>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: "tony.luck@intel.com" <tony.luck@intel.com>,
	linux-ia64@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	nickpiggin@yahoo.com.au
Subject: Re: [PATCH] flush icache before set_pte on ia64 take3
Date: Thu, 26 Jul 2007 17:13:52 +0200	[thread overview]
Message-ID: <46A8BA30.7000508@bull.net> (raw)
In-Reply-To: <20070726171323.a9c17eda.kamezawa.hiroyu@jp.fujitsu.com>

I do not think changing the semantics of flush_cache_page()
is a good idea. Please have a look at Documentation/cachetlb.txt.
This is for virtual address tagged caches.

The doc. does not say much about flush_icache_page().
It is used in:

install_page(): can release any previously existing mapping
		(similarly: for virtual address tagged I-cache)

On the other hand, I do not see why it is used in these two
cases, I think the virtual address tagged I-cache must have been
flushed when the old PTE of the page was removed:

do_no_page():
	if (pte_none(*page_table)) {
		flush_icache_page(vma, new_page);

do_swap_page():
	flush_icache_page(vma, page);

Anyway, it is a NO-OP on the machines with physical address
tagged caches.

Sure, the names could have been chosen more carefully.

Unless you have a very strong reason, do not change their
semantics (would require updates for the other architectures).

I can agree:
- lazy_mmu_prot_update() is not the right place for a flush
- to flush I-cache before inserting PTE (at least for the
  machines with physical address tagged caches)

Can we have a separate, specific exec_flush_phtag_icache_page()
that is to be called before inserting a PTE with the exec bit on
for physical address tagged I-caches) ?
It is defined in asm-generic/cacheflush.h as NO-OP.

Or exec_flush_icache_page_before_set_pte() ?

Thanks,

Zoltan


  parent reply	other threads:[~2007-07-26 15:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-26  8:13 [PATCH] flush icache before set_pte on ia64 take3 KAMEZAWA Hiroyuki
2007-07-26  8:13 ` KAMEZAWA Hiroyuki
2007-07-26  8:27 ` KAMEZAWA Hiroyuki
2007-07-26  8:27   ` KAMEZAWA Hiroyuki
2007-07-26 15:13 ` Zoltan Menyhart [this message]
2007-07-26 15:13   ` Zoltan Menyhart
2007-07-26 16:08   ` KAMEZAWA Hiroyuki
2007-07-26 16:08     ` KAMEZAWA Hiroyuki
2007-07-26 16:25     ` Zoltan Menyhart
2007-07-26 16:25       ` Zoltan Menyhart
2007-07-26 15:56 ` Race condition around flush_cache_page() ? Zoltan Menyhart

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=46A8BA30.7000508@bull.net \
    --to=zoltan.menyhart@bull.net \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nickpiggin@yahoo.com.au \
    --cc=tony.luck@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.