| Message ID | 20250226033526.2769817-1-roman.gushchin@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] net: RDMA: explicitly enumerate ib_device attribute groups | expand |
> From: Roman Gushchin <roman.gushchin@linux.dev> > Sent: Wednesday, February 26, 2025 9:05 AM > > Explicitly enumerate ib_device's attribute groups. > > Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev> > Cc: Jason Gunthorpe <jgg@ziepe.ca> > Cc: Leon Romanovsky <leon@kernel.org> > Cc: Maher Sanalla <msanalla@nvidia.com> > Cc: Parav Pandit <parav@mellanox.com> > Cc: linux-rdma@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > drivers/infiniband/core/device.c | 4 ++-- > include/rdma/ib_verbs.h | 14 ++++++++------ > 2 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > index 0ded91f056f3..8dea307addf1 100644 > --- a/drivers/infiniband/core/device.c > +++ b/drivers/infiniband/core/device.c > @@ -1404,8 +1404,8 @@ int ib_register_device(struct ib_device *device, > const char *name, > return ret; > } > > - device->groups[0] = &ib_dev_attr_group; > - device->groups[1] = device->ops.device_group; > + device->groups[IB_ATTR_GROUP_DEV_ATTR] = &ib_dev_attr_group; > + device->groups[IB_ATTR_GROUP_DRIVER_ATTR] = device- > >ops.device_group; > ret = ib_setup_device_attrs(device); This function may initialize the hw_stats at the index IB_ATTR_GROUP_DRIVER_ATTR when it is optional. It runs through a for loop to find the first free entry. This needs to be captured in the comment above, and it will have effect on your next fix patch. > if (ret) > goto cache_cleanup; > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index > b59bf30de430..9c4c4238e6fc 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -2728,6 +2728,13 @@ struct ib_core_device { > struct ib_device *owner; /* reach back to owner ib_device */ }; > > +enum ib_device_attr_group { > + IB_ATTR_GROUP_DEV_ATTR = 0, /* Device attributes */ > + IB_ATTR_GROUP_DRIVER_ATTR = 1, /* Driver-provided attributes > */ > + IB_ATTR_GROUP_HW_STATS = 2, /* hw_stats */ > + IB_ATTR_GROUP_LAST, /* NULL pointer terminating array */ > +}; > + > struct rdma_restrack_root; > struct ib_device { > /* Do not access @dma_device directly from ULP nor from HW > drivers. */ @@ -2761,12 +2768,7 @@ struct ib_device { > struct ib_core_device coredev; > }; > > - /* First group is for device attributes, > - * Second group is for driver provided attributes (optional). This text 'optional' is missing in above enums, please add it. Also please add the optional text comment for hw_stats too, as its optional too. > - * Third group is for the hw_stats > - * It is a NULL terminated array. > - */ > - const struct attribute_group *groups[4]; > + const struct attribute_group *groups[IB_ATTR_GROUP_LAST + 1]; > > u64 uverbs_cmd_mask; > > -- > 2.48.1.658.g4767266eb4-goog
On Wed, Feb 26, 2025 at 03:35:25AM +0000, Roman Gushchin wrote: > Explicitly enumerate ib_device's attribute groups. > > Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev> > Cc: Jason Gunthorpe <jgg@ziepe.ca> > Cc: Leon Romanovsky <leon@kernel.org> > Cc: Maher Sanalla <msanalla@nvidia.com> > Cc: Parav Pandit <parav@mellanox.com> > Cc: linux-rdma@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > drivers/infiniband/core/device.c | 4 ++-- > include/rdma/ib_verbs.h | 14 ++++++++------ > 2 files changed, 10 insertions(+), 8 deletions(-) Very minor comment as you will need to address Parav's comments anyway. The title should be "RDMA/core: Explicitly ..." and not as it is written. Thanks
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index 0ded91f056f3..8dea307addf1 100644 --- a/drivers/infiniband/core/device.c +++ b/drivers/infiniband/core/device.c @@ -1404,8 +1404,8 @@ int ib_register_device(struct ib_device *device, const char *name, return ret; } - device->groups[0] = &ib_dev_attr_group; - device->groups[1] = device->ops.device_group; + device->groups[IB_ATTR_GROUP_DEV_ATTR] = &ib_dev_attr_group; + device->groups[IB_ATTR_GROUP_DRIVER_ATTR] = device->ops.device_group; ret = ib_setup_device_attrs(device); if (ret) goto cache_cleanup; diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index b59bf30de430..9c4c4238e6fc 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -2728,6 +2728,13 @@ struct ib_core_device { struct ib_device *owner; /* reach back to owner ib_device */ }; +enum ib_device_attr_group { + IB_ATTR_GROUP_DEV_ATTR = 0, /* Device attributes */ + IB_ATTR_GROUP_DRIVER_ATTR = 1, /* Driver-provided attributes */ + IB_ATTR_GROUP_HW_STATS = 2, /* hw_stats */ + IB_ATTR_GROUP_LAST, /* NULL pointer terminating array */ +}; + struct rdma_restrack_root; struct ib_device { /* Do not access @dma_device directly from ULP nor from HW drivers. */ @@ -2761,12 +2768,7 @@ struct ib_device { struct ib_core_device coredev; }; - /* First group is for device attributes, - * Second group is for driver provided attributes (optional). - * Third group is for the hw_stats - * It is a NULL terminated array. - */ - const struct attribute_group *groups[4]; + const struct attribute_group *groups[IB_ATTR_GROUP_LAST + 1]; u64 uverbs_cmd_mask;
Explicitly enumerate ib_device's attribute groups. Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: Leon Romanovsky <leon@kernel.org> Cc: Maher Sanalla <msanalla@nvidia.com> Cc: Parav Pandit <parav@mellanox.com> Cc: linux-rdma@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/infiniband/core/device.c | 4 ++-- include/rdma/ib_verbs.h | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-)