| Message ID | 20180627135510.117945-9-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 27.06.2018 15:55, Janosch Frank wrote: > From: Dominik Dingel <dingel@linux.vnet.ibm.com> > > Guests backed by huge pages could theoretically free unused pages via > the diagnose 10 instruction. We currently don't allow that, so we > don't have to refault it once it's needed again. > > Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com> > Reviewed-by: Martin Schwidefsky <schwidefsky@de.ibm.com> > --- > arch/s390/mm/gmap.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c > index af0a87eeede0..c691b9d9d223 100644 > --- a/arch/s390/mm/gmap.c > +++ b/arch/s390/mm/gmap.c > @@ -723,6 +723,9 @@ void gmap_discard(struct gmap *gmap, unsigned long from, unsigned long to) > vmaddr |= gaddr & ~PMD_MASK; > /* Find vma in the parent mm */ > vma = find_vma(gmap->mm, vmaddr); > + /* We do not discard pages that are backed by hugetlbfs */ Can you add why we don't do that? > + if (vma && is_vm_hugetlb_page(vma)) > + continue; > size = min(to - gaddr, PMD_SIZE - (gaddr & ~PMD_MASK)); > zap_page_range(vma, vmaddr, size); > } > Apart from that Reviewed-by: David Hildenbrand <david@redhat.com>
On 28.06.2018 09:42, David Hildenbrand wrote: > On 27.06.2018 15:55, Janosch Frank wrote: >> From: Dominik Dingel <dingel@linux.vnet.ibm.com> >> >> Guests backed by huge pages could theoretically free unused pages via >> the diagnose 10 instruction. We currently don't allow that, so we >> don't have to refault it once it's needed again. >> >> Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com> >> Reviewed-by: Martin Schwidefsky <schwidefsky@de.ibm.com> >> --- >> arch/s390/mm/gmap.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c >> index af0a87eeede0..c691b9d9d223 100644 >> --- a/arch/s390/mm/gmap.c >> +++ b/arch/s390/mm/gmap.c >> @@ -723,6 +723,9 @@ void gmap_discard(struct gmap *gmap, unsigned long from, unsigned long to) >> vmaddr |= gaddr & ~PMD_MASK; >> /* Find vma in the parent mm */ >> vma = find_vma(gmap->mm, vmaddr); >> + /* We do not discard pages that are backed by hugetlbfs */ > > Can you add why we don't do that? It's in the patch description, but if you also need it here, sure. > >> + if (vma && is_vm_hugetlb_page(vma)) >> + continue; >> size = min(to - gaddr, PMD_SIZE - (gaddr & ~PMD_MASK)); >> zap_page_range(vma, vmaddr, size); >> } >> > > Apart from that > > Reviewed-by: David Hildenbrand <david@redhat.com> >
On 28.06.2018 12:13, Janosch Frank wrote: > On 28.06.2018 09:42, David Hildenbrand wrote: >> On 27.06.2018 15:55, Janosch Frank wrote: >>> From: Dominik Dingel <dingel@linux.vnet.ibm.com> >>> >>> Guests backed by huge pages could theoretically free unused pages via >>> the diagnose 10 instruction. We currently don't allow that, so we >>> don't have to refault it once it's needed again. >>> >>> Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com> >>> Reviewed-by: Martin Schwidefsky <schwidefsky@de.ibm.com> >>> --- >>> arch/s390/mm/gmap.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c >>> index af0a87eeede0..c691b9d9d223 100644 >>> --- a/arch/s390/mm/gmap.c >>> +++ b/arch/s390/mm/gmap.c >>> @@ -723,6 +723,9 @@ void gmap_discard(struct gmap *gmap, unsigned long from, unsigned long to) >>> vmaddr |= gaddr & ~PMD_MASK; >>> /* Find vma in the parent mm */ >>> vma = find_vma(gmap->mm, vmaddr); >>> + /* We do not discard pages that are backed by hugetlbfs */ >> >> Can you add why we don't do that? > > It's in the patch description, but if you also need it here, sure. "We currently don't allow that, so we don't have to refault it once it's needed again." why exactly can't we do that? Is it an artificial limitation? > >> >>> + if (vma && is_vm_hugetlb_page(vma)) >>> + continue; >>> size = min(to - gaddr, PMD_SIZE - (gaddr & ~PMD_MASK)); >>> zap_page_range(vma, vmaddr, size); >>> } >>> >> >> Apart from that >> >> Reviewed-by: David Hildenbrand <david@redhat.com> >> > >
On 28.06.2018 12:32, David Hildenbrand wrote: > On 28.06.2018 12:13, Janosch Frank wrote: >> On 28.06.2018 09:42, David Hildenbrand wrote: >>> On 27.06.2018 15:55, Janosch Frank wrote: >>>> From: Dominik Dingel <dingel@linux.vnet.ibm.com> >>>> >>>> Guests backed by huge pages could theoretically free unused pages via >>>> the diagnose 10 instruction. We currently don't allow that, so we >>>> don't have to refault it once it's needed again. >>>> >>>> Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com> >>>> Reviewed-by: Martin Schwidefsky <schwidefsky@de.ibm.com> >>>> --- >>>> arch/s390/mm/gmap.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c >>>> index af0a87eeede0..c691b9d9d223 100644 >>>> --- a/arch/s390/mm/gmap.c >>>> +++ b/arch/s390/mm/gmap.c >>>> @@ -723,6 +723,9 @@ void gmap_discard(struct gmap *gmap, unsigned long from, unsigned long to) >>>> vmaddr |= gaddr & ~PMD_MASK; >>>> /* Find vma in the parent mm */ >>>> vma = find_vma(gmap->mm, vmaddr); >>>> + /* We do not discard pages that are backed by hugetlbfs */ >>> >>> Can you add why we don't do that? >> >> It's in the patch description, but if you also need it here, sure. > > "We currently don't allow that, so we don't have to refault it once it's > needed again." why exactly can't we do that? Is it an artificial limitation? diag44 doesn't make sense on areas that are not pmd aligned and sized for huge guests. Also I'm not sure if a hpage would go back to the VMs pool or the general pool and both cases are not beneficial. > >> >>> >>>> + if (vma && is_vm_hugetlb_page(vma)) >>>> + continue; >>>> size = min(to - gaddr, PMD_SIZE - (gaddr & ~PMD_MASK)); >>>> zap_page_range(vma, vmaddr, size); >>>> } >>>> >>> >>> Apart from that >>> >>> Reviewed-by: David Hildenbrand <david@redhat.com> >>> >> >> > >
diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c index af0a87eeede0..c691b9d9d223 100644 --- a/arch/s390/mm/gmap.c +++ b/arch/s390/mm/gmap.c @@ -723,6 +723,9 @@ void gmap_discard(struct gmap *gmap, unsigned long from, unsigned long to) vmaddr |= gaddr & ~PMD_MASK; /* Find vma in the parent mm */ vma = find_vma(gmap->mm, vmaddr); + /* We do not discard pages that are backed by hugetlbfs */ + if (vma && is_vm_hugetlb_page(vma)) + continue; size = min(to - gaddr, PMD_SIZE - (gaddr & ~PMD_MASK)); zap_page_range(vma, vmaddr, size); }