RE: [PATCH rdma-next v3 07/10] RDMA/core: Implement compat device/sysfs tree in net namespace

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

 




> -----Original Message-----
> From: Jason Gunthorpe <jgg@xxxxxxxx>
> Sent: Friday, January 11, 2019 5:31 PM
> To: Leon Romanovsky <leon@xxxxxxxxxx>
> Cc: Doug Ledford <dledford@xxxxxxxxxx>; Leon Romanovsky
> <leonro@xxxxxxxxxxxx>; RDMA mailing list <linux-rdma@xxxxxxxxxxxxxxx>;
> Parav Pandit <parav@xxxxxxxxxxxx>
> Subject: Re: [PATCH rdma-next v3 07/10] RDMA/core: Implement compat
> device/sysfs tree in net namespace
> 
> On Tue, Dec 18, 2018 at 02:16:01PM +0200, Leon Romanovsky wrote:
> > +static __net_init int rdma_dev_init_net(struct net *net) {
> > +	struct rdma_dev_net *rdma_net =	net_generic(net,
> rdma_dev_net_id);
> > +	struct ib_device *device;
> > +
> > +	INIT_LIST_HEAD(&rdma_net->compatdev_list);
> > +	init_rwsem(&rdma_net->compat_rwsem);
> > +
> > +	/* No need to create any compat devices in init_net. */
> > +	if (net_eq(net, &init_net))
> > +		return 0;
> > +
> > +	/* Hold device mutex to synchronize with ib_register_device()
> > +	 * which also tries to add compat devices.
> > +	 */
> > +	mutex_lock(&ib_device_mutex);
> > +	/* Hold ib_lists_rwsem read lock; thereby not assume that
> > +	 * ib_device_mutex is always locked while accessing ib_device_list.
> > +	 */
> > +	down_read(&ib_lists_rwsem);
> > +	list_for_each_entry(device, &ib_device_list, core_list)
> > +		rdma_compatdev_create(device, net);
> > +	up_read(&ib_lists_rwsem);
> > +	mutex_unlock(&ib_device_mutex);
> 
> This is racey
> 
No. it is race free.

rdma_dev_init_net() holds ib_device_mutex and ib_register_device().
mutex synchronizes it as described in Documentation and in comment in init_net() above.
Will discuss that in other thread where you have comment for the lock.

>   CPU 0                                           CPU 1
> setup_net()
>    rdma_dev_init_net()
>                                                  ib_device_register()
>    list_add_tail_rcu(&net->list..)
> 
> Only thing I can think of it is to have our own list of current nets, with our
> own locking and do not use for_each_net()
No. I would prefer to avoid such list. Creating another list and protecting it using some different lock doesn't offer much gain.
Let's discuss in other thread where there are more comments for lock.
> 
> Again, ib_device_mutex is not intended as a registration lock, and should
> not be used as such.
> 
> 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