Re: [PATCH rdma-next 5/9] RDMA/core: Restrict sysfs entries view to init_net

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 27, 2018 at 07:44:16PM +0000, Parav Pandit wrote:
> 
> 
> > From: Jason Gunthorpe <jgg@xxxxxxxx>
> > Sent: Monday, November 26, 2018 11:42 AM
> > To: Parav Pandit <parav@xxxxxxxxxxxx>
> > Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford
> > <dledford@xxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA
> > mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Daniel Jurgens
> > <danielj@xxxxxxxxxxxx>
> > Subject: Re: [PATCH rdma-next 5/9] RDMA/core: Restrict sysfs entries view to
> > init_net
> > 
> > On Mon, Nov 26, 2018 at 04:10:06AM +0000, Parav Pandit wrote:
> > >
> > >
> > > > From: Jason Gunthorpe <jgg@xxxxxxxx>
> > > > Sent: Sunday, November 25, 2018 9:16 PM
> > > > To: Parav Pandit <parav@xxxxxxxxxxxx>
> > > > Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford
> > > > <dledford@xxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxxxx>;
> > RDMA
> > > > mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Daniel Jurgens
> > > > <danielj@xxxxxxxxxxxx>
> > > > Subject: Re: [PATCH rdma-next 5/9] RDMA/core: Restrict sysfs entries
> > > > view to init_net
> > > >
> > > > On Sat, Nov 24, 2018 at 02:18:24PM +0000, Parav Pandit wrote:
> > > >
> > > > > > >  static struct class ib_class = {
> > > > > > > -	.name    = "infiniband",
> > > > > > > -	.dev_release = ib_device_release,
> > > > > > > -	.dev_uevent = ib_device_uevent,
> > > > > > > +	.name		= "infiniband",
> > > > > > > +	.dev_release	= ib_device_release,
> > > > > > > +	.dev_uevent	= ib_device_uevent,
> > > > > > > +	.ns_type	= &net_ns_type_operations,
> > > > > > > +	.namespace	= net_namespace,
> > > > > > >  };
> > > > > >
> > > > > > and lets not reformat code just to add horizonal whitespace please.
> > > > > >
> > > > > I see. 4 out of the 5 fields of structure follows right alignment,
> > > > > but keep old
> > > > one as is?
> > > > > But ok.
> > > >
> > > > Don't do vertical alignment then only 1 out of 5 is wrong.
> > > >
> > > Ok.
> > >
> > > > > > > +fs_initcall(ib_core_init);
> > > > > >
> > > > > > Really? So very strange.
> > > > > >
> > > > > Yes. when module is compile as in-built to kernel, init sequence
> > > > > needs first
> > > > initialize the net's init function first.
> > > > > Otherwise ib_core_init gets called and accesses uninitialized
> > > > net_ns_operations.
> > > > > This is similar to 80211 net/wireless/core.c
> > > >
> > > > And subsyste_late_initcall() is not OK?
> > > >
> > > late_initcall() is after device_initcall().
> > > Apparently commit a9cd1a673737 needs before device initialization.
> > > So late_initcall() is late and bug fixed in  commit a9cd1a673737 will surface
> > again.
> > > So that is why fs_initcall is chosen, after net init and before device init.
> > 
> > This is exactly the sort of thing I'm worried about.. We have FS's that use
> > RDMA, it is scary to have RDMA init at the same level as FS..
> 
> If there are two modules at same level, one depend on another,
> sequencing their init should be done by core kernel.  I changed
> ib_umad.ko to fs_initcall for an experiment along with init debug
> logs and I see below trace.

That only works if you are using modules, it doesn't work the same if
it is linked in with no modules. In fact modules delete all init call
ordering levels..

> I do not know how is this magic done, but seems like intuitive as
> later module have symbols dependency on previous one.  

It happens as a consequence of module load ordering, initcalls in
one module complete before the next module starts loading..

Jason



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux