Skip to content

Commit 4452b80

Browse files
vwaxgregkh
authored andcommitted
CIFS: fix oplock break deadlocks
commit 3998e6b upstream. When the final cifsFileInfo_put() is called from cifsiod and an oplock break work is queued, lockdep complains loudly: ============================================= [ INFO: possible recursive locking detected ] 4.11.0+ #21 Not tainted --------------------------------------------- kworker/0:2/78 is trying to acquire lock: ("cifsiod"){++++.+}, at: flush_work+0x215/0x350 but task is already holding lock: ("cifsiod"){++++.+}, at: process_one_work+0x255/0x8e0 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock("cifsiod"); lock("cifsiod"); *** DEADLOCK *** May be due to missing lock nesting notation 2 locks held by kworker/0:2/78: #0: ("cifsiod"){++++.+}, at: process_one_work+0x255/0x8e0 #1: ((&wdata->work)){+.+...}, at: process_one_work+0x255/0x8e0 stack backtrace: CPU: 0 PID: 78 Comm: kworker/0:2 Not tainted 4.11.0+ #21 Workqueue: cifsiod cifs_writev_complete Call Trace: dump_stack+0x85/0xc2 __lock_acquire+0x17dd/0x2260 ? match_held_lock+0x20/0x2b0 ? trace_hardirqs_off_caller+0x86/0x130 ? mark_lock+0xa6/0x920 lock_acquire+0xcc/0x260 ? lock_acquire+0xcc/0x260 ? flush_work+0x215/0x350 flush_work+0x236/0x350 ? flush_work+0x215/0x350 ? destroy_worker+0x170/0x170 __cancel_work_timer+0x17d/0x210 ? ___preempt_schedule+0x16/0x18 cancel_work_sync+0x10/0x20 cifsFileInfo_put+0x338/0x7f0 cifs_writedata_release+0x2a/0x40 ? cifs_writedata_release+0x2a/0x40 cifs_writev_complete+0x29d/0x850 ? preempt_count_sub+0x18/0xd0 process_one_work+0x304/0x8e0 worker_thread+0x9b/0x6a0 kthread+0x1b2/0x200 ? process_one_work+0x8e0/0x8e0 ? kthread_create_on_node+0x40/0x40 ret_from_fork+0x31/0x40 This is a real warning. Since the oplock is queued on the same workqueue this can deadlock if there is only one worker thread active for the workqueue (which will be the case during memory pressure when the rescuer thread is handling it). Furthermore, there is at least one other kind of hang possible due to the oplock break handling if there is only worker. (This can be reproduced without introducing memory pressure by having passing 1 for the max_active parameter of cifsiod.) cifs_oplock_break() can wait indefintely in the filemap_fdatawait() while the cifs_writev_complete() work is blocked: sysrq: SysRq : Show Blocked State task PC stack pid father kworker/0:1 D 0 16 2 0x00000000 Workqueue: cifsiod cifs_oplock_break Call Trace: __schedule+0x562/0xf40 ? mark_held_locks+0x4a/0xb0 schedule+0x57/0xe0 io_schedule+0x21/0x50 wait_on_page_bit+0x143/0x190 ? add_to_page_cache_lru+0x150/0x150 __filemap_fdatawait_range+0x134/0x190 ? do_writepages+0x51/0x70 filemap_fdatawait_range+0x14/0x30 filemap_fdatawait+0x3b/0x40 cifs_oplock_break+0x651/0x710 ? preempt_count_sub+0x18/0xd0 process_one_work+0x304/0x8e0 worker_thread+0x9b/0x6a0 kthread+0x1b2/0x200 ? process_one_work+0x8e0/0x8e0 ? kthread_create_on_node+0x40/0x40 ret_from_fork+0x31/0x40 dd D 0 683 171 0x00000000 Call Trace: __schedule+0x562/0xf40 ? mark_held_locks+0x29/0xb0 schedule+0x57/0xe0 io_schedule+0x21/0x50 wait_on_page_bit+0x143/0x190 ? add_to_page_cache_lru+0x150/0x150 __filemap_fdatawait_range+0x134/0x190 ? do_writepages+0x51/0x70 filemap_fdatawait_range+0x14/0x30 filemap_fdatawait+0x3b/0x40 filemap_write_and_wait+0x4e/0x70 cifs_flush+0x6a/0xb0 filp_close+0x52/0xa0 __close_fd+0xdc/0x150 SyS_close+0x33/0x60 entry_SYSCALL_64_fastpath+0x1f/0xbe Showing all locks held in the system: 2 locks held by kworker/0:1/16: #0: ("cifsiod"){.+.+.+}, at: process_one_work+0x255/0x8e0 #1: ((&cfile->oplock_break)){+.+.+.}, at: process_one_work+0x255/0x8e0 Showing busy workqueues and worker pools: workqueue cifsiod: flags=0xc pwq 0: cpus=0 node=0 flags=0x0 nice=0 active=1/1 in-flight: 16:cifs_oplock_break delayed: cifs_writev_complete, cifs_echo_request pool 0: cpus=0 node=0 flags=0x0 nice=0 hung=0s workers=3 idle: 750 3 Fix these problems by creating a a new workqueue (with a rescuer) for the oplock break work. Signed-off-by: Rabin Vincent <[email protected]> Signed-off-by: Steve French <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent cd01b99 commit 4452b80

