All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@primarydata.com>
To: "green@linuxhacker.ru" <green@linuxhacker.ru>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v2 03/12] NFS: Cache aggressively when file is open for writing
Date: Wed, 22 Jun 2016 13:06:49 +0000	[thread overview]
Message-ID: <1466600806.90993.2.camel@primarydata.com> (raw)
In-Reply-To: <43F206E4-FCA8-4ED3-A4FC-4150AB927026@linuxhacker.ru>

[-- Attachment #1: Type: text/plain, Size: 6053 bytes --]

Hi Oleg,

Thanks for testing!

On Tue, 2016-06-21 at 18:25 -0400, Oleg Drokin wrote:
> This patch still crashes for me.
> 
> [  183.814855] BUG: unable to handle kernel paging request at
> ffff88001b18cfe8
> [  183.815403] IP: [<ffffffff81381b63>] nfs_update_inode+0x53/0xa30
> [  183.815868] PGD 3580067 PUD 3581067 PMD 77f69067 PTE
> 800000001b18c060
> [  183.816640] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
> [  183.817252] Modules linked in: loop rpcsec_gss_krb5 joydev
> acpi_cpufreq tpm_tis virtio_console tpm i2c_piix4 pcspkr nfsd ttm
> drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops
> virtio_blk serio_raw drm floppy
> [  183.820203] CPU: 6 PID: 40935 Comm: rm Not tainted 4.7.0-rc3-vm-
> nfst+ #6
> [  183.820818] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [  183.821329] task: ffff88001b430c80 ti: ffff88001cf2c000 task.ti:
> ffff88001cf2c000
> [  183.822223] RIP: 0010:[<ffffffff81381b63>]  [<ffffffff81381b63>]
> nfs_update_inode+0x53/0xa30
> [  183.823007] RSP: 0018:ffff88001cf2fb08  EFLAGS: 00010283
> [  183.823417] RAX: ffff88001b18d000 RBX: ffff880021b582a8 RCX:
> 0000000000000000
> [  183.823845] RDX: ffff88001b18d000 RSI: ffff88001c3cdf00 RDI:
> ffff880021b582a8
> [  183.824276] RBP: ffff88001cf2fb58 R08: 0000000000000000 R09:
> 0000000000000001
> [  183.824701] R10: ffff88001b430c80 R11: 00000000fffe3818 R12:
> ffff88001c3cdf00
> [  183.825190] R13: ffff88001c3cdf00 R14: ffff88001c3cdf00 R15:
> 0000000000000001
> [  183.825651] FS:  00007fa0e9c5b700(0000) GS:ffff880075200000(0000)
> knlGS:0000000000000000
> [  183.826396] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  183.826884] CR2: ffff88001b18cfe8 CR3: 000000001bc1f000 CR4:
> 00000000000006e0
> [  183.827460] Stack:
> [  183.827825]  ffff880000000000 ffff88001cf2fb58 0000000000000246
> ffffffff81382f33
> [  183.828734]  0000000000000246 ffff880021b582a8 ffff880021b582a8
> ffff88001c3cdf00
> [  183.829634]  ffff88001c3cdf00 0000000000000001 ffff88001cf2fb80
> ffffffff81382aa1
> [  183.830562] Call Trace:
> [  183.830938]  [<ffffffff81382f33>] ?
> nfs_refresh_inode.part.24+0x23/0x50
> [  183.831368]  [<ffffffff81382aa1>]
> nfs_refresh_inode_locked+0x71/0x4e0
> [  183.831785]  [<ffffffff81382f3e>]
> nfs_refresh_inode.part.24+0x2e/0x50
> [  183.832209]  [<ffffffff81384024>]
> __nfs_revalidate_inode+0x314/0x4b0
> [  183.832632]  [<ffffffff8137c6fe>] nfs_do_access+0x47e/0x620
> [  183.833036]  [<ffffffff8137c2d1>] ? nfs_do_access+0x51/0x620
> [  183.833456]  [<ffffffff818507ea>] ? generic_lookup_cred+0x1a/0x20
> [  183.833872]  [<ffffffff8184ef7e>] ? rpcauth_lookupcred+0x8e/0xd0
> [  183.834288]  [<ffffffff8137cb69>] nfs_permission+0x289/0x2e0
> [  183.834699]  [<ffffffff8137c943>] ? nfs_permission+0x63/0x2e0
> [  183.835108]  [<ffffffff812768da>] __inode_permission+0x6a/0xb0
> [  183.835521]  [<ffffffff81276934>] inode_permission+0x14/0x50
> [  183.835928]  [<ffffffff8127b2cf>] link_path_walk+0x2ef/0x6b0
> [  183.837327]  [<ffffffff8127b6a9>] ? path_parentat+0x19/0x80
> [  183.837988]  [<ffffffff8127b6be>] path_parentat+0x2e/0x80
> [  183.838653]  [<ffffffff8127cb13>] filename_parentat+0xb3/0x190
> [  183.839328]  [<ffffffff8123bc4c>] ?
> cache_alloc_debugcheck_after.isra.52+0x19c/0x1f0
> [  183.840533]  [<ffffffff8123e0a0>] ? kmem_cache_alloc+0x300/0x3d0
> [  183.841217]  [<ffffffff8127c89f>] ? getname_flags+0x4f/0x1f0
> [  183.841888]  [<ffffffff810e6d0d>] ? trace_hardirqs_on+0xd/0x10
> [  183.842570]  [<ffffffff8127d455>] do_unlinkat+0x65/0x2f0
> [  183.843243]  [<ffffffff8100201a>] ?
> trace_hardirqs_on_thunk+0x1a/0x1c
> [  183.843910]  [<ffffffff8127e17b>] SyS_unlinkat+0x1b/0x30
> [  183.844582]  [<ffffffff8188aebc>]
> entry_SYSCALL_64_fastpath+0x1f/0xbd
> 
> (gdb) l *(nfs_update_inode+0x53)
> 0xffffffff81381b63 is in nfs_update_inode (/home/green/bk/linux-
> test/fs/nfs/inode.c:1233).
> 1228		 *       being placed at the head of the list.
> 1229		 *       See nfs_inode_attach_open_context()
> 1230		 */
> 1231		return (list_first_entry(&nfsi->open_files,
> 1232				struct nfs_open_context,
> 1233				list)->mode & FMODE_WRITE) ==
> FMODE_WRITE;
> 1234	}
> 1235	
> 1236	static unsigned long nfs_wcc_update_inode(struct inode
> *inode, struct nfs_fattr *fattr)
> 1237	{
> 
> Reverting it makes the crash go away.
> 
> But there's also this other problem that I have not tracked to a
> particular patch yet:
> 
> [   25.830288] =============================================
> [   25.830468] [ INFO: possible recursive locking detected ]
> [   25.830658] 4.7.0-rc3-vm-nfst+ #8 Not tainted
> [   25.830822] ---------------------------------------------
> [   25.831009] cat/6588 is trying to acquire lock:
> [   25.831179]  (&sb->s_type->i_mutex_key#13){++++++}, at:
> [<ffffffff813841f4>] __nfs_revalidate_mapping+0x124/0x3e0
> [   25.832488] 
>                but task is already holding lock:
> [   25.833214]  (&sb->s_type->i_mutex_key#13){++++++}, at:
> [<ffffffff81388b7e>] nfs_start_io_read+0x1e/0x50
> [   25.834209] 
>                other info that might help us debug this:
> [   25.834970]  Possible unsafe locking scenario:
> 
> [   25.835699]        CPU0
> [   25.836077]        ----
> [   25.836459]   lock(&sb->s_type->i_mutex_key#13);
> [   25.836988]   lock(&sb->s_type->i_mutex_key#13);
> [   25.837636] 
>                 *** DEADLOCK ***
> 

The 2 attached patches should hopefully fix these 2 issues and will be
rolled into v3 together with a cleanup patch to remove the now-obsolete 
function nfs_revalidate_mapping_protected().

Cheers
  Trond
  
-- 

Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fixup-NFS-Cache-aggressively-when-file-is-open-for-w.patch --]
[-- Type: text/x-patch; name=0001-fixup-NFS-Cache-aggressively-when-file-is-open-for-w.patch, Size: 968 bytes --]

From df4c863d69f5c333648d8c8e077f00b2cc93aadc Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@primarydata.com>
Date: Wed, 22 Jun 2016 08:04:15 -0400
Subject: [PATCH 1/2] fixup! NFS: Cache aggressively when file is open for
 writing

---
 fs/nfs/inode.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 6bc65ffc3c6c..9521fa6154c8 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1219,8 +1219,12 @@ int nfs_revalidate_mapping_protected(struct inode *inode, struct address_space *
 
 static bool nfs_file_has_writers(struct nfs_inode *nfsi)
 {
-	assert_spin_locked(&nfsi->vfs_inode.i_lock);
+	struct inode *inode = &nfsi->vfs_inode;
 
+	assert_spin_locked(&inode->i_lock);
+
+	if (!S_ISREG(inode->i_mode))
+		return false;
 	if (list_empty(&nfsi->open_files))
 		return false;
 	/* Note: This relies on nfsi->open_files being ordered with writers
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-fixup-NFS-Do-not-serialise-O_DIRECT-reads-and-writes.patch --]
[-- Type: text/x-patch; name=0002-fixup-NFS-Do-not-serialise-O_DIRECT-reads-and-writes.patch, Size: 1259 bytes --]

From 65d27ea6cc51cb653c2674666353cf3b8d0f765f Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@primarydata.com>
Date: Wed, 22 Jun 2016 08:16:54 -0400
Subject: [PATCH 2/2] fixup! NFS: Do not serialise O_DIRECT reads and writes

---
 fs/nfs/file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index fb7a1b0b6a20..9081afacb457 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -171,7 +171,7 @@ nfs_file_read(struct kiocb *iocb, struct iov_iter *to)
 		iov_iter_count(to), (unsigned long) iocb->ki_pos);
 
 	nfs_start_io_read(inode);
-	result = nfs_revalidate_mapping_protected(inode, iocb->ki_filp->f_mapping);
+	result = nfs_revalidate_mapping(inode, iocb->ki_filp->f_mapping);
 	if (!result) {
 		result = generic_file_read_iter(iocb, to);
 		if (result > 0)
@@ -194,7 +194,7 @@ nfs_file_splice_read(struct file *filp, loff_t *ppos,
 		filp, (unsigned long) count, (unsigned long long) *ppos);
 
 	nfs_start_io_read(inode);
-	res = nfs_revalidate_mapping_protected(inode, filp->f_mapping);
+	res = nfs_revalidate_mapping(inode, filp->f_mapping);
 	if (!res) {
 		res = generic_file_splice_read(filp, ppos, pipe, count, flags);
 		if (res > 0)
-- 
2.7.4


  reply	other threads:[~2016-06-22 13:07 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-21 21:34 [PATCH v2 01/12] NFS: Don't flush caches for a getattr that races with writeback Trond Myklebust
2016-06-21 21:34 ` [PATCH v2 02/12] NFS: Cache access checks more aggressively Trond Myklebust
2016-06-21 21:34   ` [PATCH v2 03/12] NFS: Cache aggressively when file is open for writing Trond Myklebust
2016-06-21 21:34     ` [PATCH v2 04/12] NFS: Kill NFS_INO_NFS_INO_FLUSHING: it is a performance killer Trond Myklebust
2016-06-21 21:34       ` [PATCH v2 05/12] NFS: writepage of a single page should not be synchronous Trond Myklebust
2016-06-21 21:34         ` [PATCH v2 06/12] NFS: Don't hold the inode lock across fsync() Trond Myklebust
2016-06-21 21:34           ` [PATCH v2 07/12] NFS: Don't call COMMIT in ->releasepage() Trond Myklebust
2016-06-21 21:34             ` [PATCH v2 08/12] NFS: Fix O_DIRECT verifier problems Trond Myklebust
2016-06-21 21:34               ` [PATCH v2 09/12] NFS: Ensure we reset the write verifier 'committed' value on resend Trond Myklebust
2016-06-21 21:34                 ` [PATCH v2 10/12] NFS: Do not serialise O_DIRECT reads and writes Trond Myklebust
2016-06-21 21:34                   ` [PATCH v2 11/12] NFS: Remove inode->i_dio_count from the NFS O_DIRECT code Trond Myklebust
2016-06-21 21:34                     ` [PATCH v2 12/12] NFS: Clean up nfs_direct_complete() Trond Myklebust
2016-06-22 16:43                       ` Christoph Hellwig
2016-06-22 16:42                     ` [PATCH v2 11/12] NFS: Remove inode->i_dio_count from the NFS O_DIRECT code Christoph Hellwig
2016-06-22 16:58                       ` Trond Myklebust
2016-06-23 10:19                         ` Christoph Hellwig
2016-06-22 17:58                     ` Anna Schumaker
2016-06-22 18:06                       ` Trond Myklebust
2016-06-22 18:08                         ` Anna Schumaker
2016-06-22 18:51                           ` Anna Schumaker
2016-06-22 19:42                             ` Trond Myklebust
2016-06-22 16:47                   ` [PATCH v2 10/12] NFS: Do not serialise O_DIRECT reads and writes Christoph Hellwig
2016-06-22 17:24                     ` Trond Myklebust
2016-06-23 11:00                       ` Christoph Hellwig
2016-06-23 11:00                         ` Christoph Hellwig
2016-06-21 22:25     ` [PATCH v2 03/12] NFS: Cache aggressively when file is open for writing Oleg Drokin
2016-06-22 13:06       ` Trond Myklebust [this message]
2016-06-22 16:19         ` Oleg Drokin
2016-06-22 15:48 ` [PATCH v2 01/12] NFS: Don't flush caches for a getattr that races with writeback Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1466600806.90993.2.camel@primarydata.com \
    --to=trondmy@primarydata.com \
    --cc=green@linuxhacker.ru \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.