On Wed, 2018-03-07 at 14:53 -0500, Scott Mayhew wrote: > On Mon, 05 Mar 2018, Trond Myklebust wrote: > > > On Mon, 2018-03-05 at 16:16 -0500, J. Bruce Fields wrote: > > > On Fri, Mar 02, 2018 at 11:00:38AM -0500, Scott Mayhew wrote: > > > > It seems that nfs_commit_inode can be called where the > > > > nfs_inode > > > > has > > > > outstanding requests and the commit lists are empty. That can > > > > lead > > > > to > > > > invalidate_complete_page2 failing due to the associated page > > > > having > > > > private data which in turn leads to > > > > invalidate_inode_pages2_range > > > > returning -EBUSY. > > > > > > For what it's worth, I verified that this fixes the EBUSY I was > > > seeing: > > > > > > http://marc.info/?i=20180223160350.GF15876@xxxxxxxxxxxx > > > > > > > Fine, but the patch will also cause the inode to be marked as dirty > > in > > cases where there are no unstable writes to commit, but there are > > pages > > undergoing writeback. > > IOW: it regresses the fix that was made in dc4fd9ab01 > > > > So please do look into fixing do_launder_page(). > > > > Yes, sorry... so I've been testing with this change since Friday > afternoon: > > diff --git a/mm/truncate.c b/mm/truncate.c > index c34e2fd4f583..909734a5d3a3 100644 > --- a/mm/truncate.c > +++ b/mm/truncate.c > @@ -647,7 +647,7 @@ invalidate_complete_page2(struct address_space > *mapping, struct page *page) > > static int do_launder_page(struct address_space *mapping, struct > page *page) > { > - if (!PageDirty(page)) > + if (!PageDirty(page) && !PagePrivate(page)) > return 0; > if (page->mapping != mapping || mapping->a_ops->launder_page > == NULL) > return 0; > > But I'm frequently seeing soft lockups though, on both 4.16-rc4 and > on > the latest RHEL 7 kernel. > > Mar 7 13:52:08 localhost kernel: watchdog: BUG: soft lockup - CPU#5 > stuck for 23s! [xfs_io:17667] > Mar 7 13:52:08 localhost kernel: Modules linked in: rpcsec_gss_krb5 > auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache sunrpc > crct10dif_pclmul crc32_pclmul ghash_clmulni_intel virtio_balloon > i2c_piix4 joydev xfs libcrc32c qxl drm_kms_helper ttm virtio_console > virtio_net drm virtio_scsi serio_raw crc32c_intel ata_generic > virtio_pci pata_acpi qemu_fw_cfg virtio_rng virtio_ring virtio > Mar 7 13:52:08 localhost kernel: CPU: 5 PID: 17667 Comm: xfs_io > Tainted: G L 4.16.0-rc4+ #2 > Mar 7 13:52:08 localhost kernel: Hardware name: Red Hat RHEV > Hypervisor, BIOS 1.10.2-3.el7_4.1 04/01/2014 > Mar 7 13:52:08 localhost kernel: RIP: > 0010:nfs_commit_inode+0x87/0x160 [nfs] > Mar 7 13:52:08 localhost kernel: RSP: 0018:ffffab310e627b00 EFLAGS: > 00000202 ORIG_RAX: ffffffffffffff12 > Mar 7 13:52:08 localhost kernel: RAX: 0000000000000000 RBX: > ffff8cd834f0a3e0 RCX: 0000000000000000 > Mar 7 13:52:08 localhost kernel: RDX: ffff8cd834f0a300 RSI: > 0000000000000001 RDI: ffff8cd834f0a3e0 > Mar 7 13:52:08 localhost kernel: RBP: 0000000000000001 R08: > ffffab310e627c30 R09: 000000000001d400 > Mar 7 13:52:08 localhost kernel: R10: ffff8cd836c02480 R11: > ffff8cd83302043c R12: ffffab310e627b70 > Mar 7 13:52:08 localhost kernel: R13: ffffffffffffffff R14: > 0000000000000000 R15: ffffcd0147055f00 > Mar 7 13:52:08 localhost kernel: FS: 00007feae2d97b80(0000) > GS:ffff8cd837340000(0000) knlGS:0000000000000000 > Mar 7 13:52:08 localhost kernel: CS: 0010 DS: 0000 ES: 0000 CR0: > 0000000080050033 > Mar 7 13:52:08 localhost kernel: CR2: 00007feae2103fb8 CR3: > 0000000120fc2002 CR4: 00000000003606e0 > Mar 7 13:52:08 localhost kernel: DR0: 0000000000000000 DR1: > 0000000000000000 DR2: 0000000000000000 > Mar 7 13:52:08 localhost kernel: DR3: 0000000000000000 DR6: > 00000000fffe0ff0 DR7: 0000000000000400 > Mar 7 13:52:08 localhost kernel: Call Trace: > Mar 7 13:52:08 localhost kernel: nfs_wb_page+0xd7/0x1b0 [nfs] Ah... So the real problem is that we're not waiting for the outstanding commit? OK, so how about something like the following then? 8<------------------------------------------ >From f2b7634d8a05100631ab019d4fb5092ed5fe3c03 Mon Sep 17 00:00:00 2001 From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> Date: Wed, 7 Mar 2018 15:22:31 -0500 Subject: [PATCH] NFS: Don't circumvent wait for commit completion We do want to respect the FLUSH_SYNC argument to nfs_commit_inode() to ensure that all outstanding COMMIT requests to the inode in question are complete. Currently we will exit early if we did not have to schedule a new COMMIT request. Fixes: dc4fd9ab01ab3 ("nfs: don't wait on commit in nfs_commit_inode()...") Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> Cc: stable@xxxxxxxxxxxxxxx # 4.5+ --- fs/nfs/write.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/nfs/write.c b/fs/nfs/write.c index 93460f1cf5a4..89ca7b725454 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -1886,8 +1886,6 @@ int nfs_commit_inode(struct inode *inode, int how) if (res) error = nfs_generic_commit_list(inode, &head, how, &cinfo); nfs_commit_end(cinfo.mds); - if (res == 0) - return res; if (error < 0) goto out_error; if (!may_wait) @@ -1904,7 +1902,8 @@ int nfs_commit_inode(struct inode *inode, int how) * that the data is on the disk. */ out_mark_dirty: - __mark_inode_dirty(inode, I_DIRTY_DATASYNC); + if (atomic_read(&cinfo.mds->rpcs_out)) + __mark_inode_dirty(inode, I_DIRTY_DATASYNC); return res; } EXPORT_SYMBOL_GPL(nfs_commit_inode); -- 2.14.3 -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@xxxxxxxxxxxxxxx ��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
