From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH] bus/vmbus: fix race in sub channel creation Date: Mon, 3 Dec 2018 08:48:44 -0800 Message-ID: <20181203084844.39c82962@shemminger-XPS-13-9360> References: <1543575881.5400.33.camel@redhat.com> <20181130202457.10888-1-stephen@networkplumber.org> <1543816975.5400.35.camel@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org, Stephen Hemminger To: Mohammed Gamal Return-path: Received: from mail-pg1-f193.google.com (mail-pg1-f193.google.com [209.85.215.193]) by dpdk.org (Postfix) with ESMTP id 318651B432 for ; Mon, 3 Dec 2018 17:48:48 +0100 (CET) Received: by mail-pg1-f193.google.com with SMTP id t13so5945405pgr.11 for ; Mon, 03 Dec 2018 08:48:48 -0800 (PST) In-Reply-To: <1543816975.5400.35.camel@redhat.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, 03 Dec 2018 07:02:55 +0100 Mohammed Gamal wrote: > On Fri, 2018-11-30 at 12:24 -0800, Stephen Hemminger wrote: > > When using multiple queues, there was a race with the kernel > > in setting up the second channel. This is do due to a kernel change > > whiche does not allow accessing sysfs files for Hyper-V > > channels that are not opened. > >=20 > > The fix is simple, just move the logic to detect not ready > > sub channels earlier in the existing loop. > >=20 > > Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support") > > Reported-by:Mohammed Gamal > > Signed-off-by: Stephen Hemminger > > --- > > =C2=A0drivers/bus/vmbus/linux/vmbus_uio.c | 12 ++++++------ > > =C2=A01 file changed, 6 insertions(+), 6 deletions(-) > >=20 > > diff --git a/drivers/bus/vmbus/linux/vmbus_uio.c > > b/drivers/bus/vmbus/linux/vmbus_uio.c > > index 12e97e3a420a..38df4d724ed5 100644 > > --- a/drivers/bus/vmbus/linux/vmbus_uio.c > > +++ b/drivers/bus/vmbus/linux/vmbus_uio.c > > @@ -357,6 +357,12 @@ int vmbus_uio_get_subchan(struct vmbus_channel > > *primary, > > =C2=A0 continue; > > =C2=A0 } > > =C2=A0 > > + if (!vmbus_isnew_subchannel(primary, relid)) > > + continue; /* Already know about you > > */ > > + > > + if (!vmbus_uio_ring_present(dev, relid)) > > + continue; /* Ring may not be ready > > yet */ > > + > > =C2=A0 snprintf(subchan_path, sizeof(subchan_path), > > "%s/%lu", > > =C2=A0 =C2=A0chan_path, relid); > > =C2=A0 err =3D vmbus_uio_sysfs_read(subchan_path, > > "subchannel_id", > > @@ -370,12 +376,6 @@ int vmbus_uio_get_subchan(struct vmbus_channel > > *primary, > > =C2=A0 if (subid =3D=3D 0) > > =C2=A0 continue; /* skip primary channel */ > > =C2=A0 > > - if (!vmbus_isnew_subchannel(primary, relid)) > > - continue; > > - > > - if (!vmbus_uio_ring_present(dev, relid)) > > - continue; /* Ring may not be ready > > yet */ > > - > > =C2=A0 err =3D vmbus_uio_sysfs_read(subchan_path, > > "monitor_id", > > =C2=A0 =C2=A0=C2=A0=C2=A0&monid, UINT8_MAX); > > =C2=A0 if (err) { =20 >=20 > With this patch I am now getting the following error: > [...] > Configuring Port 0 (socket 0) > hn_dev_configure():=C2=A0=C2=A0>> =20 > hn_rndis_link_status(): link status 0x40020006 > hn_subchan_configure(): open 1 subchannels > hn_subchan_configure(): open subchannel failed: -2 > hn_dev_configure(): subchannel configuration failed > Port0 dev_configure =3D -2 > hn_dev_rx_queue_release():=C2=A0=C2=A0>> > hn_dev_rx_queue_release():=C2=A0=C2=A0>> > hn_dev_tx_queue_release():=C2=A0=C2=A0>> > hn_dev_tx_queue_release():=C2=A0=C2=A0>> =20 > Fail to configure port 0 > EAL: Error - exiting with code: 1 > =C2=A0 Cause: Start ports failed >=20 > Apparently, no subchannels were ready. Anything I may have missed or > misconfigured? >=20 > Regards, > Mohammed Could you check the kernel log? The way sub channel configuration works is that the userspace code in DPDK sends a message to the hypervisor that it would like N subchannels, then the response from the hypervisor is processed by the kernel causing sysfs files to be created. Meanwhile the userspace is polling waiting for the sysfs files to show up (for 10 seconds). You could increas the timeout or go looking in the sysfs directory to see what is present. There is no good way to handle errors here, the hypervisor doesn't really give much feedback.