All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hanging swapoff with HAVE_ARCH_SOFT_DIRTY=y
@ 2015-09-17  8:58 Martin Schwidefsky
  2015-09-17  8:58 ` [PATCH] mm/swapfile: fix swapoff vs. software dirty bits Martin Schwidefsky
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Schwidefsky @ 2015-09-17  8:58 UTC (permalink / raw)
  To: Cyrill Gorcunov, Andrew Morton, Ingo Molnar, Michal Hocko
  Cc: linux-mm, Martin Schwidefsky

Greetings,

while implementing software dirty bits for s390 we noticed that the swapoff
command at shutdown caused the system to hang. After some debugging I found
the maybe_same_pte() function to be the cause of this.

The bug shows up for any configuration with CONFIG_HAVE_ARCH_SOFT_DIRTY=y
and CONFIG_MEM_SOFT_DIRTY=n. Currently this affects x86_64 only.

Martin Schwidefsky (1):
  mm/swapfile: fix swapoff vs. software dirty bits

 mm/swapfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
1.9.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-17  8:58 [PATCH] hanging swapoff with HAVE_ARCH_SOFT_DIRTY=y Martin Schwidefsky
@ 2015-09-17  8:58 ` Martin Schwidefsky
  2015-09-17  9:53   ` Cyrill Gorcunov
  2015-09-17 19:31   ` Cyrill Gorcunov
  0 siblings, 2 replies; 16+ messages in thread
From: Martin Schwidefsky @ 2015-09-17  8:58 UTC (permalink / raw)
  To: Cyrill Gorcunov, Andrew Morton, Ingo Molnar, Michal Hocko
  Cc: linux-mm, Martin Schwidefsky

Fixes a regression introduced with commit 179ef71cbc085252
"mm: save soft-dirty bits on swapped pages"

The maybe_same_pte() function is used to match a swap pte independent
of the swap software dirty bit set with pte_swp_mksoft_dirty().

For CONFIG_HAVE_ARCH_SOFT_DIRTY=y but CONFIG_MEM_SOFT_DIRTY=n the
software dirty bit may be set but maybe_same_pte() will not recognize
a software dirty swap pte. Due to this a 'swapoff -a' will hang.

The straightforward solution is to replace CONFIG_MEM_SOFT_DIRTY
with HAVE_ARCH_SOFT_DIRTY in maybe_same_pte().

