Re: [PATCH 4/4] MIPS: Sync icache & dcache in set_pte_at

[Ralf Baechle]

On Tue, Mar 01, 2016 at 05:19:40PM +0000, Paul Burton wrote:

> > >+static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
> > >+			      pte_t *ptep, pte_t pteval)
> > >+{
> > >+	extern void __update_cache(unsigned long address, pte_t pte);
> > >+
> > >+	if (!pte_present(pteval))
> > >+		goto cache_sync_done;
> > >+
> > >+	if (pte_present(*ptep) && (pte_pfn(*ptep) == pte_pfn(pteval)))
> > >+		goto cache_sync_done;
> > >+
> > >+	__update_cache(addr, pteval);
> > >+cache_sync_done:
> > >+	set_pte(ptep, pteval);
> > >+}
> > >+
> > 
> > This seems crazy.
> 
> Perhaps, but also correct...
> 
> > I don't think any other architecture does this type of work in set_pte_at().
> 
> Yes they do. As mentioned in the commit message see arm, ia64 or powerpc
> for architectures that all do the same sort of thing in set_pte_at.
> 
> > Can you look into finding a better way?
> 
> Not that I can see.

FYI, this is the original commit message adding set_pte_at().  The commit
predates Linus' git history but is in the various history trees, for example
https://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/?id=ae3d0a847f4b38812241e4a5dc3371965c752a8c

Or for your convenience:

commit ae3d0a847f4b38812241e4a5dc3371965c752a8c
Author: David S. Miller <davem@xxxxxxxxxxxxxxxxxx>
Date:   Tue Feb 22 23:42:56 2005 -0800

    [MM]: Add set_pte_at() which takes 'mm' and 'addr' args.
    
    I'm taking a slightly different approach this time around so things
    are easier to integrate.  Here is the first patch which builds the
    infrastructure.  Basically:
    
    1) Add set_pte_at() which is set_pte() with 'mm' and 'addr' arguments
       added.  All generic code uses set_pte_at().
    
       Most platforms simply get this define:
        #define set_pte_at(mm,addr,ptep,pteval) set_pte(ptep,pteval)
    
       I chose this method over simply changing all set_pte() call sites
       because many platforms implement this in assembler and it would
       take forever to preserve the build and stabilize things if modifying
       that was necessary.
    
       Soon, with platform maintainer's help, we can kill of set_pte() entirely.
       To be honest, there are only a handful of set_pte() call sites in the
       arch specific code.
    
       Actually, in this patch ppc64 is completely set_pte() free and does not
       define it.
    
    2) pte_clear() gets 'mm' and 'addr' arguments now.
       This had a cascading effect on many ptep_test_and_*() routines.  Specifically:
       a) ptep_test_and_clear_{young,dirty}() now take 'vma' and 'address' args.
       b) ptep_get_and_clear now take 'mm' and 'address' args.
       c) ptep_mkdirty was deleted, unused by any code.
       d) ptep_set_wrprotect now takes 'mm' and 'address' args.
    
    I've tested this patch as follows:
    
    1) compile and run tested on sparc64/SMP
    2) compile tested on:
       a) ppc64/SMP
       b) i386 both with and without PAE enabled
    
    Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>

Which doesn't specify if the function is meant for cache management nor
is it documented in Documentation/cachetlb.txt.

  Ralf