Thread (32 messages) 32 messages, 6 authors, 2007-05-23

Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2007-05-22 22:59:08
Also in: linux-mm, sparclinux

Looks pretty good to me.

There was a minor build error in x86 (see below), and ia64 is missing
(again see below).  I've now built and am running this on x86, x86_64
and powerpc64; but I'm very unlikely to be doing anything which
actually tickles these changes, or Andrea's original handle_pte_fault
optimization.
Ok.
Would the "__changed && __dirty" architectures (x86, x86_64, ia64)
be better off saying __changed = __dirty && pte_same?  I doubt it's
worth bothering about.
I'd say let gcc figure it out :-)
You've updated do_wp_page to do "if (ptep_set_access_flags(...",
but not updated set_huge_ptep_writable in the same way: I'd have
thought you'd either leave both alone, or update them both: any
reason for one not the other?  But again, not really an issue.
Nah, I must have missed set_huge_ptep_writable(). I don't think the wp
code path matters much anyway, it's likely to always be different.
These changes came about because the sun4c needs to update_mmu_cache
even in the pte_same case: might it also need to flush_tlb_page then?
Well, I don't know which is why I'm waiting for Tom Callaway to test.
Davem mentioned update_mmu_cache only though when we discussed the
problem initially.
quoted
 #define  __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
 #define ptep_set_access_flags(vma, address, ptep, entry, dirty)		\
-do {									\
-	if (dirty) {							\
+({									\
+	int __changed = !pte_same(*(__ptep), __entry);			\
That just needs to be:

  +	int __changed = !pte_same(*(ptep), entry);			\
Ah yes, sorry about that. I need to setup an x86 toolchain somewhere :-)
Here's what I think the ia64 hunk would be, unbuilt and untested.
Ok.

I'll respin a patch later today.

Cheers,
Ben.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help