ppc64le soft dirty tracking on 4.14

Submitted by Andrey Vagin on Oct. 3, 2017, 11:01 p.m.

Details

Message ID 20171003230136.GG25180@outlook.office365.com
State Not Applicable
Series "ppc64le soft dirty tracking on 4.14"
Headers show

Commit Message

Andrey Vagin Oct. 3, 2017, 11:01 p.m.
On Mon, Sep 25, 2017 at 06:08:26PM +0200, Adrian Reber wrote:
> Hello Laurent,
> 
> I had a closer look and soft dirty tracking works correctly. I was just
> confused by the page size. The file was 65536 and not the expected
> 4096.
> 
> CRIU still cannot detect if CONFIG_MEM_SOFT_DIRTY is enabled or not.
> The check always returns true and the criu pre-dump command is always
> successful. In the case where CONFIG_MEM_SOFT_DIRTY=n CRIU still does
> pre-dumps but it seems to always dump the complete memory.

I think it is a bug in a kernel and it can be fixed by a patch like
this:



I haven't try to test it, just got an idea from arch/x86/include/asm/pgtable_types.h.

Without this patch, the kernel has some logic about _PAGE_SOFT_DIRTY,
even when CONFIG_MEM_SOFT_DIRTY isn't set.


> 
> 'criu/criu check --feature mem_dirty_track' is always successful.
> 
> Disabling CONFIG_PPC_RADIX_MMU makes also no difference to me.
> 
> So it seems the check for CONFIG_MEM_SOFT_DIRTY in CRIU on ppc64le is
> broken. The kernel does that right thing.
> 
> 		Adrian
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu

Patch hide | download patch | download mbox

--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -78,7 +78,12 @@ 
  */
 #define _PAGE_PA_MAX           53
 
+#ifdef CONFIG_MEM_SOFT_DIRTY
 #define _PAGE_SOFT_DIRTY       _RPAGE_SW3 /* software: software dirty tracking */
+#elif
+#define _PAGE_SOFT_DIRTY       0
+#endif
+
 #define _PAGE_SPECIAL          _RPAGE_SW2 /* software: special page */
 #define _PAGE_DEVMAP           _RPAGE_SW1 /* software: ZONE_DEVICE page */
 #define __HAVE_ARCH_PTE_DEVMAP

Comments

Adrian Reber Oct. 4, 2017, 6:23 a.m.
On Tue, Oct 03, 2017 at 04:01:37PM -0700, Andrei Vagin wrote:
> On Mon, Sep 25, 2017 at 06:08:26PM +0200, Adrian Reber wrote:
> > Hello Laurent,
> > 
> > I had a closer look and soft dirty tracking works correctly. I was just
> > confused by the page size. The file was 65536 and not the expected
> > 4096.
> > 
> > CRIU still cannot detect if CONFIG_MEM_SOFT_DIRTY is enabled or not.
> > The check always returns true and the criu pre-dump command is always
> > successful. In the case where CONFIG_MEM_SOFT_DIRTY=n CRIU still does
> > pre-dumps but it seems to always dump the complete memory.
> 
> I think it is a bug in a kernel and it can be fixed by a patch like
> this:

Thanks, Andrei. I will test it later today and let you know.

> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -78,7 +78,12 @@
>   */
>  #define _PAGE_PA_MAX           53
>  
> +#ifdef CONFIG_MEM_SOFT_DIRTY
>  #define _PAGE_SOFT_DIRTY       _RPAGE_SW3 /* software: software dirty tracking */
> +#elif
> +#define _PAGE_SOFT_DIRTY       0
> +#endif
> +
>  #define _PAGE_SPECIAL          _RPAGE_SW2 /* software: special page */
>  #define _PAGE_DEVMAP           _RPAGE_SW1 /* software: ZONE_DEVICE page */
>  #define __HAVE_ARCH_PTE_DEVMAP
> 
> I haven't try to test it, just got an idea from arch/x86/include/asm/pgtable_types.h.
> 
> Without this patch, the kernel has some logic about _PAGE_SOFT_DIRTY,
> even when CONFIG_MEM_SOFT_DIRTY isn't set.
> 
> 
> > 
> > 'criu/criu check --feature mem_dirty_track' is always successful.
> > 
> > Disabling CONFIG_PPC_RADIX_MMU makes also no difference to me.
> > 
> > So it seems the check for CONFIG_MEM_SOFT_DIRTY in CRIU on ppc64le is
> > broken. The kernel does that right thing.
> > 
> > 		Adrian
> > _______________________________________________
> > CRIU mailing list
> > CRIU@openvz.org
> > https://lists.openvz.org/mailman/listinfo/criu
Laurent Dufour Oct. 4, 2017, 6:41 a.m.
On 04/10/2017 01:01, Andrei Vagin wrote:
> On Mon, Sep 25, 2017 at 06:08:26PM +0200, Adrian Reber wrote:
>> Hello Laurent,
>>
>> I had a closer look and soft dirty tracking works correctly. I was just
>> confused by the page size. The file was 65536 and not the expected
>> 4096.
>>
>> CRIU still cannot detect if CONFIG_MEM_SOFT_DIRTY is enabled or not.
>> The check always returns true and the criu pre-dump command is always
>> successful. In the case where CONFIG_MEM_SOFT_DIRTY=n CRIU still does
>> pre-dumps but it seems to always dump the complete memory.

