| Message ID | 20200327092356.25171-1-borntraeger@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] s390/gmap: return proper error code on ksm unsharing | expand |
On 27.03.20 10:23, Christian Borntraeger wrote: > If a signal is pending we might return -ENOMEM instead of -EINTR. > We should propagate the proper error during KSM unsharing. > > Fixes: 3ac8e38015d4 ("s390/mm: disable KSM for storage key enabled pages") > Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > arch/s390/mm/gmap.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c > index 27926a06df32..1ef864c252e6 100644 > --- a/arch/s390/mm/gmap.c > +++ b/arch/s390/mm/gmap.c > @@ -2552,12 +2552,13 @@ int gmap_mark_unmergeable(void) > { > struct mm_struct *mm = current->mm; > struct vm_area_struct *vma; > + int ret; > > for (vma = mm->mmap; vma; vma = vma->vm_next) { > - if (ksm_madvise(vma, vma->vm_start, vma->vm_end, > - MADV_UNMERGEABLE, &vma->vm_flags)) { > - return -ENOMEM; > - } > + ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, > + MADV_UNMERGEABLE, &vma->vm_flags); > + if (ret) > + return ret; > } > mm->def_flags &= ~VM_MERGEABLE; > return 0; > Reviewed-by: David Hildenbrand <david@redhat.com>
On Fri, 27 Mar 2020 05:23:56 -0400 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > If a signal is pending we might return -ENOMEM instead of -EINTR. > We should propagate the proper error during KSM unsharing. > > Fixes: 3ac8e38015d4 ("s390/mm: disable KSM for storage key enabled pages") > Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > arch/s390/mm/gmap.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On 27.03.20 10:23, Christian Borntraeger wrote: > If a signal is pending we might return -ENOMEM instead of -EINTR. > We should propagate the proper error during KSM unsharing. > > Fixes: 3ac8e38015d4 ("s390/mm: disable KSM for storage key enabled pages") > Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > arch/s390/mm/gmap.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c > index 27926a06df32..1ef864c252e6 100644 > --- a/arch/s390/mm/gmap.c > +++ b/arch/s390/mm/gmap.c > @@ -2552,12 +2552,13 @@ int gmap_mark_unmergeable(void) > { > struct mm_struct *mm = current->mm; > struct vm_area_struct *vma; > + int ret; > > for (vma = mm->mmap; vma; vma = vma->vm_next) { > - if (ksm_madvise(vma, vma->vm_start, vma->vm_end, > - MADV_UNMERGEABLE, &vma->vm_flags)) { > - return -ENOMEM; > - } > + ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, > + MADV_UNMERGEABLE, &vma->vm_flags); > + if (ret) > + return ret; > } > mm->def_flags &= ~VM_MERGEABLE; > return 0; After the qemu patch discussion I would add diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 0e268b3d1591..ba8f9cbe4376 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -4680,6 +4680,12 @@ KVM_PV_ENABLE command has succeeded, any CPU added via hotplug will become protected during its creation as well. + Errors: + + ===== ============================= + EINTR an unmasked signal is pending + ===== ============================= + KVM_PV_DISABLE Deregister the VM from the Ultravisor and reclaim the memory that and change the patch description to something like s390/gmap: return proper error code on ksm unsharing If a signal is pending we might return -ENOMEM instead of -EINTR. We should propagate the proper error during KSM unsharing. unmerge_ksm_pages returns -ERESTARTSYS on signal_pending. This gets translated by entry.S to -EINTR. It is important to get this error code so that userspace can retry. Fixes: 3ac8e38015d4 ("s390/mm: disable KSM for storage key enabled pages") Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> Tested-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: David Hildenbrand <david@redhat.com> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
On 27.03.20 11:23, Christian Borntraeger wrote: > On 27.03.20 10:23, Christian Borntraeger wrote: >> If a signal is pending we might return -ENOMEM instead of -EINTR. >> We should propagate the proper error during KSM unsharing. >> >> Fixes: 3ac8e38015d4 ("s390/mm: disable KSM for storage key enabled pages") >> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> arch/s390/mm/gmap.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c >> index 27926a06df32..1ef864c252e6 100644 >> --- a/arch/s390/mm/gmap.c >> +++ b/arch/s390/mm/gmap.c >> @@ -2552,12 +2552,13 @@ int gmap_mark_unmergeable(void) >> { >> struct mm_struct *mm = current->mm; >> struct vm_area_struct *vma; >> + int ret; >> >> for (vma = mm->mmap; vma; vma = vma->vm_next) { >> - if (ksm_madvise(vma, vma->vm_start, vma->vm_end, >> - MADV_UNMERGEABLE, &vma->vm_flags)) { >> - return -ENOMEM; >> - } >> + ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, >> + MADV_UNMERGEABLE, &vma->vm_flags); >> + if (ret) >> + return ret; >> } >> mm->def_flags &= ~VM_MERGEABLE; >> return 0; > > > After the qemu patch discussion I would add > > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index 0e268b3d1591..ba8f9cbe4376 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -4680,6 +4680,12 @@ KVM_PV_ENABLE > command has succeeded, any CPU added via hotplug will become > protected during its creation as well. > > + Errors: > + > + ===== ============================= > + EINTR an unmasked signal is pending > + ===== ============================= > + > KVM_PV_DISABLE > > Deregister the VM from the Ultravisor and reclaim the memory that > > > and change the patch description to something like > > > s390/gmap: return proper error code on ksm unsharing > > If a signal is pending we might return -ENOMEM instead of -EINTR. > We should propagate the proper error during KSM unsharing. > unmerge_ksm_pages returns -ERESTARTSYS on signal_pending. This gets > translated by entry.S to -EINTR. It is important to get this error > code so that userspace can retry. > > Fixes: 3ac8e38015d4 ("s390/mm: disable KSM for storage key enabled pages") > Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> > Tested-by: Marc Hartmayer <mhartmay@linux.ibm.com> > Reviewed-by: David Hildenbrand <david@redhat.com> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > Looks good.
On Fri, 27 Mar 2020 11:23:33 +0100 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 27.03.20 10:23, Christian Borntraeger wrote: > After the qemu patch discussion I would add > > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index 0e268b3d1591..ba8f9cbe4376 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -4680,6 +4680,12 @@ KVM_PV_ENABLE > command has succeeded, any CPU added via hotplug will become > protected during its creation as well. > > + Errors: > + > + ===== ============================= > + EINTR an unmasked signal is pending > + ===== ============================= > + > KVM_PV_DISABLE > > Deregister the VM from the Ultravisor and reclaim the memory that > > > and change the patch description to something like > > > s390/gmap: return proper error code on ksm unsharing > > If a signal is pending we might return -ENOMEM instead of -EINTR. > We should propagate the proper error during KSM unsharing. > unmerge_ksm_pages returns -ERESTARTSYS on signal_pending. This gets > translated by entry.S to -EINTR. It is important to get this error > code so that userspace can retry. > > Fixes: 3ac8e38015d4 ("s390/mm: disable KSM for storage key enabled pages") > Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> > Tested-by: Marc Hartmayer <mhartmay@linux.ibm.com> > Reviewed-by: David Hildenbrand <david@redhat.com> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > LGTM
On Fri, 27 Mar 2020 11:39:54 +0100 Cornelia Huck <cohuck@redhat.com> wrote: > On Fri, 27 Mar 2020 11:23:33 +0100 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > > > On 27.03.20 10:23, Christian Borntraeger wrote: > > > After the qemu patch discussion I would add > > > > > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > > index 0e268b3d1591..ba8f9cbe4376 100644 > > --- a/Documentation/virt/kvm/api.rst > > +++ b/Documentation/virt/kvm/api.rst > > @@ -4680,6 +4680,12 @@ KVM_PV_ENABLE > > command has succeeded, any CPU added via hotplug will become > > protected during its creation as well. > > > > + Errors: > > + > > + ===== ============================= > > + EINTR an unmasked signal is pending > > + ===== ============================= > > + > > KVM_PV_DISABLE > > > > Deregister the VM from the Ultravisor and reclaim the memory that > > > > > > and change the patch description to something like > > > > > > s390/gmap: return proper error code on ksm unsharing > > > > If a signal is pending we might return -ENOMEM instead of -EINTR. > > We should propagate the proper error during KSM unsharing. > > unmerge_ksm_pages returns -ERESTARTSYS on signal_pending. This gets > > translated by entry.S to -EINTR. It is important to get this error > > code so that userspace can retry. > > > > Fixes: 3ac8e38015d4 ("s390/mm: disable KSM for storage key enabled pages") > > Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > > Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> > > Tested-by: Marc Hartmayer <mhartmay@linux.ibm.com> > > Reviewed-by: David Hildenbrand <david@redhat.com> > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > > > > LGTM ...well, if you add my Reviewed-by: Cornelia Huck <cohuck@redhat.com>
diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c index 27926a06df32..1ef864c252e6 100644 --- a/arch/s390/mm/gmap.c +++ b/arch/s390/mm/gmap.c @@ -2552,12 +2552,13 @@ int gmap_mark_unmergeable(void) { struct mm_struct *mm = current->mm; struct vm_area_struct *vma; + int ret; for (vma = mm->mmap; vma; vma = vma->vm_next) { - if (ksm_madvise(vma, vma->vm_start, vma->vm_end, - MADV_UNMERGEABLE, &vma->vm_flags)) { - return -ENOMEM; - } + ret = ksm_madvise(vma, vma->vm_start, vma->vm_end, + MADV_UNMERGEABLE, &vma->vm_flags); + if (ret) + return ret; } mm->def_flags &= ~VM_MERGEABLE; return 0;
If a signal is pending we might return -ENOMEM instead of -EINTR. We should propagate the proper error during KSM unsharing. Fixes: 3ac8e38015d4 ("s390/mm: disable KSM for storage key enabled pages") Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- arch/s390/mm/gmap.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)