From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 7EF26A0A0C;
	Fri, 23 Jul 2021 09:25:17 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id EDD6440040;
	Fri, 23 Jul 2021 09:25:16 +0200 (CEST)
Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com
 [66.111.4.29]) by mails.dpdk.org (Postfix) with ESMTP id DC2874003C
 for <dev@dpdk.org>; Fri, 23 Jul 2021 09:25:14 +0200 (CEST)
Received: from compute5.internal (compute5.nyi.internal [10.202.2.45])
 by mailout.nyi.internal (Postfix) with ESMTP id 770875C00B4;
 Fri, 23 Jul 2021 03:25:12 -0400 (EDT)
Received: from mailfrontend2 ([10.202.2.163])
 by compute5.internal (MEProxy); Fri, 23 Jul 2021 03:25:12 -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=
 Cbu0dTCn30uPSZBG74qC1w24ZfL5ZuAWQo+pE1S3H/w=; b=ftqAT6EwkzCBu325
 BiKS9WE0mZvMW1KvQg2MQE7o5/MyF1ITK7Nb8hP4mIgCuHclnOSzU2hbXfo/Ps4x
 pgGauoQVapP1pCq3Db7jJlv9kY0QPA8nOPiTVaZNhMQY/ENcmD8YfnguONqm8LcA
 tKkfM27cHNdaJo24uuySQZJMAgP1mhoX+WW4V+XA5BZWKujgb/S5Jgb1YHyMta7D
 pzmI9CeKJQrZ+zmPMppA/hAH1H3/3Y6kOde2rwXy/HpzVIoS5WyZ25iOT3tzrUCC
 zZG29RPyZFAmPgNFUZn1Y/ytXXXAx4+cs/2hWCYPJrWT55X56gm7YKgaYYMrCiUY
 8nUKug==
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=Cbu0dTCn30uPSZBG74qC1w24ZfL5ZuAWQo+pE1S3H
 /w=; b=gwIMFknoJ4Jk4q7axIsleqhqEJuDeAYnRz1bAWtO3N15+iE8IDvz/97tI
 t4TMGCpKIgxYJ6vnZrEiePq0UKdnnsFhJaE1WE4tYor1s6J+zmojlodWPXeQzLhr
 BvoU7uTCCzFvbEGQ4G21+HjJ6kmhFnWay2FMqTHqAIa2eERCD6MerV91QfwV/umH
 Pd1DWoFml/rDakNzgcvqghnXtNayYrj/buCz5INcxi/k6HQsyQ9Jb+Od8iMqFihA
 IMlEbY0MF1LtwZ7pHTMhX5b2lIj47XlXF/N+7e33bBLAZlP9cQwL24DyZli6bTnq
 Zxy2aUAkG0O/ra52FBSvlljsgeoEg==
X-ME-Sender: <xms:1276YILno7m7t_ET9HoBCdvmk1ku122zBlPrRxM7b5Mj6YAD1ELlbQ>
 <xme:1276YIJ6lrwi2553ZaGjJYj3nUNmabgemYL2Ky8isiQ5JkKbT2G_dnqxakgBPFPrv
 m_frBFj_TNkmkevFQ>
X-ME-Received: <xmr:1276YItNWyEICYJcShnhOT6ft2r7_hLNexylXgnhYKV4AF9FP_MZn7hNTYAr31dIq-L8KcUYTJ2WtBhOrgIsRJ8wag>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrfeejgdduudegucetufdoteggodetrfdotf
 fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen
 uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne
 cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr
 shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg
 ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu
 ieeivdffgeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh
 homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth
X-ME-Proxy: <xmx:1276YFZSkoaekE28xTYRIVSDq-pfDcm0ufYWcnmhSW0ZhEy8qqrbgA>
 <xmx:1276YPYW0nk2EOZLFO-TDcRxS8qvgE44AmkqP7EQ0sou1ifpT8krUQ>
 <xmx:1276YBBnw579WN35lRmFLA_bvupHBnfjC_4vuSr1xrIhS2IMOKIDSw>
 <xmx:2G76YEMxwrxrhWVZSY6aeZkhSS1H3CVVKTgH5aBTH-xtLiKM_fuBtQ>
Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri,
 23 Jul 2021 03:25:10 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: "maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>, "Xia,
 Chenbo" <chenbo.xia@intel.com>
Cc: "Jiang, Cheng1" <cheng1.jiang@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
 "Hu, Jiayu" <jiayu.hu@intel.com>, "Yang, YvonneX" <yvonnex.yang@intel.com>,
 "david.marchand@redhat.com" <david.marchand@redhat.com>,
 ferruh.yigit@intel.com
Date: Fri, 23 Jul 2021 09:25:29 +0200
Message-ID: <6867079.l8GmTB2vYg@thomas>
In-Reply-To: <MN2PR11MB40639AD5A2B0029ED1F851859CE59@MN2PR11MB4063.namprd11.prod.outlook.com>
References: <20210602042802.31943-1-cheng1.jiang@intel.com>
 <10705747.pk50nUbaNJ@thomas>
 <MN2PR11MB40639AD5A2B0029ED1F851859CE59@MN2PR11MB4063.namprd11.prod.outlook.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

23/07/2021 07:06, Xia, Chenbo:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 22/07/2021 07:07, Xia, Chenbo:
> > > From: Jiang, Cheng1 <cheng1.jiang@intel.com>
> > > > 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.

> > - 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.

> > 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.

> @Maxime What do you think?