From patchwork Thu Aug 2 14:50:22 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 10553917 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 DF9D613B4 for ; Thu, 2 Aug 2018 15:31:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CF7832C228 for ; Thu, 2 Aug 2018 15:31:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CDD822C25B; Thu, 2 Aug 2018 15:31: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=-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 69C8A2C26D for ; Thu, 2 Aug 2018 15:31:40 +0000 (UTC) Received: from localhost ([::1]:46336 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1flFZj-0007nt-Ir for patchwork-qemu-devel@patchwork.kernel.org; Thu, 02 Aug 2018 11:31:39 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37168) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1flEwz-0004mB-T2 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-0001LJ-Pa for qemu-devel@nongnu.org; Thu, 02 Aug 2018 10:51:37 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:34119) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1flEww-00010q-DG; 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=Message-Id:Date:Subject:Cc:To:From; bh=wHoFC/f5r4rHjzLmQojQm8k+dBDmFomJHQ69mkXBvCg=; b=T9OSdBHiqSkmdbrjQHp86zf/VRrv1IUNLoQCmKxdPUDq02vASeIsx/li2B4Lm2jAU7qxlN5aMJkxwUHgKas1bkDiCv4UktKrigcdxTDLV/mOrqI337CoLN38qGmYH0tqO2S2sdYJXp/w2Zn6qgIkCZCNcq5HNAdVBTBCWvLT3IKWpJ46s1cuSk/1DlNRM/obj/vs5KEq84LKOyI8GBK16ovhAGawOe46o1TJv536TyrCDa5Aj4qXYsL7xT4jtn466EaDBiNvb3sBH9/0tZlRpnxhnrhj8ADlu0s5JgaZBNPWfag1BMAf9rgt5w+VpCHXpewkoj3geFsj9j3OnX2pBA==; 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-0006Dm-Ht; Thu, 02 Aug 2018 16:50:55 +0200 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1flEw0-0007ad-O5; Thu, 02 Aug 2018 17:50:36 +0300 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 2 Aug 2018 17:50:22 +0300 Message-Id: X-Mailer: git-send-email 2.11.0 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 0/4] throttle: Race condition fixes and test cases 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 Hi all, here are the patches that I promised yesterday. I was originally thinking to propose this for the v3.0 release, but after debugging and fixing the problem I think that it's not essential (details below). The important patch is the second one. The first and the third are just test cases and the last is an alternative solution for the bug that Stefan fixed in 6fccbb475bc6effc313ee9481726a1748b6dae57. There are details in the patches themselves, but here's an explanation of the problem: consider a scenario with two drives A and B that are part of the same throttle group. Both of them have throttled requests and they're waiting for a timer that is set on drive A. (timer here) --> [A] --- req1, req2 [B] --- req3 If we drain drive [A] (e.g. by disabling its I/O limits) then its queue is restarted. req1 is processed immediately, and before finishing it calls schedule_next_request(). This follows the round-robin algorithm, selects req3 and puts a timer in [B]. But we're still not done with draining [A], and now we have a BDRV_POLL_WHILE() loop at the end of bdrv_do_drained_begin() waiting for req2 to finish. That won't happen until the timer in [B] fires and req3 is done. If there are more drives in the group and more requests in the queue this can take a while. That's why disabling a drive's I/O limits can be noticeably slow: we disabled the I/O limits but they're still being enforced in practice. The QEMU I/O tests run in qtest mode (with QEMU_CLOCK_VIRTUAL). The clock must be advanced manually, which means that the scenario that I just described hangs QEMU because BDRV_POLL_WHILE() loops forever (you can reproduce this with patch 3). In a real world scenario this only results in the aforementioned slowdown (probably negligible in practice), which is not a critical thing, and that's why I think it's safe to keep the current code for QEMU 3. I think that's all. Questions and commend are welcome. Berto Alberto Garcia (4): qemu-iotests: Test removing a throttle group member with a pending timer throttle-groups: Skip the round-robin if a member is being drained qemu-iotests: Update 093 to improve the draining test throttle-groups: Don't allow timers without throttled requests block/throttle-groups.c | 41 +++++++++++++++++++++++++--------- tests/qemu-iotests/093 | 55 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/093.out | 4 ++-- 3 files changed, 88 insertions(+), 12 deletions(-)