From patchwork Mon Sep 3 04:31:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Xu X-Patchwork-Id: 965196 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 423cXQ3mdXz9s0n for ; Mon, 3 Sep 2018 14:33:10 +1000 (AEST) Received: from localhost ([::1]:43310 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fwgY0-00075J-2u for incoming@patchwork.ozlabs.org; Mon, 03 Sep 2018 00:33:08 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53695) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fwgXJ-0006yZ-C2 for qemu-devel@nongnu.org; Mon, 03 Sep 2018 00:32:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fwgXI-0000HU-Gm for qemu-devel@nongnu.org; Mon, 03 Sep 2018 00:32:25 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:57436 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fwgXI-0000HA-Bg for qemu-devel@nongnu.org; Mon, 03 Sep 2018 00:32:24 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 65BF087A6D for ; Mon, 3 Sep 2018 04:32:23 +0000 (UTC) Received: from xz-x1.redhat.com (ovpn-12-52.pek2.redhat.com [10.72.12.52]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7402A2026D6B; Mon, 3 Sep 2018 04:32:14 +0000 (UTC) From: Peter Xu To: qemu-devel@nongnu.org Date: Mon, 3 Sep 2018 12:31:45 +0800 Message-Id: <20180903043149.4076-4-peterx@redhat.com> In-Reply-To: <20180903043149.4076-1-peterx@redhat.com> References: <20180903043149.4076-1-peterx@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 03 Sep 2018 04:32:23 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 03 Sep 2018 04:32:23 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'peterx@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v7 3/7] monitor: suspend monitor instead of send CMD_DROP 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: "Dr . David Alan Gilbert" , peterx@redhat.com, Markus Armbruster , =?utf-8?q?Marc-Andr=C3=A9_Lure?= =?utf-8?q?au?= Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" When we received too many qmp commands, previously we'll send COMMAND_DROPPED events to monitors, then we'll drop the requests. Now instead of dropping the command we stop reading from input when we notice the queue is getting full. Note that here since we removed the need_resume flag we need to be _very_ careful on the suspend/resume paring on the conditions since unmatched condition checks will hang death the monitor. Meanwhile, now we will need to read the queue length to decide whether we'll need to resume the monitor so we need to take the queue lock again even after popping from it. Signed-off-by: Peter Xu --- monitor.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/monitor.c b/monitor.c index 3b90c9eb5f..9e6cad2af6 100644 --- a/monitor.c +++ b/monitor.c @@ -89,6 +89,8 @@ #include "hw/s390x/storage-attributes.h" #endif +#define QMP_REQ_QUEUE_LEN_MAX (8) + /* * Supported types: * @@ -4124,29 +4126,33 @@ static QMPRequest *monitor_qmp_requests_pop_any(void) static void monitor_qmp_bh_dispatcher(void *data) { QMPRequest *req_obj = monitor_qmp_requests_pop_any(); + Monitor *mon; QDict *rsp; bool need_resume; if (!req_obj) { return; } - + mon = req_obj->mon; /* qmp_oob_enabled() might change after "qmp_capabilities" */ - need_resume = !qmp_oob_enabled(req_obj->mon); + qemu_mutex_lock(&mon->qmp.qmp_queue_lock); + need_resume = !qmp_oob_enabled(req_obj->mon) || + mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1; + qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); if (req_obj->req) { trace_monitor_qmp_cmd_in_band(qobject_get_try_str(req_obj->id) ?: ""); - monitor_qmp_dispatch(req_obj->mon, req_obj->req, req_obj->id); + monitor_qmp_dispatch(mon, req_obj->req, req_obj->id); } else { assert(req_obj->err); rsp = qmp_error_response(req_obj->err); req_obj->err = NULL; - monitor_qmp_respond(req_obj->mon, rsp, NULL); + monitor_qmp_respond(mon, rsp, NULL); qobject_unref(rsp); } if (need_resume) { /* Pairs with the monitor_suspend() in handle_qmp_command() */ - monitor_resume(req_obj->mon); + monitor_resume(mon); } qmp_request_free(req_obj); @@ -4154,8 +4160,6 @@ static void monitor_qmp_bh_dispatcher(void *data) qemu_bh_schedule(qmp_dispatcher_bh); } -#define QMP_REQ_QUEUE_LEN_MAX (8) - static void handle_qmp_command(void *opaque, QObject *req, Error *err) { Monitor *mon = opaque; @@ -4205,19 +4209,12 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) if (!qmp_oob_enabled(mon)) { monitor_suspend(mon); } else { - /* Drop the request if queue is full. */ - if (mon->qmp.qmp_requests->length >= QMP_REQ_QUEUE_LEN_MAX) { - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); + if (mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) { /* - * FIXME @id's scope is just @mon, and broadcasting it is - * wrong. If another monitor's client has a command with - * the same ID in flight, the event will incorrectly claim - * that command was dropped. + * If this is _exactly_ the last request that we can + * queue, we suspend the monitor right now. */ - qapi_event_send_command_dropped(id, - COMMAND_DROP_REASON_QUEUE_FULL); - qmp_request_free(req_obj); - return; + monitor_suspend(mon); } }