Re: [PATCH 1/3] Revert "thp: make MADV_HUGEPAGE check formm->def_flags"

From: Gerald Schaefer
Date: Mon Feb 03 2014 - 08:53:41 EST


On Fri, 31 Jan 2014 14:52:24 -0800
Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Fri, 31 Jan 2014 12:23:43 -0600 Alex Thorlton <athorlton@xxxxxxx> wrote:
>
> > This reverts commit 8e72033f2a489b6c98c4e3c7cc281b1afd6cb85cm, and adds
>
> 'm' is not a hex digit ;)
>
> > in code to fix up any issues caused by the revert.
> >
> > The revert is necessary because hugepage_madvise would return -EINVAL
> > when VM_NOHUGEPAGE is set, which will break subsequent chunks of this
> > patch set.
>
> This is a bit skimpy. Why doesn't the patch re-break kvm-on-s390?
>
> it would be nice to have a lot more detail here, please. What was the
> intent of 8e72033f2a48, how this patch retains 8e72033f2a48's behavior,
> etc.

The intent of 8e72033f2a48 was to guard against any future programming
errors that may result in an madvice(MADV_HUGEPAGE) on guest mappings,
which would crash the kernel.

Martin suggested adding the bit to arch/s390/mm/pgtable.c, if 8e72033f2a48
was to be reverted, because that check will also prevent a kernel crash
in the case described above, it will now send a SIGSEGV instead.

This would now also allow to do the madvise on other parts, if needed,
so it is a more flexible approach. One could also say that it would have
been better to do it this way right from the beginning...

> > --- a/arch/s390/mm/pgtable.c
> > +++ b/arch/s390/mm/pgtable.c
> > @@ -504,6 +504,9 @@ static int gmap_connect_pgtable(unsigned long address, unsigned long segment,
> > if (!pmd_present(*pmd) &&
> > __pte_alloc(mm, vma, pmd, vmaddr))
> > return -ENOMEM;
> > + /* large pmds cannot yet be handled */
> > + if (pmd_large(*pmd))
> > + return -EFAULT;
>
> This bit wasn't in 8e72033f2a48.

Yes, in order to be on the safe side regarding potential distribution
backports, it would be good to have the revert and the "replacement"
in the same patch.

>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-s390" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

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