2018-03-17 07:54:20

by Fengguang Wu

[permalink] [raw]
Subject: [mm] b1f0502d04: INFO:trying_to_register_non-static_key

FYI, we noticed the following commit (built with gcc-7):

commit: b1f0502d04537ef55b0c296823affe332b100eb5 ("mm: VMA sequence count")
url: https://github.com/0day-ci/linux/commits/Laurent-Dufour/Speculative-page-faults/20180316-151833


in testcase: trinity
with following parameters:

runtime: 300s

test-description: Trinity is a linux system call fuzz tester.
test-url: http://codemonkey.org.uk/projects/trinity/


on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -m 512M

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


+----------------------------------------+------------+------------+
| | 6a4ce82339 | b1f0502d04 |
+----------------------------------------+------------+------------+
| boot_successes | 8 | 4 |
| boot_failures | 0 | 4 |
| INFO:trying_to_register_non-static_key | 0 | 4 |
+----------------------------------------+------------+------------+



[ 22.212940] INFO: trying to register non-static key.
[ 22.213687] the code is fine but needs lockdep annotation.
[ 22.214459] turning off the locking correctness validator.
[ 22.227459] CPU: 0 PID: 547 Comm: trinity-main Not tainted 4.16.0-rc4-next-20180309-00007-gb1f0502 #239
[ 22.228904] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[ 22.230043] Call Trace:
[ 22.230409] dump_stack+0x5d/0x79
[ 22.231025] register_lock_class+0x226/0x45e
[ 22.231827] ? kvm_clock_read+0x21/0x30
[ 22.232544] ? kvm_sched_clock_read+0x5/0xd
[ 22.233330] __lock_acquire+0xa2/0x774
[ 22.234152] lock_acquire+0x4b/0x66
[ 22.234805] ? unmap_vmas+0x30/0x3d
[ 22.245680] unmap_page_range+0x56/0x48c
[ 22.248127] ? unmap_vmas+0x30/0x3d
[ 22.248741] ? lru_deactivate_file_fn+0x2c6/0x2c6
[ 22.249537] ? pagevec_lru_move_fn+0x9a/0xa9
[ 22.250244] unmap_vmas+0x30/0x3d
[ 22.250791] unmap_region+0xad/0x105
[ 22.251419] mmap_region+0x3cc/0x455
[ 22.252011] do_mmap+0x394/0x3e9
[ 22.261224] vm_mmap_pgoff+0x9c/0xe5
[ 22.261798] SyS_mmap_pgoff+0x19a/0x1d4
[ 22.262475] ? task_work_run+0x5e/0x9c
[ 22.263163] do_syscall_64+0x6d/0x103
[ 22.263814] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[ 22.264697] RIP: 0033:0x4573da
[ 22.267248] RSP: 002b:00007fffa22f1398 EFLAGS: 00000246 ORIG_RAX: 0000000000000009
[ 22.274720] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00000000004573da
[ 22.276083] RDX: 0000000000000001 RSI: 0000000000001000 RDI: 0000000000000000
[ 22.277343] RBP: 000000000000001c R08: 000000000000001c R09: 0000000000000000
[ 22.278686] R10: 0000000000000002 R11: 0000000000000246 R12: 0000000000000000
[ 22.279930] R13: 0000000000001000 R14: 0000000000000002 R15: 0000000000000000
[ 22.391866] trinity-main uses obsolete (PF_INET,SOCK_PACKET)
[ 327.566956] sysrq: SysRq : Emergency Sync
[ 327.567849] Emergency Sync complete
[ 327.569975] sysrq: SysRq : Resetting

Elapsed time: 330

#!/bin/bash

# To reproduce,
# 1) save job-script and this script (both are attached in 0day report email)
# 2) run this script with your compiled kernel and optional env $INSTALL_MOD_PATH

kernel=$1

initrds=(
/osimage/yocto/yocto-minimal-x86_64-2016-04-22.cgz
/lkp/lkp/lkp-x86_64.cgz
/osimage/pkg/debian-x86_64-2016-08-31.cgz/trinity-static-x86_64-x86_64-6ddabfd2_2017-11-10.cgz
)

HTTP_PREFIX=https://github.com/0day-ci/lkp-qemu/raw/master
wget --timestamping "${initrds[@]/#/$HTTP_PREFIX}"

