From patchwork Wed Jul 11 13:09:36 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 1182081 Return-Path: X-Original-To: patchwork-cifs-client@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id DCB7BDF25A for ; Wed, 11 Jul 2012 13:09:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754740Ab2GKNJs (ORCPT ); Wed, 11 Jul 2012 09:09:48 -0400 Received: from mail-gh0-f174.google.com ([209.85.160.174]:54912 "EHLO mail-gh0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754642Ab2GKNJr (ORCPT ); Wed, 11 Jul 2012 09:09:47 -0400 Received: by ghrr11 with SMTP id r11so1162591ghr.19 for ; Wed, 11 Jul 2012 06:09:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references:x-gm-message-state; bh=Dd+F0kANw92nZDfaOIfmtdUsGS6D2dkfZMQhsK4/KNE=; b=GrIxoFCjkKWO3Vpe4Qz5EdJXr3KufF92TTftX+RfFlB7utMS9vP8Rye5VETl2ZVn3Y L2Sbk6HjhH9B2ORo4vA3ZGsOiDIiCFBcjyRTwlUE/pcjotNtMyQeUQBKPosHuQhSU6SP 2Elm/URiqWfZoM0icltn8VB9pdLpMX7MeE3nh9P7VTkJbberGcoce9/5KWSR/4qj70aD sGlpjrc8VZqMI4eqAU3EfjK6web5WKQq0c2bAagPEi6JgbyCJYYAAynWqp9fd9YURj3i bDT15LQSpVFrlxobQYe2J4Hlwk4xQ4Zh4N90rScsTBoporNeiB9bZuNyOo79ULySlNdM 9wNQ== Received: by 10.236.76.103 with SMTP id a67mr16526533yhe.69.1342012187033; Wed, 11 Jul 2012 06:09:47 -0700 (PDT) Received: from salusa.poochiereds.net (cpe-076-182-054-194.nc.res.rr.com. [76.182.54.194]) by mx.google.com with ESMTPS id p14sm1367051ani.8.2012.07.11.06.09.45 (version=SSLv3 cipher=OTHER); Wed, 11 Jul 2012 06:09:46 -0700 (PDT) From: Jeff Layton To: smfrench@gmail.com Cc: linux-cifs@vger.kernel.org, jiali@redhat.com Subject: [PATCH 2/2] cifs: when CONFIG_HIGHMEM is set, serialize the read/write kmaps Date: Wed, 11 Jul 2012 09:09:36 -0400 Message-Id: <1342012176-14783-3-git-send-email-jlayton@redhat.com> X-Mailer: git-send-email 1.7.7.6 In-Reply-To: <1342012176-14783-1-git-send-email-jlayton@redhat.com> References: <1342012176-14783-1-git-send-email-jlayton@redhat.com> X-Gm-Message-State: ALoCoQlMnWXWk3DZDp0O+WlJfE3ru2ciBahoOOdk2VDUDn+2v5KCIbuzRqgQySICVhnT4rDl4dND Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org 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: Reported-by: Jian Li Signed-off-by: Jeff Layton --- fs/cifs/cifssmb.c | 30 +++++++++++++++++++++++++++++- 1 files changed, 29 insertions(+), 1 deletions(-) diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 0170ee8..684a072 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -86,7 +86,31 @@ static struct { #endif /* CONFIG_CIFS_WEAK_PW_HASH */ #endif /* CIFS_POSIX */ -/* Forward declarations */ +#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 */ @@ -1503,7 +1527,9 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid) } /* marshal up the page array */ + cifs_kmap_lock(); len = rdata->marshal_iov(rdata, data_len); + cifs_kmap_unlock(); data_len -= len; /* issue the read if we have any iovecs left to fill */ @@ -2069,7 +2095,9 @@ cifs_async_writev(struct cifs_writedata *wdata) * and set the iov_len properly for each one. It may also set * wdata->bytes too. */ + cifs_kmap_lock(); wdata->marshal_iov(iov, wdata); + cifs_kmap_unlock(); cFYI(1, "async write at %llu %u bytes", wdata->offset, wdata->bytes);