Messages in this thread |  | | | Subject | Re: Fw: [PATCH] ia64: race flushing icache in do_no_page path | | From | Rohit Seth <> | | Date | Fri, 27 Apr 2007 18:31:03 -0700 |
| |
On Fri, 2007-04-27 at 15:18 +0100, Hugh Dickins wrote: > 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. >
That sounds about right. Before installing new mapping, kernel should ensure there are no stale contents in caches or TLB. lazy_mmu_prot_update needs to be called whenever the permissions on pte (about to) change. So if remapping is causing change in protection then lazy_mmu_prot_update needs to be called.
> 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). > > I believe lazy_mmu_prot_update(pteval) came into existence primarily > for mprotect's change_pte_range() case.
Yup.
> 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, 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. >
Right. Extra flush_icache_page routines will add cost to archs that have non-null definition of this routine. BTW, isn't flush_icache_page marked for deprecation?
> I believe that was the secondary reason for lazy_mmu_prot_update(), > perhaps better called ia64_flush_icache_page(): to allow calls to > be added where ia64 was (mistakenly) thought to want them, without > needing a protracted audit of how other architectures might be > impacted. >
lazy_mmu_prot_update was added specifically for notifying change in protection. So, in a way it is closer to update_mmu_cache (Which is for change in mappings itself). Though for ia64 implementation, this ends up flushing the icaches when needed.
Hopefully my reply is useful.
-rohit
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |