Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
Le 18/09/2019 à 09:32, Anshuman Khandual a écrit : On 09/13/2019 11:53 AM, Christophe Leroy wrote: Fix build failure on powerpc. Fix preemption imbalance. Signed-off-by: Christophe Leroy --- mm/arch_pgtable_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c index 8b4a92756ad8..f2b3c9ec35fa 100644 --- a/mm/arch_pgtable_test.c +++ b/mm/arch_pgtable_test.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void) p4d_clear_tests(p4dp); pgd_clear_tests(mm, pgdp); + pte_unmap(ptep); + pmd_populate_tests(mm, pmdp, saved_ptep); pud_populate_tests(mm, pudp, saved_pmdp); p4d_populate_tests(mm, p4dp, saved_pudp); Hello Christophe, I am planning to fold this fix into the current patch and retain your Signed-off-by. Are you okay with it ? No problem, do whatever is convenient for you. You can keep the signed-off-by, or use tested-by: as I tested it on PPC32. Christophe ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH V2 2/2] mm/pgtable/debug: Add test validating architecture page table helpers
Le 19/09/2019 à 06:56, Anshuman Khandual a écrit :
On 09/18/2019 09:56 PM, Christophe Leroy wrote:
Le 18/09/2019 à 07:04, Anshuman Khandual a écrit :
On 09/13/2019 03:31 PM, Christophe Leroy wrote:
Le 13/09/2019 à 11:02, Anshuman Khandual a écrit :
+#if !defined(__PAGETABLE_PMD_FOLDED) && !defined(__ARCH_HAS_4LEVEL_HACK)
#ifdefs have to be avoided as much as possible, see below
Yeah but it has been bit difficult to avoid all these $ifdef because of the
availability (or lack of it) for all these pgtable helpers in various config
combinations on all platforms.
As far as I can see these pgtable helpers should exist everywhere at least via
asm-generic/ files.
But they might not actually do the right thing.
Can you spot a particular config which fails ?
Lets consider the following example (after removing the $ifdefs around it)
which though builds successfully but fails to pass the intended test. This
is with arm64 config 4K pages sizes with 39 bits VA space which ends up
with a 3 level page table arrangement.
static void __init p4d_clear_tests(p4d_t *p4dp)
{
p4d_t p4d = READ_ONCE(*p4dp);
My suggestion was not to completely drop the #ifdef but to do like you did in
pgd_clear_tests() for instance, ie to add the following test on top of the
function:
if (mm_pud_folded(mm) || is_defined(__ARCH_HAS_5LEVEL_HACK))
return;
Sometimes this does not really work. On some platforms, combination of
__PAGETABLE_PUD_FOLDED and __ARCH_HAS_5LEVEL_HACK decide whether the
helpers such as __pud() or __pgd() is even available for that platform.
Ideally it should have been through generic falls backs in include/*/
but I guess there might be bugs on the platform or it has not been
changed to adopt 5 level page table framework with required folding
macros etc.
Yes. As I suggested below, most likely that's better to retain the
#ifdef __ARCH_HAS_5LEVEL_HACK but change the #ifdef
__PAGETABLE_PUD_FOLDED by a runtime test of mm_pud_folded(mm)
As pointed by Gerald, some arches don't have __PAGETABLE_PUD_FOLDED
because they are deciding dynamically if they fold the level on not, but
have mm_pud_folded(mm)
p4d = __p4d(p4d_val(p4d) | RANDOM_ORVALUE);
WRITE_ONCE(*p4dp, p4d);
p4d_clear(p4dp);
p4d = READ_ONCE(*p4dp);
WARN_ON(!p4d_none(p4d));
}
The following test hits an error at WARN_ON(!p4d_none(p4d))
[ 16.757333] [ cut here ]
[ 16.758019] WARNING: CPU: 11 PID: 1 at mm/arch_pgtable_test.c:187
arch_pgtable_tests_init+0x24c/0x474
[...]
[ 16.781282] ---[ end trace 042e6c40c0a3b038 ]---
On arm64 (4K page size|39 bits VA|3 level page table)
#elif CONFIG_PGTABLE_LEVELS == 3 /* Applicable here */
#define __ARCH_USE_5LEVEL_HACK
#include
Which pulls in
#include
which pulls in
#include
which defines
static inline int p4d_none(p4d_t p4d)
{
return 0;
}
which will invariably trigger WARN_ON(!p4d_none(p4d)).
Similarly for next test p4d_populate_tests() which will always be
successful because p4d_bad() invariably returns negative.
static inline int p4d_bad(p4d_t p4d)
{
return 0;
}
static void __init p4d_populate_tests(struct mm_struct *mm, p4d_t *p4dp,
pud_t *pudp)
{
p4d_t p4d;
/*
* This entry points to next level page table page.
* Hence this must not qualify as p4d_bad().
*/
pud_clear(pudp);
p4d_clear(p4dp);
p4d_populate(mm, p4dp, pudp);
p4d = READ_ONCE(*p4dp);
WARN_ON(p4d_bad(p4d));
}
We should not run these tests for the above config because they are
not applicable and will invariably produce same result.
[...]
So it shouldn't be an issue. Maybe if a couple of arches miss them, the best
would be to fix the arches, since that's the purpose of your testsuite isn't it
?
The run time failures as explained previously is because of the folding which
needs to be protected as they are not even applicable. The compile time
failures are because pxx_populate() signatures are platform specific depending
on how many page table levels they really support.
So IIUC, the compiletime problem is around __ARCH_HAS_5LEVEL_HACK. For all #if
!defined(__PAGETABLE_PXX_FOLDED), something equivalent to the following should
make the trick.
if (mm_pxx_folded())
return;
For the __ARCH_HAS_5LEVEL_HACK stuff, I think we should be able to regroup all
impacted functions inside a single #ifdef __ARCH_HAS_5LEVEL_HACK
I was wondering if it will be better to
1) Minimize all #ifdefs in the code which might fail on some platforms
2) Restrict proposed test module to platforms where it builds and runs
3) Enable other platforms afterwards after fixing their build problems or other
requirements
I understand that __ARCH_HAS_5LEVEL_HACK is an HACK as its name
suggests, so you can't expect all platforms to go for an
Re: [PATCH V2 2/2] mm/pgtable/debug: Add test validating architecture page table helpers
On 09/18/2019 09:56 PM, Christophe Leroy wrote:
>
>
> Le 18/09/2019 à 07:04, Anshuman Khandual a écrit :
>>
>>
>> On 09/13/2019 03:31 PM, Christophe Leroy wrote:
>>>
>>>
>>> Le 13/09/2019 à 11:02, Anshuman Khandual a écrit :
>> +#if !defined(__PAGETABLE_PMD_FOLDED) && !defined(__ARCH_HAS_4LEVEL_HACK)
>
> #ifdefs have to be avoided as much as possible, see below
Yeah but it has been bit difficult to avoid all these $ifdef because of the
availability (or lack of it) for all these pgtable helpers in various
config
combinations on all platforms.
>>>
>>> As far as I can see these pgtable helpers should exist everywhere at least
>>> via asm-generic/ files.
>>
>> But they might not actually do the right thing.
>>
>>>
>>> Can you spot a particular config which fails ?
>>
>> Lets consider the following example (after removing the $ifdefs around it)
>> which though builds successfully but fails to pass the intended test. This
>> is with arm64 config 4K pages sizes with 39 bits VA space which ends up
>> with a 3 level page table arrangement.
>>
>> static void __init p4d_clear_tests(p4d_t *p4dp)
>> {
>> p4d_t p4d = READ_ONCE(*p4dp);
>
> My suggestion was not to completely drop the #ifdef but to do like you did in
> pgd_clear_tests() for instance, ie to add the following test on top of the
> function:
>
> if (mm_pud_folded(mm) || is_defined(__ARCH_HAS_5LEVEL_HACK))
> return;
>
Sometimes this does not really work. On some platforms, combination of
__PAGETABLE_PUD_FOLDED and __ARCH_HAS_5LEVEL_HACK decide whether the
helpers such as __pud() or __pgd() is even available for that platform.
Ideally it should have been through generic falls backs in include/*/
but I guess there might be bugs on the platform or it has not been
changed to adopt 5 level page table framework with required folding
macros etc.
>>
>> p4d = __p4d(p4d_val(p4d) | RANDOM_ORVALUE);
>> WRITE_ONCE(*p4dp, p4d);
>> p4d_clear(p4dp);
>> p4d = READ_ONCE(*p4dp);
>> WARN_ON(!p4d_none(p4d));
>> }
>>
>> The following test hits an error at WARN_ON(!p4d_none(p4d))
>>
>> [ 16.757333] [ cut here ]
>> [ 16.758019] WARNING: CPU: 11 PID: 1 at mm/arch_pgtable_test.c:187
>> arch_pgtable_tests_init+0x24c/0x474
>> [ 16.759455] Modules linked in:
>> [ 16.759952] CPU: 11 PID: 1 Comm: swapper/0 Not tainted
>> 5.3.0-next-20190916-5-g61c218153bb8-dirty #222
>> [ 16.761449] Hardware name: linux,dummy-virt (DT)
>> [ 16.762185] pstate: 0045 (nzcv daif +PAN -UAO)
>> [ 16.762964] pc : arch_pgtable_tests_init+0x24c/0x474
>> [ 16.763750] lr : arch_pgtable_tests_init+0x174/0x474
>> [ 16.764534] sp : ffc011d7bd50
>> [ 16.765065] x29: ffc011d7bd50 x28: 1756bac0
>> [ 16.765908] x27: ff85ddaf3000 x26: 02e8
>> [ 16.766767] x25: ffc0111ce000 x24: ff85ddaf32e8
>> [ 16.767606] x23: ff85ddaef278 x22: 0045cc844000
>> [ 16.768445] x21: 00065daef003 x20: 1754
>> [ 16.769283] x19: ff85ddb6 x18: 0014
>> [ 16.770122] x17: 980426bb x16: 698594c6
>> [ 16.770976] x15: 66e25a88 x14:
>> [ 16.771813] x13: 1754 x12: 000a
>> [ 16.772651] x11: ff85fcfd0a40 x10: 0001
>> [ 16.773488] x9 : 0008 x8 : ffc01143ab26
>> [ 16.774336] x7 : x6 :
>> [ 16.775180] x5 : x4 :
>> [ 16.776018] x3 : 1756bbe8 x2 : 00065daeb003
>> [ 16.776856] x1 : 0065daeb x0 : f000
>> [ 16.777693] Call trace:
>> [ 16.778092] arch_pgtable_tests_init+0x24c/0x474
>> [ 16.778843] do_one_initcall+0x74/0x1b0
>> [ 16.779458] kernel_init_freeable+0x1cc/0x290
>> [ 16.780151] kernel_init+0x10/0x100
>> [ 16.780710] ret_from_fork+0x10/0x18
>> [ 16.781282] ---[ end trace 042e6c40c0a3b038 ]---
>>
>> On arm64 (4K page size|39 bits VA|3 level page table)
>>
>> #elif CONFIG_PGTABLE_LEVELS == 3 /* Applicable here */
>> #define __ARCH_USE_5LEVEL_HACK
>> #include
>>
>> Which pulls in
>>
>> #include
>>
>> which pulls in
>>
>> #include
>>
>> which defines
>>
>> static inline int p4d_none(p4d_t p4d)
>> {
>> return 0;
>> }
>>
>> which will invariably trigger WARN_ON(!p4d_none(p4d)).
>>
>> Similarly for next test p4d_populate_tests() which will always be
>> successful because p4d_bad() invariably returns negative.
>>
>> static inline int p4d_bad(p4d_t p4d)
>> {
>> return 0;
>> }
>>
>> static void __init p4d_populate_tests(struct mm_struct *mm, p4d_t *p4dp,
>> pud_t *pudp)
>> {
>> p4d_t p4d;
>>
>> /*
>> * This entry points to next level page table page.
>> * Hence this must not qualify as p4d_bad().
>> */
>> pud_clear(pudp);
>> p4d_clear(p4dp);
>>
Re: stable backport for dc8635b78cd8669
On 9/18/19 11:56 AM, Greg Kroah-Hartman wrote: > So is this only needed in 4.9.y and 4.4.y? Yes indeed ! Thx, -Vineet ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: stable backport for dc8635b78cd8669
On Wed, Sep 18, 2019 at 10:40:32AM -0700, Vineet Gupta wrote:
> Hi Stable team,
>
> Can we please backport dc8635b78cd8669c37e230058d18c33af7451ab1
> ("kernel/exit.c:
> export abort() to modules")
>
> 0-Day kernel test infra reports ARC 4.x.y builds failing after backport of
> af1be2e21203867cb958aace ("ARC: handle gcc generated __builtin_trap for older
> compiler")
So is this only needed in 4.9.y and 4.4.y?
thanks,
greg k-h
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH V2 2/2] mm/pgtable/debug: Add test validating architecture page table helpers
On Wed, 18 Sep 2019 18:26:03 +0200 Christophe Leroy wrote: [..] > My suggestion was not to completely drop the #ifdef but to do like you > did in pgd_clear_tests() for instance, ie to add the following test on > top of the function: > > if (mm_pud_folded(mm) || is_defined(__ARCH_HAS_5LEVEL_HACK)) > return; > Ah, very nice, this would also fix the remaining issues for s390. Since we have dynamic page table folding, neither __PAGETABLE_PXX_FOLDED nor __ARCH_HAS_XLEVEL_HACK is defined, but mm_pxx_folded() will work. mm_alloc() returns with a 3-level page table by default on s390, so we will run into issues in p4d_clear/populate_tests(), and also at the end with p4d/pud_free() (double free). So, adding the mm_pud_folded() check to p4d_clear/populate_tests(), and also adding mm_p4d/pud_folded() checks at the end before calling p4d/pud_free(), would make it all work on s390. BTW, regarding p4d/pud_free(), I'm not sure if we should rather check the folding inside our s390 functions, similar to how we do it for p4d/pud_free_tlb(), instead of relying on not being called for folded p4d/pud. So far, I see no problem with this behavior, all callers of p4d/pud_free() should be fine because of our folding check within p4d/pud_present/none(). But that doesn't mean that it is correct not to check for the folding inside p4d/pud_free(). At least, with this test module we do now have a caller of p4d/pud_free() on potentially folded entries, so instead of adding pxx_folded() checks to this test module, we could add them to our p4d/pud_free() functions. Any thoughts on this? Regards, Gerald ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
stable backport for dc8635b78cd8669
Hi Stable team,
Can we please backport dc8635b78cd8669c37e230058d18c33af7451ab1 ("kernel/exit.c:
export abort() to modules")
0-Day kernel test infra reports ARC 4.x.y builds failing after backport of
af1be2e21203867cb958aace ("ARC: handle gcc generated __builtin_trap for older
compiler")
Thx
-Vineet
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH V2 2/2] mm/pgtable/debug: Add test validating architecture page table helpers
Le 18/09/2019 à 07:04, Anshuman Khandual a écrit :
On 09/13/2019 03:31 PM, Christophe Leroy wrote:
Le 13/09/2019 à 11:02, Anshuman Khandual a écrit :
+#if !defined(__PAGETABLE_PMD_FOLDED) && !defined(__ARCH_HAS_4LEVEL_HACK)
#ifdefs have to be avoided as much as possible, see below
Yeah but it has been bit difficult to avoid all these $ifdef because of the
availability (or lack of it) for all these pgtable helpers in various config
combinations on all platforms.
As far as I can see these pgtable helpers should exist everywhere at least via
asm-generic/ files.
But they might not actually do the right thing.
Can you spot a particular config which fails ?
Lets consider the following example (after removing the $ifdefs around it)
which though builds successfully but fails to pass the intended test. This
is with arm64 config 4K pages sizes with 39 bits VA space which ends up
with a 3 level page table arrangement.
static void __init p4d_clear_tests(p4d_t *p4dp)
{
p4d_t p4d = READ_ONCE(*p4dp);
My suggestion was not to completely drop the #ifdef but to do like you
did in pgd_clear_tests() for instance, ie to add the following test on
top of the function:
if (mm_pud_folded(mm) || is_defined(__ARCH_HAS_5LEVEL_HACK))
return;
p4d = __p4d(p4d_val(p4d) | RANDOM_ORVALUE);
WRITE_ONCE(*p4dp, p4d);
p4d_clear(p4dp);
p4d = READ_ONCE(*p4dp);
WARN_ON(!p4d_none(p4d));
}
The following test hits an error at WARN_ON(!p4d_none(p4d))
[ 16.757333] [ cut here ]
[ 16.758019] WARNING: CPU: 11 PID: 1 at mm/arch_pgtable_test.c:187
arch_pgtable_tests_init+0x24c/0x474
[ 16.759455] Modules linked in:
[ 16.759952] CPU: 11 PID: 1 Comm: swapper/0 Not tainted
5.3.0-next-20190916-5-g61c218153bb8-dirty #222
[ 16.761449] Hardware name: linux,dummy-virt (DT)
[ 16.762185] pstate: 0045 (nzcv daif +PAN -UAO)
[ 16.762964] pc : arch_pgtable_tests_init+0x24c/0x474
[ 16.763750] lr : arch_pgtable_tests_init+0x174/0x474
[ 16.764534] sp : ffc011d7bd50
[ 16.765065] x29: ffc011d7bd50 x28: 1756bac0
[ 16.765908] x27: ff85ddaf3000 x26: 02e8
[ 16.766767] x25: ffc0111ce000 x24: ff85ddaf32e8
[ 16.767606] x23: ff85ddaef278 x22: 0045cc844000
[ 16.768445] x21: 00065daef003 x20: 1754
[ 16.769283] x19: ff85ddb6 x18: 0014
[ 16.770122] x17: 980426bb x16: 698594c6
[ 16.770976] x15: 66e25a88 x14:
[ 16.771813] x13: 1754 x12: 000a
[ 16.772651] x11: ff85fcfd0a40 x10: 0001
[ 16.773488] x9 : 0008 x8 : ffc01143ab26
[ 16.774336] x7 : x6 :
[ 16.775180] x5 : x4 :
[ 16.776018] x3 : 1756bbe8 x2 : 00065daeb003
[ 16.776856] x1 : 0065daeb x0 : f000
[ 16.777693] Call trace:
[ 16.778092] arch_pgtable_tests_init+0x24c/0x474
[ 16.778843] do_one_initcall+0x74/0x1b0
[ 16.779458] kernel_init_freeable+0x1cc/0x290
[ 16.780151] kernel_init+0x10/0x100
[ 16.780710] ret_from_fork+0x10/0x18
[ 16.781282] ---[ end trace 042e6c40c0a3b038 ]---
On arm64 (4K page size|39 bits VA|3 level page table)
#elif CONFIG_PGTABLE_LEVELS == 3/* Applicable here */
#define __ARCH_USE_5LEVEL_HACK
#include
Which pulls in
#include
which pulls in
#include
which defines
static inline int p4d_none(p4d_t p4d)
{
return 0;
}
which will invariably trigger WARN_ON(!p4d_none(p4d)).
Similarly for next test p4d_populate_tests() which will always be
successful because p4d_bad() invariably returns negative.
static inline int p4d_bad(p4d_t p4d)
{
return 0;
}
static void __init p4d_populate_tests(struct mm_struct *mm, p4d_t *p4dp,
pud_t *pudp)
{
p4d_t p4d;
/*
* This entry points to next level page table page.
* Hence this must not qualify as p4d_bad().
*/
pud_clear(pudp);
p4d_clear(p4dp);
p4d_populate(mm, p4dp, pudp);
p4d = READ_ONCE(*p4dp);
WARN_ON(p4d_bad(p4d));
}
We should not run these tests for the above config because they are
not applicable and will invariably produce same result.
[...]
+#if !defined(__PAGETABLE_PUD_FOLDED) && !defined(__ARCH_HAS_5LEVEL_HACK)
The same can be done here.
IIRC not only the page table helpers but there are data types (pxx_t) which
were not present on various configs and these wrappers help prevent build
failures. Any ways will try and see if this can be improved further. But
meanwhile if you have some suggestions, please do let me know.
pgt_t and pmd_t are everywhere I guess.
then pud_t and p4d_t have fallbacks in asm-generic files.
Lets take another example where it fails to compile. On arm64 with 16K
page size, 48 bits VA, 4
Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
On 09/13/2019 11:53 AM, Christophe Leroy wrote: > Fix build failure on powerpc. > > Fix preemption imbalance. > > Signed-off-by: Christophe Leroy > --- > mm/arch_pgtable_test.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c > index 8b4a92756ad8..f2b3c9ec35fa 100644 > --- a/mm/arch_pgtable_test.c > +++ b/mm/arch_pgtable_test.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void) > p4d_clear_tests(p4dp); > pgd_clear_tests(mm, pgdp); > > + pte_unmap(ptep); > + > pmd_populate_tests(mm, pmdp, saved_ptep); > pud_populate_tests(mm, pudp, saved_pmdp); > p4d_populate_tests(mm, p4dp, saved_pudp); > Hello Christophe, I am planning to fold this fix into the current patch and retain your Signed-off-by. Are you okay with it ? - Anshuman ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
