On 5/25/16, 12:40, "Anna Schumaker" <Anna.Schumaker@xxxxxxxxxx> wrote:
>Thanks, Dros!
>
>How often are you hitting this? Do you want me to send it to stable?
>
Agreed that it should go to stable.
Dros, do you know how far back? I’d assume 4.0.x, if it still applies to those kernels.
>Anna
>
>On 05/25/2016 10:07 AM, Weston Andros Adamson wrote:
>> Since the patch "NFS: Allow multiple commit requests in flight per file"
>> we can run multiple simultaneous commits on the same inode. This
>> introduced a race over collecting pages to commit that made it possible
>> to call nfs_init_commit() with an empty list - which causes crashes like
>> the one below.
>>
>> The fix is to catch this race and avoid calling nfs_init_commit and
>> initiate_commit when there is no work to do.
>>
>> Here is the crash:
>>
>> [600522.076832] BUG: unable to handle kernel NULL pointer dereference at 0000000000000040
>> [600522.078475] IP: [<ffffffffa0479e72>] nfs_init_commit+0x22/0x130 [nfs]
>> [600522.078745] PGD 4272b1067 PUD 4272cb067 PMD 0
>> [600522.078972] Oops: 0000 [#1] SMP
>> [600522.079204] Modules linked in: nfsv3 nfs_layout_flexfiles rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache dcdbas ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw vmw_vsock_vmci_transport vsock bonding ipmi_devintf ipmi_msghandler coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel ppdev vmw_balloon parport_pc parport acpi_cpufreq vmw_vmci i2c_piix4 shpchp nfsd auth_rpcgss nfs_acl lockd grace sunrpc xfs libcrc32c vmwgfx drm_kms_helper ttm drm crc32c_intel serio_raw vmxnet3
>> [600522.081380] vmw_pvscsi ata_generic pata_acpi
>> [600522.081809] CPU: 3 PID: 15667 Comm: /usr/bin/python Not tainted 4.1.9-100.pd.88.el7.x86_64 #1
>> [600522.082281] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 09/30/2014
>> [600522.082814] task: ffff8800bbbfa780 ti: ffff88042ae84000 task.ti: ffff88042ae84000
>> [600522.083378] RIP: 0010:[<ffffffffa0479e72>] [<ffffffffa0479e72>] nfs_init_commit+0x22/0x130 [nfs]
>> [600522.083973] RSP: 0018:ffff88042ae87438 EFLAGS: 00010246
>> [600522.084571] RAX: 0000000000000000 RBX: ffff880003485e40 RCX: ffff88042ae87588
>> [600522.085188] RDX: 0000000000000000 RSI: ffff88042ae874b0 RDI: ffff880003485e40
>> [600522.085756] RBP: ffff88042ae87448 R08: ffff880003486010 R09: ffff88042ae874b0
>> [600522.086332] R10: 0000000000000000 R11: 0000000000000005 R12: ffff88042ae872d0
>> [600522.086905] R13: ffff88042ae874b0 R14: ffff880003485e40 R15: ffff88042704c840
>> [600522.087484] FS: 00007f4728ff2740(0000) GS:ffff88043fd80000(0000) knlGS:0000000000000000
>> [600522.088070] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>> [600522.088663] CR2: 0000000000000040 CR3: 000000042b6aa000 CR4: 00000000001406e0
>> [600522.089327] Stack:
>> [600522.089926] 0000000000000001 ffff88042ae87588 ffff88042ae874f8 ffffffffa04f09fa
>> [600522.090549] 0000000000017840 0000000000017840 ffff88042ae87588 ffff8803258d9930
>> [600522.091169] ffff88042ae87578 ffffffffa0563d80 0000000000000000 ffff88042704c840
>> [600522.091789] Call Trace:
>> [600522.092420] [<ffffffffa04f09fa>] pnfs_generic_commit_pagelist+0x1da/0x320 [nfsv4]
>> [600522.093052] [<ffffffffa0563d80>] ? ff_layout_commit_prepare_v3+0x30/0x30 [nfs_layout_flexfiles]
>> [600522.093696] [<ffffffffa0562645>] ff_layout_commit_pagelist+0x15/0x20 [nfs_layout_flexfiles]
>> [600522.094359] [<ffffffffa047bc78>] nfs_generic_commit_list+0xe8/0x120 [nfs]
>> [600522.095032] [<ffffffffa047bd6a>] nfs_commit_inode+0xba/0x110 [nfs]
>> [600522.095719] [<ffffffffa046ac54>] nfs_release_page+0x44/0xd0 [nfs]
>> [600522.096410] [<ffffffff811a8122>] try_to_release_page+0x32/0x50
>> [600522.097109] [<ffffffff811bd4f1>] shrink_page_list+0x961/0xb30
>> [600522.097812] [<ffffffff811bdced>] shrink_inactive_list+0x1cd/0x550
>> [600522.098530] [<ffffffff811bea65>] shrink_lruvec+0x635/0x840
>> [600522.099250] [<ffffffff811bed60>] shrink_zone+0xf0/0x2f0
>> [600522.099974] [<ffffffff811bf312>] do_try_to_free_pages+0x192/0x470
>> [600522.100709] [<ffffffff811bf6ca>] try_to_free_pages+0xda/0x170
>> [600522.101464] [<ffffffff811b2198>] __alloc_pages_nodemask+0x588/0x970
>> [600522.102235] [<ffffffff811fbbd5>] alloc_pages_vma+0xb5/0x230
>> [600522.103000] [<ffffffff813a1589>] ? cpumask_any_but+0x39/0x50
>> [600522.103774] [<ffffffff811d6115>] wp_page_copy.isra.55+0x95/0x490
>> [600522.104558] [<ffffffff810e3438>] ? __wake_up+0x48/0x60
>> [600522.105357] [<ffffffff811d7d3b>] do_wp_page+0xab/0x4f0
>> [600522.106137] [<ffffffff810a1bbb>] ? release_task+0x36b/0x470
>> [600522.106902] [<ffffffff8126dbd7>] ? eventfd_ctx_read+0x67/0x1c0
>> [600522.107659] [<ffffffff811da2a8>] handle_mm_fault+0xc78/0x1900
>> [600522.108431] [<ffffffff81067ef1>] __do_page_fault+0x181/0x420
>> [600522.109173] [<ffffffff811446a6>] ? __audit_syscall_exit+0x1e6/0x280
>> [600522.109893] [<ffffffff810681c0>] do_page_fault+0x30/0x80
>> [600522.110594] [<ffffffff81024f36>] ? syscall_trace_leave+0xc6/0x120
>> [600522.111288] [<ffffffff81790a58>] page_fault+0x28/0x30
>> [600522.111947] Code: 5d c3 0f 1f 80 00 00 00 00 0f 1f 44 00 00 55 4c 8d 87 d0 01 00 00 48 89 e5 53 48 89 fb 48 83 ec 08 4c 8b 0e 49 8b 41 18 4c 39 ce <48> 8b 40 40 4c 8b 50 30 74 24 48 8b 87 d0 01 00 00 48 8b 7e 08
>> [600522.113343] RIP [<ffffffffa0479e72>] nfs_init_commit+0x22/0x130 [nfs]
>> [600522.114003] RSP <ffff88042ae87438>
>> [600522.114636] CR2: 0000000000000040
>>
>> Signed-off-by: Weston Andros Adamson <dros@xxxxxxxxxxxxxxx>
>> ---
>>
>> V2 has some cleanup requested by Anna.
>>
>> -dros
>>
>> fs/nfs/pnfs_nfs.c | 28 ++++++++++++++++++++++++++++
>> fs/nfs/write.c | 4 ++++
>> 2 files changed, 32 insertions(+)
>>
>> diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
>> index d2a7c9f7aa94..0dfc476da3e1 100644
>> --- a/fs/nfs/pnfs_nfs.c
>> +++ b/fs/nfs/pnfs_nfs.c
>> @@ -246,6 +246,23 @@ void pnfs_fetch_commit_bucket_list(struct list_head *pages,
>>
>> }
>>
>> +/* Helper function for pnfs_generic_commit_pagelist to catch an empty
>> + * page list. This can happen when two commits race. */
>> +static bool
>> +pnfs_generic_commit_cancel_empty_pagelist(struct list_head *pages,
>> + struct nfs_commit_data *data,
>> + struct nfs_commit_info *cinfo)
>> +{
>> + if (list_empty(pages)) {
>> + if (atomic_dec_and_test(&cinfo->mds->rpcs_out))
>> + wake_up_atomic_t(&cinfo->mds->rpcs_out);
>> + nfs_commitdata_release(data);
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> /* This follows nfs_commit_list pretty closely */
>> int
>> pnfs_generic_commit_pagelist(struct inode *inode, struct list_head *mds_pages,
>> @@ -280,6 +297,11 @@ pnfs_generic_commit_pagelist(struct inode *inode, struct list_head *mds_pages,
>> list_for_each_entry_safe(data, tmp, &list, pages) {
>> list_del_init(&data->pages);
>> if (data->ds_commit_index < 0) {
>> + /* another commit raced with us */
>> + if (pnfs_generic_commit_cancel_empty_pagelist(mds_pages,
>> + data, cinfo))
>> + continue;
>> +
>> nfs_init_commit(data, mds_pages, NULL, cinfo);
>> nfs_initiate_commit(NFS_CLIENT(inode), data,
>> NFS_PROTO(data->inode),
>> @@ -288,6 +310,12 @@ pnfs_generic_commit_pagelist(struct inode *inode, struct list_head *mds_pages,
>> LIST_HEAD(pages);
>>
>> pnfs_fetch_commit_bucket_list(&pages, data, cinfo);
>> +
>> + /* another commit raced with us */
>> + if (pnfs_generic_commit_cancel_empty_pagelist(&pages,
>> + data, cinfo))
>> + continue;
>> +
>> nfs_init_commit(data, &pages, data->lseg, cinfo);
>> initiate_commit(data, how);
>> }
>> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
>> index 4dac51ba1f7e..438c00110fc9 100644
>> --- a/fs/nfs/write.c
>> +++ b/fs/nfs/write.c
>> @@ -1710,6 +1710,10 @@ nfs_commit_list(struct inode *inode, struct list_head *head, int how,
>> {
>> struct nfs_commit_data *data;
>>
>> + /* another commit raced with us */
>> + if (list_empty(head))
>> + return 0;
>> +
>> data = nfs_commitdata_alloc();
>>
>> if (!data)
>>
>
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
