From patchwork Tue Dec 12 11:55:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wei Wang X-Patchwork-Id: 10106839 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 4C71E602B3 for ; Tue, 12 Dec 2017 12:16:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4565629B15 for ; Tue, 12 Dec 2017 12:16:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3A2A529B8E; Tue, 12 Dec 2017 12:16:22 +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.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A9C3529B7D for ; Tue, 12 Dec 2017 12:16:21 +0000 (UTC) Received: from localhost ([::1]:57884 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eOjTw-0001wJ-Po for patchwork-qemu-devel@patchwork.kernel.org; Tue, 12 Dec 2017 07:16:20 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56522) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eOjQW-0006uk-J1 for qemu-devel@nongnu.org; Tue, 12 Dec 2017 07:12:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eOjQV-0006Sy-5V for qemu-devel@nongnu.org; Tue, 12 Dec 2017 07:12:48 -0500 Received: from mga17.intel.com ([192.55.52.151]:39928) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eOjQU-0006Pn-QZ for qemu-devel@nongnu.org; Tue, 12 Dec 2017 07:12:47 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Dec 2017 04:12:46 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,395,1508828400"; d="scan'208";a="1896099" Received: from devel-ww.sh.intel.com ([10.239.48.110]) by fmsmga008.fm.intel.com with ESMTP; 12 Dec 2017 04:12:43 -0800 From: Wei Wang To: virtio-dev@lists.oasis-open.org, linux-kernel@vger.kernel.org, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org, kvm@vger.kernel.org, linux-mm@kvack.org, mst@redhat.com, mhocko@kernel.org, akpm@linux-foundation.org, mawilcox@microsoft.com Date: Tue, 12 Dec 2017 19:55:54 +0800 Message-Id: <1513079759-14169-3-git-send-email-wei.w.wang@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1513079759-14169-1-git-send-email-wei.w.wang@intel.com> References: <1513079759-14169-1-git-send-email-wei.w.wang@intel.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 192.55.52.151 Subject: [Qemu-devel] [PATCH v19 2/7] xbitmap: potential improvement X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: aarcange@redhat.com, yang.zhang.wz@gmail.com, david@redhat.com, penguin-kernel@I-love.SAKURA.ne.jp, liliang.opensource@gmail.com, willy@infradead.org, amit.shah@redhat.com, wei.w.wang@intel.com, quan.xu@aliyun.com, cornelia.huck@de.ibm.com, pbonzini@redhat.com, nilal@redhat.com, mgorman@techsingularity.net Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP This patch made some changes to the original xbitmap implementation from the linux-dax tree: - remove xb_fill() and xb_zero() from xbitmap.h since they are not implemented; - xb_test_bit: changed "ebit > BITS_PER_LONG" to "ebit >= BITS_PER_LONG", because bit 64 beyonds the "unsigned long" exceptional entry (0 to 63); - xb_set_bit: delete the new inserted radix_tree_node when failing to get the per cpu ida bitmap, this avoids the kind of memory leak of the unused radix tree node left in the tree. - xb_clear_bit: change it to be a void function, since the original implementation reurns nothing than a 0. - remove the comment above "#define XB_INDEX_BITS", because it causes confusion based on the feedbacks from the previous discussion; - xb_preload: with the original implementation, the CPU that successfully do __radix_tree_preload() may get into sleep by kmalloc(), which has a risk of getting the caller of xb_preload() scheduled to another CPU after waken up, and the new CPU may not have radix_tree_node pre-allocated there, this will be a problem when inserting a node to the tree later. This patch moves __radix_tree_preload() after kmalloc() and returns a boolean to indicate the success or failure. Also, add the __must_check annotation to xb_preload for prudence purpose. Signed-off-by: Wei Wang Cc: Matthew Wilcox Cc: Andrew Morton Cc: Michal Hocko Cc: Michael S. Tsirkin Cc: Tetsuo Handa --- include/linux/xbitmap.h | 5 +---- lib/radix-tree.c | 27 +++++++++++++++++++++------ lib/xbitmap.c | 24 +++++++++++++----------- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/include/linux/xbitmap.h b/include/linux/xbitmap.h index ed75d87..b4d8375 100644 --- a/include/linux/xbitmap.h +++ b/include/linux/xbitmap.h @@ -36,15 +36,12 @@ int xb_set_bit(struct xb *xb, unsigned long bit); bool xb_test_bit(const struct xb *xb, unsigned long bit); int xb_clear_bit(struct xb *xb, unsigned long bit); -int xb_zero(struct xb *xb, unsigned long start, unsigned long nbits); -int xb_fill(struct xb *xb, unsigned long start, unsigned long nbits); - static inline bool xb_empty(const struct xb *xb) { return radix_tree_empty(&xb->xbrt); } -void xb_preload(gfp_t); +bool xb_preload(gfp_t); static inline void xb_preload_end(void) { diff --git a/lib/radix-tree.c b/lib/radix-tree.c index 7000ad6..a039588 100644 --- a/lib/radix-tree.c +++ b/lib/radix-tree.c @@ -77,9 +77,6 @@ static struct kmem_cache *radix_tree_node_cachep; RADIX_TREE_MAP_SHIFT)) #define IDA_PRELOAD_SIZE (IDA_MAX_PATH * 2 - 1) -/* - * The XB can go up to unsigned long, but also uses a bitmap. - */ #define XB_INDEX_BITS (BITS_PER_LONG - ilog2(IDA_BITMAP_BITS)) #define XB_MAX_PATH (DIV_ROUND_UP(XB_INDEX_BITS, \ RADIX_TREE_MAP_SHIFT)) @@ -2145,17 +2142,35 @@ int ida_pre_get(struct ida *ida, gfp_t gfp) } EXPORT_SYMBOL(ida_pre_get); -void xb_preload(gfp_t gfp) +/** + * xb_preload - preload for xb_set_bit() + * @gfp_mask: allocation mask to use for preloading + * + * Preallocate memory to use for the next call to xb_set_bit(). On success, + * return true, with preemption disabled. On error, return false with + * preemption not disabled. + */ +__must_check bool xb_preload(gfp_t gfp) { - __radix_tree_preload(gfp, XB_PRELOAD_SIZE); if (!this_cpu_read(ida_bitmap)) { struct ida_bitmap *bitmap = kmalloc(sizeof(*bitmap), gfp); if (!bitmap) - return; + return false; + /* + * The per-CPU variable is updated with preemption enabled. + * If the calling task is unlucky to be scheduled to another + * CPU which has no ida_bitmap allocation, it will be detected + * when setting a bit (i.e. __xb_set_bit()). + */ bitmap = this_cpu_cmpxchg(ida_bitmap, NULL, bitmap); kfree(bitmap); } + + if (__radix_tree_preload(gfp, XB_PRELOAD_SIZE) < 0) + return false; + + return true; } EXPORT_SYMBOL(xb_preload); diff --git a/lib/xbitmap.c b/lib/xbitmap.c index 2b547a73..182aa29 100644 --- a/lib/xbitmap.c +++ b/lib/xbitmap.c @@ -39,8 +39,10 @@ int xb_set_bit(struct xb *xb, unsigned long bit) return 0; } bitmap = this_cpu_xchg(ida_bitmap, NULL); - if (!bitmap) + if (!bitmap) { + __radix_tree_delete(root, node, slot); return -EAGAIN; + } memset(bitmap, 0, sizeof(*bitmap)); bitmap->bitmap[0] = tmp >> RADIX_TREE_EXCEPTIONAL_SHIFT; rcu_assign_pointer(*slot, bitmap); @@ -54,8 +56,10 @@ int xb_set_bit(struct xb *xb, unsigned long bit) return 0; } bitmap = this_cpu_xchg(ida_bitmap, NULL); - if (!bitmap) + if (!bitmap) { + __radix_tree_delete(root, node, slot); return -EAGAIN; + } memset(bitmap, 0, sizeof(*bitmap)); __radix_tree_replace(root, node, slot, bitmap, NULL); } @@ -73,7 +77,7 @@ EXPORT_SYMBOL(xb_set_bit); * This function is used to clear a bit in the xbitmap. If all the bits of the * bitmap are 0, the bitmap will be freed. */ -int xb_clear_bit(struct xb *xb, unsigned long bit) +void xb_clear_bit(struct xb *xb, unsigned long bit) { unsigned long index = bit / IDA_BITMAP_BITS; struct radix_tree_root *root = &xb->xbrt; @@ -90,25 +94,23 @@ int xb_clear_bit(struct xb *xb, unsigned long bit) unsigned long tmp = (unsigned long)bitmap; if (ebit >= BITS_PER_LONG) - return 0; + return; tmp &= ~(1UL << ebit); if (tmp == RADIX_TREE_EXCEPTIONAL_ENTRY) __radix_tree_delete(root, node, slot); else rcu_assign_pointer(*slot, (void *)tmp); - return 0; + return; } if (!bitmap) - return 0; + return; __clear_bit(bit, bitmap->bitmap); if (bitmap_empty(bitmap->bitmap, IDA_BITMAP_BITS)) { kfree(bitmap); __radix_tree_delete(root, node, slot); } - - return 0; } EXPORT_SYMBOL(xb_clear_bit); @@ -133,7 +135,7 @@ bool xb_test_bit(const struct xb *xb, unsigned long bit) return false; if (radix_tree_exception(bitmap)) { bit += RADIX_TREE_EXCEPTIONAL_SHIFT; - if (bit > BITS_PER_LONG) + if (bit >= BITS_PER_LONG) return false; return (unsigned long)bitmap & (1UL << bit); } @@ -151,9 +153,9 @@ void xbitmap_check_bit(unsigned long bit) assert(!xb_test_bit(&xb1, bit)); assert(xb_set_bit(&xb1, bit) == 0); assert(xb_test_bit(&xb1, bit)); - assert(xb_clear_bit(&xb1, bit) == 0); + xb_clear_bit(&xb1, bit); assert(xb_empty(&xb1)); - assert(xb_clear_bit(&xb1, bit) == 0); + xb_clear_bit(&xb1, bit); assert(xb_empty(&xb1)); xb_preload_end(); }