Re: [PATCH V2 2/2] mm/pgtable/debug: Add test validating architecture page table helpers
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);
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 level page table arrangement in the following
test, pgd_populate() does not have the required signature.
static void pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp, p4d_t *p4dp)
{
pgd_t pgd;
if (mm_p4d_folded(mm))
kisskb: OK linus/axs101_defconfig/arcompact Wed Sep 18, 13:22
OK linus/axs101_defconfig/arcompact Wed Sep 18, 13:22 http://kisskb.ellerman.id.au/kisskb/buildresult/13960599/ Commit: Merge tag 'docs-5.4' of git://git.lwn.net/linux 7c672abc120a55f678e5571ae2ee93f06ca4d7f9 Compiler: arc-buildroot-linux-uclibc-gcc (Buildroot 2015.08.1) 4.8.4 / GNU ld (GNU Binutils) 2.23.2 No errors found in log Possible warnings (3) -- :1511:2: warning: #warning syscall clone3 not implemented [-Wcpp] net/ipv4/tcp_input.c:4341:49: warning: array subscript is above array bounds [-Warray-bounds] net/xfrm/xfrm_policy.c:3496:15: warning: array subscript is above array bounds [-Warray-bounds] ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
kisskb: OK linus/axs103_smp_defconfig/arcv2 Wed Sep 18, 13:22
OK linus/axs103_smp_defconfig/arcv2 Wed Sep 18, 13:22
http://kisskb.ellerman.id.au/kisskb/buildresult/13960598/
Commit: Merge tag 'docs-5.4' of git://git.lwn.net/linux
7c672abc120a55f678e5571ae2ee93f06ca4d7f9
Compiler: arc-linux-gcc.br_real (Buildroot 2016.11-git-00613-ge98b4dd) 6.2.1
20160824 / GNU ld (GNU Binutils) 2.27.51.20160928
Possible errors
---
#define KERN_ERR KERN_SOH "3" /* error conditions */
#define KERN_ERR KERN_SOH "3" /* error conditions */
#define KERN_ERR KERN_SOH "3" /* error conditions */
#define KERN_ERR KERN_SOH "3" /* error conditions */
Possible warnings (91)
--
:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
init/main.c:381:35: warning: format '%zu' expects argument of type 'size_t',
but argument 3 has type 'unsigned int' [-Wformat=]
init/main.c:385:35: warning: format '%zu' expects argument of type 'size_t',
but argument 3 has type 'unsigned int' [-Wformat=]
init/main.c:389:35: warning: format '%zu' expects argument of type 'size_t',
but argument 3 has type 'unsigned int' [-Wformat=]
init/main.c:825:37: warning: format '%zu' expects argument of type 'size_t',
but argument 3 has type '__kernel_size_t {aka unsigned int}' [-Wformat=]
include/linux/kernel.h:821:29: warning: comparison of distinct pointer types
lacks a cast
include/linux/kernel.h:821:29: warning: comparison of distinct pointer types
lacks a cast
include/linux/kernel.h:821:29: warning: comparison of distinct pointer types
lacks a cast
include/linux/kernel.h:821:29: warning: comparison of distinct pointer types
lacks a cast
include/linux/kernel.h:821:29: warning: comparison of distinct pointer types
lacks a cast
include/linux/kernel.h:821:29: warning: comparison of distinct pointer types
lacks a cast
kernel/dma/direct.c:32:4: warning: format '%zu' expects argument of type
'size_t', but argument 4 has type 'unsigned int' [-Wformat=]
include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of
type 'size_t', but argument 2 has type 'unsigned int' [-Wformat=]
include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of
type 'size_t', but argument 2 has type 'unsigned int' [-Wformat=]
drivers/base/component.c:196:24: warning: format '%zu' expects argument of type
'size_t', but argument 4 has type 'unsigned int' [-Wformat=]
drivers/base/regmap/regmap.c:1526:22: warning: format '%zu' expects argument of
type 'size_t', but argument 5 has type 'unsigned int' [-Wformat=]
include/linux/overflow.h:59:15: warning: comparison of distinct pointer types
lacks a cast
include/linux/overflow.h:60:15: warning: comparison of distinct pointer types
lacks a cast
drivers/base/regmap/regcache.c:715:20: warning: format '%zu' expects argument
of type 'size_t', but argument 4 has type 'unsigned int' [-Wformat=]
mm/percpu.c:1334:35: warning: format '%zu' expects argument of type 'size_t',
but argument 3 has type 'unsigned int' [-Wformat=]
mm/percpu.c:1349:35: warning: format '%zu' expects argument of type 'size_t',
but argument 3 has type 'unsigned int' [-Wformat=]
mm/percpu.c:1356:35: warning: format '%zu' expects argument of type 'size_t',
but argument 3 has type 'unsigned int' [-Wformat=]
mm/percpu.c:1362:35: warning: format '%zu' expects argument of type 'size_t',
but argument 3 has type 'unsigned int' [-Wformat=]
mm/percpu.c:1616:17: warning: format '%zu' expects argument of type 'size_t',
but argument 4 has type 'unsigned int' [-Wformat=]
mm/percpu.c:1616:17: warning: format '%zu' expects argument of type 'size_t',
but argument 5 has type 'unsigned int' [-Wformat=]
include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of
type 'size_t', but argument 2 has type 'unsigned int' [-Wformat=]
#define KERN_WARNING KERN_SOH "4" /* warning conditions */
include/linux/kern_levels.h:5:18: warning: format '%zu' expects argument of
type 'size_t', but argument 3 has type 'unsigned int' [-Wformat=]
#define KERN_WARNING KERN_SOH "4" /* warning conditions */
mm/percpu.c:2189:27: warning: format '%zu' expects argument of type 'size_t',
but argument 3 has type 'unsigned int' [-Wformat=]
mm/percpu.c:2189:32: warning: format '%zu' expects argument of type 'size_t',
but argument 4 has type 'unsigned int' [-Wformat=]
mm/percpu.c:2189:37: warning: format '%zu' expects argument of type 'size_t',
but argument 5 has type 'unsigned int' [-Wformat=]
mm/percpu.c:2189:42: warning: format '%zu' expects argument of type 'size_t',
but argument 6 has type 'unsigned int' [-Wformat=]
mm/percpu.c:2189:52: warning: format '%zu' expects argument of type 'size_t',
but argument 7 has type 'unsigned int' [-Wformat=]
mm/percpu.c:2189:56: warning: format '%zu' expects argument of type 'size_t',
but argument 8 has type 'unsigned int' [-Wformat=]
mm/percpu.c:2320:35: warning: format '%zu' expects argument of type 'size_t',
but argument 3 has type 'unsigned int' [-Wformat=]
mm/percpu.c:2326:35: warning: format '%zu' expects argument of
Re: [PATCH 3/6] ARC: mm: TLB Miss optim: avoid re-reading ECR
On 9/16/19 4:36 PM, Alexey Brodkin wrote: >> >> ; >> ; UPDATE_PTE: Let Linux VM know that page was accessed/dirty > > I'd suggest to put a BOLD comment here saying that we rely on previously > set condition flag so that whoever reads or (even worse) modifies that or > previous code keeps in mind that we shouldn't clobber a particular flag. The flag setting code is only a few lines prior. It would be messy to annotate for flag checking instruction where the flags are clobbered. This is low level assembly code - not for faint hearted. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
