From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 955ECA0096 for ; Thu, 14 Mar 2019 08:17:34 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id EB0BB3572; Thu, 14 Mar 2019 08:17:33 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 05F39324D; Thu, 14 Mar 2019 08:17:31 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Mar 2019 00:17:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,477,1544515200"; d="scan'208";a="141924834" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga002.jf.intel.com with ESMTP; 14 Mar 2019 00:17:30 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 14 Mar 2019 00:17:29 -0700 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 14 Mar 2019 00:17:28 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.134]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.57]) with mapi id 14.03.0415.000; Thu, 14 Mar 2019 15:15:54 +0800 From: "Zhang, Qi Z" To: Thomas Monjalon , "dev@dpdk.org" CC: "stable@dpdk.org" Thread-Topic: [PATCH 3/3] eal: fix multi-process probe failure handling Thread-Index: AQHU0KHC8a3cb3kxS0aDZccKyGGzwKYKyI+g Date: Thu, 14 Mar 2019 07:15:53 +0000 Message-ID: <039ED4275CED7440929022BC67E706115334C7D0@SHSMSX103.ccr.corp.intel.com> References: <20190302024253.15594-1-thomas@monjalon.net> <20190302024253.15594-4-thomas@monjalon.net> In-Reply-To: <20190302024253.15594-4-thomas@monjalon.net> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTI3MjVjYjEtNzRiOS00MDJlLWJlNWEtOTgxYjQ1NGY3MDZlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiV2JzNU1PYVhEWlp1Q3lpZEZVcEx5QUZzaHZSQlh1V29tTWlPbFZ1OFwvT0xobmlMMmtrdkE4cjFcLzRjZFV2b3BSIn0= 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="UTF-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 3/3] eal: fix multi-process probe failure handling 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190314071553.s0mCn2c1T_RsCM4WoIFftuxirPzu3UnmyAmKNbAb0no@z> > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Saturday, March 2, 2019 10:43 AM > To: dev@dpdk.org > Cc: Zhang, Qi Z ; stable@dpdk.org > Subject: [PATCH 3/3] eal: fix multi-process probe failure handling >=20 > If probe fails in multi-process context, the device must removed in other > processes for consistency. This is a rollback mechanism. > However the rollback should not happen for devices which were already pro= bed > before the current probe transaction. >=20 > When probing an already probed device, the driver may reject with -EEXIST= or > update and succeed with code 0. > In order to distinguish successful new probe from re-probe, in the functi= on > local_dev_probe(), the positive EEXIST code is returned for the latter ca= se. >=20 > The functions rte_dev_probe() and __handle_secondary_request() can test f= or > -EEXIST and +EEXIST, and skip rollback in such case. >=20 > Fixes: 244d5130719c ("eal: enable hotplug on multi-process") > Fixes: ac9e4a17370f ("eal: support attach/detach shared device from secon= dary") > Cc: qi.z.zhang@intel.com > Cc: stable@dpdk.org >=20 > Signed-off-by: Thomas Monjalon > --- > lib/librte_eal/common/eal_common_dev.c | 12 ++++++++++-- > lib/librte_eal/common/eal_private.h | 2 +- > lib/librte_eal/common/hotplug_mp.c | 8 ++++++-- > 3 files changed, 17 insertions(+), 5 deletions(-) >=20 > diff --git a/lib/librte_eal/common/eal_common_dev.c > b/lib/librte_eal/common/eal_common_dev.c > index deaaea9345..2c7b1ab071 100644 > --- a/lib/librte_eal/common/eal_common_dev.c > +++ b/lib/librte_eal/common/eal_common_dev.c > @@ -132,6 +132,7 @@ local_dev_probe(const char *devargs, struct rte_devic= e > **new_dev) { > struct rte_device *dev; > struct rte_devargs *da; > + bool already_probed; > int ret; >=20 > *new_dev =3D NULL; > @@ -171,12 +172,15 @@ local_dev_probe(const char *devargs, struct rte_dev= ice > **new_dev) > * those devargs shouldn't be removed manually anymore. > */ >=20 > + already_probed =3D rte_dev_is_probed(dev); > ret =3D dev->bus->plug(dev); > if (ret && !rte_dev_is_probed(dev)) { /* if hasn't ever succeeded */ > RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", > dev->name); > return ret; > } > + if (ret =3D=3D 0 && already_probed) > + ret =3D EEXIST; /* hint to avoid any rollback */ What if bus->plug return -EEXIST and rte_dev_is_probed return true? (See rt= e_pci_probe_one_driver) You will not give hint here, but is this expected? >=20 > *new_dev =3D dev; > return ret; > @@ -194,6 +198,7 @@ rte_dev_probe(const char *devargs) { > struct eal_dev_mp_req req; > struct rte_device *dev; > + bool already_probed; > int ret; >=20 > memset(&req, 0, sizeof(req)); > @@ -221,8 +226,8 @@ rte_dev_probe(const char *devargs) >=20 > /* primary attach the new device itself. */ > ret =3D local_dev_probe(devargs, &dev); > - > - if (ret !=3D 0 && ret !=3D -EEXIST) { > + already_probed =3D (ret =3D=3D -EEXIST || ret =3D=3D EEXIST); > + if (ret < 0 && !already_probed) { > RTE_LOG(ERR, EAL, > "Failed to attach device on primary process\n"); > return ret; > @@ -250,6 +255,9 @@ rte_dev_probe(const char *devargs) > return 0; >=20 > rollback: > + if (already_probed) > + return ret; /* skip rollback */ > + > req.t =3D EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK; >=20 > /* primary send rollback request to secondary. */ diff --git > a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private= .h > index 798ede553b..a01d252930 100644 > --- a/lib/librte_eal/common/eal_private.h > +++ b/lib/librte_eal/common/eal_private.h > @@ -304,7 +304,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, > * @param new_dev > * new device be probed as output. > * @return > - * 0 on success, negative on error. > + * >=3D0 on success (+EEXIST if already probed), negative on error. > */ > int local_dev_probe(const char *devargs, struct rte_device **new_dev); >=20 > diff --git a/lib/librte_eal/common/hotplug_mp.c > b/lib/librte_eal/common/hotplug_mp.c > index 69e9a16d6a..9f8ef28a3b 100644 > --- a/lib/librte_eal/common/hotplug_mp.c > +++ b/lib/librte_eal/common/hotplug_mp.c > @@ -90,13 +90,15 @@ __handle_secondary_request(void *param) > struct rte_devargs da; > struct rte_device *dev; > struct rte_bus *bus; > + bool already_probed =3D false; > int ret =3D 0; >=20 > tmp_req =3D *req; >=20 > if (req->t =3D=3D EAL_DEV_REQ_TYPE_ATTACH) { > ret =3D local_dev_probe(req->devargs, &dev); > - if (ret !=3D 0 && ret !=3D -EEXIST) { > + already_probed =3D (ret =3D=3D -EEXIST || ret =3D=3D EEXIST); > + if (ret < 0 && !already_probed) { > RTE_LOG(ERR, EAL, "Failed to hotplug add device on primary\n"); > goto finish; > } > @@ -159,7 +161,7 @@ __handle_secondary_request(void *param) > goto finish; >=20 > rollback: > - if (req->t =3D=3D EAL_DEV_REQ_TYPE_ATTACH) { > + if (req->t =3D=3D EAL_DEV_REQ_TYPE_ATTACH && !already_probed) { > tmp_req.t =3D EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK; > eal_dev_hotplug_request_to_secondary(&tmp_req); > local_dev_remove(dev); > @@ -238,6 +240,8 @@ static void __handle_primary_request(void *param) > case EAL_DEV_REQ_TYPE_ATTACH: > case EAL_DEV_REQ_TYPE_DETACH_ROLLBACK: > ret =3D local_dev_probe(req->devargs, &dev); > + if (ret > 0) > + ret =3D 0; /* return only errors */ > break; > case EAL_DEV_REQ_TYPE_DETACH: > case EAL_DEV_REQ_TYPE_ATTACH_ROLLBACK: > -- > 2.20.1