From patchwork Wed May 25 14:07:23 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Weston Andros Adamson X-Patchwork-Id: 9135439 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8D141607D7 for ; Wed, 25 May 2016 14:07:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7F02428110 for ; Wed, 25 May 2016 14:07:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 73B512822B; Wed, 25 May 2016 14:07:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 77E8A28164 for ; Wed, 25 May 2016 14:07:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751028AbcEYOH3 (ORCPT ); Wed, 25 May 2016 10:07:29 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:34585 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750924AbcEYOH1 (ORCPT ); Wed, 25 May 2016 10:07:27 -0400 Received: by mail-io0-f194.google.com with SMTP id d197so5280612ioe.1 for ; Wed, 25 May 2016 07:07:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id; bh=wxvgasUt6ItrIPZl/5XGELDsvDRrqPfLE8SERe44AlY=; b=Qjk1fu5VwQchluDyytH3BfKhbVSltOE2b9o2t49c6Cu92fTJU0ILzMa2p49owu2tbH eCXULiqCEUnoDNl3QzjTU8IA1BQ7D6T+HrzYwZ8C52FGmXuwzAVKIyb5/cwPPycIuVPe lRAUQBrdzGJy16XXXXFkT/vy2MY8zw/ziP/4mTgrYw3NYNhQIpVBfIx0ao7BoEoSZL/i TFqj9WexQyopARuTBqKWDlue0/yNZ2KzemuCUm1s1DL6rjxI4Le30AqXHVnPmEgcCIuq 1qyBzKZnDEI5jBwrtrujBKK5YGuxeyKWAk+bPkTnNkCVU9BHH1jgjeUM+lIUsrRdtiCw hNrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id; bh=wxvgasUt6ItrIPZl/5XGELDsvDRrqPfLE8SERe44AlY=; b=hDMkGxWcWYpky6XM1q3G+Vgpqo90SD0YNCB9KBwZE8fGbyewNT6rtdr6Sk5UdIWkJm 61XTt8z1L5lDd/sSHMt0oO5pPjgVHfmN9Pk8qJRBcmuO0L0XnwwoQNYOmw0WpgnAddu1 whNMXe4Xx/RzReICLX0ONDAKGW/M+5ur3IYnkMTfbOYqRgG99syDm/meI9c9SePC7cT0 +93v6Qset2UOPzW7BzZgYR37lve7l8aH1m30gs/IoWM5raLNMpPYTfIr0elcirfbxfYZ alGFf/AGx4ZJ7kL/LOmNLDU11s9F4a+RtNrAa2X7HByZuMH9iKFT7bivuV8kQjzBBGrs PxSw== X-Gm-Message-State: ALyK8tJ/NApEoTmZXFaNJh+qimv4r/pczrVB5yj5pdSL0eNcPQ6N1NOMIGsL/k60ajb2QA== X-Received: by 10.107.199.67 with SMTP id x64mr3714948iof.45.1464185246010; Wed, 25 May 2016 07:07:26 -0700 (PDT) Received: from dhcp-56.robotsandstuff.fake (c-68-49-175-159.hsd1.mi.comcast.net. [68.49.175.159]) by smtp.gmail.com with ESMTPSA id g18sm2844506iod.11.2016.05.25.07.07.25 (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 25 May 2016 07:07:25 -0700 (PDT) From: Weston Andros Adamson X-Google-Original-From: Weston Andros Adamson To: trond.myklebust@primarydata.com Cc: linux-nfs@vger.kernel.org, Weston Andros Adamson Subject: [PATCH v2] nfs: avoid race that crashes nfs_init_commit Date: Wed, 25 May 2016 10:07:23 -0400 Message-Id: <1464185243-15556-1-git-send-email-dros@primarydata.com> X-Mailer: git-send-email 2.6.4 (Apple Git-63) Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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: [] 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:[] [] 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] [] pnfs_generic_commit_pagelist+0x1da/0x320 [nfsv4] [600522.093052] [] ? ff_layout_commit_prepare_v3+0x30/0x30 [nfs_layout_flexfiles] [600522.093696] [] ff_layout_commit_pagelist+0x15/0x20 [nfs_layout_flexfiles] [600522.094359] [] nfs_generic_commit_list+0xe8/0x120 [nfs] [600522.095032] [] nfs_commit_inode+0xba/0x110 [nfs] [600522.095719] [] nfs_release_page+0x44/0xd0 [nfs] [600522.096410] [] try_to_release_page+0x32/0x50 [600522.097109] [] shrink_page_list+0x961/0xb30 [600522.097812] [] shrink_inactive_list+0x1cd/0x550 [600522.098530] [] shrink_lruvec+0x635/0x840 [600522.099250] [] shrink_zone+0xf0/0x2f0 [600522.099974] [] do_try_to_free_pages+0x192/0x470 [600522.100709] [] try_to_free_pages+0xda/0x170 [600522.101464] [] __alloc_pages_nodemask+0x588/0x970 [600522.102235] [] alloc_pages_vma+0xb5/0x230 [600522.103000] [] ? cpumask_any_but+0x39/0x50 [600522.103774] [] wp_page_copy.isra.55+0x95/0x490 [600522.104558] [] ? __wake_up+0x48/0x60 [600522.105357] [] do_wp_page+0xab/0x4f0 [600522.106137] [] ? release_task+0x36b/0x470 [600522.106902] [] ? eventfd_ctx_read+0x67/0x1c0 [600522.107659] [] handle_mm_fault+0xc78/0x1900 [600522.108431] [] __do_page_fault+0x181/0x420 [600522.109173] [] ? __audit_syscall_exit+0x1e6/0x280 [600522.109893] [] do_page_fault+0x30/0x80 [600522.110594] [] ? syscall_trace_leave+0xc6/0x120 [600522.111288] [] 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 [] nfs_init_commit+0x22/0x130 [nfs] [600522.114003] RSP [600522.114636] CR2: 0000000000000040 Signed-off-by: Weston Andros Adamson --- 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)