Skip to content

Commit 5d0c7b4

Browse files
jtlaytongregkh
authored andcommitted
cifs: when CONFIG_HIGHMEM is set, serialize the read/write kmaps
commit 3cf003c upstream. [The async read code was broadened to include uncached reads in 3.5, so the mainline patch did not apply directly. This patch is just a backport to account for that change.] 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 [eed63db] 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: bf8db17 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. Reported-by: Jian Li <[email protected]> Signed-off-by: Jeff Layton <[email protected]> Signed-off-by: Steve French <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent a22ad13 commit 5d0c7b4

File tree

1 file changed

+30
-0
lines changed

1 file changed

+30
-0
lines changed

fs/cifs/cifssmb.c

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,32 @@ static struct {
8989
/* Forward declarations */
9090
static void cifs_readv_complete(struct work_struct *work);
9191

92+
#ifdef CONFIG_HIGHMEM
93+
/*
94+
* On arches that have high memory, kmap address space is limited. By
95+
* serializing the kmap operations on those arches, we ensure that we don't
96+
* end up with a bunch of threads in writeback with partially mapped page
97+
* arrays, stuck waiting for kmap to come back. That situation prevents
98+
* progress and can deadlock.
99+
*/
100+
static DEFINE_MUTEX(cifs_kmap_mutex);
101+
102+
static inline void
103+
cifs_kmap_lock(void)
104+
{
105+
mutex_lock(&cifs_kmap_mutex);
106+
}
107+
108+
static inline void
109+
cifs_kmap_unlock(void)
110+
{
111+
mutex_unlock(&cifs_kmap_mutex);
112+
}
113+
#else /* !CONFIG_HIGHMEM */
114+
#define cifs_kmap_lock() do { ; } while(0)
115+
#define cifs_kmap_unlock() do { ; } while(0)
116+
#endif /* CONFIG_HIGHMEM */
117+
92118
/* Mark as invalid, all open files on tree connections since they
93119
were closed when session to server was lost */
94120
static void mark_open_files_invalid(struct cifs_tcon *pTcon)
@@ -1557,6 +1583,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
15571583
eof_index = eof ? (eof - 1) >> PAGE_CACHE_SHIFT : 0;
15581584
cFYI(1, "eof=%llu eof_index=%lu", eof, eof_index);
15591585

1586+
cifs_kmap_lock();
15601587
list_for_each_entry_safe(page, tpage, &rdata->pages, lru) {
15611588
if (remaining >= PAGE_CACHE_SIZE) {
15621589
/* enough data to fill the page */
@@ -1606,6 +1633,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
16061633
page_cache_release(page);
16071634
}
16081635
}
1636+
cifs_kmap_unlock();
16091637

16101638
/* issue the read if we have any iovecs left to fill */
16111639
if (rdata->nr_iov > 1) {
@@ -2194,7 +2222,9 @@ cifs_async_writev(struct cifs_writedata *wdata)
21942222
* and set the iov_len properly for each one. It may also set
21952223
* wdata->bytes too.
21962224
*/
2225+
cifs_kmap_lock();
21972226
wdata->marshal_iov(iov, wdata);
2227+
cifs_kmap_unlock();
21982228

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

0 commit comments

Comments
 (0)