{
cat "${initrds[@]//*\//}"
[[ $INSTALL_MOD_PATH ]] && (
cd "$INSTALL_MOD_PATH"
find lib | cpio -o -H newc --quiet | gzip
)
echo job-script | cpio -o -H newc --quiet | gzip
} > initrd.img

qemu-img create -f qcow2 disk-vm-kbuild-yocto-x86_64-62-0 256G

kvm=(
qemu-system-x86_64
-enable-kvm
-cpu SandyBridge
-kernel $kernel
-initrd initrd.img
-m 512
-smp 1
-device e1000,netdev=net0
-netdev user,id=net0
-boot order=nc
-no-reboot
-watchdog i6300esb
-watchdog-action debug
-rtc base=localtime
-drive file=disk-vm-kbuild-yocto-x86_64-62-0,media=disk,if=virtio
-serial stdio
-display none
-monitor null
)

append=(
ip=::::vm-kbuild-yocto-x86_64-62::dhcp
root=/dev/ram0
user=lkp
job=/job-script
ARCH=x86_64
kconfig=x86_64-acpi-redef
branch=linux-devel/devel-catchup-201803161558
commit=b1f0502d04537ef55b0c296823affe332b100eb5
BOOT_IMAGE=/pkg/linux/x86_64-acpi-redef/gcc-7/b1f0502d04537ef55b0c296823affe332b100eb5/vmlinuz-4.16.0-rc4-next-20180309-00007-gb1f0502
max_uptime=1500
RESULT_ROOT=/result/trinity/300s/vm-kbuild-yocto-x86_64/yocto-minimal-x86_64-2016-04-22.cgz/x86_64-acpi-redef/gcc-7/b1f0502d04537ef55b0c296823affe332b100eb5/0


To reproduce:

git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email



Thanks,
lkp


Attachments:
(No filename) (4.95 kB)
config-4.16.0-rc4-next-20180309-00007-gb1f0502 (127.49 kB)
job-script (3.79 kB)
dmesg.xz (13.42 kB)
Download all attachments

2018-03-21 12:22:33

by Laurent Dufour

[permalink] [raw]
Subject: Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key



On 17/03/2018 08:51, kernel test robot wrote:
> FYI, we noticed the following commit (built with gcc-7):
>
> commit: b1f0502d04537ef55b0c296823affe332b100eb5 ("mm: VMA sequence count")
> url: https://github.com/0day-ci/linux/commits/Laurent-Dufour/Speculative-page-faults/20180316-151833
>
>
> in testcase: trinity
> with following parameters:
>
> runtime: 300s
>
> test-description: Trinity is a linux system call fuzz tester.
> test-url: http://codemonkey.org.uk/projects/trinity/
>
>
> on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -m 512M
>
> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
>
>
> +----------------------------------------+------------+------------+
> | | 6a4ce82339 | b1f0502d04 |
> +----------------------------------------+------------+------------+
> | boot_successes | 8 | 4 |
> | boot_failures | 0 | 4 |
> | INFO:trying_to_register_non-static_key | 0 | 4 |
> +----------------------------------------+------------+------------+
>
>
>
> [ 22.212940] INFO: trying to register non-static key.
> [ 22.213687] the code is fine but needs lockdep annotation.
> [ 22.214459] turning off the locking correctness validator.
> [ 22.227459] CPU: 0 PID: 547 Comm: trinity-main Not tainted 4.16.0-rc4-next-20180309-00007-gb1f0502 #239
> [ 22.228904] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> [ 22.230043] Call Trace:
> [ 22.230409] dump_stack+0x5d/0x79
> [ 22.231025] register_lock_class+0x226/0x45e
> [ 22.231827] ? kvm_clock_read+0x21/0x30
> [ 22.232544] ? kvm_sched_clock_read+0x5/0xd
> [ 22.233330] __lock_acquire+0xa2/0x774
> [ 22.234152] lock_acquire+0x4b/0x66
> [ 22.234805] ? unmap_vmas+0x30/0x3d
> [ 22.245680] unmap_page_range+0x56/0x48c
> [ 22.248127] ? unmap_vmas+0x30/0x3d
> [ 22.248741] ? lru_deactivate_file_fn+0x2c6/0x2c6
> [ 22.249537] ? pagevec_lru_move_fn+0x9a/0xa9
> [ 22.250244] unmap_vmas+0x30/0x3d
> [ 22.250791] unmap_region+0xad/0x105
> [ 22.251419] mmap_region+0x3cc/0x455
> [ 22.252011] do_mmap+0x394/0x3e9
> [ 22.261224] vm_mmap_pgoff+0x9c/0xe5
> [ 22.261798] SyS_mmap_pgoff+0x19a/0x1d4
> [ 22.262475] ? task_work_run+0x5e/0x9c
> [ 22.263163] do_syscall_64+0x6d/0x103
> [ 22.263814] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> [ 22.264697] RIP: 0033:0x4573da
> [ 22.267248] RSP: 002b:00007fffa22f1398 EFLAGS: 00000246 ORIG_RAX: 0000000000000009
> [ 22.274720] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00000000004573da
> [ 22.276083] RDX: 0000000000000001 RSI: 0000000000001000 RDI: 0000000000000000
> [ 22.277343] RBP: 000000000000001c R08: 000000000000001c R09: 0000000000000000
> [ 22.278686] R10: 0000000000000002 R11: 0000000000000246 R12: 0000000000000000
> [ 22.279930] R13: 0000000000001000 R14: 0000000000000002 R15: 0000000000000000
> [ 22.391866] trinity-main uses obsolete (PF_INET,SOCK_PACKET)
> [ 327.566956] sysrq: SysRq : Emergency Sync
> [ 327.567849] Emergency Sync complete
> [ 327.569975] sysrq: SysRq : Resetting

I found the root cause of this lockdep warning.

In mmap_region(), unmap_region() may be called while vma_link() has not been
called. This happens during the error path if call_mmap() failed.

The only to fix that particular case is to call
seqcount_init(&vma->vm_sequence) when initializing the vma in mmap_region().

Thanks,
Laurent.


2018-03-25 22:12:25

by David Rientjes

[permalink] [raw]
Subject: Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key

On Wed, 21 Mar 2018, Laurent Dufour wrote:

> I found the root cause of this lockdep warning.
>
> In mmap_region(), unmap_region() may be called while vma_link() has not been
> called. This happens during the error path if call_mmap() failed.
>
> The only to fix that particular case is to call
> seqcount_init(&vma->vm_sequence) when initializing the vma in mmap_region().
>

Ack, although that would require a fixup to dup_mmap() as well.

2018-03-28 13:32:26

by Laurent Dufour

[permalink] [raw]
Subject: Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key

On 26/03/2018 00:10, David Rientjes wrote:
> On Wed, 21 Mar 2018, Laurent Dufour wrote:
>
>> I found the root cause of this lockdep warning.
>>
>> In mmap_region(), unmap_region() may be called while vma_link() has not been
>> called. This happens during the error path if call_mmap() failed.
>>
>> The only to fix that particular case is to call
>> seqcount_init(&vma->vm_sequence) when initializing the vma in mmap_region().
>>
>
> Ack, although that would require a fixup to dup_mmap() as well.

You're right, I'll fix that too.

Thanks a lot.
Laurent.


2018-04-04 00:49:55

by David Rientjes

[permalink] [raw]
Subject: Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key

On Wed, 28 Mar 2018, Laurent Dufour wrote:

> On 26/03/2018 00:10, David Rientjes wrote:
> > On Wed, 21 Mar 2018, Laurent Dufour wrote:
> >
> >> I found the root cause of this lockdep warning.
> >>
> >> In mmap_region(), unmap_region() may be called while vma_link() has not been
> >> called. This happens during the error path if call_mmap() failed.
> >>
> >> The only to fix that particular case is to call
> >> seqcount_init(&vma->vm_sequence) when initializing the vma in mmap_region().
> >>
> >
> > Ack, although that would require a fixup to dup_mmap() as well.
>
> You're right, I'll fix that too.
>

I also think the following is needed:

diff --git a/fs/exec.c b/fs/exec.c
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -312,6 +312,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
INIT_LIST_HEAD(&vma->anon_vma_chain);
+#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
+ seqcount_init(&vma->vm_sequence);
+ atomic_set(&vma->vm_ref_count, 0);
+#endif

err = insert_vm_struct(mm, vma);
if (err)

2018-04-04 01:04:49

by David Rientjes

[permalink] [raw]
Subject: Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key

On Tue, 3 Apr 2018, David Rientjes wrote:

> > >> I found the root cause of this lockdep warning.
> > >>
> > >> In mmap_region(), unmap_region() may be called while vma_link() has not been
> > >> called. This happens during the error path if call_mmap() failed.
> > >>
> > >> The only to fix that particular case is to call
> > >> seqcount_init(&vma->vm_sequence) when initializing the vma in mmap_region().
> > >>
> > >
> > > Ack, although that would require a fixup to dup_mmap() as well.
> >
> > You're right, I'll fix that too.
> >
>
> I also think the following is needed:
>
> diff --git a/fs/exec.c b/fs/exec.c
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -312,6 +312,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
> vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
> vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> INIT_LIST_HEAD(&vma->anon_vma_chain);
> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
> + seqcount_init(&vma->vm_sequence);
> + atomic_set(&vma->vm_ref_count, 0);
> +#endif
>
> err = insert_vm_struct(mm, vma);
> if (err)
>

Ugh, I think there are a number of other places where this is needed as
well in mm/mmap.c. I think it would be better to just create a new
alloc_vma(unsigned long flags) that all vma allocators can use and for
CONFIG_SPECULATIVE_PAGE_FAULT will initialize the seqcount_t and atomic_t.

2018-04-04 10:21:32

by Laurent Dufour

[permalink] [raw]
Subject: Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key



On 04/04/2018 02:48, David Rientjes wrote:
> On Wed, 28 Mar 2018, Laurent Dufour wrote:
>
>> On 26/03/2018 00:10, David Rientjes wrote:
>>> On Wed, 21 Mar 2018, Laurent Dufour wrote:
>>>
>>>> I found the root cause of this lockdep warning.
>>>>
>>>> In mmap_region(), unmap_region() may be called while vma_link() has not been
>>>> called. This happens during the error path if call_mmap() failed.
>>>>
>>>> The only to fix that particular case is to call
>>>> seqcount_init(&vma->vm_sequence) when initializing the vma in mmap_region().
>>>>
>>>
>>> Ack, although that would require a fixup to dup_mmap() as well.
>>
>> You're right, I'll fix that too.
>>
>
> I also think the following is needed:
>
> diff --git a/fs/exec.c b/fs/exec.c
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -312,6 +312,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
> vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
> vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> INIT_LIST_HEAD(&vma->anon_vma_chain);
> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
> + seqcount_init(&vma->vm_sequence);
> + atomic_set(&vma->vm_ref_count, 0);
> +#endif
>
> err = insert_vm_struct(mm, vma);
> if (err)

No, this not needed because the vma is allocated with kmem_cache_zalloc() so
vm_ref_count is 0, and insert_vm_struc() will later call
__vma_link_rb() which will call seqcount_init().

Furhtermore, in case of error, the vma structure is freed without calling
get_vma() so there is risk of lockdep warning.


2018-04-04 10:31:51

by Laurent Dufour

[permalink] [raw]
Subject: Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key



On 04/04/2018 03:03, David Rientjes wrote:
> On Tue, 3 Apr 2018, David Rientjes wrote:
>
>>>>> I found the root cause of this lockdep warning.
>>>>>
>>>>> In mmap_region(), unmap_region() may be called while vma_link() has not been
>>>>> called. This happens during the error path if call_mmap() failed.
>>>>>
>>>>> The only to fix that particular case is to call
>>>>> seqcount_init(&vma->vm_sequence) when initializing the vma in mmap_region().
>>>>>
>>>>
>>>> Ack, although that would require a fixup to dup_mmap() as well.
>>>
>>> You're right, I'll fix that too.
>>>
>>
>> I also think the following is needed:
>>
>> diff --git a/fs/exec.c b/fs/exec.c
>> --- a/fs/exec.c
>> +++ b/fs/exec.c
>> @@ -312,6 +312,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
>> vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
>> vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>> INIT_LIST_HEAD(&vma->anon_vma_chain);
>> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
>> + seqcount_init(&vma->vm_sequence);
>> + atomic_set(&vma->vm_ref_count, 0);
>> +#endif
>>
>> err = insert_vm_struct(mm, vma);
>> if (err)
>>
>
> Ugh, I think there are a number of other places where this is needed as
> well in mm/mmap.c. I think it would be better to just create a new
> alloc_vma(unsigned long flags) that all vma allocators can use and for
> CONFIG_SPECULATIVE_PAGE_FAULT will initialize the seqcount_t and atomic_t.
>

I don't think this is really needed, most of the time the vma structure is
allocated cleared and is then link to rb tree via vma_link.

The only case generating a locked warning is when the vma is referenced in the
error path before being linked in the mm tree and that is not usual.


2018-04-04 21:55:11

by David Rientjes

[permalink] [raw]
Subject: Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key

On Wed, 4 Apr 2018, Laurent Dufour wrote:

> > I also think the following is needed:
> >
> > diff --git a/fs/exec.c b/fs/exec.c
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -312,6 +312,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
> > vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
> > vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> > INIT_LIST_HEAD(&vma->anon_vma_chain);
> > +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
> > + seqcount_init(&vma->vm_sequence);
> > + atomic_set(&vma->vm_ref_count, 0);
> > +#endif
> >
> > err = insert_vm_struct(mm, vma);
> > if (err)
>
> No, this not needed because the vma is allocated with kmem_cache_zalloc() so
> vm_ref_count is 0, and insert_vm_struc() will later call
> __vma_link_rb() which will call seqcount_init().
>
> Furhtermore, in case of error, the vma structure is freed without calling
> get_vma() so there is risk of lockdep warning.
>

Perhaps you're working from a different tree than I am, or you fixed the
lockdep warning differently when adding to dup_mmap() and mmap_region().

I got the following two lockdep errors.

I fixed it locally by doing the seqcount_init() and atomic_set()
everywhere a vma could be initialized.

INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 12 PID: 1 Comm: init Not tainted
Call Trace:
[<ffffffff8b12026f>] dump_stack+0x67/0x98
[<ffffffff8a92b616>] register_lock_class+0x1e6/0x4e0
[<ffffffff8a92cfe9>] __lock_acquire+0xb9/0x1710
[<ffffffff8a92ef3a>] lock_acquire+0xba/0x200
[<ffffffff8aa827df>] mprotect_fixup+0x10f/0x310
[<ffffffff8aade3fd>] setup_arg_pages+0x12d/0x230
[<ffffffff8ab4564a>] load_elf_binary+0x44a/0x1740
[<ffffffff8aadde9b>] search_binary_handler+0x9b/0x1e0
[<ffffffff8ab44e96>] load_script+0x206/0x270
[<ffffffff8aadde9b>] search_binary_handler+0x9b/0x1e0
[<ffffffff8aae0355>] do_execveat_common.isra.32+0x6b5/0x9d0
[<ffffffff8aae069c>] do_execve+0x2c/0x30
[<ffffffff8a80047b>] run_init_process+0x2b/0x30
[<ffffffff8b1358d4>] kernel_init+0x54/0x110
[<ffffffff8b2001ca>] ret_from_fork+0x3a/0x50

and

INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 21 PID: 1926 Comm: mkdir Not tainted
Call Trace:
[<ffffffff985202af>] dump_stack+0x67/0x98
[<ffffffff97d2b616>] register_lock_class+0x1e6/0x4e0
[<ffffffff97d2cfe9>] __lock_acquire+0xb9/0x1710
[<ffffffff97d2ef3a>] lock_acquire+0xba/0x200
[<ffffffff97e73c09>] unmap_page_range+0x89/0xaa0
[<ffffffff97e746af>] unmap_single_vma+0x8f/0x100
[<ffffffff97e74a1b>] unmap_vmas+0x4b/0x90
[<ffffffff97e7f833>] exit_mmap+0xa3/0x1c0
[<ffffffff97cc1b23>] mmput+0x73/0x120
[<ffffffff97ccbacd>] do_exit+0x2bd/0xd60
[<ffffffff97ccc5b7>] SyS_exit+0x17/0x20
[<ffffffff97c01f1d>] do_syscall_64+0x6d/0x1a0
[<ffffffff9860005a>] entry_SYSCALL_64_after_hwframe+0x26/0x9b

I think it would just be better to generalize vma allocation to initialize
certain fields and init both spf fields properly for
CONFIG_SPECULATIVE_PAGE_FAULT. It's obviously too delicate as is.

2018-04-05 16:59:08

by Laurent Dufour

[permalink] [raw]
Subject: Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key

On 04/04/2018 23:53, David Rientjes wrote:
> On Wed, 4 Apr 2018, Laurent Dufour wrote:
>
>>> I also think the following is needed:
>>>
>>> diff --git a/fs/exec.c b/fs/exec.c
>>> --- a/fs/exec.c
>>> +++ b/fs/exec.c
>>> @@ -312,6 +312,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
>>> vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
>>> vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>>> INIT_LIST_HEAD(&vma->anon_vma_chain);
>>> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
>>> + seqcount_init(&vma->vm_sequence);
>>> + atomic_set(&vma->vm_ref_count, 0);
>>> +#endif
>>>
>>> err = insert_vm_struct(mm, vma);
>>> if (err)
>>
>> No, this not needed because the vma is allocated with kmem_cache_zalloc() so
>> vm_ref_count is 0, and insert_vm_struc() will later call
>> __vma_link_rb() which will call seqcount_init().
>>
>> Furhtermore, in case of error, the vma structure is freed without calling
>> get_vma() so there is risk of lockdep warning.
>>
>
> Perhaps you're working from a different tree than I am, or you fixed the
> lockdep warning differently when adding to dup_mmap() and mmap_region().
>
> I got the following two lockdep errors.
>
> I fixed it locally by doing the seqcount_init() and atomic_set()
> everywhere a vma could be initialized.

That's weird, I don't get that on my side with lockdep activated.

There is a call to seqcount_init() in dup_mmap(), in mmap_region() and
__vma_link_rb() and that's enough to cover all the case.

That's being said, it'll be better call seqcount_init each time as soon as a
vma structure is allocated. For the vm_ref_count value, as most of the time the
vma is zero allocated, I don't think this is needed.
I just have to check when new_vma = *old_vma is done, but this often just
follow a vma allocation.
>
> INFO: trying to register non-static key.
> the code is fine but needs lockdep annotation.
> turning off the locking correctness validator.
> CPU: 12 PID: 1 Comm: init Not tainted
> Call Trace:
> [<ffffffff8b12026f>] dump_stack+0x67/0x98
> [<ffffffff8a92b616>] register_lock_class+0x1e6/0x4e0
> [<ffffffff8a92cfe9>] __lock_acquire+0xb9/0x1710
> [<ffffffff8a92ef3a>] lock_acquire+0xba/0x200
> [<ffffffff8aa827df>] mprotect_fixup+0x10f/0x310
> [<ffffffff8aade3fd>] setup_arg_pages+0x12d/0x230
> [<ffffffff8ab4564a>] load_elf_binary+0x44a/0x1740
> [<ffffffff8aadde9b>] search_binary_handler+0x9b/0x1e0
> [<ffffffff8ab44e96>] load_script+0x206/0x270
> [<ffffffff8aadde9b>] search_binary_handler+0x9b/0x1e0
> [<ffffffff8aae0355>] do_execveat_common.isra.32+0x6b5/0x9d0
> [<ffffffff8aae069c>] do_execve+0x2c/0x30
> [<ffffffff8a80047b>] run_init_process+0x2b/0x30
> [<ffffffff8b1358d4>] kernel_init+0x54/0x110
> [<ffffffff8b2001ca>] ret_from_fork+0x3a/0x50
>
> and
>
> INFO: trying to register non-static key.
> the code is fine but needs lockdep annotation.
> turning off the locking correctness validator.
> CPU: 21 PID: 1926 Comm: mkdir Not tainted
> Call Trace:
> [<ffffffff985202af>] dump_stack+0x67/0x98
> [<ffffffff97d2b616>] register_lock_class+0x1e6/0x4e0
> [<ffffffff97d2cfe9>] __lock_acquire+0xb9/0x1710
> [<ffffffff97d2ef3a>] lock_acquire+0xba/0x200
> [<ffffffff97e73c09>] unmap_page_range+0x89/0xaa0
> [<ffffffff97e746af>] unmap_single_vma+0x8f/0x100
> [<ffffffff97e74a1b>] unmap_vmas+0x4b/0x90
> [<ffffffff97e7f833>] exit_mmap+0xa3/0x1c0
> [<ffffffff97cc1b23>] mmput+0x73/0x120
> [<ffffffff97ccbacd>] do_exit+0x2bd/0xd60
> [<ffffffff97ccc5b7>] SyS_exit+0x17/0x20
> [<ffffffff97c01f1d>] do_syscall_64+0x6d/0x1a0
> [<ffffffff9860005a>] entry_SYSCALL_64_after_hwframe+0x26/0x9b
>
> I think it would just be better to generalize vma allocation to initialize
> certain fields and init both spf fields properly for
> CONFIG_SPECULATIVE_PAGE_FAULT. It's obviously too delicate as is.
>