Skip to content

Commit 3cf003c

Browse files
jtlaytonsmfrench
authored andcommitted
cifs: when CONFIG_HIGHMEM is set, serialize the read/write kmaps
Jian found that when he ran fsx on a 32 bit arch with a large wsize the process and one of the bdi writeback kthreads would sometimes deadlock with a stack trace like this: crash> bt PID: 2789 TASK: f02edaa0 CPU: 3 COMMAND: "fsx" #0 [eed63cbc] schedule at c083c5b3 #1 [eed63d80] kmap_high at c0500ec8 #2 [eed63db0] cifs_async_writev at f7fabcd7 [cifs] #3 [eed63df0] cifs_writepages at f7fb7f5c [cifs] #4 [eed63e50] do_writepages at c04f3e32 #5 [eed63e54] __filemap_fdatawrite_range at c04e152a #6 [eed63ea4] filemap_fdatawrite at c04e1b3e #7 [eed63eb4] cifs_file_aio_write at f7fa111a [cifs] #8 [eed63ecc] do_sync_write at c052d202 #9 [eed63f74] vfs_write at c052d4ee #10 [eed63f94] sys_write at c052df4c #11 [eed63fb0] ia32_sysenter_target at c0409a98 EAX: 00000004 EBX: 00000003 ECX: abd73b73 EDX: 012a65c6 DS: 007b ESI: 012a65c6 ES: 007b EDI: 00000000 SS: 007b ESP: bf8db178 EBP: bf8db1f8 GS: 0033 CS: 0073 EIP: 40000424 ERR: 00000004 EFLAGS: 00000246 Each task would kmap part of its address array before getting stuck, but not enough to actually issue the write. This patch fixes this by serializing the marshal_iov operations for async reads and writes. The idea here is to ensure that cifs aggressively tries to populate a request before attempting to fulfill another one. As soon as all of the pages are kmapped for a request, then we can unlock and allow another one to proceed. There's no need to do this serialization on non-CONFIG_HIGHMEM arches however, so optimize all of this out when CONFIG_HIGHMEM isn't set. Cc: <[email protected]> Reported-by: Jian Li <[email protected]> Signed-off-by: Jeff Layton <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent 3ae629d commit 3cf003c

File tree

1 file changed

+29
-1
lines changed

1 file changed

+29
-1
lines changed

fs/cifs/cifssmb.c

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,31 @@ static struct {
8686
#endif /* CONFIG_CIFS_WEAK_PW_HASH */
8787
#endif /* CIFS_POSIX */
8888

89-
/* Forward declarations */
89+
#ifdef CONFIG_HIGHMEM
90+
/*
91+
* On arches that have high memory, kmap address space is limited. By
92+
* serializing the kmap operations on those arches, we ensure that we don't
93+
* end up with a bunch of threads in writeback with partially mapped page
94+
* arrays, stuck waiting for kmap to come back. That situation prevents
95+
* progress and can deadlock.
96+
*/
97+
static DEFINE_MUTEX(cifs_kmap_mutex);
98+
99+
static inline void
100+
cifs_kmap_lock(void)
101+
{
102+
mutex_lock(&cifs_kmap_mutex);
103+
}
104+
105+
static inline void
106+
cifs_kmap_unlock(void)
107+
{
108+
mutex_unlock(&cifs_kmap_mutex);
109+
}
110+
#else /* !CONFIG_HIGHMEM */
111+
#define cifs_kmap_lock() do { ; } while(0)
112+
#define cifs_kmap_unlock() do { ; } while(0)
113+
#endif /* CONFIG_HIGHMEM */
90114

91115
/* Mark as invalid, all open files on tree connections since they
92116
were closed when session to server was lost */
@@ -1503,7 +1527,9 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
15031527
}
15041528

15051529
/* marshal up the page array */
1530+
cifs_kmap_lock();
15061531
len = rdata->marshal_iov(rdata, data_len);
1532+
cifs_kmap_unlock();
15071533
data_len -= len;
15081534

15091535
/* issue the read if we have any iovecs left to fill */
@@ -2069,7 +2095,9 @@ cifs_async_writev(struct cifs_writedata *wdata)
20692095
* and set the iov_len properly for each one. It may also set
20702096
* wdata->bytes too.
20712097
*/
2098+
cifs_kmap_lock();
20722099
wdata->marshal_iov(iov, wdata);
2100+
cifs_kmap_unlock();
20732101

20742102
cFYI(1, "async write at %llu %u bytes", wdata->offset, wdata->bytes);
20752103

0 commit comments

Comments
 (0)