From: Gerald Schaefer <gerald.schaefer@de.ibm.com>
To: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: linux-doc@vger.kernel.org,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Heiko Carstens <heiko.carstens@de.ibm.com>,
linux-mm@kvack.org, Paul Mackerras <paulus@samba.org>,
"H. Peter Anvin" <hpa@zytor.com>,
linux-riscv@lists.infradead.org, Will Deacon <will@kernel.org>,
linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
Jonathan Corbet <corbet@lwn.net>,
Michael Ellerman <mpe@ellerman.id.au>,
x86@kernel.org, Mike Rapoport <rppt@linux.ibm.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
Ingo Molnar <mingo@redhat.com>,
Gerald Schaefer <gerald.schaefer@de.ibm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
linux-snps-arc@lists.infradead.org,
Vasily Gorbik <gor@linux.ibm.com>, Borislav Petkov <bp@alien8.de>,
Paul Walmsley <paul.walmsley@sifive.com>,
"Kirill A . Shutemov" <kirill@shutemov.name>,
Thomas Gleixner <tglx@linutronix.de>,
linux-arm-kernel@lists.infradead.org, christophe.leroy@c-s.fr,
Vineet Gupta <vgupta@synopsys.com>,
linux-kernel@vger.kernel.org, Palmer Dabbelt <palmer@dabbelt.com>,
Andrew Morton <akpm@linux-foundation.org>,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH V2 0/3] mm/debug: Add more arch page table helper tests
Date: Wed, 8 Apr 2020 14:15:00 +0200 [thread overview]
Message-ID: <20200408141500.75b2e1a7@thinkpad> (raw)
In-Reply-To: <253cf5c8-e43e-5737-24e8-3eda3b6ba7b3@arm.com>
On Wed, 8 Apr 2020 12:41:51 +0530
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
[...]
> >
> >>
> >> Some thing like this instead.
> >>
> >> pte_t pte = READ_ONCE(*ptep);
> >> pte = pte_mkhuge(__pte((pte_val(pte) | RANDOM_ORVALUE) & PMD_MASK));
> >>
> >> We cannot use mk_pte_phys() as it is defined only on some platforms
> >> without any generic fallback for others.
> >
> > Oh, didn't know that, sorry. What about using mk_pte() instead, at least
> > it would result in a present pte:
> >
> > pte = pte_mkhuge(mk_pte(phys_to_page(RANDOM_ORVALUE & PMD_MASK), prot));
>
> Lets use mk_pte() here but can we do this instead
>
> paddr = (__pfn_to_phys(pfn) | RANDOM_ORVALUE) & PMD_MASK;
> pte = pte_mkhuge(mk_pte(phys_to_page(paddr), prot));
>
Sure, that will also work.
BTW, this RANDOM_ORVALUE is not really very random, the way it is
defined. For s390 we already changed it to mask out some arch bits,
but I guess there are other archs and bits that would always be
set with this "not so random" value, and I wonder if/how that would
affect all the tests using this value, see also below.
> >
> > And if you also want to do some with the existing value, which seems
> > to be an empty pte, then maybe just check if writing and reading that
> > value with set_huge_pte_at() / huge_ptep_get() returns the same,
> > i.e. initially w/o RANDOM_ORVALUE.
> >
> > So, in combination, like this (BTW, why is the barrier() needed, it
> > is not used for the other set_huge_pte_at() calls later?):
>
> Ahh missed, will add them. Earlier we faced problem without it after
> set_pte_at() for a test on powerpc (64) platform. Hence just added it
> here to be extra careful.
>
> >
> > @@ -733,24 +733,28 @@ static void __init hugetlb_advanced_test
> > struct page *page = pfn_to_page(pfn);
> > pte_t pte = READ_ONCE(*ptep);
> >
> > - pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
> > + set_huge_pte_at(mm, vaddr, ptep, pte);
> > + WARN_ON(!pte_same(pte, huge_ptep_get(ptep)));
> > +
> > + pte = pte_mkhuge(mk_pte(phys_to_page(RANDOM_ORVALUE & PMD_MASK), prot));
> > set_huge_pte_at(mm, vaddr, ptep, pte);
> > barrier();
> > WARN_ON(!pte_same(pte, huge_ptep_get(ptep)));
> >
> > This would actually add a new test "write empty pte with
> > set_huge_pte_at(), then verify with huge_ptep_get()", which happens
> > to trigger a warning on s390 :-)
>
> On arm64 as well which checks for pte_present() in set_huge_pte_at().
> But PTE present check is not really present in each set_huge_pte_at()
> implementation especially without __HAVE_ARCH_HUGE_SET_HUGE_PTE_AT.
> Hence wondering if we should add this new test here which will keep
> giving warnings on s390 and arm64 (at the least).
Hmm, interesting. I forgot about huge swap / migration, which is not
(and probably cannot be) supported on s390. The pte_present() check
on arm64 seems to check for such huge swap / migration entries,
according to the comment.
The new test "write empty pte with set_huge_pte_at(), then verify
with huge_ptep_get()" would then probably trigger the
WARN_ON(!pte_present(pte)) in arm64 code. So I guess "writing empty
ptes with set_huge_pte_at()" is not really a valid use case in practice,
or else you would have seen this warning before. In that case, it
might not be a good idea to add this test.
I also do wonder now, why the original test with
"pte = __pte(pte_val(pte) | RANDOM_ORVALUE);"
did not also trigger that warning on arm64. On s390 this test failed
exactly because the constructed pte was not present (initially empty,
or'ing RANDOM_ORVALUE does not make it present for s390). I guess this
just worked by chance on arm64, because the bits from RANDOM_ORVALUE
also happened to mark the pte present for arm64.
This brings us back to the question above, regarding the "randomness"
of RANDOM_ORVALUE. Not really sure what the intention behind that was,
but maybe it would make sense to restrict this RANDOM_ORVALUE to
non-arch-specific bits, i.e. only bits that would be part of the
address value within a page table entry? Or was it intentionally
chosen to also mess with other bits?
Regards,
Gerald
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
next prev parent reply other threads:[~2020-04-08 12:15 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-24 5:22 [PATCH V2 0/3] mm/debug: Add more arch page table helper tests Anshuman Khandual
2020-03-24 5:22 ` [PATCH V2 1/3] mm/debug: Add tests validating arch page table helpers for core features Anshuman Khandual
2020-03-24 13:29 ` Zi Yan
2020-03-26 2:18 ` Anshuman Khandual
2020-03-26 17:08 ` Zi Yan
2020-03-24 5:22 ` [PATCH V2 2/3] mm/debug: Add tests validating arch advanced page table helpers Anshuman Khandual
2020-03-27 7:41 ` [mm/debug] d157503f6f: WARNING:at_mm/debug_vm_pgtable.c:#debug_vm_pgtable kernel test robot
2020-03-26 2:23 ` [PATCH V2 0/3] mm/debug: Add more arch page table helper tests Anshuman Khandual
2020-03-26 15:23 ` Christophe Leroy
2020-03-27 6:46 ` Anshuman Khandual
2020-03-27 7:00 ` Christophe Leroy
2020-03-29 14:21 ` Anshuman Khandual
2020-03-31 12:30 ` Gerald Schaefer
2020-04-05 12:28 ` Anshuman Khandual
2020-04-07 15:54 ` Gerald Schaefer
2020-04-08 7:11 ` Anshuman Khandual
2020-04-08 12:15 ` Gerald Schaefer [this message]
2020-04-09 1:06 ` Anshuman Khandual
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=20200408141500.75b2e1a7@thinkpad \
--to=gerald.schaefer@de.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=anshuman.khandual@arm.com \
--cc=benh@kernel.crashing.org \
--cc=borntraeger@de.ibm.com \
--cc=bp@alien8.de \
--cc=catalin.marinas@arm.com \
--cc=christophe.leroy@c-s.fr \
--cc=corbet@lwn.net \
--cc=gor@linux.ibm.com \
--cc=heiko.carstens@de.ibm.com \
--cc=hpa@zytor.com \
--cc=kirill@shutemov.name \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-snps-arc@lists.infradead.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mingo@redhat.com \
--cc=mpe@ellerman.id.au \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=paulus@samba.org \
--cc=rppt@linux.ibm.com \
--cc=tglx@linutronix.de \
--cc=vgupta@synopsys.com \
--cc=will@kernel.org \
--cc=x86@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).