From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 58917A0C47; Thu, 14 Oct 2021 14:01:05 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 032E14112E; Thu, 14 Oct 2021 14:01:05 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 676FA40041 for ; Thu, 14 Oct 2021 14:01:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1634212863; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=c+LayhBsLuEC22xddaVZAvwKUxEYGc2yuBOZqvvL+28=; b=AjqK7Ly2Wx5HezpKUHyGNSbZff3F0Rrxl4xJHFbGPRO77YtWP0BpUkYSFb4JSlLZyBwOM8 92jNJCX3vFfgJBW4zjgFpRq/wM8c7uNyFn9KqUA06GS3azZvZu5Dtg20T0v2TF9fHDzJH5 wYeBBWdJYZYBSPez7JxzzRV22gAB5Hs= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-562-4QtWYiJxPsS91bdK1pbb0Q-1; Thu, 14 Oct 2021 08:00:59 -0400 X-MC-Unique: 4QtWYiJxPsS91bdK1pbb0Q-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3A09D19200C4; Thu, 14 Oct 2021 12:00:58 +0000 (UTC) Received: from [10.39.208.20] (unknown [10.39.208.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5023819D9F; Thu, 14 Oct 2021 12:00:56 +0000 (UTC) Message-ID: <34b02edd-b44c-0f75-a1ff-5c78803783cf@redhat.com> Date: Thu, 14 Oct 2021 14:00:55 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 To: Li Feng , Chenbo Xia Cc: dev@dpdk.org References: <20210903075637.2201185-1-fengli@smartx.com> <20210903080203.2495559-1-fengli@smartx.com> From: Maxime Coquelin In-Reply-To: <20210903080203.2495559-1-fengli@smartx.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] vhost: call destroy_device always in vhost_destroy_device_notify X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Li, The commit message is not compliant with the contributors guidelines: https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-subject-line On 9/3/21 10:02, Li Feng wrote: > Vhost-user client must send the mem table, kick fd, call fd on all > virtqueues, then the device will be VIRTIO_DEV_RUNNING. > > If the vhost-user communication is initialized partly, e.g. > - When initializing the vhost-user, try to restart the vhost-user > backend; > - Seabios only initialized the vhost-scsi req vq. > The device is not with flags VIRTIO_DEV_RUNNING.. > > Root Cause: > The vhost session has been created, and added the scsi/blk requestq > poller into reactor, but when destroying the device, the requestq is not > unregistered. > > Reproduce the crash on spdk vhost-user backend: > 1. Create a VM; > 2. Mount a ISO to a VM, start the VM, don't install the OS; > 3. Restart the spdk_tgt; > > Another discusstion is in seabiso: > https://patchew.org/Seabios/20210831122339.2591585-1-fengli@smartx.com/ This is a fix, so you need to add the Fixes tag and cc stable. > Signed-off-by: Li Feng > --- > v2: > Fix the commit msg typo: vas -> virtqueues. > -- > lib/vhost/vhost.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c > index 355ff37651..191ba82c41 100644 > --- a/lib/vhost/vhost.c > +++ b/lib/vhost/vhost.c > @@ -710,8 +710,8 @@ vhost_destroy_device_notify(struct virtio_net *dev) > if (vdpa_dev) > vdpa_dev->ops->dev_close(dev->vid); > dev->flags &= ~VIRTIO_DEV_RUNNING; > - dev->notify_ops->destroy_device(dev->vid); > } > + dev->notify_ops->destroy_device(dev->vid); .destroy_device() is the counter-part of .new_device(). VIRTIO_DEV_RUNNING is set only when .new_device() has been called with success, and cleared when .destroy_device() is called. So I disagree with the fix, we want to keep the correlation between VIRTIO_DEV_RUNNING and .new_device()/.destroy_device(). Doing otherwise could lead to regressions with other applications than yours. What is not clear from the commit message or the discussion you link is where does it crash exactly. Is it in SPDK, in DPDK? Regards, Maxime > } > > /* >