Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-21 Thread Dominik Dingel
On Tue, 21 Oct 2014 10:11:43 +0200
Paolo Bonzini  wrote:

> 
> 
> On 10/21/2014 08:11 AM, Martin Schwidefsky wrote:
> >> I agree with Dave (I thought I disagreed, but I changed my mind while
> >> writing down my thoughts).  Just define mm_forbids_zeropage in
> >> arch/s390/include/asm, and make it return mm->context.use_skey---with a
> >> comment explaining how this is only for processes that use KVM, and then
> >> only for guests that use storage keys.
> >
> > The mm_forbids_zeropage() sure will work for now, but I think a vma flag
> > is the better solution. This is analog to VM_MERGEABLE or VM_NOHUGEPAGE,
> > the best solution would be to only mark those vmas that are mapped to
> > the guest. That we have not found a way to do that yet in a sensible way
> > does not change the fact that "no-zero-page" is a per-vma property, no?
> 
> I agree it should be per-VMA.  However, right now the code is 
> complicated unnecessarily by making it a per-VMA flag.  Also, setting 
> the flag per VMA should probably be done in 
> kvm_arch_prepare_memory_region together with some kind of storage key 
> notifier.  This is not very much like Dominik's patch.  All in all, 
> mm_forbids_zeropage() provides a non-intrusive and non-controversial way 
> to fix the bug.  Later on, switching to vma_forbids_zeropage() will be 
> trivial as far as mm/ code is concerned.
> 

Thank you for all the feedback, will cook up a new version.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-21 Thread Paolo Bonzini



On 10/21/2014 08:11 AM, Martin Schwidefsky wrote:

I agree with Dave (I thought I disagreed, but I changed my mind while
writing down my thoughts).  Just define mm_forbids_zeropage in
arch/s390/include/asm, and make it return mm->context.use_skey---with a
comment explaining how this is only for processes that use KVM, and then
only for guests that use storage keys.


The mm_forbids_zeropage() sure will work for now, but I think a vma flag
is the better solution. This is analog to VM_MERGEABLE or VM_NOHUGEPAGE,
the best solution would be to only mark those vmas that are mapped to
the guest. That we have not found a way to do that yet in a sensible way
does not change the fact that "no-zero-page" is a per-vma property, no?


I agree it should be per-VMA.  However, right now the code is 
complicated unnecessarily by making it a per-VMA flag.  Also, setting 
the flag per VMA should probably be done in 
kvm_arch_prepare_memory_region together with some kind of storage key 
notifier.  This is not very much like Dominik's patch.  All in all, 
mm_forbids_zeropage() provides a non-intrusive and non-controversial way 
to fix the bug.  Later on, switching to vma_forbids_zeropage() will be 
trivial as far as mm/ code is concerned.



But if you insist we go with the mm_forbids_zeropage() until we find a
clever way to distinguish the guest vmas from the qemu ones.


Yeah, I think it is simpler for now.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-21 Thread Martin Schwidefsky
On Mon, 20 Oct 2014 20:14:53 +0200
Paolo Bonzini  wrote:

> On 10/18/2014 06:28 PM, Dave Hansen wrote:
> > > Currently it is an all or nothing thing, but for a future change we might 
> > > want to just
> > > tag the guest memory instead of the complete user address space.
> >
> > I think it's a bad idea to reserve a flag for potential future use.  If
> > you_need_  it in the future, let's have the discussion then.  For now, I
> > think it should probably just be stored in the mm somewhere.
> 
> I agree with Dave (I thought I disagreed, but I changed my mind while 
> writing down my thoughts).  Just define mm_forbids_zeropage in 
> arch/s390/include/asm, and make it return mm->context.use_skey---with a 
> comment explaining how this is only for processes that use KVM, and then 
> only for guests that use storage keys.

The mm_forbids_zeropage() sure will work for now, but I think a vma flag
is the better solution. This is analog to VM_MERGEABLE or VM_NOHUGEPAGE,
the best solution would be to only mark those vmas that are mapped to
the guest. That we have not found a way to do that yet in a sensible way
does not change the fact that "no-zero-page" is a per-vma property, no?

