On Wed, 07 Mar 2018, Trond Myklebust wrote: > 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? > Yes, this works. I ran it through a dozen fio runs on v4.1 and 1000 runs of generic/247 on v3/v4.0/v4.1/v4.2 and didn't see any EBUSY errors. Also ran the xfstests "quick" group (~80-90 tests) plus generic/074 on v3/v4.0/v4.1/v4.2. Finally, I double checked the panic on umount issue that dc4fd9ab01ab3 fixed and that still works too. Thanks, Scott > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
