lkml.org 
[lkml]   [2005]   [Oct]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/9] mm: parisc pte atomicity
    On Sat, Oct 22, 2005 at 05:23:27PM +0100, Hugh Dickins wrote:
    > There's a worrying function translation_exists in parisc cacheflush.h,
    > unaffected by split ptlock since flush_dcache_page is using it on some
    > other mm, without any relevant lock. Oh well, make it a slightly more
    > robust by factoring the pfn check within it. And it looked liable to
    > confuse a camouflaged swap or file entry with a good pte: fix that too.

    I have to say I really don't understand VM at all. cc'ing the
    parisc-linux list in case anyone there has a better understanding than I
    do.

    > Signed-off-by: Hugh Dickins <hugh@veritas.com>
    > ---
    >
    > arch/parisc/kernel/cache.c | 24 +++++++++---------------
    > include/asm-parisc/cacheflush.h | 35 +++++++++++++++++++----------------
    > 2 files changed, 28 insertions(+), 31 deletions(-)
    >
    > --- mm2/arch/parisc/kernel/cache.c 2005-03-02 07:38:56.000000000 +0000
    > +++ mm3/arch/parisc/kernel/cache.c 2005-10-22 14:06:30.000000000 +0100
    > @@ -266,7 +266,6 @@ void flush_dcache_page(struct page *page
    > unsigned long offset;
    > unsigned long addr;
    > pgoff_t pgoff;
    > - pte_t *pte;
    > unsigned long pfn = page_to_pfn(page);
    >
    >
    > @@ -297,21 +296,16 @@ void flush_dcache_page(struct page *page
    > * taking a page fault if the pte doesn't exist.
    > * This is just for speed. If the page translation
    > * isn't there, there's no point exciting the
    > - * nadtlb handler into a nullification frenzy */
    > -
    > -
    > - if(!(pte = translation_exists(mpnt, addr)))
    > - continue;
    > -
    > - /* make sure we really have this page: the private
    > + * nadtlb handler into a nullification frenzy.
    > + *
    > + * Make sure we really have this page: the private
    > * mappings may cover this area but have COW'd this
    > - * particular page */
    > - if(pte_pfn(*pte) != pfn)
    > - continue;
    > -
    > - __flush_cache_page(mpnt, addr);
    > -
    > - break;
    > + * particular page.
    > + */
    > + if (translation_exists(mpnt, addr, pfn)) {
    > + __flush_cache_page(mpnt, addr);
    > + break;
    > + }
    > }
    > flush_dcache_mmap_unlock(mapping);
    > }
    > --- mm2/include/asm-parisc/cacheflush.h 2005-10-11 12:07:49.000000000 +0100
    > +++ mm3/include/asm-parisc/cacheflush.h 2005-10-22 14:06:30.000000000 +0100
    > @@ -100,30 +100,34 @@ static inline void flush_cache_range(str
    >
    > /* Simple function to work out if we have an existing address translation
    > * for a user space vma. */
    > -static inline pte_t *__translation_exists(struct mm_struct *mm,
    > - unsigned long addr)
    > +static inline int translation_exists(struct vm_area_struct *vma,
    > + unsigned long addr, unsigned long pfn)
    > {
    > - pgd_t *pgd = pgd_offset(mm, addr);
    > + pgd_t *pgd = pgd_offset(vma->vm_mm, addr);
    > pmd_t *pmd;
    > - pte_t *pte;
    > + pte_t pte;
    >
    > if(pgd_none(*pgd))
    > - return NULL;
    > + return 0;
    >
    > pmd = pmd_offset(pgd, addr);
    > if(pmd_none(*pmd) || pmd_bad(*pmd))
    > - return NULL;
    > + return 0;
    >
    > - pte = pte_offset_map(pmd, addr);
    > + /* We cannot take the pte lock here: flush_cache_page is usually
    > + * called with pte lock already held. Whereas flush_dcache_page
    > + * takes flush_dcache_mmap_lock, which is lower in the hierarchy:
    > + * the vma itself is secure, but the pte might come or go racily.
    > + */
    > + pte = *pte_offset_map(pmd, addr);
    > + /* But pte_unmap() does nothing on this architecture */
    > +
    > + /* Filter out coincidental file entries and swap entries */
    > + if (!(pte_val(pte) & (_PAGE_FLUSH|_PAGE_PRESENT)))
    > + return 0;
    >
    > - /* The PA flush mappings show up as pte_none, but they're
    > - * valid none the less */
    > - if(pte_none(*pte) && ((pte_val(*pte) & _PAGE_FLUSH) == 0))
    > - return NULL;
    > - return pte;
    > + return pte_pfn(pte) == pfn;
    > }
    > -#define translation_exists(vma, addr) __translation_exists((vma)->vm_mm, addr)
    > -
    >
    > /* Private function to flush a page from the cache of a non-current
    > * process. cr25 contains the Page Directory of the current user
    > @@ -175,9 +179,8 @@ flush_cache_page(struct vm_area_struct *
    > {
    > BUG_ON(!vma->vm_mm->context);
    >
    > - if(likely(translation_exists(vma, vmaddr)))
    > + if (likely(translation_exists(vma, vmaddr, pfn)))
    > __flush_cache_page(vma, vmaddr);
    >
    > }
    > #endif
    > -
    -
    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/

    \
     
     \ /
      Last update: 2005-10-22 18:36    [W:0.028 / U:61.612 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site