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.