| Message ID | 20200327085602.24535-1-borntraeger@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390/gmap: return proper error code on ksm unsharing | expand |
On 27.03.20 09:56, 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") > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > arch/s390/mm/gmap.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c > index 27926a06df32..2bf63035a295 100644 > --- a/arch/s390/mm/gmap.c > +++ b/arch/s390/mm/gmap.c > @@ -2552,15 +2552,16 @@ int gmap_mark_unmergeable(void) > { > struct mm_struct *mm = current->mm; > struct vm_area_struct *vma; > + int ret = 0; > extra whitespace > 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; > + return ret; > } > EXPORT_SYMBOL_GPL(gmap_mark_unmergeable); > >
On 27.03.20 09:56, 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") > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > arch/s390/mm/gmap.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c > index 27926a06df32..2bf63035a295 100644 > --- a/arch/s390/mm/gmap.c > +++ b/arch/s390/mm/gmap.c > @@ -2552,15 +2552,16 @@ int gmap_mark_unmergeable(void) > { > struct mm_struct *mm = current->mm; > struct vm_area_struct *vma; > + int ret = 0; > > 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; > + return ret; How can this ever be !0 ? Also, once removed, no need to initialize ret to 0. > } > EXPORT_SYMBOL_GPL(gmap_mark_unmergeable); > >
On 27.03.20 10:07, David Hildenbrand wrote: > On 27.03.20 09:56, 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") >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> arch/s390/mm/gmap.c | 11 ++++++----- >> 1 file changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c >> index 27926a06df32..2bf63035a295 100644 >> --- a/arch/s390/mm/gmap.c >> +++ b/arch/s390/mm/gmap.c >> @@ -2552,15 +2552,16 @@ int gmap_mark_unmergeable(void) >> { >> struct mm_struct *mm = current->mm; >> struct vm_area_struct *vma; >> + int ret = 0; >> >> 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; >> + return ret; > > How can this ever be !0 ? > > Also, once removed, no need to initialize ret to 0. Right, I can make the patch smaller by keeping the return 0; > >> } >> EXPORT_SYMBOL_GPL(gmap_mark_unmergeable); >> >> > >
On 3/27/20 9:56 AM, 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") > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > --- > arch/s390/mm/gmap.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c > index 27926a06df32..2bf63035a295 100644 > --- a/arch/s390/mm/gmap.c > +++ b/arch/s390/mm/gmap.c > @@ -2552,15 +2552,16 @@ int gmap_mark_unmergeable(void) > { > struct mm_struct *mm = current->mm; > struct vm_area_struct *vma; > + int ret = 0; > > 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; > + return ret; That part is not really necessary but also doesn't hurt. > } > EXPORT_SYMBOL_GPL(gmap_mark_unmergeable); >
diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c index 27926a06df32..2bf63035a295 100644 --- a/arch/s390/mm/gmap.c +++ b/arch/s390/mm/gmap.c @@ -2552,15 +2552,16 @@ int gmap_mark_unmergeable(void) { struct mm_struct *mm = current->mm; struct vm_area_struct *vma; + int ret = 0; 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; + return ret; } EXPORT_SYMBOL_GPL(gmap_mark_unmergeable);
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") Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- arch/s390/mm/gmap.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)