On Wed, Sep 26, 2018 at 10:34:46AM -0400, Dennis Dalessandro wrote: > On 9/26/2018 10:18 AM, Leon Romanovsky wrote: > > On Wed, Sep 26, 2018 at 01:30:21PM +0000, Ruhl, Michael J wrote: > > > > -----Original Message----- > > > > From: linux-rdma-owner@xxxxxxxxxxxxxxx [mailto:linux-rdma- > > > > owner@xxxxxxxxxxxxxxx] On Behalf Of Leon Romanovsky > > > > Sent: Wednesday, September 26, 2018 2:46 AM > > > > To: Doug Ledford <dledford@xxxxxxxxxx>; Jason Gunthorpe > > > > <jgg@xxxxxxxxxxxx> > > > > Cc: Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA mailing list <linux- > > > > rdma@xxxxxxxxxxxxxxx> > > > > Subject: [PATCH rdma-next v2 1/2] RDMA/core: Implement IB device rename > > > > function > > > > > > > > From: Leon Romanovsky <leonro@xxxxxxxxxxxx> > > > > > > > > Generic implementation of IB device rename function. > > > > > > > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx> > > > > --- > > > > drivers/infiniband/core/core_priv.h | 1 + > > > > drivers/infiniband/core/device.c | 25 +++++++++++++++++++++++++ > > > > 2 files changed, 26 insertions(+) > > > > > > > > diff --git a/drivers/infiniband/core/core_priv.h > > > > b/drivers/infiniband/core/core_priv.h > > > > index f60c7b65aea9..bb9007a0cca7 100644 > > > > --- a/drivers/infiniband/core/core_priv.h > > > > +++ b/drivers/infiniband/core/core_priv.h > > > > @@ -87,6 +87,7 @@ int ib_device_register_sysfs(struct ib_device *device, > > > > int (*port_callback)(struct ib_device *, > > > > u8, struct kobject *)); > > > > void ib_device_unregister_sysfs(struct ib_device *device); > > > > +int ib_device_rename(struct ib_device *ibdev, const char *name); > > > > > > > > typedef void (*roce_netdev_callback)(struct ib_device *device, u8 port, > > > > struct net_device *idev, void *cookie); > > > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > > > > index d105b9b2d118..5e70f5e1cfd9 100644 > > > > --- a/drivers/infiniband/core/device.c > > > > +++ b/drivers/infiniband/core/device.c > > > > @@ -171,6 +171,31 @@ static struct ib_device > > > > *__ib_device_get_by_name(const char *name) > > > > return NULL; > > > > } > > > > > > > > +int ib_device_rename(struct ib_device *ibdev, const char *name) > > > > +{ > > > > + struct ib_device *device; > > > > + int ret = 0; > > > > + > > > > + if (!strcmp(name, dev_name(&ibdev->dev))) > > > > + return ret; > > > > + > > > > + mutex_lock(&device_mutex); > > > > + list_for_each_entry(device, &device_list, core_list) { > > > > + if (!strcmp(name, dev_name(&device->dev))) { > > > > + ret = -EEXIST; > > > > + goto out; > > > > + } > > > > + } > > > > + > > > > + ret = device_rename(&ibdev->dev, name); > > > > > > The comments for device_rename() say: > > > > > > * Note: Don't call this function. > > > > > > Is this a concern? > > > > I wrote about it in cover letter: > > "This functionality is done through RDMA netlink and implemented by > > device_rename() function, despite the comment from 2010, which warns > > about downsides of this function, the netdev is still uses, so we will > > use too." > > > > I think that it is ok to use as long as netdev uses it and no one > > complained about it :) > > So what are the "downsides" to calling this function? I think you should > mention that in the commit message and make the justification for why > this is OK rather than, someone else did so we can to. Actually, I reread again the comment above device_rename() and think that "downsides" mentioned there can be races with symlinks only. We are holding lock which prevent addition of new ib_device with same name, so from name point of view, we are safe. Regarding comment, I don't know what else can I add to comment in device_rename() section. Thanks > > -Denny >
Attachment:
signature.asc
Description: PGP signature