But if you insist we go with the mm_forbids_zeropage() until we find a
clever way to distinguish the guest vmas from the qemu ones.

-- 
blue skies,
   Martin.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-21 Thread Martin Schwidefsky
On Mon, 20 Oct 2014 20:14:53 +0200
Paolo Bonzini pbonz...@redhat.com wrote:

 On 10/18/2014 06:28 PM, Dave Hansen wrote:
   Currently it is an all or nothing thing, but for a future change we might 
   want to just
   tag the guest memory instead of the complete user address space.
 
  I think it's a bad idea to reserve a flag for potential future use.  If
  you_need_  it in the future, let's have the discussion then.  For now, I
  think it should probably just be stored in the mm somewhere.
 
 I agree with Dave (I thought I disagreed, but I changed my mind while 
 writing down my thoughts).  Just define mm_forbids_zeropage in 
 arch/s390/include/asm, and make it return mm-context.use_skey---with a 
 comment explaining how this is only for processes that use KVM, and then 
 only for guests that use storage keys.

The mm_forbids_zeropage() sure will work for now, but I think a vma flag
is the better solution. This is analog to VM_MERGEABLE or VM_NOHUGEPAGE,
the best solution would be to only mark those vmas that are mapped to
the guest. That we have not found a way to do that yet in a sensible way
does not change the fact that no-zero-page is a per-vma property, no?

But if you insist we go with the mm_forbids_zeropage() until we find a
clever way to distinguish the guest vmas from the qemu ones.

-- 
blue skies,
   Martin.

Reality continues to ruin my life. - Calvin.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-21 Thread Paolo Bonzini



On 10/21/2014 08:11 AM, Martin Schwidefsky wrote:

I agree with Dave (I thought I disagreed, but I changed my mind while
writing down my thoughts).  Just define mm_forbids_zeropage in
arch/s390/include/asm, and make it return mm-context.use_skey---with a
comment explaining how this is only for processes that use KVM, and then
only for guests that use storage keys.


The mm_forbids_zeropage() sure will work for now, but I think a vma flag
is the better solution. This is analog to VM_MERGEABLE or VM_NOHUGEPAGE,
the best solution would be to only mark those vmas that are mapped to
the guest. That we have not found a way to do that yet in a sensible way
does not change the fact that no-zero-page is a per-vma property, no?


I agree it should be per-VMA.  However, right now the code is 
complicated unnecessarily by making it a per-VMA flag.  Also, setting 
the flag per VMA should probably be done in 
kvm_arch_prepare_memory_region together with some kind of storage key 
notifier.  This is not very much like Dominik's patch.  All in all, 
mm_forbids_zeropage() provides a non-intrusive and non-controversial way 
to fix the bug.  Later on, switching to vma_forbids_zeropage() will be 
trivial as far as mm/ code is concerned.



But if you insist we go with the mm_forbids_zeropage() until we find a
clever way to distinguish the guest vmas from the qemu ones.


Yeah, I think it is simpler for now.

Paolo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-21 Thread Dominik Dingel
On Tue, 21 Oct 2014 10:11:43 +0200
Paolo Bonzini pbonz...@redhat.com wrote:

 
 
 On 10/21/2014 08:11 AM, Martin Schwidefsky wrote:
  I agree with Dave (I thought I disagreed, but I changed my mind while
  writing down my thoughts).  Just define mm_forbids_zeropage in
  arch/s390/include/asm, and make it return mm-context.use_skey---with a
  comment explaining how this is only for processes that use KVM, and then
  only for guests that use storage keys.
 
  The mm_forbids_zeropage() sure will work for now, but I think a vma flag
  is the better solution. This is analog to VM_MERGEABLE or VM_NOHUGEPAGE,
  the best solution would be to only mark those vmas that are mapped to
  the guest. That we have not found a way to do that yet in a sensible way
  does not change the fact that no-zero-page is a per-vma property, no?
 
 I agree it should be per-VMA.  However, right now the code is 
 complicated unnecessarily by making it a per-VMA flag.  Also, setting 
 the flag per VMA should probably be done in 
 kvm_arch_prepare_memory_region together with some kind of storage key 
 notifier.  This is not very much like Dominik's patch.  All in all, 
 mm_forbids_zeropage() provides a non-intrusive and non-controversial way 
 to fix the bug.  Later on, switching to vma_forbids_zeropage() will be 
 trivial as far as mm/ code is concerned.
 

Thank you for all the feedback, will cook up a new version.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-20 Thread Paolo Bonzini

On 10/18/2014 06:28 PM, Dave Hansen wrote:

> Currently it is an all or nothing thing, but for a future change we might 
want to just
> tag the guest memory instead of the complete user address space.

I think it's a bad idea to reserve a flag for potential future use.  If
you_need_  it in the future, let's have the discussion then.  For now, I
think it should probably just be stored in the mm somewhere.


I agree with Dave (I thought I disagreed, but I changed my mind while 
writing down my thoughts).  Just define mm_forbids_zeropage in 
arch/s390/include/asm, and make it return mm->context.use_skey---with a 
comment explaining how this is only for processes that use KVM, and then 
only for guests that use storage keys.


Paolo (who was just taught what storage keys really are)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-20 Thread Paolo Bonzini

On 10/18/2014 06:28 PM, Dave Hansen wrote:

 Currently it is an all or nothing thing, but for a future change we might 
want to just
 tag the guest memory instead of the complete user address space.

I think it's a bad idea to reserve a flag for potential future use.  If
you_need_  it in the future, let's have the discussion then.  For now, I
think it should probably just be stored in the mm somewhere.


I agree with Dave (I thought I disagreed, but I changed my mind while 
writing down my thoughts).  Just define mm_forbids_zeropage in 
arch/s390/include/asm, and make it return mm-context.use_skey---with a 
comment explaining how this is only for processes that use KVM, and then 
only for guests that use storage keys.


Paolo (who was just taught what storage keys really are)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-18 Thread Dave Hansen
On 10/18/2014 07:49 AM, Dominik Dingel wrote:
> On Fri, 17 Oct 2014 15:04:21 -0700
> Dave Hansen  wrote:
>> Is there ever a time where the VMAs under an mm have mixed VM_NOZEROPAGE
>> status?  Reading the patches, it _looks_ like it might be an all or
>> nothing thing.
> 
> Currently it is an all or nothing thing, but for a future change we might 
> want to just
> tag the guest memory instead of the complete user address space.

I think it's a bad idea to reserve a flag for potential future use.  If
you _need_ it in the future, let's have the discussion then.  For now, I
think it should probably just be stored in the mm somewhere.

>> Full disclosure: I've got an x86-specific feature I want to steal a flag
>> for.  Maybe we should just define another VM_ARCH bit.
>>
> 
> So you think of something like:
> 
> #if defined(CONFIG_S390)
> # define VM_NOZEROPAGEVM_ARCH_1
> #endif
> 
> #ifndef VM_NOZEROPAGE
> # define VM_NOZEROPAGEVM_NONE
> #endif
> 
> right?

Yeah, something like that.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-18 Thread Dominik Dingel
On Fri, 17 Oct 2014 15:04:21 -0700
Dave Hansen  wrote:
 
> Is there ever a time where the VMAs under an mm have mixed VM_NOZEROPAGE
> status?  Reading the patches, it _looks_ like it might be an all or
> nothing thing.

Currently it is an all or nothing thing, but for a future change we might want 
to just
tag the guest memory instead of the complete user address space.

> Full disclosure: I've got an x86-specific feature I want to steal a flag
> for.  Maybe we should just define another VM_ARCH bit.
> 

So you think of something like:

#if defined(CONFIG_S390)
# define VM_NOZEROPAGE  VM_ARCH_1
#endif

#ifndef VM_NOZEROPAGE
# define VM_NOZEROPAGE  VM_NONE
#endif

right?


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-18 Thread Dominik Dingel
On Fri, 17 Oct 2014 15:04:21 -0700
Dave Hansen dave.han...@intel.com wrote:
 
 Is there ever a time where the VMAs under an mm have mixed VM_NOZEROPAGE
 status?  Reading the patches, it _looks_ like it might be an all or
 nothing thing.

