From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stephen@networkplumber.org>
Received: from mail-pa0-f47.google.com (mail-pa0-f47.google.com
 [209.85.220.47]) by dpdk.org (Postfix) with ESMTP id E8F3D8D9A
 for <dev@dpdk.org>; Wed, 14 Oct 2015 02:05:22 +0200 (CEST)
Received: by pabrc13 with SMTP id rc13so35554949pab.0
 for <dev@dpdk.org>; Tue, 13 Oct 2015 17:05:22 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to
 :references:mime-version:content-type:content-transfer-encoding;
 bh=KgdABDSpd/a8aHyukOp0DYTA44JMhupAFqi/XtWWcsM=;
 b=Fri7TgZM5zq88qArk7cUbjtDcZXoy3f/uu6uzWaTVoI5lHCuYh1Tb7HjOMKWamaN4K
 MI18H+680h3Wdv7xvHpYo6xUWFxliXxv6OVsYYPlDF5RKQOuzKGzGSI19c22Cr94py6t
 B8SA+N+Oeb9dImahXscyPWB7faghRhUGV3qbWCgQGgClfDFocCJshe5CqGkYAq2e5FJ3
 nlWdcxD349fsD2xFzqcBTzT3KiL4AYiYQb++uuW69LW0lBD7/T3rjKbtlF2LzvYJhEPu
 NiVoH7gqnW+IYCAZzETjVFRaUzux0PjxfuIzUAJtaWtpNtlwVYei3LB5aCxG3XpT8GeA
 vG7g==
X-Gm-Message-State: ALoCoQkJ0CBdmaR55KRApeEAnLaJp4uYcxbBI2p+GoQf5lAbHJ/FXsvfJyMZ1wgKWwZQ7ouJblTw
X-Received: by 10.68.69.40 with SMTP id b8mr263455pbu.84.1444781122041;
 Tue, 13 Oct 2015 17:05:22 -0700 (PDT)
Received: from xeon-e3 (static-50-53-82-155.bvtn.or.frontiernet.net.
 [50.53.82.155])
 by smtp.gmail.com with ESMTPSA id qv5sm5928979pbc.71.2015.10.13.17.05.21
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Tue, 13 Oct 2015 17:05:21 -0700 (PDT)
Date: Tue, 13 Oct 2015 17:05:32 -0700
From: Stephen Hemminger <stephen@networkplumber.org>
To: Neil Horman <nhorman@tuxdriver.com>
Message-ID: <20151013170532.05e44007@xeon-e3>
In-Reply-To: <20150310105541.GA7873@hmsreliant.think-freely.org>
References: <CALwxeUuQtv685KnbmpZKCPkrAqmjLs558xeCW7c=-TPTsB423w@mail.gmail.com>
 <1425912999-13118-1-git-send-email-david.marchand@6wind.com>
 <1425912999-13118-2-git-send-email-david.marchand@6wind.com>
 <20150309152106.GA24326@hmsreliant.think-freely.org>
 <CALwxeUs4hPbYDPBUfz9u2AoiCoj_wwTsAyj=_1xxzuT6LLW6nw@mail.gmail.com>
 <20150310105541.GA7873@hmsreliant.think-freely.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/2] eal/linux: move plugin load to very
 start of eal init
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 14 Oct 2015 00:05:23 -0000

On Tue, 10 Mar 2015 06:55:41 -0400
Neil Horman <nhorman@tuxdriver.com> wrote:

> On Tue, Mar 10, 2015 at 10:08:24AM +0100, David Marchand wrote:
> > Hello Neil,
> > 
> > On Mon, Mar 9, 2015 at 4:21 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > 
> > > On Mon, Mar 09, 2015 at 03:56:38PM +0100, David Marchand wrote:
> > > > Loading shared libraries should be done at the very start of eal init so
> > > that
> > > > the code statically built in dpdk and the code loaded from shared
> > > objects is
> > > > handled (almost) the same way wrt to call to rte_eal_init().
> > > > The only thing that must be done before is filling the solib_list which
> > > is done
> > > > by eal_parse_args().
> > > >
> > >
> > >
> > > I don't see anything explicitly wrong with this, but at the same time it
> > > doesn't
> > > seem to fix anything.  Is there a particular bug that you're fixing in
> > > relation
> > > to your cover letter here?  Or is there some expectation that PMD's loaded
> > > in
> > > this fashion expect the dpdk to be completely uninitalized?  That would
> > > seem
> > > like a strange operational requirement to me.
> > >
> > 
> > Well, at first, I wanted to fix the virtio pmd init issue (iopl() not
> > called at the right place wrt to other pthreads created in rte_eal_init()).
> Ah, this is what you were addressing:
> http://dpdk.org/ml/archives/dev/2015-March/014765.html
> 
> > With next patch, this issue is fixed for statically builtin virtio pmd, but
> > for virtio pmd as a shared object, the dlopen comes too late.
> > So, yes, I moved the dlopen() for this reason.
> > 
> But this doesn't do anything to help you.  The goal, according to the above
> thread, is to initalize the pmd earlier so that you can call iopl prior to doing
> any forks (so that io privlidges are inherited).  But both static and dynamic
> pmd have constructors that just register their driver structures.  No
> initalization happens until rte_eal_dev_init is called.  So this movement does
> nothing to change the time any given drivers init routine is called.
> 
> > From a more general point of view, since we support both static and dso
> > pmds, I would say that this is more logical to have dlopen comes very
> > early, since static code is "loaded" even earlier : if the current pmds
> > needed more than just register to the driver list, they would already have
> > triggered segfaults and/or bugs.
> > 
> No, not really.  I suppose it doesn't hurt anything, but moving this earlier in
> a function doesn't really buy you anything, as statically allocate pmds are
> called by the gcc start code prior to an applications main routine running, so
> we're never actually going to get close to parity there, nor do we need to,
> because the actual init happens at rte_eal_dev_init, which is in parity for both
> static and dynamic drivers.
> 
> > 
> > I know this change comes really late for 2.0.
> > I am open to other ideas but I don't want to see more #ifdef <some feature>
> > in eal.c (especially for a pmd), this is a non sense.
> > 
> > I would say that at least the patch 2 is needed for 2.0 : it fixes the
> > static case, but without patch 1 virtio pmd triggers a segfault on
> > interrupt receipt when built as a dso.
> > 
> The static case suffers from problems as well I think, in that its possible to
> architect multiple processes that are not started from fork that use the same
> pmd, which would create the same issue.  I think a better course of action would
> be to document the need for an application to call iopl before rte_eal_init.
> 

Given all this, I recommend that Thomas not apply this patch.
Please resubmit if there is a real problem with drivers (something in tree).
There are enough other bugs to fix without chasing ghosts.