From patchwork Sun Feb 18 13:05:06 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejun Heo X-Patchwork-Id: 10226749 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 BD30660467 for ; Sun, 18 Feb 2018 13:05:13 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A51BF28B05 for ; Sun, 18 Feb 2018 13:05:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 99F8E28AFA; Sun, 18 Feb 2018 13:05:13 +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.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2F0AB28AF7 for ; Sun, 18 Feb 2018 13:05:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751337AbeBRNFM (ORCPT ); Sun, 18 Feb 2018 08:05:12 -0500 Received: from mail-qt0-f195.google.com ([209.85.216.195]:36341 "EHLO mail-qt0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751292AbeBRNFL (ORCPT ); Sun, 18 Feb 2018 08:05:11 -0500 Received: by mail-qt0-f195.google.com with SMTP id q18so9240271qtl.3 for ; Sun, 18 Feb 2018 05:05:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:mime-version :content-disposition:user-agent; bh=fnxpghCSq7nZ2/H5tB1Z25BzrAGIYGJFGPodgkGThxk=; b=UjPFtED0AVys588+l5FqkxFBeRyDEDEK9xAkSRLXnyafMbv7sgiUQhznqgPOEw36hI ttuJCbhMRojTjXEC0CmRfB4smq8FGosVEfNPr7yEkEcNDfazc7YdGAxw37xlS8iJnAFR 0/f14ROOuGu5o70xWEjz/S1T80QceYW8G8Se0Y122jJBJsvyoHMZFybr4EDGhZfdPNrW /rkXLf5ygc9u0QM3Qa+L1NVevBo6vIIX89f5SlwgrFn1jCymnlqvjsMHxSrngdYDwd6D BvX+bTSmrZ0ex+9obv4iZW+ijDgarZVCR+NsiY+i39RM9/VIjjY3GG55QSSn5A97m7n+ AB/A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :mime-version:content-disposition:user-agent; bh=fnxpghCSq7nZ2/H5tB1Z25BzrAGIYGJFGPodgkGThxk=; b=fI5QFL9pY4kHbZ4RN1OOtGpBC4qIbBRLWMJHxopbfpHSozIrXM3p4J1lFJNYckMo38 G0gZQcOV3EJeRbvHFPpPF9cGOVt5aSSKk7H4n+lO86QfSfUaLbMc26cnLpBIZVtzYtUu 2BW59T2oohSaFjmC7bj6L9IMJRHX1RDX4hFOGrQLrdkEm3FuyC7nVZoPjCmbo/oUzG0/ vULbFZr/tQTKYA2XoB3gsdrsgZAZfs+AkA6POY1SXOVvlmVdBy6A5qE4MHvA8aO1pqYz Qav2piHgdBqv5mQuUjZQPA5S9QY6tzj4NWbjh7SIbeTj5il+XiNh4DfrVdr7nov7o7jO Oq9g== X-Gm-Message-State: APf1xPBZeV64b3LVJjdmhdh4WAa8nUc6KypuXTFqujo7jazh1WPm3wQf lSS0P3tU/u2X7JWh37AZHog= X-Google-Smtp-Source: AH8x225WMkJUOeUmbrQS4WqQVd14u/9luUo/5RN7MF8bzOmPJtBf8mM4ahhWqd19WEy28CAGq1TBQw== X-Received: by 10.200.3.110 with SMTP id w46mr13414661qtg.149.1518959110973; Sun, 18 Feb 2018 05:05:10 -0800 (PST) Received: from localhost (dhcp-ec-8-6b-ed-7a-cf.cpe.echoes.net. [72.28.5.223]) by smtp.gmail.com with ESMTPSA id q26sm15850154qtl.68.2018.02.18.05.05.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 18 Feb 2018 05:05:09 -0800 (PST) Date: Sun, 18 Feb 2018 05:05:06 -0800 From: Tejun Heo To: Jens Axboe Cc: linux-block@vger.kernel.org, kernel-team@fb.com Subject: [PATCH RFC] sbitmap: Use lock/unlock atomic bitops Message-ID: <20180218130506.GW695913@devbig577.frc2.facebook.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP sbitmap is used to allocate tags. The free and alloc paths use a memory ordering scheme similar to the one used by waitqueue, where the waiter and waker synchronize around set_current_state(). This doesn't seem sufficient for sbitmap given that a tag may get released and re-acquired without the allocator blocking at all. Once the bit for the tag is cleared, the tag may be reused immediately and due to the lack of memory ordering around bit clearing, its memory accesses may race against the ones from before the clearing. Given that the bits are the primary synchronization mechanism, they should be ordered memory-wise. This patch replaces waitqueue-style memory barriers with clear_bit_unlock() in sbitmap_clear_bit() and test_and_set_bit_lock() in __sbitmap_get_word(). Signed-off-by: Tejun Heo --- Hello, Spotted this while verifying the timeout fix. I didn't check the whole code, so although unlikely it's possible that the removed mb's are needed from elsewhere, so the RFC. Only boot tested. Thanks. include/linux/sbitmap.h | 3 ++- lib/sbitmap.c | 17 ++++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) --- a/include/linux/sbitmap.h +++ b/include/linux/sbitmap.h @@ -297,7 +297,8 @@ static inline void sbitmap_set_bit(struc static inline void sbitmap_clear_bit(struct sbitmap *sb, unsigned int bitnr) { - clear_bit(SB_NR_TO_BIT(sb, bitnr), __sbitmap_word(sb, bitnr)); + /* paired with test_and_set_bit_lock() in __sbitmap_get_word() */ + clear_bit_unlock(SB_NR_TO_BIT(sb, bitnr), __sbitmap_word(sb, bitnr)); } static inline int sbitmap_test_bit(struct sbitmap *sb, unsigned int bitnr) --- a/lib/sbitmap.c +++ b/lib/sbitmap.c @@ -100,7 +100,8 @@ static int __sbitmap_get_word(unsigned l return -1; } - if (!test_and_set_bit(nr, word)) + /* paired with clear_bit_unlock() in sbitmap_clear_bit() */ + if (!test_and_set_bit_lock(nr, word)) break; hint = nr + 1; @@ -432,14 +433,9 @@ static void sbq_wake_up(struct sbitmap_q int wait_cnt; /* - * Pairs with the memory barrier in set_current_state() to ensure the - * proper ordering of clear_bit()/waitqueue_active() in the waker and - * test_and_set_bit()/prepare_to_wait()/finish_wait() in the waiter. See - * the comment on waitqueue_active(). This is __after_atomic because we - * just did clear_bit() in the caller. + * Memory ordering is handled by sbitmap_clear_bit() and + * __sbitmap_get_word(). */ - smp_mb__after_atomic(); - ws = sbq_wake_ptr(sbq); if (!ws) return; @@ -481,10 +477,9 @@ void sbitmap_queue_wake_all(struct sbitm int i, wake_index; /* - * Pairs with the memory barrier in set_current_state() like in - * sbq_wake_up(). + * Memory ordering is handled by sbitmap_clear_bit() and + * __sbitmap_get_word(). */ - smp_mb(); wake_index = atomic_read(&sbq->wake_index); for (i = 0; i < SBQ_WAIT_QUEUES; i++) { struct sbq_wait_state *ws = &sbq->ws[wake_index];