On 5/18/2021 12:47 AM, Jason Gunthorpe wrote:
External email: Use caution opening links or attachments This is being used to implement both the port and device global stats, which is causing some confusion in the drivers. For instance EFA and i40iw both seem to be misusing the device stats. Split it into two ops so drivers that don't support one or the other can leave the op NULL'd, making the calling code a little simpler to understand. Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx> --- drivers/infiniband/core/counters.c | 4 +- drivers/infiniband/core/device.c | 3 +- drivers/infiniband/core/nldev.c | 2 +- drivers/infiniband/core/sysfs.c | 15 +++- drivers/infiniband/hw/bnxt_re/hw_counters.c | 7 +- drivers/infiniband/hw/bnxt_re/hw_counters.h | 4 +- drivers/infiniband/hw/bnxt_re/main.c | 2 +- drivers/infiniband/hw/cxgb4/provider.c | 9 +-- drivers/infiniband/hw/efa/efa.h | 3 +- drivers/infiniband/hw/efa/efa_main.c | 3 +- drivers/infiniband/hw/efa/efa_verbs.c | 11 ++- drivers/infiniband/hw/hfi1/verbs.c | 86 ++++++++++----------- drivers/infiniband/hw/i40iw/i40iw_verbs.c | 19 ++++- drivers/infiniband/hw/mlx4/main.c | 25 ++++-- drivers/infiniband/hw/mlx5/counters.c | 42 +++++++--- drivers/infiniband/sw/rxe/rxe_hw_counters.c | 7 +- drivers/infiniband/sw/rxe/rxe_hw_counters.h | 4 +- drivers/infiniband/sw/rxe/rxe_verbs.c | 2 +- include/rdma/ib_verbs.h | 13 ++-- 19 files changed, 158 insertions(+), 103 deletions(-)
[...]
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c index 05b702de00e89b..29082d8d45fc4f 100644 --- a/drivers/infiniband/core/sysfs.c +++ b/drivers/infiniband/core/sysfs.c @@ -981,8 +981,15 @@ static void setup_hw_stats(struct ib_device *device, struct ib_port *port, struct rdma_hw_stats *stats; int i, ret; - stats = device->ops.alloc_hw_stats(device, port_num); - + if (port_num) { + if (!device->ops.alloc_hw_port_stats) + return;
Do we need this "if (!device->ops.alloc_hw_port_stats)" here?
+ stats = device->ops.alloc_hw_port_stats(device, port_num); + } else { + if (!device->ops.alloc_hw_device_stats) + return;
And here?
+ stats = device->ops.alloc_hw_device_stats(device); + } if (!stats) return; @@ -1165,7 +1172,7 @@ static int add_port(struct ib_core_device *coredev, int port_num) * port, so holder should be device. Therefore skip per port conunter * initialization. */ - if (device->ops.alloc_hw_stats && port_num && is_full_dev) + if (device->ops.alloc_hw_port_stats && port_num && is_full_dev) setup_hw_stats(device, p, port_num); list_add_tail(&p->kobj.entry, &coredev->port_list); @@ -1409,7 +1416,7 @@ int ib_device_register_sysfs(struct ib_device *device) if (ret) return ret; - if (device->ops.alloc_hw_stats) + if (device->ops.alloc_hw_device_stats) setup_hw_stats(device, NULL, 0); return 0;
[...]
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c index 3f1893e180ddf3..c0f01799f4a0b9 100644 --- a/drivers/infiniband/hw/cxgb4/provider.c +++ b/drivers/infiniband/hw/cxgb4/provider.c @@ -377,14 +377,11 @@ static const char * const names[] = { [IP6OUTRSTS] = "ip6OutRsts" }; -static struct rdma_hw_stats *c4iw_alloc_stats(struct ib_device *ibdev, - u32 port_num) +static struct rdma_hw_stats *c4iw_alloc_port_stats(struct ib_device *ibdev, + u32 port_num) { BUILD_BUG_ON(ARRAY_SIZE(names) != NR_COUNTERS); - if (port_num != 0) - return NULL; -
I'm not familiar with this driver, but if port_num must be 0 here, does it mean this is per-device not per-port?