From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: virtio-dev-return-2884-cohuck=redhat.com@lists.oasis-open.org Sender: List-Post: List-Help: List-Unsubscribe: List-Subscribe: Received: from lists.oasis-open.org (oasis-open.org [66.179.20.138]) by lists.oasis-open.org (Postfix) with ESMTP id 84C2E5818F5D for ; Sat, 23 Dec 2017 23:29:28 -0800 (PST) Message-ID: <5A3F57D0.9050007@intel.com> Date: Sun, 24 Dec 2017 15:31:28 +0800 From: Wei Wang MIME-Version: 1.0 References: <1513823406-43632-1-git-send-email-wei.w.wang@intel.com> <20171221210327.GB25009@bombadil.infradead.org> <201712231159.ECI73411.tFFFJOHOVMOLQS@I-love.SAKURA.ne.jp> <20171223032959.GA11578@bombadil.infradead.org> <201712232333.BAH82874.FFFtOMHSLVQOOJ@I-love.SAKURA.ne.jp> In-Reply-To: <201712232333.BAH82874.FFFtOMHSLVQOOJ@I-love.SAKURA.ne.jp> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: [virtio-dev] Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations To: Tetsuo Handa , willy@infradead.org Cc: 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 List-ID: On 12/23/2017 10:33 PM, Tetsuo Handa wrote: >>>> + bitmap = rcu_dereference_raw(*slot); >>>> + if (!bitmap) { >>>> + bitmap = this_cpu_xchg(ida_bitmap, NULL); >>>> + if (!bitmap) >>>> + return -ENOMEM; >>> I can't understand this. I can understand if it were >>> >>> BUG_ON(!bitmap); >>> >>> because you called xb_preload(). >>> >>> But >>> >>> /* >>> * Regular test 2 >>> * set bit 2000, 2001, 2040 >>> * Next 1 in [0, 2048) --> 2000 >>> * Next 1 in [2000, 2002) --> 2000 >>> * Next 1 in [2002, 2041) --> 2040 >>> * Next 1 in [2002, 2040) --> none >>> * Next 0 in [2000, 2048) --> 2002 >>> * Next 0 in [2048, 2060) --> 2048 >>> */ >>> xb_preload(GFP_KERNEL); >>> assert(!xb_set_bit(&xb1, 2000)); >>> assert(!xb_set_bit(&xb1, 2001)); >>> assert(!xb_set_bit(&xb1, 2040)); >> [...] >>> xb_preload_end(); >>> >>> you are not calling xb_preload() prior to each xb_set_bit() call. >>> This means that, if each xb_set_bit() is not surrounded with >>> xb_preload()/xb_preload_end(), there is possibility of hitting >>> this_cpu_xchg(ida_bitmap, NULL) == NULL. >> This is just a lazy test. We "know" that the bits in the range 1024-2047 >> will all land in the same bitmap, so there's no need to preload for each >> of them. > Testcases also serves as how to use that API. > Assuming such thing leads to incorrect usage. If callers are aware that the bits that they going to record locate in the same bitmap, I think they should also perform the xb_ APIs with only one preload. So the test cases here have shown them a correct example. We can probably add some comments above to explain this. Best, Wei --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Wang Subject: Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations Date: Sun, 24 Dec 2017 15:31:28 +0800 Message-ID: <5A3F57D0.9050007@intel.com> References: <1513823406-43632-1-git-send-email-wei.w.wang@intel.com> <20171221210327.GB25009@bombadil.infradead.org> <201712231159.ECI73411.tFFFJOHOVMOLQS@I-love.SAKURA.ne.jp> <20171223032959.GA11578@bombadil.infradead.org> <201712232333.BAH82874.FFFtOMHSLVQOOJ@I-love.SAKURA.ne.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: 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 To: Tetsuo Handa , willy@infradead.org Return-path: In-Reply-To: <201712232333.BAH82874.FFFtOMHSLVQOOJ@I-love.SAKURA.ne.jp> Sender: owner-linux-mm@kvack.org List-Id: kvm.vger.kernel.org On 12/23/2017 10:33 PM, Tetsuo Handa wrote: >>>> + bitmap = rcu_dereference_raw(*slot); >>>> + if (!bitmap) { >>>> + bitmap = this_cpu_xchg(ida_bitmap, NULL); >>>> + if (!bitmap) >>>> + return -ENOMEM; >>> I can't understand this. I can understand if it were >>> >>> BUG_ON(!bitmap); >>> >>> because you called xb_preload(). >>> >>> But >>> >>> /* >>> * Regular test 2 >>> * set bit 2000, 2001, 2040 >>> * Next 1 in [0, 2048) --> 2000 >>> * Next 1 in [2000, 2002) --> 2000 >>> * Next 1 in [2002, 2041) --> 2040 >>> * Next 1 in [2002, 2040) --> none >>> * Next 0 in [2000, 2048) --> 2002 >>> * Next 0 in [2048, 2060) --> 2048 >>> */ >>> xb_preload(GFP_KERNEL); >>> assert(!xb_set_bit(&xb1, 2000)); >>> assert(!xb_set_bit(&xb1, 2001)); >>> assert(!xb_set_bit(&xb1, 2040)); >> [...] >>> xb_preload_end(); >>> >>> you are not calling xb_preload() prior to each xb_set_bit() call. >>> This means that, if each xb_set_bit() is not surrounded with >>> xb_preload()/xb_preload_end(), there is possibility of hitting >>> this_cpu_xchg(ida_bitmap, NULL) == NULL. >> This is just a lazy test. We "know" that the bits in the range 1024-2047 >> will all land in the same bitmap, so there's no need to preload for each >> of them. > Testcases also serves as how to use that API. > Assuming such thing leads to incorrect usage. If callers are aware that the bits that they going to record locate in the same bitmap, I think they should also perform the xb_ APIs with only one preload. So the test cases here have shown them a correct example. We can probably add some comments above to explain this. Best, Wei -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752034AbdLXH3V (ORCPT ); Sun, 24 Dec 2017 02:29:21 -0500 Received: from mga07.intel.com ([134.134.136.100]:51568 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751023AbdLXH3T (ORCPT ); Sun, 24 Dec 2017 02:29:19 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,449,1508828400"; d="scan'208";a="4914977" Message-ID: <5A3F57D0.9050007@intel.com> Date: Sun, 24 Dec 2017 15:31:28 +0800 From: Wei Wang User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Tetsuo Handa , willy@infradead.org CC: 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 Subject: Re: [PATCH v20 3/7 RESEND] xbitmap: add more operations References: <1513823406-43632-1-git-send-email-wei.w.wang@intel.com> <20171221210327.GB25009@bombadil.infradead.org> <201712231159.ECI73411.tFFFJOHOVMOLQS@I-love.SAKURA.ne.jp> <20171223032959.GA11578@bombadil.infradead.org> <201712232333.BAH82874.FFFtOMHSLVQOOJ@I-love.SAKURA.ne.jp> In-Reply-To: <201712232333.BAH82874.FFFtOMHSLVQOOJ@I-love.SAKURA.ne.jp> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/23/2017 10:33 PM, Tetsuo Handa wrote: >>>> + bitmap = rcu_dereference_raw(*slot); >>>> + if (!bitmap) { >>>> + bitmap = this_cpu_xchg(ida_bitmap, NULL); >>>> + if (!bitmap) >>>> + return -ENOMEM; >>> I can't understand this. I can understand if it were >>> >>> BUG_ON(!bitmap); >>> >>> because you called xb_preload(). >>> >>> But >>> >>> /* >>> * Regular test 2 >>> * set bit 2000, 2001, 2040 >>> * Next 1 in [0, 2048) --> 2000 >>> * Next 1 in [2000, 2002) --> 2000 >>> * Next 1 in [2002, 2041) --> 2040 >>> * Next 1 in [2002, 2040) --> none >>> * Next 0 in [2000, 2048) --> 2002 >>> * Next 0 in [2048, 2060) --> 2048 >>> */ >>> xb_preload(GFP_KERNEL); >>> assert(!xb_set_bit(&xb1, 2000)); >>> assert(!xb_set_bit(&xb1, 2001)); >>> assert(!xb_set_bit(&xb1, 2040)); >> [...] >>> xb_preload_end(); >>> >>> you are not calling xb_preload() prior to each xb_set_bit() call. >>> This means that, if each xb_set_bit() is not surrounded with >>> xb_preload()/xb_preload_end(), there is possibility of hitting >>> this_cpu_xchg(ida_bitmap, NULL) == NULL. >> This is just a lazy test. We "know" that the bits in the range 1024-2047 >> will all land in the same bitmap, so there's no need to preload for each >> of them. > Testcases also serves as how to use that API. > Assuming such thing leads to incorrect usage. If callers are aware that the bits that they going to record locate in the same bitmap, I think they should also perform the xb_ APIs with only one preload. So the test cases here have shown them a correct example. We can probably add some comments above to explain this. Best, Wei