Currently it is an all or nothing thing, but for a future change we might want 
to just
tag the guest memory instead of the complete user address space.

 Full disclosure: I've got an x86-specific feature I want to steal a flag
 for.  Maybe we should just define another VM_ARCH bit.
 

So you think of something like:

#if defined(CONFIG_S390)
# define VM_NOZEROPAGE  VM_ARCH_1
#endif

#ifndef VM_NOZEROPAGE
# define VM_NOZEROPAGE  VM_NONE
#endif

right?


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-18 Thread Dave Hansen
On 10/18/2014 07:49 AM, Dominik Dingel wrote:
 On Fri, 17 Oct 2014 15:04:21 -0700
 Dave Hansen dave.han...@intel.com wrote:
 Is there ever a time where the VMAs under an mm have mixed VM_NOZEROPAGE
 status?  Reading the patches, it _looks_ like it might be an all or
 nothing thing.
 
 Currently it is an all or nothing thing, but for a future change we might 
 want to just
 tag the guest memory instead of the complete user address space.

I think it's a bad idea to reserve a flag for potential future use.  If
you _need_ it in the future, let's have the discussion then.  For now, I
think it should probably just be stored in the mm somewhere.

 Full disclosure: I've got an x86-specific feature I want to steal a flag
 for.  Maybe we should just define another VM_ARCH bit.

 
 So you think of something like:
 
 #if defined(CONFIG_S390)
 # define VM_NOZEROPAGEVM_ARCH_1
 #endif
 
 #ifndef VM_NOZEROPAGE
 # define VM_NOZEROPAGEVM_NONE
 #endif
 
 right?

Yeah, something like that.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-17 Thread Dave Hansen
On 10/17/2014 07:09 AM, Dominik Dingel wrote:
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index cd33ae2..8f09c91 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -113,7 +113,7 @@ extern unsigned int kobjsize(const void *objp);
>  #define VM_GROWSDOWN 0x0100  /* general info on the segment */
>  #define VM_PFNMAP0x0400  /* Page-ranges managed without "struct 
> page", just pure PFN */
>  #define VM_DENYWRITE 0x0800  /* ETXTBSY on write attempts.. */
> -
> +#define VM_NOZEROPAGE0x1000  /* forbid new zero page 
> mappings */
>  #define VM_LOCKED0x2000
>  #define VM_IO   0x4000   /* Memory mapped I/O or similar */

This seems like an awfully obscure use for a very constrained resource
(VM_ flags).

Is there ever a time where the VMAs under an mm have mixed VM_NOZEROPAGE
status?  Reading the patches, it _looks_ like it might be an all or
nothing thing.

Full disclosure: I've got an x86-specific feature I want to steal a flag
for.  Maybe we should just define another VM_ARCH bit.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] mm: introduce new VM_NOZEROPAGE flag

2014-10-17 Thread Dave Hansen
On 10/17/2014 07:09 AM, Dominik Dingel wrote:
 diff --git a/include/linux/mm.h b/include/linux/mm.h
 index cd33ae2..8f09c91 100644
 --- a/include/linux/mm.h
 +++ b/include/linux/mm.h
 @@ -113,7 +113,7 @@ extern unsigned int kobjsize(const void *objp);
  #define VM_GROWSDOWN 0x0100  /* general info on the segment */
  #define VM_PFNMAP0x0400  /* Page-ranges managed without struct 
 page, just pure PFN */
  #define VM_DENYWRITE 0x0800  /* ETXTBSY on write attempts.. */
 -
 +#define VM_NOZEROPAGE0x1000  /* forbid new zero page 
 mappings */
  #define VM_LOCKED0x2000
  #define VM_IO   0x4000   /* Memory mapped I/O or similar */

This seems like an awfully obscure use for a very constrained resource
(VM_ flags).

Is there ever a time where the VMAs under an mm have mixed VM_NOZEROPAGE
status?  Reading the patches, it _looks_ like it might be an all or
nothing thing.

Full disclosure: I've got an x86-specific feature I want to steal a flag
for.  Maybe we should just define another VM_ARCH bit.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/