On Wed, Apr 10, 2019 at 02:57:47PM -0300, Jason Gunthorpe wrote:
> On Wed, Apr 10, 2019 at 08:54:14PM +0300, Leon Romanovsky wrote:
>
> > > infiniband/core uses the
> > > /*
> > > * foo
> > > */
> > >
> > > Style of multi-line comment for some reason.
> >
> > It is preferable coding style for whole kernel, except netdev
> > and Parav used netdev coding style.
> >
> > During my reviews, I'm allowing to use netdev codding so people will be
> > able easily submit their patches to netdev without need to respin due to
> > differences in coding style.
>
> Lets try and keep infiniband/core in the mainstream style..
>
> > > > +static int __rdma_dev_change_netns(struct ib_device *device, struct net *net)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + /* If the refcount is zero, it means disable_device() has progressed
> > > > + * first during unregistration phase. There is no point in changing net
> > > > + * namespace of device which is on path of removal.
> > > > + */
> > > > + if (refcount_read(&device->refcount) == 0)
> > > > + return -ENODEV;
> > > > +
> > > > + disable_device(device);
> > > > +
> > > > + /* At this point no one would be using the device,
> > > > + * so it is safe to change the device net namespace.
> > > > + */
> > > > + write_pnet(&device->coredev.rdma_net, net);
> > > > + /* Currently rdma devices are system wide unique. So device
> > > > + * name is guaranteed free in new namespace. Publish new
> > > > + * namespace at sysfs level.
> > > > + */
> > > > + ret = device_rename(&device->dev, dev_name(&device->dev));
> > > > + if (ret) {
> > > > + dev_warn(&device->dev, "%s Couldn't rename device\n", __func__);
> > > > + return ret;
> > >
> > > Hurm.. Technically we are supposed to hold the devices_rwsem if we are
> > > manipulating the name. Inferring that we don't have to because the
> > > refcount is 0 is a little over optimizing for my tastes
> > >
> > > .. and if this fails the device is now permanently stuck with a 0
> > > refcount.. So we should re-enable it anyhow. The sysfs will be screwed
> > > up but what can you do..
> >
> > Why? We will enter device_rename only in case refcount != 0, see check
> > above.
>
> disable_device() makes it 0.
Thanks, I went down the stack and found the place where it happens.
>
> Jason
Attachment:
signature.asc
Description: PGP signature
