From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 7B0196833; Thu, 25 Oct 2018 16:57:00 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Oct 2018 07:56:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,424,1534834800"; d="scan'208";a="102618671" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga001.jf.intel.com with ESMTP; 25 Oct 2018 07:56:59 -0700 Received: from fmsmsx155.amr.corp.intel.com (10.18.116.71) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 25 Oct 2018 07:56:59 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX155.amr.corp.intel.com (10.18.116.71) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 25 Oct 2018 07:56:58 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.161]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.199]) with mapi id 14.03.0415.000; Thu, 25 Oct 2018 22:56:56 +0800 From: "Zhang, Qi Z" To: =?iso-8859-1?Q?Ga=EBtan_Rivet?= CC: "thomas@monjalon.net" , "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [PATCH] bus/vdev: fix device argument corrupt after bus scan Thread-Index: AQHUbBL3TYeZulvOwUOzGQInCVlqWKUvMksAgADS0DA= Date: Thu, 25 Oct 2018 14:56:55 +0000 Message-ID: <039ED4275CED7440929022BC67E70611532DB6EF@SHSMSX103.ccr.corp.intel.com> References: <20181025033036.23680-1-qi.z.zhang@intel.com> <20181025095118.gnc75zvkuvfajtha@bidouze.vm.6wind.com> In-Reply-To: <20181025095118.gnc75zvkuvfajtha@bidouze.vm.6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTdhMmExODItNmFlZS00MDAwLTgxYmUtODlhZTgzZDMyZTEyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiam8zdkZqVnFRQnJjYmtXMDFMR2tQcStENGx1Z0ZERUtWRGRhUTZEXC9qQ1l1WDVPK2o5RjI1cE12WVFxelM1ZDgifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 14:57:01 -0000 > -----Original Message----- > From: Ga=EBtan 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 >=20 > 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) >=20 > Why is vdev the only bus not using hotplug API itself and reimplementing = it > on its own? I don't know the history,=20 but replace insert_vdev with hotplug API will not solve all the problem (se= e my below comments) >=20 > It should not have to insert devargs at all, not even in the primary proc= ess. If > it called rte_dev_probe(), this would normally already be properly handle= d 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_vde= v here. 2. during secondary's vdev->scan when it receive a SCAN_ONE event from prim= ary, it should not call rte_devargs_insert,=20 The patch is going to fix the issue on secondary scenario and we can do the= cleanup for first issue in a separate patch >=20 > > { > > 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 =3D 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 =3D insert_vdev(name, args, &dev); > > + ret =3D insert_vdev(name, args, &dev, true); > > if (ret =3D=3D 0) { > > ret =3D 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 =3D insert_vdev(in->name, NULL, NULL); > > + ret =3D insert_vdev(in->name, NULL, NULL, false); > > if (ret =3D=3D -EEXIST) > > VDEV_LOG(DEBUG, "device already exist, %s", in->name); > > else if (ret < 0) > > -- > > 2.13.6 > > >=20 > -- > Ga=EBtan Rivet > 6WIND