Attachment 595703 Details for Bug 832252 – patch -- serialize kmaps in async writev marshalling code and cap wsize at available kmap address space

[patch] patch -- serialize kmaps in async writev marshalling code and cap wsize at available kmap address space

rhel6-cifs-kmap.patch (text/plain), 6.28 KB, created by Jeff Layton on 2012-07-02 11:41:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Layton

Created: 2012-07-02 11:41:41 UTC

Size: 6.28 KB

patch

obsolete

>-------------------------------------------------------------------------------
>cifs-on-config_highmem-machine
>-------------------------------------------------------------------------------
>cifs: on CONFIG_HIGHMEM machines, limit the rsize/wsize to the kmap space
>
>From: Jeff Layton <jlayton@redhat.com>
>
>We currently rely on being able to kmap all of the pages in an async
>read or write request. If you're on a machine that has CONFIG_HIGHMEM
>set then that kmap space is limited, sometimes to as low as 512 slots.
>
>With 512 slots, we can only support up to a 2M r/wsize, and that's
>assuming that we can get our greedy little hands on all of them. There
>are other users however, so it's possible we'll end up stuck with a
>size that large.
>
>Since we can't handle a rsize or wsize larger than that currently, cap
>those options at the number of kmap slots we have. We could consider
>capping it even lower, but we currently default to a max of 1M. Might as
>well allow those luddites on 32 bit arches enough rope to hang
>themselves.
>
>A more robust fix would be to teach the send and receive routines how
>to contend with an array of pages so we don't need to marshal up a kvec
>array at all. That's a fairly significant overhaul though, so we'll need
>this limit in place until someone is willing to do that work.
>
>Signed-off-by: Jeff Layton <jlayton@redhat.com>
>---
>
> fs/cifs/connect.c |   15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)
>
>
>diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>index 2a9bbc6..6b6e9c0 100644
>--- a/fs/cifs/connect.c
>+++ b/fs/cifs/connect.c
>@@ -2713,6 +2713,18 @@ static void setup_cifs_sb(struct smb_vol *pvolume_info,
>  */
> #define CIFS_DEFAULT_NON_POSIX_WSIZE (65536)
> 
>+/*
>+ * On hosts with high memory, we can't currently support wsize/rsize that are
>+ * larger than we can kmap at once. Cap the rsize/wsize at
>+ * LAST_PKMAP * PAGE_SIZE. We'll never be able to fill a read or write request
>+ * larger than that anyway.
>+ */
>+#ifdef CONFIG_HIGHMEM
>+#define CIFS_KMAP_SIZE_LIMIT	(LAST_PKMAP * PAGE_CACHE_SIZE)
>+#else /* CONFIG_HIGHMEM */
>+#define CIFS_KMAP_SIZE_LIMIT	(1<<24)
>+#endif /* CONFIG_HIGHMEM */
>+
> static unsigned int
> cifs_negotiate_wsize(struct cifsTconInfo *tcon, struct smb_vol *pvolume_info)
> {
>@@ -2743,6 +2755,9 @@ cifs_negotiate_wsize(struct cifsTconInfo *tcon, struct smb_vol *pvolume_info)
> 		wsize = min_t(unsigned int, wsize,
> 				server->maxBuf - sizeof(WRITE_REQ) + 4);
> 
>+	/* limit to the amount that we can kmap at once */
>+	wsize = min_t(unsigned int, wsize, CIFS_KMAP_SIZE_LIMIT);
>+
> 	/* hard limit of CIFS_MAX_WSIZE */
> 	wsize = min_t(unsigned int, wsize, CIFS_MAX_WSIZE);
> 
>-------------------------------------------------------------------------------
>cifs-when-config_highmem-is-se
>-------------------------------------------------------------------------------
>BZ#832252: cifs: when CONFIG_HIGHMEM is set, serialize the read/write kmaps
>
>From: Jeff Layton <jlayton@redhat.com>
>
>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.
>
>Reported-by: Jian Li <jiali@redhat.com>
>Signed-off-by: Jeff Layton <jlayton@redhat.com>
>---
>
> fs/cifs/cifssmb.c |   28 ++++++++++++++++++++++++++++
> 1 files changed, 28 insertions(+), 0 deletions(-)
>
>
>diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
>index b485659..ff69808 100644
>--- a/fs/cifs/cifssmb.c
>+++ b/fs/cifs/cifssmb.c
>@@ -82,6 +82,32 @@ static struct {
> #endif /* CONFIG_CIFS_WEAK_PW_HASH */
> #endif /* CIFS_POSIX */
> 
>+#ifdef CONFIG_HIGHMEM
>+/*
>+ * On arches that have high memory, kmap address space is limited. By
>+ * serializing the kmap operations on those arches, we ensure that we don't
>+ * end up with a bunch of threads in writeback with partially mapped page
>+ * arrays, stuck waiting for kmap to come back. That situation prevents
>+ * progress and can deadlock.
>+ */
>+static DEFINE_MUTEX(cifs_kmap_mutex);
>+
>+static inline void
>+cifs_kmap_lock(void)
>+{
>+	mutex_lock(&cifs_kmap_mutex);
>+}
>+
>+static inline void
>+cifs_kmap_unlock(void)
>+{
>+	mutex_unlock(&cifs_kmap_mutex);
>+}
>+#else /* !CONFIG_HIGHMEM */
>+#define cifs_kmap_lock() do { ; } while(0)
>+#define cifs_kmap_unlock() do { ; } while(0)
>+#endif /* CONFIG_HIGHMEM */
>+
> /* Mark as invalid, all open files on tree connections since they
>    were closed when session to server was lost */
> static void mark_open_files_invalid(struct cifsTconInfo *pTcon)
>@@ -1801,6 +1827,7 @@ cifs_async_writev(struct cifs_writedata *wdata)
> 
> 	/* marshal up the pages into iov array */
> 	wdata->bytes = 0;
>+	cifs_kmap_lock();
> 	for (i = 0; i < wdata->nr_pages; i++) {
> 		iov[i + 1].iov_len = min(inode->i_size -
> 				      page_offset(wdata->pages[i]),
>@@ -1808,6 +1835,7 @@ cifs_async_writev(struct cifs_writedata *wdata)
> 		iov[i + 1].iov_base = kmap(wdata->pages[i]);
> 		wdata->bytes += iov[i + 1].iov_len;
> 	}
>+	cifs_kmap_unlock();
> 
> 	cFYI(1, "async write at %llu %u bytes", wdata->offset, wdata->bytes);
>

Actions: View | Diff

Attachments on bug 832252: 592100 | 595703