Cc: linux-mm@kvack.org
Cc: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.cz>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
 mm/swapfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 5887731..bf7da58 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1113,7 +1113,7 @@ unsigned int count_swap_pages(int type, int free)
 
 static inline int maybe_same_pte(pte_t pte, pte_t swp_pte)
 {
-#ifdef CONFIG_MEM_SOFT_DIRTY
+#ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
 	/*
 	 * When pte keeps soft dirty bit the pte generated
 	 * from swap entry does not has it, still it's same
-- 
1.9.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-17  8:58 ` [PATCH] mm/swapfile: fix swapoff vs. software dirty bits Martin Schwidefsky
@ 2015-09-17  9:53   ` Cyrill Gorcunov
  2015-09-17 19:31   ` Cyrill Gorcunov
  1 sibling, 0 replies; 16+ messages in thread
From: Cyrill Gorcunov @ 2015-09-17  9:53 UTC (permalink / raw)
  To: Martin Schwidefsky; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Thu, Sep 17, 2015 at 10:58:59AM +0200, Martin Schwidefsky wrote:
> Fixes a regression introduced with commit 179ef71cbc085252
> "mm: save soft-dirty bits on swapped pages"
> 
> The maybe_same_pte() function is used to match a swap pte independent
> of the swap software dirty bit set with pte_swp_mksoft_dirty().
> 
> For CONFIG_HAVE_ARCH_SOFT_DIRTY=y but CONFIG_MEM_SOFT_DIRTY=n the
> software dirty bit may be set but maybe_same_pte() will not recognize
> a software dirty swap pte. Due to this a 'swapoff -a' will hang.
> 
> The straightforward solution is to replace CONFIG_MEM_SOFT_DIRTY
> with HAVE_ARCH_SOFT_DIRTY in maybe_same_pte().
> 
> Cc: linux-mm@kvack.org
> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.cz>
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

Thanks a huge, Martin! I'll take a look today.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-17  8:58 ` [PATCH] mm/swapfile: fix swapoff vs. software dirty bits Martin Schwidefsky
  2015-09-17  9:53   ` Cyrill Gorcunov
@ 2015-09-17 19:31   ` Cyrill Gorcunov
  2015-09-18  6:58     ` Martin Schwidefsky
  1 sibling, 1 reply; 16+ messages in thread
From: Cyrill Gorcunov @ 2015-09-17 19:31 UTC (permalink / raw)
  To: Martin Schwidefsky; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Thu, Sep 17, 2015 at 10:58:59AM +0200, Martin Schwidefsky wrote:
> Fixes a regression introduced with commit 179ef71cbc085252
> "mm: save soft-dirty bits on swapped pages"
> 
> The maybe_same_pte() function is used to match a swap pte independent
> of the swap software dirty bit set with pte_swp_mksoft_dirty().
> 
> For CONFIG_HAVE_ARCH_SOFT_DIRTY=y but CONFIG_MEM_SOFT_DIRTY=n the
> software dirty bit may be set but maybe_same_pte() will not recognize
> a software dirty swap pte. Due to this a 'swapoff -a' will hang.
> 
> The straightforward solution is to replace CONFIG_MEM_SOFT_DIRTY
> with HAVE_ARCH_SOFT_DIRTY in maybe_same_pte().
> 
> Cc: linux-mm@kvack.org
> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.cz>
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
> ---
>  mm/swapfile.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 5887731..bf7da58 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1113,7 +1113,7 @@ unsigned int count_swap_pages(int type, int free)
>  
>  static inline int maybe_same_pte(pte_t pte, pte_t swp_pte)
>  {
> -#ifdef CONFIG_MEM_SOFT_DIRTY
> +#ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
>  	/*
>  	 * When pte keeps soft dirty bit the pte generated
>  	 * from swap entry does not has it, still it's same

You know, I seem to miss how this might help. If CONFIG_MEM_SOFT_DIRTY=n
then all related helpers are nop'ed.

In particular in the commit you mentioned

+static inline int maybe_same_pte(pte_t pte, pte_t swp_pte)
+{
+#ifdef CONFIG_MEM_SOFT_DIRTY
+       /*
+        * When pte keeps soft dirty bit the pte generated
+        * from swap entry does not has it, still it's same
+        * pte from logical point of view.
+        */
+       pte_t swp_pte_dirty = pte_swp_mksoft_dirty(swp_pte);
+       return pte_same(pte, swp_pte) || pte_same(pte, swp_pte_dirty);
+#else
+       return pte_same(pte, swp_pte);
+#endif
+}
+
...
@@ -892,7 +907,7 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
        }
 
        pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