File tree

4 files changed

+18
-5
lines changed

4 files changed

+18
-5
lines changed

fs/cifs/cifsfs.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ extern mempool_t *cifs_req_poolp;
8787
extern mempool_t *cifs_mid_poolp;
8888

8989
struct workqueue_struct *cifsiod_wq;
90+
struct workqueue_struct *cifsoplockd_wq;
9091
__u32 cifs_lock_secret;
9192

9293
/*
@@ -1282,9 +1283,16 @@ init_cifs(void)
12821283
goto out_clean_proc;
12831284
}
12841285

1286+
cifsoplockd_wq = alloc_workqueue("cifsoplockd",
1287+
WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
1288+
if (!cifsoplockd_wq) {
1289+
rc = -ENOMEM;
1290+
goto out_destroy_cifsiod_wq;
1291+
}
1292+
12851293
rc = cifs_fscache_register();
12861294
if (rc)
1287-
goto out_destroy_wq;
1295+
goto out_destroy_cifsoplockd_wq;
12881296

12891297
rc = cifs_init_inodecache();
12901298
if (rc)
@@ -1332,7 +1340,9 @@ init_cifs(void)
13321340
cifs_destroy_inodecache();
13331341
out_unreg_fscache:
13341342
cifs_fscache_unregister();
1335-
out_destroy_wq:
1343+
out_destroy_cifsoplockd_wq:
1344+
destroy_workqueue(cifsoplockd_wq);
1345+
out_destroy_cifsiod_wq:
13361346
destroy_workqueue(cifsiod_wq);
13371347
out_clean_proc:
13381348
cifs_proc_clean();
@@ -1355,6 +1365,7 @@ exit_cifs(void)
13551365
cifs_destroy_mids();
13561366
cifs_destroy_inodecache();
13571367
cifs_fscache_unregister();
1368+
destroy_workqueue(cifsoplockd_wq);
13581369
destroy_workqueue(cifsiod_wq);
13591370
cifs_proc_clean();
13601371
}

fs/cifs/cifsglob.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,6 +1655,7 @@ void cifs_oplock_break(struct work_struct *work);
16551655

16561656
extern const struct slow_work_ops cifs_oplock_break_ops;
16571657
extern struct workqueue_struct *cifsiod_wq;
1658+
extern struct workqueue_struct *cifsoplockd_wq;
16581659
extern __u32 cifs_lock_secret;
16591660

16601661
extern mempool_t *cifs_mid_poolp;

fs/cifs/misc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ is_valid_oplock_break(char *buffer, struct TCP_Server_Info *srv)
492492
CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
493493
&pCifsInode->flags);
494494

495-
queue_work(cifsiod_wq,
495+
queue_work(cifsoplockd_wq,
496496
&netfile->oplock_break);
497497
netfile->oplock_break_cancelled = false;
498498

fs/cifs/smb2misc.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp,
494494
else
495495
cfile->oplock_break_cancelled = true;
496496

497-
queue_work(cifsiod_wq, &cfile->oplock_break);
497+
queue_work(cifsoplockd_wq, &cfile->oplock_break);
498498
kfree(lw);
499499
return true;
500500
}
@@ -638,7 +638,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
638638
CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
639639
&cinode->flags);
640640
spin_unlock(&cfile->file_info_lock);
641-
queue_work(cifsiod_wq, &cfile->oplock_break);
641+
queue_work(cifsoplockd_wq,
642+
&cfile->oplock_break);
642643

643644
spin_unlock(&tcon->open_file_lock);
644645
spin_unlock(&cifs_tcp_ses_lock);

0 commit comments

Comments
 (0)