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 2A8C1A0A0C; Fri, 23 Jul 2021 09:39:26 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A9F404014D; Fri, 23 Jul 2021 09:39:25 +0200 (CEST) Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by mails.dpdk.org (Postfix) with ESMTP id 58FFB40040 for ; Fri, 23 Jul 2021 09:39:24 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id F136F5C0174; Fri, 23 Jul 2021 03:39:23 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Fri, 23 Jul 2021 03:39:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm1; bh= jPa7UQbWqE5Y8g1a+PKQwO5R3w8cZciWXlcgi2AFQkc=; b=qypoHYcNRJh1mlZI lx2uZ9BESXPpHxP/BqNYC6bfxhvkjN4xVwfPdMHKU7LWH3H+YB8+sYUkKFK6J239 bnVWuUgOsI6txo4BC7sx7wi6ikg6Rj3ejCMJcp5h5qMaVUJydNagqZGoxD1xlHzc jkcNltkNkrCLDl3ObwCUvQhRG7XQaPbyl23tzpfNrJAylHEzIi34TpVSmHoueaoP bjpsx1lV0lT4nf6r/OS15TjcIx1JRO2U2ighrplDeDBUrtNPJEtdSrinbdZGaIDo XWQCcbRsTpajaLFv3HM1XNJm81IWuZFlc2fQymnvDWK2+6eK2oVVDkzXkw6htk9h LztSog== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=jPa7UQbWqE5Y8g1a+PKQwO5R3w8cZciWXlcgi2AFQ kc=; b=p/+yjihkvfAD32BqRUz/u4V0WeZurPKJzJ176m+v2rfGXwjPggj3P0eo/ M0yAIrGojEv7CglapIG+Er6mHrjmQtCbRiPLc3XyQGR/qOwWtznM1SLYdyrfxZbw x3z6HtQla4KVjUGbs7SM7IxXf5jqvfyJ2JvIU1uVcmyRzWhplP0tetbQsso3l92h 4maMGKbwmAEioOHDXUb9bst4035Zln8oKZwyjMF83BXTt3zhv8TLZ6FOvE2x4AIC YwF5mIunglZIkICPSvwG3q6G1KP3Ieq75dNLM5QjZmQ4a2KTklU73GL0iFyBvS7E 5IPm5OJirVsbaDP0S0hmnM/fPD1fw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrfeejgdduudejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Jul 2021 03:39:22 -0400 (EDT) From: Thomas Monjalon To: "maxime.coquelin@redhat.com" , "Xia, Chenbo" Cc: "Jiang, Cheng1" , "dev@dpdk.org" , "Hu, Jiayu" , "Yang, YvonneX" , "david.marchand@redhat.com" , "Yigit, Ferruh" Date: Fri, 23 Jul 2021 09:39:41 +0200 Message-ID: <2246168.clQEStCGqb@thomas> In-Reply-To: References: <20210602042802.31943-1-cheng1.jiang@intel.com> <6867079.l8GmTB2vYg@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v7 0/5] vhost: handle memory hotplug for async vhost 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" 23/07/2021 09:34, Xia, Chenbo: > From: Thomas Monjalon > > 23/07/2021 07:06, Xia, Chenbo: > > > From: Thomas Monjalon > > > > 22/07/2021 07:07, Xia, Chenbo: > > > > > From: Jiang, Cheng1 > > > > > > When the guest memory is hotplugged, the vhost application which > > > > > > enables DMA acceleration must stop DMA transfers before the vhost > > > > > > re-maps the guest memory. > > > > > > > > > > > > This patch set is to provide an unsafe API to drain inflight pkts > > > > > > which are submitted to DMA engine in vhost async data path, and > > > > > > notify the vhost application of stopping DMA transfers. And enable it > > > > > > in vhost example. > > > > > > > > > > Series applied to next-virtio/main. Thanks > > > > > > > > I cannot pull this series in main branch. > > > > > > > > There is a compilation error seen on Arm cross-compilation: > > > > > > > > examples/vhost/main.c:1493:51: error: assignment to 'int32_t (*)(int, > > > > uint16_t, struct rte_vhost_async_desc *, struct rte_vhost_async_status *, > > > > uint16_t)' {aka 'int (*)(int, short unsigned int, struct > > > > rte_vhost_async_desc *, struct rte_vhost_async_status *, short unsigned > > int)'} > > > > from incompatible pointer type 'uint32_t (*)(int, uint16_t, struct > > > > rte_vhost_async_desc *, struct rte_vhost_async_status *, uint16_t)' {aka > > > > 'unsigned int (*)(int, short unsigned int, struct rte_vhost_async_desc *, > > > > struct rte_vhost_async_status *, short unsigned int)'} [- > > Werror=incompatible- > > > > pointer-types] > > > > 1493 | channel_ops.transfer_data = > > > > ioat_transfer_data_cb; > > > > | ^ > > > > > > I see. @Cheng, please fix it in new version. > > > > > > > > > > > Other comments about the last patch: > > > > - it is updating doc out of the original patch doing the code changes > > > > - there is not even a reference to the code patch (Fixes: line) > > > > > > I think the doc patch could be combined with the code patch in the same > > series. > > > But personally, sometimes I am not very clear when doc patch should be split. > > > For example, in this case we can combine as the update in release note is > > related > > > only to the code patch. What if it's related to multiple patch? Should we > > split or > > > add doc changes to every related patches? Just a bit confused. Maybe you can > > give > > > me some general guidance so that we will be on the same page. > > > > The doc must be updated in each patch. > > Sometimes, the same line is updated to add a word related to the patch. > > Thanks for the guidance! > > > > > > > - the addition in the release notes is not sorted > > > > > > Not very clear on this. The change is put in the bottom. Is there any > > sorting > > > rules? > > > > Read the comment at the beginning of the section, it explains > > how things must be sorted: > > > > Suggested order in release notes items: > > * Core libs (EAL, mempool, ring, mbuf, buses) > > * Device abstraction libs and PMDs (ordered alphabetically by vendor name) > > - ethdev (lib, PMDs) > > - cryptodev (lib, PMDs) > > - eventdev (lib, PMDs) > > - etc > > * Other libs > > * Apps, Examples, Tools (if significant) > > > > vhost is usually at the end of ethdev PMDs. > > Oops.. I should notice it.. > > > > > > > Last question while at it, why having the API documentation > > > > in the vhost guide (rst file)? > > > > Doxygen is not enough to describe the functions? > > > > > > Good point. To be honest, I have not thought about it :P > > > > > > I think it could be moved to the doxygen later (maybe in another patch). The > > only > > > concern of mine is some API description in the vhost guide is a bit long. > > > > So you can improve doxygen and remove this part of the guide. > > The guide should be an overview, a tutorial and an internal design reference. > > Make sense to me. For this patch, I suggest to keep the api doc in vhost guide. Yes of course, don't change everything for this patch :) > Then I will send a patch to move them all if we all agree on this. Thank you.