From nobody Wed Apr 24 23:32:25 2024 Delivered-To: importer@patchew.org Received-SPF: none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org ARC-Seal: i=1; a=rsa-sha256; t=1557311382; cv=none; d=zoho.com; s=zohoarc; b=d/hjzZQS4cypNO+pJ0MV+HqN8DLsawaQQSCwDkxAtSmauhKpJZtD651eo7wgl/NTlnlGFhABWiZHArmFC0s/s5c26JByANsoO2xFUoJbccmeDbvBBb3vGZ3pKJ4lpJAWjANlTEb6Zz1X4Barxfj3sHhTjUBB+vWOvfpc8wbSaW8= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1557311382; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To:ARC-Authentication-Results; bh=ohU/rO0WWzbLin/DG8Dowpo+TPXbjdOCENZhFjOQLqE=; b=SxnMoNj10n7fBm9uJ6K9iX7q4XuKVoISZcyuqYuQXo+h0pP7pesXyQ8Dql7SmX8wUruhiDFrfmI2h5XBaYrN/b75ow+cL5o6KA29DS4T+hTmLYD78FQN3vciSK2UfWsKBx243f5uUiLPY8Vig8EhasUtbHq5jxscvpI+gsFtOPk= ARC-Authentication-Results: i=1; mx.zoho.com; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 155731138200388.56884419661912; Wed, 8 May 2019 03:29:42 -0700 (PDT) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hOJoG-0000Xx-6S; Wed, 08 May 2019 10:28:24 +0000 Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hOJoF-0000Xs-Je for xen-devel@lists.xenproject.org; Wed, 08 May 2019 10:28:23 +0000 Received: from SMTP03.CITRIX.COM (unknown [162.221.156.55]) by us1-rack-dfw2.inumbo.com (Halon) with ESMTPS id 00a5f9b8-717c-11e9-843c-bc764e045a96; Wed, 08 May 2019 10:28:21 +0000 (UTC) X-Inumbo-ID: 00a5f9b8-717c-11e9-843c-bc764e045a96 X-IronPort-AV: E=Sophos;i="5.60,445,1549929600"; d="scan'208";a="85259235" From: Ross Lagerwall To: Boris Ostrovsky , Juergen Gross Date: Wed, 8 May 2019 11:28:07 +0100 Message-ID: <20190508102807.7096-1-ross.lagerwall@citrix.com> X-Mailer: git-send-email 2.17.2 MIME-Version: 1.0 Subject: [Xen-devel] [PATCH] xenbus: Avoid deadlock during suspend due to open transactions X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: xen-devel@lists.xenproject.org, Ross Lagerwall Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" During a suspend/resume, the xenwatch thread waits for all outstanding xenstore requests and transactions to complete. This does not work correctly for transactions started by userspace because it waits for them to complete after freezing userspace threads which means the transactions has no way of completing, resulting in a deadlock. This is trivial to reproduce by running this script and then suspending the VM: import pyxs, time c =3D pyxs.client.Client(xen_bus_path=3D"/dev/xen/xenbus") c.connect() c.transaction() time.sleep(3600) Even if this deadlock were resolved, misbehaving userspace should not prevent a VM from being migrated. So, instead of waiting for these transactions to complete, ignore them during suspend and mark them as aborted during the return path. If the caller commits the transaction, return EAGAIN so that they try again. If the caller discards the transaction, return OK since no changes were made anyway. This only affects users of the xenbus file interface. In-kernel users of xenbus are assumed to be well-behaved and complete all transactions before freezing. Signed-off-by: Ross Lagerwall --- drivers/xen/xenbus/xenbus.h | 2 + drivers/xen/xenbus/xenbus_dev_frontend.c | 60 ++++++++++++++++++++++++ drivers/xen/xenbus/xenbus_xs.c | 16 ++++++- 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/drivers/xen/xenbus/xenbus.h b/drivers/xen/xenbus/xenbus.h index 092981171df1..a977e1396149 100644 --- a/drivers/xen/xenbus/xenbus.h +++ b/drivers/xen/xenbus/xenbus.h @@ -133,4 +133,6 @@ void xenbus_ring_ops_init(void); int xenbus_dev_request_and_reply(struct xsd_sockmsg *msg, void *par); void xenbus_dev_queue_reply(struct xb_req_data *req); =20 +unsigned int xenbus_file_abort_trans(bool abort); + #endif diff --git a/drivers/xen/xenbus/xenbus_dev_frontend.c b/drivers/xen/xenbus/= xenbus_dev_frontend.c index 0782ff3c2273..623218a2a165 100644 --- a/drivers/xen/xenbus/xenbus_dev_frontend.c +++ b/drivers/xen/xenbus/xenbus_dev_frontend.c @@ -69,6 +69,7 @@ struct xenbus_transaction_holder { struct list_head list; struct xenbus_transaction handle; + bool aborted; }; =20 /* @@ -113,8 +114,49 @@ struct xenbus_file_priv { wait_queue_head_t read_waitq; =20 struct kref kref; + + struct list_head file_list; }; =20 +static DEFINE_SPINLOCK(file_list_lock); +static LIST_HEAD(file_list); + +static void register_xenbus_file(struct xenbus_file_priv *u) +{ + spin_lock(&file_list_lock); + list_add(&u->file_list, &file_list); + spin_unlock(&file_list_lock); +} + +static void unregister_xenbus_file(struct xenbus_file_priv *u) +{ + spin_lock(&file_list_lock); + list_del(&u->file_list); + spin_unlock(&file_list_lock); +} + +unsigned int xenbus_file_abort_trans(bool abort) +{ + struct xenbus_file_priv *u; + struct xenbus_transaction_holder *trans; + unsigned int count =3D 0; + + spin_lock(&file_list_lock); + list_for_each_entry(u, &file_list, file_list) { + mutex_lock(&u->msgbuffer_mutex); + list_for_each_entry(trans, &u->transactions, list) { + if (!trans->aborted) { + count++; + trans->aborted =3D abort; + } + } + mutex_unlock(&u->msgbuffer_mutex); + } + spin_unlock(&file_list_lock); + + return count; +} + /* Read out any raw xenbus messages queued up. */ static ssize_t xenbus_file_read(struct file *filp, char __user *ubuf, @@ -306,6 +348,8 @@ static void xenbus_file_free(struct kref *kref) =20 u =3D container_of(kref, struct xenbus_file_priv, kref); =20 + unregister_xenbus_file(u); + /* * No need for locking here because there are no other users, * by definition. @@ -449,6 +493,20 @@ static int xenbus_write_transaction(unsigned msg_type, !(msg->hdr.len =3D=3D 2 && (!strcmp(msg->body, "T") || !strcmp(msg->body, "F")))) return xenbus_command_reply(u, XS_ERROR, "EINVAL"); + else if (msg_type =3D=3D XS_TRANSACTION_END) { + trans =3D xenbus_get_transaction(u, msg->hdr.tx_id); + if (trans && trans->aborted) { + list_del(&trans->list); + kfree(trans); + if (!strcmp(msg->body, "T")) + return xenbus_command_reply(u, XS_ERROR, + "EAGAIN"); + else + return xenbus_command_reply(u, + XS_TRANSACTION_END, + "OK"); + } + } =20 rc =3D xenbus_dev_request_and_reply(&msg->hdr, u); if (rc && trans) { @@ -640,6 +698,8 @@ static int xenbus_file_open(struct inode *inode, struct= file *filp) =20 filp->private_data =3D u; =20 + register_xenbus_file(u); + return 0; } =20 diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c index 49a3874ae6bb..9abff635fc20 100644 --- a/drivers/xen/xenbus/xenbus_xs.c +++ b/drivers/xen/xenbus/xenbus_xs.c @@ -95,12 +95,23 @@ static pid_t xenwatch_pid; static DEFINE_MUTEX(xenwatch_mutex); static DECLARE_WAIT_QUEUE_HEAD(watch_events_waitq); =20 +static unsigned int xs_state_count_users(void) +{ + unsigned int count; + + spin_lock(&xs_state_lock); + count =3D xs_state_users - xenbus_file_abort_trans(false); + spin_unlock(&xs_state_lock); + + return count; +} + static void xs_suspend_enter(void) { spin_lock(&xs_state_lock); xs_suspend_active++; spin_unlock(&xs_state_lock); - wait_event(xs_state_exit_wq, xs_state_users =3D=3D 0); + wait_event(xs_state_exit_wq, xs_state_count_users() =3D=3D 0); } =20 static void xs_suspend_exit(void) @@ -838,6 +849,9 @@ void xs_resume(void) =20 mutex_unlock(&xs_response_mutex); =20 + spin_lock(&xs_state_lock); + xs_state_users -=3D xenbus_file_abort_trans(true); + spin_unlock(&xs_state_lock); xs_suspend_exit(); =20 /* No need for watches_lock: the xs_watch_rwsem is sufficient. */ --=20 2.17.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel