From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by dpdk.org (Postfix) with ESMTP id 5447C14EC for ; Tue, 6 Nov 2018 09:54:08 +0100 (CET) Received: by mail-wr1-f68.google.com with SMTP id 74-v6so12470892wrb.13 for ; Tue, 06 Nov 2018 00:54:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=Bxh9/xrnaNMRjpg4z+hU+Z9Nt3cXG+/QcJasPl2H2/8=; b=jXTmhO2aBMN3oiirRZ5G+QEsYDz8V/VPgIu4qgjeHOYM01Wm4iXm5ah50LvcZCmcuO l/503mR+xOMlr5VxP5kn3r29Fy/g7fZqpQy3kkIpVBAlKNTwDepGjSzm6fX7g1I0b556 4xCJ6FzXmtnJwKzC7rEgFwtxs5PGmOO7Ev4Q+nYlg6DiO/OvYauzxhDtSxRYHv+8m/pC Ue+3hwh+1TClJ/WJqFXFh965qM30ZoOPbf3XJQrBHDD7SHrW1G+CXmfwjcXF/ZpxXmYi y2qmSlgMmR+KYqKJZAOH5qlhGBsQLybFdmCXMVFSjNtR9mtvhAn/H1ey9TGB1tA+a3y5 iaHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=Bxh9/xrnaNMRjpg4z+hU+Z9Nt3cXG+/QcJasPl2H2/8=; b=Nb0wemwxhjk6Z9XPyL5DXdxhLVFSfat++X7i47+1NsVE7aSe6dxgsxgMgiLdwXtw6p 1FZR6KdEGwDtl14uvt1BaFrB7OjafpVRwK6d1790xJfjT8f8Kyptxg3kn1EY9EfH0gg6 Jaue6+JNEIGTw3uokojv4203UngKGK8VMGzm/Z2itq9jTL7HaAgAsm2jF4u4kMq8FeN2 8pwQpDIKGdmNIDz33uuE3UwZdniRs2c2aXbg6wxGJnVK6X3KJmETrHNykuoaJoz47u0N mZcT5jQEjh71ii0mtgYlk8bRng0jy1sp/yVh2naTY4t/aRm7+ghodtmiSa7vR8/eMUnL Pejg== X-Gm-Message-State: AGRZ1gJoTo48EK5iqoveHAMASnm1dQ12l7/CjxyOk2EBcEqPB3n0Nor4 aapK48GHO2R0Zha8AxyyuuKsBDBnv+E= X-Google-Smtp-Source: AJdET5cHmGlAhxiKZoioP0G+0o7aUXNgY+eD4stGpR7qFCLG+KuyLWtD6dPO3Or2vTLC0kxMFlM6Lw== X-Received: by 2002:a5d:618c:: with SMTP id j12-v6mr15689099wru.300.1541494447760; Tue, 06 Nov 2018 00:54:07 -0800 (PST) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id i7-v6sm70257125wrb.30.2018.11.06.00.54.06 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 06 Nov 2018 00:54:06 -0800 (PST) Date: Tue, 6 Nov 2018 09:53:47 +0100 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Qi Zhang Cc: thomas@monjalon.net, ferruh.yigit@intel.com, dev@dpdk.org Message-ID: <20181106085347.4pthgcq6j6kj3bme@bidouze.vm.6wind.com> References: <20181106003150.10560-1-qi.z.zhang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20181106003150.10560-1-qi.z.zhang@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] bus/vdev: fix probe same device twice X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Nov 2018 08:54:08 -0000 Hi, On Tue, Nov 06, 2018 at 08:31:50AM +0800, Qi Zhang wrote: > When probe the same device at second time, devargs will be > replaced in devargs_list, old version is destoried but they > are still referenced by vdev->device. So we break the link > between vdev->device to devargs_list by clone, and the copy > one will be freed by vdev bus itself. > Please don't add even more cruft that will need to be cleaned from vdev anyway. This subject was already discussed[1], the solution is not to add a devargs-dependent function clone that you did not declare part of the devargs API to simplify its inclusion, but still relies on the devargs structure and operation (meaning the same maintenance cost, only deported to vdev and that future devargs work will need to follow). You see a feature as such missing, propose a new experimental API in rte_devargs, it will be cleaner and simpler, and might also be useful to others. [1]: https://mails.dpdk.org/archives/dev/2018-November/118274.html > Signed-off-by: Qi Zhang > --- > drivers/bus/vdev/vdev.c | 44 ++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 40 insertions(+), 4 deletions(-) > > diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c > index 9c66bdc78..2e2b6dc57 100644 > --- a/drivers/bus/vdev/vdev.c > +++ b/drivers/bus/vdev/vdev.c > @@ -176,6 +176,21 @@ find_vdev(const char *name) > } > > static struct rte_devargs * > +clone_devargs(struct rte_devargs *devargs) > +{ > + struct rte_devargs *da; > + > + da = calloc(1, sizeof(*devargs)); > + if (da == NULL) > + return NULL; > + > + *da = *devargs; > + da->args = strdup(devargs->args); > + > + return da; > +} > + > +static struct rte_devargs * > alloc_devargs(const char *name, const char *args) > { > struct rte_devargs *devargs; > @@ -208,6 +223,7 @@ insert_vdev(const char *name, const char *args, > { > struct rte_vdev_device *dev; > struct rte_devargs *devargs; > + struct rte_devargs *devargs2; > int ret; > > if (name == NULL) > @@ -217,6 +233,13 @@ insert_vdev(const char *name, const char *args, > if (!devargs) > return -ENOMEM; > > + devargs2 = clone_devargs(devargs); > + if (!devargs2) { > + free(devargs->args); > + free(devargs); > + return -ENOMEM; > + } > + > dev = calloc(1, sizeof(*dev)); > if (!dev) { > ret = -ENOMEM; > @@ -224,9 +247,9 @@ insert_vdev(const char *name, const char *args, > } > > dev->device.bus = &rte_vdev_bus; > - dev->device.devargs = devargs; > + dev->device.devargs = devargs2; > dev->device.numa_node = SOCKET_ID_ANY; > - dev->device.name = devargs->name; > + dev->device.name = devargs2->name; > > if (find_vdev(name)) { > /* > @@ -249,6 +272,8 @@ insert_vdev(const char *name, const char *args, > fail: > free(devargs->args); > free(devargs); > + free(devargs2->args); > + free(devargs2); > free(dev); > return ret; > } > @@ -269,6 +294,8 @@ rte_vdev_init(const char *name, const char *args) > /* If fails, remove it from vdev list */ > TAILQ_REMOVE(&vdev_device_list, dev, next); > rte_devargs_remove(dev->device.devargs); > + free(dev->device.devargs->args); > + free(dev->device.devargs); > free(dev); > } > } > @@ -315,6 +342,8 @@ rte_vdev_uninit(const char *name) > > TAILQ_REMOVE(&vdev_device_list, dev, next); > rte_devargs_remove(dev->device.devargs); > + free(dev->device.devargs->args); > + free(dev->device.devargs); > free(dev); > > unlock: > @@ -404,6 +433,7 @@ vdev_scan(void) > { > struct rte_vdev_device *dev; > struct rte_devargs *devargs; > + struct rte_devargs *devargs2; > struct vdev_custom_scan *custom_scan; > > if (rte_mp_action_register(VDEV_MP_KEY, vdev_action) < 0 && > @@ -457,18 +487,24 @@ vdev_scan(void) > if (!dev) > return -1; > > + devargs2 = clone_devargs(devargs); > + if (!devargs2) { > + free(dev); > + return -1; > + } > rte_spinlock_recursive_lock(&vdev_device_list_lock); > > if (find_vdev(devargs->name)) { > rte_spinlock_recursive_unlock(&vdev_device_list_lock); > + free(devargs2); > free(dev); > continue; > } > > dev->device.bus = &rte_vdev_bus; > - dev->device.devargs = devargs; > + dev->device.devargs = devargs2; > dev->device.numa_node = SOCKET_ID_ANY; > - dev->device.name = devargs->name; > + dev->device.name = devargs2->name; > > TAILQ_INSERT_TAIL(&vdev_device_list, dev, next); > > -- > 2.13.6 > -- Gaëtan Rivet 6WIND