From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id D67507D4A for ; Thu, 25 Oct 2018 17:03:33 +0200 (CEST) Received: by mail-wr1-f66.google.com with SMTP id w5-v6so9701303wrt.2 for ; Thu, 25 Oct 2018 08:03:33 -0700 (PDT) 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=C9PKNKtqx9dsFYbxRIiko0ZYXD0/VhDR3qiT8yltSeg=; b=w31HPJvZA5CRoSVz7+DtYLDFE6n/i5zZSBRYxoPLETEmVV/B8ldoSrIwlkJGwi+I0h 32SivKlIM/RokCXe1kcagunSdCRoMbIQzyi0dzliABS+JXSA1neEvNsgIavUCvsPUkzn SEWSdfM0IeWpqPOCc/EDoL1S6lFTR21zE6PJNvl5ahNyUZDKs0qaOmfmawBivKxjmSpE 4vpbj35HLaLMyjK14StCsGgK9jCnOPia5AuMExhJzN6NaAeyTIP00vd1V3CWlZs+NSAh NDk2/RB9pgXZiZfh/6YowfAn1SWC5VIfCz6zvfPlTLEWKxOYkQQ7ddYXXAmilBs0mwTV WtWg== 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=C9PKNKtqx9dsFYbxRIiko0ZYXD0/VhDR3qiT8yltSeg=; b=Wv6YE9aWiVuQCdcQwP5+IDWIuY7/Oj5kHwR56un/cmZinVrqJ+WS76mgABtLKRh0yX GMZc3aEJwDeDZf48RNKG2935TDIH21qtWgA+0SWk2AS+yLz5kohvNORmYaw5n7Gf2aGB k9kmgao3Gac3ELBsA8YLAhJs7CJKC/9NCB7nOqAuXiD9f7+GwZ2Un5jehits7K0F4XLP 5w+HLgBvdUrlI5Xs8dt0lwWF8SibIvysjB/AvxgFmhdNBIW2t+MGHXaaHMrgb8157zpe 7enB65oZQsrtYERR5LcFAk2VxUSW5KIDJEEftAZw0vTR/WZMYoYfbhHOiYn0ic5v3afl rSxg== X-Gm-Message-State: AGRZ1gKsuBSw9ZgSYtWLJxVMgWHvCRRUssmdlUXGfp10cZ//z+sswWz/ Y0ad5njC8/E42wHHZvMcxQMUNw== X-Google-Smtp-Source: AJdET5dsCQmPIgiZ5eWQ597HNJZgXbZgw899fU54Ju5TTjyoNSC8dkKIiQnP6nIf8DbRJYRjxFGrBg== X-Received: by 2002:adf:81e1:: with SMTP id 88-v6mr2567872wra.19.1540479812958; Thu, 25 Oct 2018 08:03:32 -0700 (PDT) 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 c64-v6sm1066882wma.44.2018.10.25.08.03.31 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 25 Oct 2018 08:03:32 -0700 (PDT) Date: Thu, 25 Oct 2018 17:03:13 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: "Zhang, Qi Z" Cc: "thomas@monjalon.net" , "dev@dpdk.org" , "stable@dpdk.org" Message-ID: <20181025150313.g6d7tfhmsiz7wwim@bidouze.vm.6wind.com> References: <20181025033036.23680-1-qi.z.zhang@intel.com> <20181025095118.gnc75zvkuvfajtha@bidouze.vm.6wind.com> <039ED4275CED7440929022BC67E70611532DB6EF@SHSMSX103.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <039ED4275CED7440929022BC67E70611532DB6EF@SHSMSX103.ccr.corp.intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] bus/vdev: fix device argument corrupt after bus scan 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: Thu, 25 Oct 2018 15:03:34 -0000 On Thu, Oct 25, 2018 at 02:56:55PM +0000, Zhang, Qi Z wrote: > > > > -----Original Message----- > > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com] > > Sent: Thursday, October 25, 2018 4:51 AM > > To: Zhang, Qi Z > > Cc: thomas@monjalon.net; dev@dpdk.org; stable@dpdk.org > > Subject: Re: [PATCH] bus/vdev: fix device argument corrupt after bus scan > > > > On Thu, Oct 25, 2018 at 11:30:36AM +0800, Qi Zhang wrote: > > > It's not necessary to insert device argment to devargs_list during bus > > > scan, but this happens when we try to attach a device on secondary > > > process. The patch fix the issue. > > > > > > Fixes: cdb068f031c6 ("bus/vdev: scan by multi-process channel") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Qi Zhang > > > --- > > > drivers/bus/vdev/vdev.c | 11 +++++++---- > > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c index > > > 688e31c21..818a2bfc2 100644 > > > --- a/drivers/bus/vdev/vdev.c > > > +++ b/drivers/bus/vdev/vdev.c > > > @@ -202,7 +202,9 @@ alloc_devargs(const char *name, const char *args) > > > } > > > > > > static int > > > -insert_vdev(const char *name, const char *args, struct > > > rte_vdev_device **p_dev) > > > +insert_vdev(const char *name, const char *args, > > > + struct rte_vdev_device **p_dev, > > > + bool init) > > > > Why is vdev the only bus not using hotplug API itself and reimplementing it > > on its own? > > I don't know the history, > but replace insert_vdev with hotplug API will not solve all the problem (see my below comments) > > > > > It should not have to insert devargs at all, not even in the primary process. If > > it called rte_dev_probe(), this would normally already be properly handled I > > think. > > Currently insert_vdev is shared by two scenarios > 1. rte_vdev_init, which is called by application to attach a new device, I agree it's better to have rte_dev_probe here so, no need to have insert_vdev here. > 2. during secondary's vdev->scan when it receive a SCAN_ONE event from primary, it should not call rte_devargs_insert, > The patch is going to fix the issue on secondary scenario and we can do the cleanup for first issue in a separate patch In 2., won't dev_probe() check for secondary process context and in this case, send the request to primary, which will see that the device is already probed, which would thus fix your issue? My take on it is that it seems to be fixing both your issue and cleaning up history. > > > > > > > > { > > > struct rte_vdev_device *dev; > > > struct rte_devargs *devargs; > > > @@ -237,7 +239,8 @@ insert_vdev(const char *name, const char *args, > > struct rte_vdev_device **p_dev) > > > } > > > > > > TAILQ_INSERT_TAIL(&vdev_device_list, dev, next); > > > - rte_devargs_insert(devargs); > > > + if (init) > > > + rte_devargs_insert(devargs); > > > > > > if (p_dev) > > > *p_dev = dev; > > > @@ -257,7 +260,7 @@ rte_vdev_init(const char *name, const char *args) > > > int ret; > > > > > > rte_spinlock_recursive_lock(&vdev_device_list_lock); > > > - ret = insert_vdev(name, args, &dev); > > > + ret = insert_vdev(name, args, &dev, true); > > > if (ret == 0) { > > > ret = vdev_probe_all_drivers(dev); > > > if (ret) { > > > @@ -383,7 +386,7 @@ vdev_action(const struct rte_mp_msg *mp_msg, > > const void *peer) > > > break; > > > case VDEV_SCAN_ONE: > > > VDEV_LOG(INFO, "receive vdev, %s", in->name); > > > - ret = insert_vdev(in->name, NULL, NULL); > > > + ret = insert_vdev(in->name, NULL, NULL, false); > > > if (ret == -EEXIST) > > > VDEV_LOG(DEBUG, "device already exist, %s", in->name); > > > else if (ret < 0) > > > -- > > > 2.13.6 > > > > > > > -- > > Gaëtan Rivet > > 6WIND -- Gaëtan Rivet 6WIND