-       if (unlikely(!pte_same(*pte, swp_entry_to_pte(entry)))) {
+       if (unlikely(!maybe_same_pte(*pte, swp_entry_to_pte(entry)))) {
                mem_cgroup_cancel_charge_swapin(memcg);
                ret = 0;
                goto out;
@@ -947,7 +962,7 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
                 * swapoff spends a _lot_ of time in this loop!
                 * Test inline before going to call unuse_pte.
                 */
-               if (unlikely(pte_same(*pte, swp_pte))) {
+               if (unlikely(maybe_same_pte(*pte, swp_pte))) {
                        pte_unmap(pte);
                        ret = unuse_pte(vma, pmd, addr, entry, page);
                        if (ret)

Thus when CONFIG_MEM_SOFT_DIRTY = n, the unuse_pte will be the same
as it were without the patch, calling pte_same.

Now to the bit itself

#ifdef CONFIG_MEM_SOFT_DIRTY
#define _PAGE_SWP_SOFT_DIRTY_PAGE_PSE
#else
#define _PAGE_SWP_SOFT_DIRTY_PAGE_PSE(_AT(pteval_t, 0))
#endif

it's 0 if CONFIG_MEM_SOFT_DIRTY=n, so any setup of this
bit will simply become nop

#ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
{
	return pte_set_flags(pte, _PAGE_SWP_SOFT_DIRTY);
}

static inline int pte_swp_soft_dirty(pte_t pte)
{
	return pte_flags(pte) & _PAGE_SWP_SOFT_DIRTY;
}

static inline pte_t pte_swp_clear_soft_dirty(pte_t pte)
{
	return pte_clear_flags(pte, _PAGE_SWP_SOFT_DIRTY);
}
#endif

So I fear I'm lost where this "set" of the bit comes from
when CONFIG_MEM_SOFT_DIRTY=n.

Martin, could you please elaborate? Seems I'm missing
something obvious.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-17 19:31   ` Cyrill Gorcunov
@ 2015-09-18  6:58     ` Martin Schwidefsky
  2015-09-18  7:15       ` Cyrill Gorcunov
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Schwidefsky @ 2015-09-18  6:58 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Thu, 17 Sep 2015 22:31:52 +0300
Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> Thus when CONFIG_MEM_SOFT_DIRTY = n, the unuse_pte will be the same
> as it were without the patch, calling pte_same.
> 
> Now to the bit itself
> 
> #ifdef CONFIG_MEM_SOFT_DIRTY
> #define _PAGE_SWP_SOFT_DIRTY_PAGE_PSE
> #else
> #define _PAGE_SWP_SOFT_DIRTY_PAGE_PSE(_AT(pteval_t, 0))
> #endif
>
> it's 0 if CONFIG_MEM_SOFT_DIRTY=n, so any setup of this
> bit will simply become nop

Ok, that is what I have been missing with my soft dirty patch for s390.
_PAGE_BIT_SOFT_DIRTY is always defined but the _PAGE_SOFT_DIRTY and
_PAGE_SWP_SOFT_DIRTY are conditional. The primitives are always
defined but turn into nops with CONFIG_MEM_SOFT_DIRTY=n.

> #ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
> static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
> {
> 	return pte_set_flags(pte, _PAGE_SWP_SOFT_DIRTY);
> }
> 
> static inline int pte_swp_soft_dirty(pte_t pte)
> {
> 	return pte_flags(pte) & _PAGE_SWP_SOFT_DIRTY;
> }
> 
> static inline pte_t pte_swp_clear_soft_dirty(pte_t pte)
> {
> 	return pte_clear_flags(pte, _PAGE_SWP_SOFT_DIRTY);
> }
> #endif
> 
> So I fear I'm lost where this "set" of the bit comes from
> when CONFIG_MEM_SOFT_DIRTY=n.
> 
> Martin, could you please elaborate? Seems I'm missing
> something obvious.
 
It is me who missed something.. thanks for the explanation.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-18  6:58     ` Martin Schwidefsky
@ 2015-09-18  7:15       ` Cyrill Gorcunov
  2015-09-18  8:20         ` Martin Schwidefsky
  0 siblings, 1 reply; 16+ messages in thread
From: Cyrill Gorcunov @ 2015-09-18  7:15 UTC (permalink / raw)
  To: Martin Schwidefsky; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Fri, Sep 18, 2015 at 08:58:35AM +0200, Martin Schwidefsky wrote:
> > 
> > Martin, could you please elaborate? Seems I'm missing
> > something obvious.
>  
> It is me who missed something.. thanks for the explanation.

Sure thing! Ping me if any.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-18  7:15       ` Cyrill Gorcunov
@ 2015-09-18  8:20         ` Martin Schwidefsky
  2015-09-18  8:53           ` Cyrill Gorcunov
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Schwidefsky @ 2015-09-18  8:20 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Fri, 18 Sep 2015 10:15:50 +0300
Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> On Fri, Sep 18, 2015 at 08:58:35AM +0200, Martin Schwidefsky wrote:
> > > 
> > > Martin, could you please elaborate? Seems I'm missing
> > > something obvious.
> >  
> > It is me who missed something.. thanks for the explanation.
> 
> Sure thing! Ping me if any.
 
Ok, with the conditional define for the _PAGE_SOFT_DIRTY bit my
s390 code now works. But there is one common code patch that would
make sense, see below. If this is ok with you I can queue this via
the linux-s390 tree.

--
Subject: [PATCH] mm: add architecture primitives for software dirty bit
 clearing

There are primitives to create and query the software dirty bits
in a pte or pmd. The clearing of the software dirty bit is done
in common code with x86 specific page table functions.

Add the missing architecture primitives to clear the software dirty
bits to allow the feature to be used on non-x86 systems.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
 arch/x86/include/asm/pgtable.h | 10 ++++++++++
 fs/proc/task_mmu.c             |  4 ++--
 include/asm-generic/pgtable.h  | 10 ++++++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 867da5b..0be49a2 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -318,6 +318,16 @@ static inline pmd_t pmd_mksoft_dirty(pmd_t pmd)
 	return pmd_set_flags(pmd, _PAGE_SOFT_DIRTY);
 }
 
+static inline pte_t pte_clear_soft_dirty(pte_t pte)
+{
+	return pte_clear_flags(pte, _PAGE_SOFT_DIRTY);
+}
+
+static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
+{
+	return pmp_clear_flags(pmd, _PAGE_SOFT_DIRTY);
+}
+
 #endif /* CONFIG_HAVE_ARCH_SOFT_DIRTY */
 
 /*
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index e2d46ad..b029d42 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -754,7 +754,7 @@ static inline void clear_soft_dirty(struct vm_area_struct *vma,
 
 	if (pte_present(ptent)) {
 		ptent = pte_wrprotect(ptent);
-		ptent = pte_clear_flags(ptent, _PAGE_SOFT_DIRTY);
+		ptent = pte_clear_soft_dirty(ptent);
 	} else if (is_swap_pte(ptent)) {
 		ptent = pte_swp_clear_soft_dirty(ptent);
 	}
@@ -768,7 +768,7 @@ static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma,
 	pmd_t pmd = *pmdp;
 
 	pmd = pmd_wrprotect(pmd);
-	pmd = pmd_clear_flags(pmd, _PAGE_SOFT_DIRTY);
+	pmd = pmd_clear_soft_dirty(pmd);
 
 	if (vma->vm_flags & VM_SOFTDIRTY)
 		vma->vm_flags &= ~VM_SOFTDIRTY;
diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index 29c57b2..f167cdd 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -482,6 +482,16 @@ static inline pmd_t pmd_mksoft_dirty(pmd_t pmd)
 	return pmd;
 }
 
+static inline pte_t pte_clear_soft_dirty(pte_t pte)
+{
+	return pte;
+}
+
+static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
+{
+	return pmd;
+}
+
 static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
 {
 	return pte;
-- 
2.3.8

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-18  8:20         ` Martin Schwidefsky
@ 2015-09-18  8:53           ` Cyrill Gorcunov
  2015-09-18  9:10             ` Martin Schwidefsky
  0 siblings, 1 reply; 16+ messages in thread
From: Cyrill Gorcunov @ 2015-09-18  8:53 UTC (permalink / raw)
  To: Martin Schwidefsky; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Fri, Sep 18, 2015 at 10:20:01AM +0200, Martin Schwidefsky wrote:
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index e2d46ad..b029d42 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -754,7 +754,7 @@ static inline void clear_soft_dirty(struct vm_area_struct *vma,
>  
>  	if (pte_present(ptent)) {
>  		ptent = pte_wrprotect(ptent);
> -		ptent = pte_clear_flags(ptent, _PAGE_SOFT_DIRTY);
> +		ptent = pte_clear_soft_dirty(ptent);
>  	} else if (is_swap_pte(ptent)) {
>  		ptent = pte_swp_clear_soft_dirty(ptent);
>  	}
> @@ -768,7 +768,7 @@ static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma,
>  	pmd_t pmd = *pmdp;
>  
>  	pmd = pmd_wrprotect(pmd);
> -	pmd = pmd_clear_flags(pmd, _PAGE_SOFT_DIRTY);
> +	pmd = pmd_clear_soft_dirty(pmd);
>  

You know, these are only two lines where we use _PAGE_SOFT_DIRTY
directly, so I don't see much point in adding 22 lines of code
for that. Maybe we can leave it as is?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-18  8:53           ` Cyrill Gorcunov
@ 2015-09-18  9:10             ` Martin Schwidefsky
  2015-09-18  9:28               ` Cyrill Gorcunov
  2015-09-18 20:21               ` Cyrill Gorcunov
  0 siblings, 2 replies; 16+ messages in thread
From: Martin Schwidefsky @ 2015-09-18  9:10 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Fri, 18 Sep 2015 11:53:01 +0300
Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> On Fri, Sep 18, 2015 at 10:20:01AM +0200, Martin Schwidefsky wrote:
> > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > index e2d46ad..b029d42 100644
> > --- a/fs/proc/task_mmu.c
> > +++ b/fs/proc/task_mmu.c
> > @@ -754,7 +754,7 @@ static inline void clear_soft_dirty(struct vm_area_struct *vma,
> >  
> >  	if (pte_present(ptent)) {
> >  		ptent = pte_wrprotect(ptent);
> > -		ptent = pte_clear_flags(ptent, _PAGE_SOFT_DIRTY);
> > +		ptent = pte_clear_soft_dirty(ptent);
> >  	} else if (is_swap_pte(ptent)) {
> >  		ptent = pte_swp_clear_soft_dirty(ptent);
> >  	}
> > @@ -768,7 +768,7 @@ static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma,
> >  	pmd_t pmd = *pmdp;
> >  
> >  	pmd = pmd_wrprotect(pmd);
> > -	pmd = pmd_clear_flags(pmd, _PAGE_SOFT_DIRTY);
> > +	pmd = pmd_clear_soft_dirty(pmd);
> >  
> 
> You know, these are only two lines where we use _PAGE_SOFT_DIRTY
> directly, so I don't see much point in adding 22 lines of code
> for that. Maybe we can leave it as is?
 
Only x86 has pte_clear_flags. And the two lines require that there is exactly
one bit in the PTE for soft-dirty. An alternative encoding will not be allowed.
And the current set of primitives is asymmetric, there are functions to query
and set the bit pte_soft_dirty and pte_mksoft_dirty but no function to clear
the bit.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-18  9:10             ` Martin Schwidefsky
@ 2015-09-18  9:28               ` Cyrill Gorcunov
  2015-09-18 10:11                 ` Martin Schwidefsky
  2015-09-18 20:21               ` Cyrill Gorcunov
  1 sibling, 1 reply; 16+ messages in thread
From: Cyrill Gorcunov @ 2015-09-18  9:28 UTC (permalink / raw)
  To: Martin Schwidefsky; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Fri, Sep 18, 2015 at 11:10:38AM +0200, Martin Schwidefsky wrote:
> > 
> > You know, these are only two lines where we use _PAGE_SOFT_DIRTY
> > directly, so I don't see much point in adding 22 lines of code
> > for that. Maybe we can leave it as is?
>  
> Only x86 has pte_clear_flags. And the two lines require that there is exactly
> one bit in the PTE for soft-dirty. An alternative encoding will not be allowed.
> And the current set of primitives is asymmetric, there are functions to query
> and set the bit pte_soft_dirty and pte_mksoft_dirty but no function to clear
> the bit.

OK, Martin, gimme some time please, I don't have code at my hands, so once
I'm back, I'll take a precise look, ok?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-18  9:28               ` Cyrill Gorcunov
@ 2015-09-18 10:11                 ` Martin Schwidefsky
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Schwidefsky @ 2015-09-18 10:11 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Fri, 18 Sep 2015 12:28:27 +0300
Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> On Fri, Sep 18, 2015 at 11:10:38AM +0200, Martin Schwidefsky wrote:
> > > 
> > > You know, these are only two lines where we use _PAGE_SOFT_DIRTY
> > > directly, so I don't see much point in adding 22 lines of code
> > > for that. Maybe we can leave it as is?
> >  
> > Only x86 has pte_clear_flags. And the two lines require that there is exactly
> > one bit in the PTE for soft-dirty. An alternative encoding will not be allowed.
> > And the current set of primitives is asymmetric, there are functions to query
> > and set the bit pte_soft_dirty and pte_mksoft_dirty but no function to clear
> > the bit.
> 
> OK, Martin, gimme some time please, I don't have code at my hands, so once
> I'm back, I'll take a precise look, ok?

Sure, thanks.
 
-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-18  9:10             ` Martin Schwidefsky
  2015-09-18  9:28               ` Cyrill Gorcunov
@ 2015-09-18 20:21               ` Cyrill Gorcunov
  2015-09-21  7:10                 ` Martin Schwidefsky
  1 sibling, 1 reply; 16+ messages in thread
From: Cyrill Gorcunov @ 2015-09-18 20:21 UTC (permalink / raw)
  To: Martin Schwidefsky; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Fri, Sep 18, 2015 at 11:10:38AM +0200, Martin Schwidefsky wrote:
> > 
> > You know, these are only two lines where we use _PAGE_SOFT_DIRTY
> > directly, so I don't see much point in adding 22 lines of code
> > for that. Maybe we can leave it as is?
>  
> Only x86 has pte_clear_flags. And the two lines require that there is exactly
> one bit in the PTE for soft-dirty. An alternative encoding will not be allowed.

Agreed, still I would defer until there is a real need for an alternative encoding.

> And the current set of primitives is asymmetric, there are functions to query
> and set the bit pte_soft_dirty and pte_mksoft_dirty but no function to clear
> the bit.

Yes, but again I don't see an urgent need for these helpers.

Anyway, there is no strong objections against this approach
from my side, but please at least compile-test the patch next
time, because this is definitely a typo

static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
{
	return pmp_clear_flags(pmd, _PAGE_SOFT_DIRTY);
}

I bet you meant pmd_clear_flags.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-18 20:21               ` Cyrill Gorcunov
@ 2015-09-21  7:10                 ` Martin Schwidefsky
  2015-09-21  7:30                   ` Cyrill Gorcunov
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Schwidefsky @ 2015-09-21  7:10 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Fri, 18 Sep 2015 23:21:09 +0300
Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> On Fri, Sep 18, 2015 at 11:10:38AM +0200, Martin Schwidefsky wrote:
> > > 
> > > You know, these are only two lines where we use _PAGE_SOFT_DIRTY
> > > directly, so I don't see much point in adding 22 lines of code
> > > for that. Maybe we can leave it as is?
> >  
> > Only x86 has pte_clear_flags. And the two lines require that there is exactly
> > one bit in the PTE for soft-dirty. An alternative encoding will not be allowed.
> 
> Agreed, still I would defer until there is a real need for an alternative encoding.

The s390 support for soft dirty ptes will need it.
 
> > And the current set of primitives is asymmetric, there are functions to query
> > and set the bit pte_soft_dirty and pte_mksoft_dirty but no function to clear
> > the bit.
> 
> Yes, but again I don't see an urgent need for these helpers.
> 
> Anyway, there is no strong objections against this approach
> from my side, but please at least compile-test the patch next
> time, because this is definitely a typo
> 
> static inline pmd_t pmd_clear_soft_dirty(pmd_t pmd)
> {
> 	return pmp_clear_flags(pmd, _PAGE_SOFT_DIRTY);
> }
> 
> I bet you meant pmd_clear_flags.

Yes, the final test is still pending. The patch was more or less for illustrative
purpose. I yet have to do the compile & boot test on an x86 system.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-21  7:10                 ` Martin Schwidefsky
@ 2015-09-21  7:30                   ` Cyrill Gorcunov
  2015-09-21  7:40                     ` Martin Schwidefsky
  0 siblings, 1 reply; 16+ messages in thread
From: Cyrill Gorcunov @ 2015-09-21  7:30 UTC (permalink / raw)
  To: Martin Schwidefsky; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Mon, Sep 21, 2015 at 09:10:33AM +0200, Martin Schwidefsky wrote:
> > 
> > Agreed, still I would defer until there is a real need for an alternative encoding.
> 
> The s390 support for soft dirty ptes will need it.

Ah, I see. Could you please note this fact in the patch
changelog.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-21  7:30                   ` Cyrill Gorcunov
@ 2015-09-21  7:40                     ` Martin Schwidefsky
  2015-09-21  7:54                       ` Cyrill Gorcunov
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Schwidefsky @ 2015-09-21  7:40 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Mon, 21 Sep 2015 10:30:33 +0300
Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> On Mon, Sep 21, 2015 at 09:10:33AM +0200, Martin Schwidefsky wrote:
> > > 
> > > Agreed, still I would defer until there is a real need for an alternative encoding.
> > 
> > The s390 support for soft dirty ptes will need it.
> 
> Ah, I see. Could you please note this fact in the patch
> changelog.
 
Sure will do. I'll send a patch set after I got the x86 test sorted out.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mm/swapfile: fix swapoff vs. software dirty bits
  2015-09-21  7:40                     ` Martin Schwidefsky
@ 2015-09-21  7:54                       ` Cyrill Gorcunov
  0 siblings, 0 replies; 16+ messages in thread
From: Cyrill Gorcunov @ 2015-09-21  7:54 UTC (permalink / raw)
  To: Martin Schwidefsky; +Cc: Andrew Morton, Ingo Molnar, Michal Hocko, linux-mm

On Mon, Sep 21, 2015 at 09:40:19AM +0200, Martin Schwidefsky wrote:
> > Ah, I see. Could you please note this fact in the patch
> > changelog.
>  
> Sure will do. I'll send a patch set after I got the x86 test sorted out.

Great! Thank you!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-09-21  7:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-17  8:58 [PATCH] hanging swapoff with HAVE_ARCH_SOFT_DIRTY=y Martin Schwidefsky
2015-09-17  8:58 ` [PATCH] mm/swapfile: fix swapoff vs. software dirty bits Martin Schwidefsky
2015-09-17  9:53   ` Cyrill Gorcunov
2015-09-17 19:31   ` Cyrill Gorcunov
2015-09-18  6:58     ` Martin Schwidefsky
2015-09-18  7:15       ` Cyrill Gorcunov
2015-09-18  8:20         ` Martin Schwidefsky
2015-09-18  8:53           ` Cyrill Gorcunov
2015-09-18  9:10             ` Martin Schwidefsky
2015-09-18  9:28               ` Cyrill Gorcunov
2015-09-18 10:11                 ` Martin Schwidefsky
2015-09-18 20:21               ` Cyrill Gorcunov
2015-09-21  7:10                 ` Martin Schwidefsky
2015-09-21  7:30                   ` Cyrill Gorcunov
2015-09-21  7:40                     ` Martin Schwidefsky
2015-09-21  7:54                       ` Cyrill Gorcunov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.