[PATCH linux-next] fs: don't call _cond_resched() from the rcu critical section

[PATCH linux-next] fs: don't call _cond_resched() from the rcu critical section

[Andrey Vagin]

The context can't be switched in rcu critical sections.

Cc: NeilBrown <neilb@suse.de>
Cci: Al Viro <viro@zeniv.linux.org.uk>
Fixes: 26c59f88c223 ("VFS/namei: make the use of touch_atime() in get_link() RCU-safe.")
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
 fs/namei.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index f884ddd..0ef3c79 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -967,14 +967,13 @@ const char *get_link(struct nameidata *nd)
 
 	if (!(nd->flags & LOOKUP_RCU)) {
 		touch_atime(&last->link);
+		_cond_resched();
 	} else if (atime_needs_update(&last->link, inode)) {
 		if (unlikely(unlazy_walk(nd, NULL, 0)))
 			return ERR_PTR(-ECHILD);
 		touch_atime(&last->link);
 	}
 
-	_cond_resched();
-
 	error = security_inode_follow_link(dentry, inode,
 					   nd->flags & LOOKUP_RCU);
 	if (unlikely(error))
-- 
2.1.0

Re: [PATCH linux-next] fs: don't call _cond_resched() from the rcu critical section

[Andrey Wagin]

2015-05-13 19:10 GMT+03:00 Andrey Vagin <avagin@openvz.org>:
> The context can't be switched in rcu critical sections.

Here is a warning which I see in dmesg without this patch:
[20570.684805] ===============================
[20570.685983] [ INFO: suspicious RCU usage. ]
[20570.687011] 4.1.0-rc3-next-20150513+ #236 Not tainted
[20570.688295] -------------------------------
[20570.689277] include/linux/rcupdate.h:568 Illegal context switch in
RCU read-side critical section!
[20570.691550]
other info that might help us debug this:

[20570.693628]
rcu_scheduler_active = 1, debug_locks = 0
[20570.695430] 1 lock held by systemd/1:
[20570.696455]  #0:  (rcu_read_lock){......}, at: [<ffffffff812166ba>]
path_init+0x54a/0x920
[20570.698734]
stack backtrace:
[20570.699896] CPU: 3 PID: 1 Comm: systemd Not tainted
4.1.0-rc3-next-20150513+ #236
[20570.701824] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[20570.703322]  0000000000000000 00000000719ddd62 ffff88007d0ffaa8
ffffffff8175e78d
[20570.705453]  0000000000000000 ffff88007d160000 ffff88007d0ffad8
ffffffff810d8b17
[20570.707485]  ffff88007fd96e40 0000000000016e40 ffff88007fd96e40
ffff88007b828868
[20570.709540] Call Trace:
[20570.710231]  [<ffffffff8175e78d>] dump_stack+0x45/0x57
[20570.711573]  [<ffffffff810d8b17>] lockdep_rcu_suspicious+0xd7/0x110
[20570.713211]  [<ffffffff81760da3>] __schedule+0x4b3/0xbf0
[20570.714553]  [<ffffffff81214714>] ? generic_permission+0x24/0x1c0
[20570.716241]  [<ffffffff817617d2>] preempt_schedule_common+0x22/0x40
[20570.717873]  [<ffffffff81761811>] _cond_resched+0x21/0x30
[20570.719287]  [<ffffffff81217d43>] link_path_walk+0x1f3/0x6a0
[20570.720785]  [<ffffffff812188f6>] ? path_lookupat+0x16/0xf0
[20570.722260]  [<ffffffff8121b544>] ? getname_kernel+0x34/0x130
[20570.723785]  [<ffffffff8121895c>] path_lookupat+0x7c/0xf0
[20570.725301]  [<ffffffff8121b544>] ? getname_kernel+0x34/0x130
[20570.726790]  [<ffffffff81218a3e>] filename_lookup+0x6e/0x120
[20570.728264]  [<ffffffff8121b941>] kern_path+0x51/0xb0
[20570.729584]  [<ffffffff8161ced1>] ? __kmalloc_reserve.isra.34+0x31/0x80
[20570.731291]  [<ffffffff816f9187>] unix_find_other+0x47/0x230
[20570.732762]  [<ffffffff816f9d4a>] unix_stream_connect+0xfa/0x4c0
[20570.734364]  [<ffffffff816133c7>] SYSC_connect+0xe7/0x120
[20570.735812]  [<ffffffff813b0cc5>] ? lockdep_sys_exit_thunk+0x12/0x14
[20570.737475]  [<ffffffff8161476e>] SyS_connect+0xe/0x10
[20570.738830]  [<ffffffff8176782e>] system_call_fastpath+0x12/0x76

>
> Cc: NeilBrown <neilb@suse.de>
> Cci: Al Viro <viro@zeniv.linux.org.uk>
> Fixes: 26c59f88c223 ("VFS/namei: make the use of touch_atime() in get_link() RCU-safe.")
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
> ---
>  fs/namei.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index f884ddd..0ef3c79 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -967,14 +967,13 @@ const char *get_link(struct nameidata *nd)
>
>         if (!(nd->flags & LOOKUP_RCU)) {
>                 touch_atime(&last->link);
> +               _cond_resched();
>         } else if (atime_needs_update(&last->link, inode)) {
>                 if (unlikely(unlazy_walk(nd, NULL, 0)))
>                         return ERR_PTR(-ECHILD);
>                 touch_atime(&last->link);
>         }
>
> -       _cond_resched();
> -
>         error = security_inode_follow_link(dentry, inode,
>                                            nd->flags & LOOKUP_RCU);
>         if (unlikely(error))
> --
> 2.1.0
>

Re: [PATCH linux-next] fs: don't call _cond_resched() from the rcu critical section

[Al Viro]

On Wed, May 13, 2015 at 07:10:01PM +0300, Andrey Vagin wrote:
> The context can't be switched in rcu critical sections.

Folded in a slightly different form and force-pushed.