From patchwork Thu Nov 17 21:16:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 9435337 X-Patchwork-Delegate: snitzer@redhat.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 45A5C60471 for ; Thu, 17 Nov 2016 21:18:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 35853296CC for ; Thu, 17 Nov 2016 21:18:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 296C6296D8; Thu, 17 Nov 2016 21:18:40 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from mx4-phx2.redhat.com (mx4-phx2.redhat.com [209.132.183.25]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 7AC48296CC for ; Thu, 17 Nov 2016 21:18:38 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx4-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id uAHLHFQ9003259; Thu, 17 Nov 2016 16:17:15 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id uAHLHEM9014735 for ; Thu, 17 Nov 2016 16:17:14 -0500 Received: from mx1.redhat.com (ext-mx06.extmail.prod.ext.phx2.redhat.com [10.5.110.30]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uAHLHEA6025878 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO) for ; Thu, 17 Nov 2016 16:17:14 -0500 Received: from mail-yb0-f182.google.com (mail-yb0-f182.google.com [209.85.213.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0E0BD3B714 for ; Thu, 17 Nov 2016 21:17:13 +0000 (UTC) Received: by mail-yb0-f182.google.com with SMTP id a184so74800975ybb.0 for ; Thu, 17 Nov 2016 13:17:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id; bh=H/pNMhOIxBcfvLOdFDGk7uhSjB0I2G0Q1p+aBHTkYFw=; b=k9h0Zu4ngvYcmBpLX2gLKqadIsSv2IEWPm+pvWQBidXAjOLL4mzerFZpL7mmayHJRR 9H1RGmosFkNoU6WfrMD6IkGytALnY1gK8jfJ7Rzl235XbQA95ENdfcchnBg769OJmg5Q 4lOL5cEQtiLxjYRxjJ2kob3HNyR+4DpQ4/Usg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=H/pNMhOIxBcfvLOdFDGk7uhSjB0I2G0Q1p+aBHTkYFw=; b=jvwEYoBf1sWzjtABE4JsKRyzag26jGeOAWl0c5XYS9WVInJPhQQQGOShLWUJVTNfX8 Bf6hO9ExWQnluhxNgg2WMsy4T1pwFWpGpKSNHtnr06KDnr3OK0qmGr3j5gF90PnnFzwH OnZehGorzmRp9Vrh/HRPaZoqaQpHWgFUp3DOqtLjonUgrgcsE2u3ISXHHPDPaPIDLO7Y 1xs0TCpKOO8Cb4q18ILUBWbMxodPuQm/sNrw70L/+C3593sfG6ruDaE1axaSlfwjYzMI ulJiy6PCrlY0nFbpOISE2fQhYBEHYiz951In59Qk12vYWt5Fpd4uFy9jnrRajtgsv1Rk dlEQ== X-Gm-Message-State: ABUngvdrBQWg0tLqFLR6zEC8HqY5g5KTFfqRNsSw9Pq7PImC1ABV4faiuyXc6O/Khz+iKT2s X-Received: by 10.13.203.69 with SMTP id n66mr4500649ywd.220.1479417432372; Thu, 17 Nov 2016 13:17:12 -0800 (PST) Received: from tictac.mtv.corp.google.com ([172.22.65.76]) by smtp.gmail.com with ESMTPSA id d136sm1629113ywh.50.2016.11.17.13.17.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 17 Nov 2016 13:17:11 -0800 (PST) From: Douglas Anderson To: Alasdair Kergon , Mike Snitzer Date: Thu, 17 Nov 2016 13:16:38 -0800 Message-Id: <1479417398-2593-1-git-send-email-dianders@chromium.org> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 17 Nov 2016 21:17:13 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 17 Nov 2016 21:17:13 +0000 (UTC) for IP:'209.85.213.182' DOMAIN:'mail-yb0-f182.google.com' HELO:'mail-yb0-f182.google.com' FROM:'dianders@chromium.org' RCPT:'' X-RedHat-Spam-Score: 0.769 (BAYES_50, DCC_REPUT_00_12, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, RCVD_IN_SORBS_SPAM, SPF_PASS) 209.85.213.182 mail-yb0-f182.google.com 209.85.213.182 mail-yb0-f182.google.com X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.30 X-loop: dm-devel@redhat.com Cc: shli@kernel.org, Dmitry Torokhov , Douglas Anderson , linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org, dm-devel@redhat.com, David Rientjes , Sonny Rao , linux@roeck-us.net Subject: [dm-devel] [PATCH] RFC: dm: avoid the mutex lock in dm_bufio_shrink_count() X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Virus-Scanned: ClamAV using ClamSMTP As reported in my recent patch ("dm: Avoid sleeping while holding the dm_bufio lock"), we've seen in-field reports of lots of tasks sitting blocked on: mutex_lock+0x4c/0x68 dm_bufio_shrink_count+0x38/0x78 shrink_slab.part.54.constprop.65+0x100/0x464 shrink_zone+0xa8/0x198 Analysis of dm_bufio_shrink_count() shows that it's not much work to avoid grabbing the mutex there. Presumably if we can avoid grabbing this mutex then other tasks (including those handling swap) may sometimes return faster. Note that it's likely that this won't be a huge savings since we'll just need to grab the mutex if dm_bufio_shrink_scan() is called, but it still seems like it would be a sane tradeoff. This does slightly change the behavior of dm_bufio_shrink_count(). If anyone was relying on dm_bufio_shrink_count() to return the total count _after_ some in-progress operation finished then they'll no longer get that behavior. It seems unlikely anyone would be relying on this behavior, though, because: - Someone would have to be certain that the operation was already in progress _before_ dm_bufio_shrink_count() was called. If the operation wasn't already in progress then we'd get the lock first. - There aren't exactly lots of long-running operations where the dm_bufio lock is held the whole time. Most functions using the dm_bufio lock grab and drop it almost constantly. That means that getting the dm_bufio doesn't mean that any in-progress dm_bufio transactions are all done. Maybe the above argument would be obvious to someone wise in the ways of dm_bufio but it's a useful argument to make for those like me who are trying to make a small fix without full comprehension of all of dm_bufio's finer details. Signed-off-by: Douglas Anderson --- It's a bit unclear if this is actually useful and I don't have any test cases showing the benefit, but posting it in case someone else has good test cases or thinks it's a clear win. Same caveat that this development was done on Chrome OS Kernel 4.4 drivers/md/dm-bufio.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index b3ba142e59a4..885ba5482d9f 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -89,6 +89,7 @@ struct dm_bufio_client { struct list_head lru[LIST_SIZE]; unsigned long n_buffers[LIST_SIZE]; + unsigned long n_all_buffers; struct block_device *bdev; unsigned block_size; @@ -485,6 +486,7 @@ static void __link_buffer(struct dm_buffer *b, sector_t block, int dirty) struct dm_bufio_client *c = b->c; c->n_buffers[dirty]++; + c->n_all_buffers++; b->block = block; b->list_mode = dirty; list_add(&b->lru_list, &c->lru[dirty]); @@ -502,6 +504,7 @@ static void __unlink_buffer(struct dm_buffer *b) BUG_ON(!c->n_buffers[b->list_mode]); c->n_buffers[b->list_mode]--; + c->n_all_buffers--; __remove(b->c, b); list_del(&b->lru_list); } @@ -515,6 +518,7 @@ static void __relink_lru(struct dm_buffer *b, int dirty) BUG_ON(!c->n_buffers[b->list_mode]); + /* NOTE: don't update n_all_buffers: -1 + 1 = 0 */ c->n_buffers[b->list_mode]--; c->n_buffers[dirty]++; b->list_mode = dirty; @@ -1588,17 +1592,10 @@ static unsigned long dm_bufio_shrink_count(struct shrinker *shrink, struct shrink_control *sc) { struct dm_bufio_client *c; - unsigned long count; c = container_of(shrink, struct dm_bufio_client, shrinker); - if (sc->gfp_mask & __GFP_FS) - dm_bufio_lock(c); - else if (!dm_bufio_trylock(c)) - return 0; - count = c->n_buffers[LIST_CLEAN] + c->n_buffers[LIST_DIRTY]; - dm_bufio_unlock(c); - return count; + return c->n_all_buffers; } /*