From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier Matz Subject: Re: Question regarding mempool changes impact to XEN PMD Date: Mon, 13 Jun 2016 10:14:24 +0200 Message-ID: <575E6B60.3030403@6wind.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable To: Christian Ehrhardt , David Marchand , dev Return-path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id A80B947D1 for ; Mon, 13 Jun 2016 10:14:32 +0200 (CEST) In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Christian, On 06/13/2016 09:34 AM, Christian Ehrhardt wrote: > Hi David, I guess this mail is for me, not for David :) > it seems to be the first time I compiled with > CONFIG_RTE_LIBRTE_PMD_XENVIRT=3Dy sinec the bigger mempool changes arou= nd > "587d684d doc: update release notes about mempool allocation". >=20 > I've seen related patch to mempool / xen in that regard "c042ba20 mempo= ol: > rework support of Xen dom0" >=20 > But with above config symbol enabled I got: > drivers/net/xenvirt/rte_xen_lib.c: In function =E2=80=98grant_gntalloc_= mbuf_pool=E2=80=99: > drivers/net/xenvirt/rte_xen_lib.c:440:69: error: =E2=80=98struct rte_me= mpool=E2=80=99 has > no member named =E2=80=98elt_va_start=E2=80=99 > if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR, > (uintptr_t)mpool->elt_va_start) =3D=3D -1) > ^ > SYMLINK-FILE include/rte_eth_bond.h > mk/internal/rte.compile-pre.mk:126: recipe for target 'rte_xen_lib.o' f= ailed > make[4]: *** [rte_xen_lib.o] Error 1 > make[4]: *** Waiting for unfinished jobs.... >=20 > The change around the mempools is complex, so I don't see on the first = look > if that needs a minor or major rework in the xen sources. > I mean I don't want it to compile, but to work and that could be more t= han > just fixing that changed structure :-) >=20 > So I wanted to ask if you as author would see if it is a trivial change > that has to be made? Sorry, I missed this reference to elt_va_start in my patches. I'm not very familiar with the xen code in dpdk, but from what I see: - in the PMD, grant_gntalloc_mbuf_pool() stores the mempool virtual address in the xen key/value database - in examples/vhost_xen, the function parse_mpool_va() retrieves it - this address is used in new_device() I think the patch would be almost similar to what I did in mlx drivers in this commit: http://dpdk.org/browse/dpdk/commit/?id=3D84121f1971873c9f45b2939c316c6612= 6d8754a1 or in librte_kni in this commit: http://dpdk.org/browse/dpdk/commit?id=3Dd1d914ebbc2514f334a3ed24057e63c8b= b76363d To give more precisions: - before the patchset, mp->elt_va_start was the virtual address of the mempool objects table. It was always virtually contiguous - now, a mempool can be fragmented in several virtually contiguous chunks. In case there is only one chunk, it can be safely replaced by STAILQ_FIRST(&mp->mem_list)->addr (=3D the virtual address of the first chunk). In case there are more chunks in the mempool, it would require deeper modifications I think. But we should keep in mind that having a virtually fragmented mempool was not possible before the patchset (it would have fail at init). If if fails later in xen code because xen does not support fragmented mempools, there is no regression compared to what we had before. I'll send a draft patch, if you could give it a try, it would be great! Thanks for reporting, Olivier