From: Gerald Schaefer <gerald.schaefer@de.ibm.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
viro@zeniv.linux.org.uk, schwidefsky@de.ibm.com,
rientjes@google.com, riel@redhat.com, peterz@infradead.org,
pbonzini@redhat.com, mingo@kernel.org, mgorman@suse.de,
kirill.shutemov@linux.intel.com, heiko.carstens@de.ibm.com,
hannes@cmpxchg.org, ebiederm@xmission.com, aarcange@redhat.com,
athorlton@sgi.com
Subject: Re: + mm-revert-thp-make-madv_hugepage-check-for-mm-def_flags.patch added to -mm tree
Date: Wed, 26 Feb 2014 17:55:33 +0100 [thread overview]
Message-ID: <20140226175533.43dc1395@thinkpad> (raw)
In-Reply-To: <20140226153144.GA15527@redhat.com>
On Wed, 26 Feb 2014 16:31:44 +0100
Oleg Nesterov <oleg@redhat.com> wrote:
> On 02/26, Christian Borntraeger wrote:
> >
> > On 26/02/14 15:50, Oleg Nesterov wrote:
> > >
> > > But perhaps qemu can be changed to avoid MADV_HUGEPAGE on s390 ?
> > > Otherwise I'd suggest the change below.
> > >
> > > Oleg.
> > >
> > >
> > > --- x/mm/huge_memory.c
> > > +++ x/mm/huge_memory.c
> > > @@ -1968,8 +1968,6 @@ out:
> > > int hugepage_madvise(struct vm_area_struct *vma,
> > > unsigned long *vm_flags, int advice)
> > > {
> > > - struct mm_struct *mm = vma->vm_mm;
> > > -
> > > switch (advice) {
> > > case MADV_HUGEPAGE:
> > > /*
> > > @@ -1977,8 +1975,16 @@ int hugepage_madvise(struct vm_area_stru
> > > */
> > > if (*vm_flags & (VM_HUGEPAGE | VM_NO_THP))
> > > return -EINVAL;
> > > - if (mm->def_flags & VM_NOHUGEPAGE)
> > > +
> > > +/*
> > > + * MADV_HUGEPAGE after PRCTL_THP_DISABLE is broken on s390 because
> > > + * qemu blindly does madvise(MADV_HUGEPAGE) after s390_enable_sie().
> > > + */
> > > +#ifdef CONFIG_S390
> > > + if (vma->vm_mm->def_flags & VM_NOHUGEPAGE)
> > > return -EINVAL;
> > > +#endif
> > > +
> >
> > Ifdefs are ugly but might be the only way of not breaking existing userspace.
>
> Yes, agreed. but note that this code is already s390-specific, nobody
> else puts VM_NOHUGEPAGE into ->def_flags (until this series of course).
>
> > If we come up with a solution for THP in KVM host processes on s390, we can
> > then remove that wart. We could even limit that hack to KVM only processes
> > to retain Alex' prctl capability by checking mm_has_pgste
>
> Yes, yes, I looked at mm->context.has_pgste too ;) But I wasn't sure
> we can rely on it. Thanks.
>
> > > +/*
> > > + * MADV_HUGEPAGE after PRCTL_THP_DISABLE is broken on s390 because
> > > + * qemu blindly does madvise(MADV_HUGEPAGE) after s390_enable_sie().
> > > + */
> > > +#ifdef CONFIG_S390
> > > + if ((vma->vm_mm->def_flags & VM_NOHUGEPAGE) && mm_has_pgste(vma->vm_mm))
> > > return -EINVAL;
>
> Hmm... but why do we need to check VM_NOHUGEPAGE then? Can't the
> change below work?
Yes, good point, that should do the trick.
>
> Oleg.
>
> --- x/arch/s390/mm/pgtable.c
> +++ x/arch/s390/mm/pgtable.c
> @@ -1084,7 +1084,6 @@ static inline void thp_split_mm(struct m
> vma->vm_flags &= ~VM_HUGEPAGE;
> vma->vm_flags |= VM_NOHUGEPAGE;
> }
> - mm->def_flags |= VM_NOHUGEPAGE;
> }
> #else
> static inline void thp_split_mm(struct mm_struct *mm)
> --- x/mm/huge_memory.c
> +++ x/mm/huge_memory.c
> @@ -1968,8 +1968,6 @@ out:
> int hugepage_madvise(struct vm_area_struct *vma,
> unsigned long *vm_flags, int advice)
> {
> - struct mm_struct *mm = vma->vm_mm;
> -
> switch (advice) {
> case MADV_HUGEPAGE:
> /*
> @@ -1977,8 +1975,12 @@ int hugepage_madvise(struct vm_area_stru
> */
> if (*vm_flags & (VM_HUGEPAGE | VM_NO_THP))
> return -EINVAL;
> - if (mm->def_flags & VM_NOHUGEPAGE)
> +
> +#ifdef CONFIG_S390
> + if (mm_has_pgste(vma->vm_mm))
> return -EINVAL;
> +#endif
> +
> *vm_flags &= ~VM_NOHUGEPAGE;
> *vm_flags |= VM_HUGEPAGE;
> /*
>
next prev parent reply other threads:[~2014-02-26 16:55 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-25 23:53 + mm-revert-thp-make-madv_hugepage-check-for-mm-def_flags.patch added to -mm tree akpm
[not found] ` <530D9F50.1080400@de.ibm.com>
2014-02-26 14:50 ` Oleg Nesterov
2014-02-26 15:06 ` Christian Borntraeger
2014-02-26 15:22 ` Kirill A. Shutemov
2014-02-26 15:31 ` Oleg Nesterov
2014-02-26 16:55 ` Gerald Schaefer [this message]
2014-02-26 16:57 ` Peter Zijlstra
2014-02-26 17:22 ` Alex Thorlton
2014-02-26 18:06 ` Oleg Nesterov
2014-02-26 19:05 ` Gerald Schaefer
2014-02-27 16:45 ` Oleg Nesterov
2014-02-26 19:27 ` Christian Borntraeger
2014-02-26 19:39 ` Alex Thorlton
2014-02-26 23:24 ` Andrew Morton
2014-02-27 0:01 ` Alex Thorlton
2014-02-27 17:26 ` Alex Thorlton
2014-02-26 20:41 ` Paolo Bonzini
2014-02-27 16:34 ` Oleg Nesterov
2014-02-26 18:08 ` Oleg Nesterov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140226175533.43dc1395@thinkpad \
--to=gerald.schaefer@de.ibm.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=athorlton@sgi.com \
--cc=borntraeger@de.ibm.com \
--cc=ebiederm@xmission.com \
--cc=hannes@cmpxchg.org \
--cc=heiko.carstens@de.ibm.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@kernel.org \
--cc=oleg@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=riel@redhat.com \
--cc=rientjes@google.com \
--cc=schwidefsky@de.ibm.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.