Skip to content

Commit bb761c9

Browse files
jtlaytonbwhacks
authored andcommitted
cifs: when CONFIG_HIGHMEM is set, serialize the read/write kmaps
commit 3cf003c upstream. 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 raspberrypi#1 [eed63d80] kmap_high at c0500ec8 raspberrypi#2 [eed63db0] cifs_async_writev at f7fabcd7 [cifs] raspberrypi#3 [eed63df0] cifs_writepages at f7fb7f5c [cifs] raspberrypi#4 [eed63e50] do_writepages at c04f3e32 raspberrypi#5 [eed63e54] __filemap_fdatawrite_range at c04e152a raspberrypi#6 [eed63ea4] filemap_fdatawrite at c04e1b3e raspberrypi#7 [eed63eb4] cifs_file_aio_write at f7fa111a [cifs] raspberrypi#8 [eed63ecc] do_sync_write at c052d202 raspberrypi#9 [eed63f74] vfs_write at c052d4ee raspberrypi#10 [eed63f94] sys_write at c052df4c raspberrypi#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. Reported-by: Jian Li <[email protected]> Signed-off-by: Jeff Layton <[email protected]> Signed-off-by: Steve French <[email protected]> [bwh: Backported to 3.2: adjust context] Signed-off-by: Ben Hutchings <[email protected]>
1 parent e67de1a commit bb761c9

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)
@@ -1540,6 +1566,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
15401566
eof_index = eof ? (eof - 1) >> PAGE_CACHE_SHIFT : 0;
15411567
cFYI(1, "eof=%llu eof_index=%lu", eof, eof_index);
15421568

1569+
cifs_kmap_lock();
15431570
list_for_each_entry_safe(page, tpage, &rdata->pages, lru) {
15441571
if (remaining >= PAGE_CACHE_SIZE) {
15451572
/* enough data to fill the page */
@@ -1589,6 +1616,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
15891616
page_cache_release(page);
15901617
}
15911618
}
1619+
cifs_kmap_unlock();
15921620

15931621
/* issue the read if we have any iovecs left to fill */
15941622
if (rdata->nr_iov > 1) {
@@ -2171,6 +2199,7 @@ cifs_async_writev(struct cifs_writedata *wdata)
21712199
iov[0].iov_base = smb;
21722200

21732201
/* marshal up the pages into iov array */
2202+
cifs_kmap_lock();
21742203
wdata->bytes = 0;
21752204
for (i = 0; i < wdata->nr_pages; i++) {
21762205
iov[i + 1].iov_len = min(inode->i_size -
@@ -2179,6 +2208,7 @@ cifs_async_writev(struct cifs_writedata *wdata)
21792208
iov[i + 1].iov_base = kmap(wdata->pages[i]);
21802209
wdata->bytes += iov[i + 1].iov_len;
21812210
}
2211+
cifs_kmap_unlock();
21822212

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

0 commit comments

Comments
 (0)