I don't think this is a good idea to change the value of _PAGE_SOFT_DIRTY
based on CONFIG_MEM_SOFT_DIRTY.

If processings are done on the soft dirty bit while CONFIG_MEM_SOFT_DIRTY
is not set, this is a bug.

Short looking to the code shows that the pte_*soft_dirty() services are
define only when CONFIG_HAVE_ARCH_SOFT_DIRTY is set.

#ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
static inline bool pte_soft_dirty(pte_t pte)
{
        return !!(pte_raw(pte) & cpu_to_be64(_PAGE_SOFT_DIRTY));
}

static inline pte_t pte_mksoft_dirty(pte_t pte)
{
        return __pte(pte_val(pte) | _PAGE_SOFT_DIRTY);
}

static inline pte_t pte_clear_soft_dirty(pte_t pte)
{
        return __pte(pte_val(pte) & ~_PAGE_SOFT_DIRTY);
}
#endif /* CONFIG_HAVE_ARCH_SOFT_DIRTY */

So I guess the issue happens when CONFIG_MEM_SOFT_DIRTY=n and
CONFIG_HAVE_ARCH_SOFT_DIRTY=y, so there should be a path where a check  to
CONFIG_MEM_SOFT_DIRTY is missing.

> I think it is a bug in a kernel and it can be fixed by a patch like
> this:
> 
> 
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -78,7 +78,12 @@
>   */
>  #define _PAGE_PA_MAX           53
> 
> +#ifdef CONFIG_MEM_SOFT_DIRTY
>  #define _PAGE_SOFT_DIRTY       _RPAGE_SW3 /* software: software dirty tracking */
> +#elif
> +#define _PAGE_SOFT_DIRTY       0
> +#endif
> +
>  #define _PAGE_SPECIAL          _RPAGE_SW2 /* software: special page */
>  #define _PAGE_DEVMAP           _RPAGE_SW1 /* software: ZONE_DEVICE page */
>  #define __HAVE_ARCH_PTE_DEVMAP
> 
> I haven't try to test it, just got an idea from arch/x86/include/asm/pgtable_types.h.
> 
> Without this patch, the kernel has some logic about _PAGE_SOFT_DIRTY,
> even when CONFIG_MEM_SOFT_DIRTY isn't set.
> 
> 
>>
>> 'criu/criu check --feature mem_dirty_track' is always successful.
>>
>> Disabling CONFIG_PPC_RADIX_MMU makes also no difference to me.
>>
>> So it seems the check for CONFIG_MEM_SOFT_DIRTY in CRIU on ppc64le is
>> broken. The kernel does that right thing.
>>
>> 		Adrian
>> _______________________________________________
>> CRIU mailing list
>> CRIU@openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu
>
Andrey Vagin Oct. 4, 2017, 11:40 p.m.
On Wed, Oct 04, 2017 at 08:41:22AM +0200, Laurent Dufour wrote:
> On 04/10/2017 01:01, Andrei Vagin wrote:
> > On Mon, Sep 25, 2017 at 06:08:26PM +0200, Adrian Reber wrote:
> >> Hello Laurent,
> >>
> >> I had a closer look and soft dirty tracking works correctly. I was just
> >> confused by the page size. The file was 65536 and not the expected
> >> 4096.
> >>
> >> CRIU still cannot detect if CONFIG_MEM_SOFT_DIRTY is enabled or not.
> >> The check always returns true and the criu pre-dump command is always
> >> successful. In the case where CONFIG_MEM_SOFT_DIRTY=n CRIU still does
> >> pre-dumps but it seems to always dump the complete memory.
> 
> I don't think this is a good idea to change the value of _PAGE_SOFT_DIRTY
> based on CONFIG_MEM_SOFT_DIRTY.
> 
> If processings are done on the soft dirty bit while CONFIG_MEM_SOFT_DIRTY
> is not set, this is a bug.

I'm not sure that it is a good idea too. But in this case, this problem is
more generic, because this way is already used for x86:

#ifdef CONFIG_MEM_SOFT_DIRTY
#define _PAGE_SOFT_DIRTY        (_AT(pteval_t, 1) << _PAGE_BIT_SOFT_DIRTY)
#else
#define _PAGE_SOFT_DIRTY        (_AT(pteval_t, 0))
#endif

And it is used even for powerpc to handle _PAGE_SWP_SOFT_DIRTY

#ifdef CONFIG_MEM_SOFT_DIRTY
#define _PAGE_SWP_SOFT_DIRTY   (1UL << (SWP_TYPE_BITS + _PAGE_BIT_SWAP_TYPE))
#else
#define _PAGE_SWP_SOFT_DIRTY    0UL
#endif /* CONFIG_MEM_SOFT_DIRTY */

Thanks,
Andrei