Re: [PATCH 1/1] IB: rxe: replace ARRAYSIZE with RXE_NUM_OF_COUNTERS

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

 




On 2018/12/16 10:16, Bart Van Assche wrote:
On 12/15/18 5:30 PM, Yanjun Zhu wrote:

On 2018/12/15 23:43, Bart Van Assche wrote:
On 12/15/18 12:09 AM, Zhu Yanjun wrote:
RXE_NUM_OF_COUNTERS is the number of counters. So it can be used
directly.

Signed-off-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxxx>
---
  drivers/infiniband/sw/rxe/rxe_hw_counters.c | 7 +++----
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_hw_counters.c b/drivers/infiniband/sw/rxe/rxe_hw_counters.c
index 6aeb7a165e46..8df4b1592d6d 100644
--- a/drivers/infiniband/sw/rxe/rxe_hw_counters.c
+++ b/drivers/infiniband/sw/rxe/rxe_hw_counters.c
@@ -58,21 +58,20 @@ int rxe_ib_get_hw_stats(struct ib_device *ibdev,
      if (!port || !stats)
          return -EINVAL;
  -    for (cnt = 0; cnt  < ARRAY_SIZE(rxe_counter_name); cnt++)
+    for (cnt = 0; cnt  < RXE_NUM_OF_COUNTERS; cnt++)
          stats->value[cnt] = dev->stats_counters[cnt];
  -    return ARRAY_SIZE(rxe_counter_name);
+    return RXE_NUM_OF_COUNTERS;
  }
    struct rdma_hw_stats *rxe_ib_alloc_hw_stats(struct ib_device *ibdev,
                          u8 port_num)
  {
-    BUILD_BUG_ON(ARRAY_SIZE(rxe_counter_name) != RXE_NUM_OF_COUNTERS);
      /* We support only per port stats */
      if (!port_num)
          return NULL;
        return rdma_alloc_hw_stats_struct(rxe_counter_name,
-                      ARRAY_SIZE(rxe_counter_name),
+                      RXE_NUM_OF_COUNTERS,
                        RDMA_HW_STATS_DEFAULT_LIFESPAN);
  }

Have you considered to make the opposite change, namely to remove the RXE_NUM_OF_COUNTERS constant and to use ARRAY_SIZE(rxe_counter_name) everywhere instead?

Interesting. Can you give me a reason? Why ARRAY_SIZE is more important than RXE_NUM_OF_COUNTERS?

Hi Yanjun,

Using ARRAY_SIZE() makes it easy to verify correctness of code that uses that macro because it is guaranteed that the ARRAY_SIZE() result matches the array size. When using a macro however an additional lookup is required to verify whether the value of that macro really matches the array size. I think in the Linux kernel using ARRAY_SIZE() is preferred instead of introducing an explicit define for the array size.

Thanks. Your concerns sound reasonable. I will send V2 soon.


Zhu Yanjun


Bart.



[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