On Tue, 10 Mar 2015 06:55:41 -0400 Neil Horman <nhorman at 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 at 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.
