The interface of flush_cache_page

The interface of flush_cache_page

[NIIBE Yutaka]

Currently, the interface is like this:

void flush_cache_page(struct vm_area_struct *vma, unsigned long address)

This doesn't work well for (virtuall indexed) physically tagged
architecture.

When it is called from vmscan.c:try_to_swap_out, as the PTE is cleared
to be zero, we have no way to know what phisical address to match.

How about adding argument: struct page *page?  With that, we have
information of physical memory.
-- 

Re: The interface of flush_cache_page

[David S. Miller]

NIIBE Yutaka writes:
 > When it is called from vmscan.c:try_to_swap_out, as the PTE is cleared
 > to be zero, we have no way to know what phisical address to match.

That is really an error, and it is only because the last time the
logic try_to_swap_out() logic got rearranges the cache flush got moved
lower down.

In fact, several architectures will take a fatal trap due to
this sequence.  On these systems the tlb must be able to translate the
virtual address given to it for the flush, and that translation must
be valid.

Thus, the code there should be:

	flush_cache_page(vma, address);
	pte = ptep_get_and_clear(page_table);
	flush_tlb_page(vma, address);

And the flush_cache_page() further down in that function then can be
removed.

Feel free to send this fix to Linus.  It is probably causing
HyperSparc sparc32 to fail to work at all once a swap happens,
if platforms using that chip work at all.

Later,
David S. Miller
davem@redhat.com

Re: The interface of flush_cache_page

[NIIBE Yutaka]

Hello Linus, 

Attached is a patch to move the call of flush_cache_page before
clearing PTE in vmscan.c:try_to_swap_out.

Here, we want to flush cache entries written from user space.
But for some archtecture, we need valid PTE to flush, so, we need to
call flush_cache_page before ptep_get_and_clear.

At least, we need this for SuperH (which has virtually indexed
physically tagged cache).

--- v2.4.8-pre4/mm/vmscan.c	Sat Aug  4 15:37:55 2001
+++ linux/mm/vmscan.c	Sat Aug  4 19:27:07 2001
@@ -65,6 +65,7 @@ static void try_to_swap_out(struct mm_st
 	 * is needed on CPUs which update the accessed and dirty
 	 * bits in hardware.
 	 */
+	flush_cache_page(vma, address);
 	pte = ptep_get_and_clear(page_table);
 	flush_tlb_page(vma, address);
 
@@ -102,7 +103,6 @@ drop_pte:
 	 * Basically, this just makes it possible for us to do
 	 * some real work in the future in "refill_inactive()".
 	 */
-	flush_cache_page(vma, address);
 	if (!pte_dirty(pte))
 		goto drop_pte;
 
--------
 
David S. Miller wrote:
 > NIIBE Yutaka writes:
 >  > When it is called from vmscan.c:try_to_swap_out, as the PTE is cleared
 >  > to be zero, we have no way to know what phisical address to match.
 > 
 > That is really an error, and it is only because the last time the
 > logic try_to_swap_out() logic got rearranges the cache flush got moved
 > lower down.
 > 
 > In fact, several architectures will take a fatal trap due to
 > this sequence.  On these systems the tlb must be able to translate the
 > virtual address given to it for the flush, and that translation must
 > be valid.
 > 
 > Thus, the code there should be:
 > 
 > 	flush_cache_page(vma, address);
 > 	pte = ptep_get_and_clear(page_table);
 > 	flush_tlb_page(vma, address);
 > 
 > And the flush_cache_page() further down in that function then can be
 > removed.
 > 
 > Feel free to send this fix to Linus.  It is probably causing
 > HyperSparc sparc32 to fail to work at all once a swap happens,
 > if platforms using that chip work at all.
--