From patchwork Thu Aug 2 14:50:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 10553923 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1C3F313B4 for ; Thu, 2 Aug 2018 15:36:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0A7EA287A9 for ; Thu, 2 Aug 2018 15:36:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F2C9F28825; Thu, 2 Aug 2018 15:36:03 +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=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham 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 83999287AA for ; Thu, 2 Aug 2018 15:36:03 +0000 (UTC) Received: from localhost ([::1]:46405 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1flFdy-00036u-KZ for patchwork-qemu-devel@patchwork.kernel.org; Thu, 02 Aug 2018 11:36:02 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37170) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1flEwz-0004mD-TI for qemu-devel@nongnu.org; Thu, 02 Aug 2018 10:51:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1flEww-0001LR-QE for qemu-devel@nongnu.org; Thu, 02 Aug 2018 10:51:37 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:34121) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1flEww-00010r-DK; Thu, 02 Aug 2018 10:51:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=qQITS0hiD8dmBsk+pBEK7NVVXfmRTr1kWbv19S/giGY=; b=X94TkeBiSX+0lCRN5mbNgJ4Iqv8vcxIKjnEvEkYTn+AriiPisKT8nNQj8dgq3UINvBlI9FL2iEl5tQGf2OkInDDf8D0ysIbXMt+MvKRGo+dffOfNbJyyRivXHlDWQ8XgjU6/Jm+3uDNl7ttBLh0yKX4NU8VnMow8BA2vwaDcV0qjV/KmF+m72/BkvJag2OMqbaWQg/3zXINIn7pbCxQqEwwpty5X4358OOvy7d5qyIxKDSDjdvDYH1ClGtBQM1BogiLgXbkMfa9y9ndLPKffL9M9CNPXsrCQtOBt2kKJ4VaDRQWRl4sWdXKjE40pQVAntul1ChNpkzYu7bRPDQQetg==; Received: from [194.100.51.2] (helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1flEwJ-0006Dp-IQ; Thu, 02 Aug 2018 16:50:55 +0200 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1flEw0-0007af-PM; Thu, 02 Aug 2018 17:50:36 +0300 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 2 Aug 2018 17:50:23 +0300 Message-Id: <7b219a0844e8eefdb170eaf3e670cdb65383361b.1533219143.git.berto@igalia.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH 1/4] qemu-iotests: Test removing a throttle group member with a pending timer 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: Kevin Wolf , Alberto Garcia , Stefan Hajnoczi , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP A throttle group can have several members, and each one of them can have several pending requests in the queue. The requests are processed in a round-robin fashion, so the algorithm decides the drive that is going to run the next request and sets a timer in it. Once the timer fires and the throttled request is run then the next drive from the group is selected and a new timer is set. If the user tried to remove a drive from a group and that drive had a timer set then the code was not taking care of setting up a new timer in one of the remaining members of the group, freezing their I/O. This problem was fixed in 6fccbb475bc6effc313ee9481726a1748b6dae57, and this patch adds a new test case that reproduces this exact scenario. Signed-off-by: Alberto Garcia --- tests/qemu-iotests/093 | 52 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/093.out | 4 ++-- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 index 68e344f8c1..b26cd34e32 100755 --- a/tests/qemu-iotests/093 +++ b/tests/qemu-iotests/093 @@ -208,6 +208,58 @@ class ThrottleTestCase(iotests.QMPTestCase): limits[tk] = rate self.do_test_throttle(ndrives, 5, limits) + # Test that removing a drive from a throttle group should not + # affect the remaining members of the group. + # https://bugzilla.redhat.com/show_bug.cgi?id=1535914 + def test_remove_group_member(self): + # Create a throttle group with two drives + # and set a 4 KB/s read limit. + params = {"bps": 0, + "bps_rd": 4096, + "bps_wr": 0, + "iops": 0, + "iops_rd": 0, + "iops_wr": 0 } + self.configure_throttle(2, params) + + # Read 4KB from drive0. This is performed immediately. + self.vm.hmp_qemu_io("drive0", "aio_read 0 4096") + + # Read 4KB again. The I/O limit has been exceeded so this + # request is throttled and a timer is set to wake it up. + self.vm.hmp_qemu_io("drive0", "aio_read 0 4096") + + # Read from drive1. We're still over the I/O limit so this + # request is also throttled. There's no timer set in drive1 + # because there's already one in drive0. Once the timer in + # drive0 fires and its throttled request is processed then the + # next request in the queue will be scheduled: this one. + self.vm.hmp_qemu_io("drive1", "aio_read 0 4096") + + # At this point only the first 4KB have been read from drive0. + # The other requests are throttled. + self.assertEqual(self.blockstats('drive0')[0], 4096) + self.assertEqual(self.blockstats('drive1')[0], 0) + + # Remove drive0 from the throttle group and disable its I/O limits. + # drive1 remains in the group with a throttled request. + params['bps_rd'] = 0 + params['device'] = 'drive0' + result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params) + self.assert_qmp(result, 'return', {}) + + # Removing the I/O limits from drive0 drains its pending request. + # The read request in drive1 is still throttled. + self.assertEqual(self.blockstats('drive0')[0], 8192) + self.assertEqual(self.blockstats('drive1')[0], 0) + + # Advance the clock 5 seconds. This completes the request in drive1 + self.vm.qtest("clock_step %d" % (5 * nsec_per_sec)) + + # Now all requests have been processed. + self.assertEqual(self.blockstats('drive0')[0], 8192) + self.assertEqual(self.blockstats('drive1')[0], 4096) + class ThrottleTestCoroutine(ThrottleTestCase): test_img = "null-co://" diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out index 594c16f49f..36376bed87 100644 --- a/tests/qemu-iotests/093.out +++ b/tests/qemu-iotests/093.out @@ -1,5 +1,5 @@ -........ +.......... ---------------------------------------------------------------------- -Ran 8 tests +Ran 10 tests OK From patchwork Thu Aug 2 14:50:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 10553921 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id F1FFD13B4 for ; Thu, 2 Aug 2018 15:34:31 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E329328F77 for ; Thu, 2 Aug 2018 15:34:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E15A62BBEC; Thu, 2 Aug 2018 15:34:31 +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=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham 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 89F0A28F77 for ; Thu, 2 Aug 2018 15:34:31 +0000 (UTC) Received: from localhost ([::1]:46368 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1flFcU-0001iR-Kh for patchwork-qemu-devel@patchwork.kernel.org; Thu, 02 Aug 2018 11:34:30 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37171) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1flEwz-0004mE-Tb for qemu-devel@nongnu.org; Thu, 02 Aug 2018 10:51:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1flEww-0001LD-PT for qemu-devel@nongnu.org; Thu, 02 Aug 2018 10:51:37 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:34116) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1flEww-00010n-DP; Thu, 02 Aug 2018 10:51:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=WO/i51RAZMMxOOLNyJMB5kIBv1nbCOd9T8MctpjNWGo=; b=kT1rHegxSqno79emN7rI33wK5AdircwLvwdT4OGAxoQQUCrpqh8qZpnRUvc3Iz0Osidq+sKkEjyCXttFWB2ZYbalpYMQZsyt6unLbl26bF+D0N1EgqNlDganOdLP7RbvpPnNIy9sYtxhVQU8BWdPnWK3HVw4IB/yuw38sCCNs1Lrt8G2/7sPPX2787csfR3fvp2n2v+xqI4xXU8/jYptpOaZ8t7db5r/GwDg80z9strV1h0FejsSFYyMrIetddDNrooxf4O248q8eluOdO/yTZ0c+ObUAuxNDagvZW5bFQttqXoa+Lz/RTkyGbVKjTdNy4x0+MpYUbIUbKKEz3m/GQ==; Received: from [194.100.51.2] (helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1flEwJ-0006Do-Hh; Thu, 02 Aug 2018 16:50:55 +0200 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1flEw0-0007ah-QP; Thu, 02 Aug 2018 17:50:36 +0300 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 2 Aug 2018 17:50:24 +0300 Message-Id: X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH 2/4] throttle-groups: Skip the round-robin if a member is being drained 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: Kevin Wolf , Alberto Garcia , Stefan Hajnoczi , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP In the throttling code after an I/O request has been completed the next one is selected from a different member using a round-robin algorithm. This ensures that all members get a chance to finish their pending I/O requests. However, if a group member has its I/O limits disabled (because it's being drained) then we should always give it priority in order to have all its pending requests finished as soon as possible. If we don't do this we could have a member in the process of being drained waiting for the throttled requests of other members, for which the I/O limits still apply. This can have additional consequences: if we're running in qtest mode (with QEMU_CLOCK_VIRTUAL) then timers can only fire if we advance the clock manually, so attempting to drain a block device can hang QEMU in the BDRV_POLL_WHILE() loop at the end of bdrv_do_drained_begin(). Signed-off-by: Alberto Garcia --- block/throttle-groups.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/block/throttle-groups.c b/block/throttle-groups.c index e297b04e17..d46c56b31e 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -221,6 +221,15 @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm, ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); ThrottleGroupMember *token, *start; + /* If this member has its I/O limits disabled then it means that + * it's being drained. Skip the round-robin search and return tgm + * immediately if it has pending requests. Otherwise we could be + * forcing it to wait for other member's throttled requests. */ + if (tgm_has_pending_reqs(tgm, is_write) && + atomic_read(&tgm->io_limits_disabled)) { + return tgm; + } + start = token = tg->tokens[is_write]; /* get next bs round in round robin style */ From patchwork Thu Aug 2 14:50:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 10553919 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A8E3D13B4 for ; Thu, 2 Aug 2018 15:33:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9A28B28D84 for ; Thu, 2 Aug 2018 15:33:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 987952C2B0; Thu, 2 Aug 2018 15:33:27 +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=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham 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 2667A2909B for ; Thu, 2 Aug 2018 15:33:26 +0000 (UTC) Received: from localhost ([::1]:46359 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1flFbR-0000p1-Sn for patchwork-qemu-devel@patchwork.kernel.org; Thu, 02 Aug 2018 11:33:25 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37174) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1flEwz-0004mF-U2 for qemu-devel@nongnu.org; Thu, 02 Aug 2018 10:51:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1flEww-0001LE-QD for qemu-devel@nongnu.org; Thu, 02 Aug 2018 10:51:37 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:34120) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1flEww-00010p-DR; Thu, 02 Aug 2018 10:51:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=ldNVuAHhwYlbI3aS2mV1ysmgCcnbfFTF4bHRbChhUJQ=; b=GGnMFCDPmI41xzcks+Lg3sUHLO7xF0+yl6PpNACgVa6sTrJo+5WnWMFY9Sv4ZP45yA0hv+PYbCmnYGOxfjuOnrkSzwz0+HM5dJJmInANjPWZ7uLtNH6CH2nlW5fgoOXAQu9flNxMhdniRm6+WfRQWHxJtR4Y/vYYalDRw8gPXL0uiCQsHNhlgbqto4iRYAC4SnZuQDgpB9adCPDRDqcE+3L0UQHDqEAmInZsyDqslyk8FUsf5u+9st+j9VQYLn49ZGsYS8UEVgdNXbfYMQT02f/ZfnPvGbAol45dXodeg49o22KFqNXvERL6ha+c6Mjc3p7Ug2q0Tqg/2MBr3VqbPA==; Received: from [194.100.51.2] (helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1flEwJ-0006Dk-Ea; Thu, 02 Aug 2018 16:50:55 +0200 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1flEw0-0007aj-RX; Thu, 02 Aug 2018 17:50:36 +0300 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 2 Aug 2018 17:50:25 +0300 Message-Id: <8fb28f638bd31f509d0db800c3fd8e9874ef2caa.1533219143.git.berto@igalia.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH 3/4] qemu-iotests: Update 093 to improve the draining test 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: Kevin Wolf , Alberto Garcia , Stefan Hajnoczi , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP The previous patch fixes a problem in which draining a block device with more than one throttled request can make it wait first for the completion of requests in other members of the same group. This patch updates test_remove_group_member() in iotest 093 to reproduce that scenario. This updated test would hang QEMU without the fix from the previous patch. Signed-off-by: Alberto Garcia --- tests/qemu-iotests/093 | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 index b26cd34e32..9d1971a56c 100755 --- a/tests/qemu-iotests/093 +++ b/tests/qemu-iotests/093 @@ -225,15 +225,18 @@ class ThrottleTestCase(iotests.QMPTestCase): # Read 4KB from drive0. This is performed immediately. self.vm.hmp_qemu_io("drive0", "aio_read 0 4096") - # Read 4KB again. The I/O limit has been exceeded so this + # Read 2KB. The I/O limit has been exceeded so this # request is throttled and a timer is set to wake it up. - self.vm.hmp_qemu_io("drive0", "aio_read 0 4096") + self.vm.hmp_qemu_io("drive0", "aio_read 0 2048") - # Read from drive1. We're still over the I/O limit so this - # request is also throttled. There's no timer set in drive1 - # because there's already one in drive0. Once the timer in - # drive0 fires and its throttled request is processed then the - # next request in the queue will be scheduled: this one. + # Read 2KB again. We're still over the I/O limit so this is + # request is also throttled, but no new timer is set since + # there's already one. + self.vm.hmp_qemu_io("drive0", "aio_read 0 2048") + + # Read from drive1. This request is also throttled, and no + # timer is set in drive1 because there's already one in + # drive0. self.vm.hmp_qemu_io("drive1", "aio_read 0 4096") # At this point only the first 4KB have been read from drive0. @@ -248,7 +251,7 @@ class ThrottleTestCase(iotests.QMPTestCase): result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params) self.assert_qmp(result, 'return', {}) - # Removing the I/O limits from drive0 drains its pending request. + # Removing the I/O limits from drive0 drains its two pending requests. # The read request in drive1 is still throttled. self.assertEqual(self.blockstats('drive0')[0], 8192) self.assertEqual(self.blockstats('drive1')[0], 0) From patchwork Thu Aug 2 14:50:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 10553907 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id CBD1413B4 for ; Thu, 2 Aug 2018 15:28:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BA8A52C1F0 for ; Thu, 2 Aug 2018 15:28:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AE4BF2C1FD; Thu, 2 Aug 2018 15:28:23 +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=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham 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 B343A2C1F0 for ; Thu, 2 Aug 2018 15:28:21 +0000 (UTC) Received: from localhost ([::1]:46305 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1flFWW-0004PD-Cu for patchwork-qemu-devel@patchwork.kernel.org; Thu, 02 Aug 2018 11:28:20 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37169) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1flEwz-0004mC-Sw for qemu-devel@nongnu.org; Thu, 02 Aug 2018 10:51:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1flEww-0001LW-RE for qemu-devel@nongnu.org; Thu, 02 Aug 2018 10:51:37 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:34112) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1flEww-00010o-DU; Thu, 02 Aug 2018 10:51:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=EetEUx292msgAXg81D3fQ6sOQdB6bkM/2jxcCHb4YT8=; b=XBfGkyua55i5n46Ww3Kc/s7LE2HdPEZcOmN1pPSnK4QuvhDzArkH1zL+Ul9fOl3oGXzb/Ap+g/y7he/OxljVSkSS4o7KuAxaUDyXej1AsCkCWeRb84GpOKV3YUP7kiPJGv+VAWTKTtgAqd6ZyqfUbUlMdkQDzGf0eL82nAOqu2CAJa/WO3NQeuEr1PR7WWExHB4AaLr7gPJnCKznQwOIlq3HdbXEfzfk6Pjs5mPnNOeFWYmAam/CTCACupWmW1XlPbkc9j2r+KCPwvgzrhMJvaQfWGEMbZWI/nxQqhujm6Zw7LXvWNJa9uavZT7Mdd2ai6JOh2qRtS+g8jwu6pOX5w==; Received: from [194.100.51.2] (helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1flEwJ-0006Dl-HB; Thu, 02 Aug 2018 16:50:55 +0200 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1flEw0-0007al-SV; Thu, 02 Aug 2018 17:50:36 +0300 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 2 Aug 2018 17:50:26 +0300 Message-Id: X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH 4/4] throttle-groups: Don't allow timers without throttled requests 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: Kevin Wolf , Alberto Garcia , Stefan Hajnoczi , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Commit 6fccbb475bc6effc313ee9481726a1748b6dae57 fixed a bug caused by QEMU attempting to remove a throttle group member with no pending requests but an active timer set. This was the result of a previous bdrv_drained_begin() call processing the throttled requests but leaving the timer untouched. Although the commit does solve the problem, the situation shouldn't happen in the first place. If we try to drain a throttle group member which has a timer set, we should cancel the timer instead of ignoring it. Signed-off-by: Alberto Garcia --- block/throttle-groups.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/block/throttle-groups.c b/block/throttle-groups.c index d46c56b31e..5d8213a443 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -36,6 +36,7 @@ static void throttle_group_obj_init(Object *obj); static void throttle_group_obj_complete(UserCreatable *obj, Error **errp); +static void timer_cb(ThrottleGroupMember *tgm, bool is_write); /* The ThrottleGroup structure (with its ThrottleState) is shared * among different ThrottleGroupMembers and it's independent from @@ -424,15 +425,31 @@ static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write rd->tgm = tgm; rd->is_write = is_write; + /* This function is called when a timer is fired or when + * throttle_group_restart_tgm() is called. Either way, there can + * be no timer pending on this tgm at this point */ + assert(!timer_pending(tgm->throttle_timers.timers[is_write])); + co = qemu_coroutine_create(throttle_group_restart_queue_entry, rd); aio_co_enter(tgm->aio_context, co); } void throttle_group_restart_tgm(ThrottleGroupMember *tgm) { + int i; + if (tgm->throttle_state) { - throttle_group_restart_queue(tgm, 0); - throttle_group_restart_queue(tgm, 1); + for (i = 0; i < 2; i++) { + QEMUTimer *t = tgm->throttle_timers.timers[i]; + if (timer_pending(t)) { + /* If there's a pending timer on this tgm, fire it now */ + timer_del(t); + timer_cb(tgm, i); + } else { + /* Else run the next request from the queue manually */ + throttle_group_restart_queue(tgm, i); + } + } } } @@ -567,16 +584,11 @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm) return; } - assert(tgm->pending_reqs[0] == 0 && tgm->pending_reqs[1] == 0); - assert(qemu_co_queue_empty(&tgm->throttled_reqs[0])); - assert(qemu_co_queue_empty(&tgm->throttled_reqs[1])); - qemu_mutex_lock(&tg->lock); for (i = 0; i < 2; i++) { - if (timer_pending(tgm->throttle_timers.timers[i])) { - tg->any_timer_armed[i] = false; - schedule_next_request(tgm, i); - } + assert(tgm->pending_reqs[i] == 0); + assert(qemu_co_queue_empty(&tgm->throttled_reqs[i])); + assert(!timer_pending(tgm->throttle_timers.timers[i])); if (tg->tokens[i] == tgm) { token = throttle_group_next_tgm(tgm); /* Take care of the case where this is the last tgm in the group */