Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1665857 > unrolled thread

[PATCH 1/3] x86/mm: Provide pmdp_mknotpresent() helper

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/3] x86/mm: Provide pmdp_mknotpresent() helper "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2017-06-14 16:00 +0200
    Re: [PATCH 1/3] x86/mm: Provide pmdp_mknotpresent() helper Andrea Arcangeli <aarcange@redhat.com> - 2017-06-14 18:10 +0200
    Re: [PATCH 1/3] x86/mm: Provide pmdp_mknotpresent() helper kbuild test robot <lkp@intel.com> - 2017-06-15 06:50 +0200

#1665857 — [PATCH 1/3] x86/mm: Provide pmdp_mknotpresent() helper

We need an atomic way to make pmd page table entry not-present.
This is required to implement pmdp_invalidate() that doesn't loose dirty
or access bits.

On x86, we need to clear two bits -- _PAGE_PRESENT and _PAGE_PROTNONE --
to make the entry non-present. The implementation uses cmpxchg() loop to
make it atomically.

PAE requires special treatment to avoid expensive cmpxchg8b(). Both
bits are in the lower part of the entry, so we can use 4-byte cmpxchg() on
this part of page table entry.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/pgtable-3level.h | 17 +++++++++++++++++
 arch/x86/include/asm/pgtable.h        | 13 +++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/arch/x86/include/asm/pgtable-3level.h b/arch/x86/include/asm/pgtable-3level.h
index 50d35e3185f5..b6efa955ecd0 100644
--- a/arch/x86/include/asm/pgtable-3level.h
+++ b/arch/x86/include/asm/pgtable-3level.h
@@ -176,8 +176,25 @@ static inline pmd_t native_pmdp_get_and_clear(pmd_t *pmdp)
 
 	return res.pmd;
 }
+
+#define pmdp_mknotpresent pmdp_mknotpresent
+static inline void pmdp_mknotpresent(pmd_t *pmdp)
+{
+	union split_pmd *p, old, new;
+
+	p = (union split_pmd *)pmdp;
+	{
+		old = *p;
+		new.pmd = pmd_mknotpresent(old.pmd);
+	} while (cmpxchg(&p->pmd_low, old.pmd_low, new.pmd_low) != old.pmd_low);
+}
 #else
 #define native_pmdp_get_and_clear(xp) native_local_pmdp_get_and_clear(xp)
+
+static inline void pmdp_mknotpresent(pmd_t *pmdp)
+{
+	*pmdp = pmd_mknotpresent(*pmdp);
+}
 #endif
 
 #ifdef CONFIG_SMP
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index f5af95a0c6b8..576420df12b8 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -1092,6 +1092,19 @@ static inline void pmdp_set_wrprotect(struct mm_struct *mm,
 	clear_bit(_PAGE_BIT_RW, (unsigned long *)pmdp);
 }
 
+#ifndef pmdp_mknotpresent
+#define pmdp_mknotpresent pmdp_mknotpresent
+static inline void pmdp_mknotpresent(pmd_t *pmdp)
+{
+	pmd_t old, new;
+
+	{
+		old = *pmdp;
+		new = pmd_mknotpresent(old);
+	} while (pmd_val(cmpxchg(pmdp, old, new)) != pmd_val(old));
+}
+#endif
+
 /*
  * clone_pgd_range(pgd_t *dst, pgd_t *src, int count);
  *
-- 
2.11.0

[toc] | [next] | [standalone]

#1666028

On Wed, Jun 14, 2017 at 04:51:41PM +0300, Kirill A. Shutemov wrote:
> We need an atomic way to make pmd page table entry not-present.
> This is required to implement pmdp_invalidate() that doesn't loose dirty
> or access bits.

What does the cmpxchg() loop achieves compared to xchg() and then
return the old value (potentially with the dirty bit set when it was
not before we called xchg)?

> index f5af95a0c6b8..576420df12b8 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -1092,6 +1092,19 @@ static inline void pmdp_set_wrprotect(struct mm_struct *mm,
>  	clear_bit(_PAGE_BIT_RW, (unsigned long *)pmdp);
>  }
>  
> +#ifndef pmdp_mknotpresent
> +#define pmdp_mknotpresent pmdp_mknotpresent
> +static inline void pmdp_mknotpresent(pmd_t *pmdp)
> +{
> +	pmd_t old, new;
> +
> +	{
> +		old = *pmdp;
> +		new = pmd_mknotpresent(old);
> +	} while (pmd_val(cmpxchg(pmdp, old, new)) != pmd_val(old));
> +}
> +#endif

Isn't it faster to do xchg(&xp->pmd, pmd_mknotpresent(pmd)) and have
the pmdp_invalidate caller can set the dirty bit in the page if it was
found set in the returned old pmd value (and skip the loop and cmpxchg)?

Thanks,
Andrea

[toc] | [prev] | [next] | [standalone]

#1666450

[Multipart message — attachments visible in raw view] — view raw

Hi Kirill,

[auto build test ERROR on mmotm/master]
[also build test ERROR on v4.12-rc5 next-20170614]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kirill-A-Shutemov/Do-not-loose-dirty-bit-on-THP-pages/20170615-115540
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: i386-randconfig-x071-06130444 (attached as .config)
compiler: gcc-6 (Debian 6.3.0-18) 6.3.0 20170516
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/mm.h:70:0,
                    from include/linux/memcontrol.h:29,
                    from include/linux/swap.h:8,
                    from include/linux/suspend.h:4,
                    from arch/x86/kernel/asm-offsets.c:12:
>> arch/x86/include/asm/pgtable.h:1096:27: error: redefinition of 'pmdp_mknotpresent'
    #define pmdp_mknotpresent pmdp_mknotpresent
                              ^
>> arch/x86/include/asm/pgtable.h:1097:20: note: in expansion of macro 'pmdp_mknotpresent'
    static inline void pmdp_mknotpresent(pmd_t *pmdp)
                       ^~~~~~~~~~~~~~~~~
   In file included from arch/x86/include/asm/pgtable_32.h:43:0,
                    from arch/x86/include/asm/pgtable.h:604,
                    from include/linux/mm.h:70,
                    from include/linux/memcontrol.h:29,
                    from include/linux/swap.h:8,
                    from include/linux/suspend.h:4,
                    from arch/x86/kernel/asm-offsets.c:12:
   arch/x86/include/asm/pgtable-3level.h:194:20: note: previous definition of 'pmdp_mknotpresent' was here
    static inline void pmdp_mknotpresent(pmd_t *pmdp)
                       ^~~~~~~~~~~~~~~~~
   make[2]: *** [arch/x86/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [sub-make] Error 2

vim +/pmdp_mknotpresent +1096 arch/x86/include/asm/pgtable.h

  1090					      unsigned long addr, pmd_t *pmdp)
  1091	{
  1092		clear_bit(_PAGE_BIT_RW, (unsigned long *)pmdp);
  1093	}
  1094	
  1095	#ifndef pmdp_mknotpresent
> 1096	#define pmdp_mknotpresent pmdp_mknotpresent
> 1097	static inline void pmdp_mknotpresent(pmd_t *pmdp)
  1098	{
  1099		pmd_t old, new;
  1100	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web