| Message ID | 1544769162-18157-1-git-send-email-parav@mellanox.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d5108e69fe013ff47ab815b849caba9cc33ca1e5 |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [PATCHv1] IB/rxe: Make counters thread safe | expand |
On Fri, Dec 14, 2018 at 12:32:42AM -0600, Parav Pandit wrote: > Current rxe device counters are not thread safe. > When multiple QPs are used, they can be racy. > Make them thread safe by making it atomic64. > > Fixes: 0b1e5b99a48b ("IB/rxe: Add port protocol stats") > Signed-off-by: Parav Pandit <parav@mellanox.com> > --- > drivers/infiniband/sw/rxe/rxe_hw_counters.c | 2 +- > drivers/infiniband/sw/rxe/rxe_verbs.h | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_hw_counters.c b/drivers/infiniband/sw/rxe/rxe_hw_counters.c > index 4a24895..636edb5 100644 > --- a/drivers/infiniband/sw/rxe/rxe_hw_counters.c > +++ b/drivers/infiniband/sw/rxe/rxe_hw_counters.c > @@ -62,7 +62,7 @@ int rxe_ib_get_hw_stats(struct ib_device *ibdev, > return -EINVAL; > > for (cnt = 0; cnt < ARRAY_SIZE(rxe_counter_name); cnt++) > - stats->value[cnt] = dev->stats_counters[cnt]; > + stats->value[cnt] = atomic64_read(&dev->stats_counters[cnt]); atomic64_read receives atomic64_t as an input and not u64 as declared by stats_counters. lib/atomic64.c: long long atomic64_read(const atomic64_t *v) Thanks
> -----Original Message----- > From: Leon Romanovsky <leon@kernel.org> > Sent: Sunday, December 16, 2018 12:58 AM > To: Parav Pandit <parav@mellanox.com> > Cc: linux-rdma@vger.kernel.org; jgg@ziepe.ca; dledford@redhat.com; Moni > Shoua <monis@mellanox.com> > Subject: Re: [PATCHv1] IB/rxe: Make counters thread safe > > On Fri, Dec 14, 2018 at 12:32:42AM -0600, Parav Pandit wrote: > > Current rxe device counters are not thread safe. > > When multiple QPs are used, they can be racy. > > Make them thread safe by making it atomic64. > > > > Fixes: 0b1e5b99a48b ("IB/rxe: Add port protocol stats") > > Signed-off-by: Parav Pandit <parav@mellanox.com> > > --- > > drivers/infiniband/sw/rxe/rxe_hw_counters.c | 2 +- > > drivers/infiniband/sw/rxe/rxe_verbs.h | 6 +++--- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_hw_counters.c > b/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > index 4a24895..636edb5 100644 > > --- a/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > +++ b/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > @@ -62,7 +62,7 @@ int rxe_ib_get_hw_stats(struct ib_device *ibdev, > > return -EINVAL; > > > > for (cnt = 0; cnt < ARRAY_SIZE(rxe_counter_name); cnt++) > > - stats->value[cnt] = dev->stats_counters[cnt]; > > + stats->value[cnt] = atomic64_read(&dev- > >stats_counters[cnt]); > > atomic64_read receives atomic64_t as an input and not u64 as declared by > stats_counters. I didn't follow you. In this patch, in rxe_verbs.h stats_counters of the device is declared as atomic64_t.
On Sun, Dec 16, 2018 at 07:19:45AM +0000, Parav Pandit wrote: > > > > -----Original Message----- > > From: Leon Romanovsky <leon@kernel.org> > > Sent: Sunday, December 16, 2018 12:58 AM > > To: Parav Pandit <parav@mellanox.com> > > Cc: linux-rdma@vger.kernel.org; jgg@ziepe.ca; dledford@redhat.com; Moni > > Shoua <monis@mellanox.com> > > Subject: Re: [PATCHv1] IB/rxe: Make counters thread safe > > > > On Fri, Dec 14, 2018 at 12:32:42AM -0600, Parav Pandit wrote: > > > Current rxe device counters are not thread safe. > > > When multiple QPs are used, they can be racy. > > > Make them thread safe by making it atomic64. > > > > > > Fixes: 0b1e5b99a48b ("IB/rxe: Add port protocol stats") > > > Signed-off-by: Parav Pandit <parav@mellanox.com> > > > --- > > > drivers/infiniband/sw/rxe/rxe_hw_counters.c | 2 +- > > > drivers/infiniband/sw/rxe/rxe_verbs.h | 6 +++--- > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > b/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > > index 4a24895..636edb5 100644 > > > --- a/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > > +++ b/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > > @@ -62,7 +62,7 @@ int rxe_ib_get_hw_stats(struct ib_device *ibdev, > > > return -EINVAL; > > > > > > for (cnt = 0; cnt < ARRAY_SIZE(rxe_counter_name); cnt++) > > > - stats->value[cnt] = dev->stats_counters[cnt]; > > > + stats->value[cnt] = atomic64_read(&dev- > > >stats_counters[cnt]); > > > > atomic64_read receives atomic64_t as an input and not u64 as declared by > > stats_counters. > I didn't follow you. In this patch, in rxe_verbs.h stats_counters of the device is declared as atomic64_t. What about stats->value[cnt]?
> -----Original Message----- > From: Leon Romanovsky <leon@kernel.org> > Sent: Sunday, December 16, 2018 2:11 AM > To: Parav Pandit <parav@mellanox.com> > Cc: linux-rdma@vger.kernel.org; jgg@ziepe.ca; dledford@redhat.com; Moni > Shoua <monis@mellanox.com> > Subject: Re: [PATCHv1] IB/rxe: Make counters thread safe > > On Sun, Dec 16, 2018 at 07:19:45AM +0000, Parav Pandit wrote: > > > > > > > -----Original Message----- > > > From: Leon Romanovsky <leon@kernel.org> > > > Sent: Sunday, December 16, 2018 12:58 AM > > > To: Parav Pandit <parav@mellanox.com> > > > Cc: linux-rdma@vger.kernel.org; jgg@ziepe.ca; dledford@redhat.com; > > > Moni Shoua <monis@mellanox.com> > > > Subject: Re: [PATCHv1] IB/rxe: Make counters thread safe > > > > > > On Fri, Dec 14, 2018 at 12:32:42AM -0600, Parav Pandit wrote: > > > > Current rxe device counters are not thread safe. > > > > When multiple QPs are used, they can be racy. > > > > Make them thread safe by making it atomic64. > > > > > > > > Fixes: 0b1e5b99a48b ("IB/rxe: Add port protocol stats") > > > > Signed-off-by: Parav Pandit <parav@mellanox.com> > > > > --- > > > > drivers/infiniband/sw/rxe/rxe_hw_counters.c | 2 +- > > > > drivers/infiniband/sw/rxe/rxe_verbs.h | 6 +++--- > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > > b/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > > > index 4a24895..636edb5 100644 > > > > --- a/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > > > +++ b/drivers/infiniband/sw/rxe/rxe_hw_counters.c > > > > @@ -62,7 +62,7 @@ int rxe_ib_get_hw_stats(struct ib_device *ibdev, > > > > return -EINVAL; > > > > > > > > for (cnt = 0; cnt < ARRAY_SIZE(rxe_counter_name); cnt++) > > > > - stats->value[cnt] = dev->stats_counters[cnt]; > > > > + stats->value[cnt] = atomic64_read(&dev- > > > >stats_counters[cnt]); > > > > > > atomic64_read receives atomic64_t as an input and not u64 as > > > declared by stats_counters. > > I didn't follow you. In this patch, in rxe_verbs.h stats_counters of the > device is declared as atomic64_t. > > What about stats->value[cnt]? atomic64_read() returns long long which is 64-bit on 64-bit and 32-bit platform. stats->value[cnt] is u64 which matches what atomic64_read() returns.
diff --git a/drivers/infiniband/sw/rxe/rxe_hw_counters.c b/drivers/infiniband/sw/rxe/rxe_hw_counters.c index 4a24895..636edb5 100644 --- a/drivers/infiniband/sw/rxe/rxe_hw_counters.c +++ b/drivers/infiniband/sw/rxe/rxe_hw_counters.c @@ -62,7 +62,7 @@ int rxe_ib_get_hw_stats(struct ib_device *ibdev, return -EINVAL; for (cnt = 0; cnt < ARRAY_SIZE(rxe_counter_name); cnt++) - stats->value[cnt] = dev->stats_counters[cnt]; + stats->value[cnt] = atomic64_read(&dev->stats_counters[cnt]); return ARRAY_SIZE(rxe_counter_name); } diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h index 831381b..74e0480 100644 --- a/drivers/infiniband/sw/rxe/rxe_verbs.h +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h @@ -409,16 +409,16 @@ struct rxe_dev { spinlock_t mmap_offset_lock; /* guard mmap_offset */ int mmap_offset; - u64 stats_counters[RXE_NUM_OF_COUNTERS]; + atomic64_t stats_counters[RXE_NUM_OF_COUNTERS]; struct rxe_port port; struct list_head list; struct crypto_shash *tfm; }; -static inline void rxe_counter_inc(struct rxe_dev *rxe, enum rxe_counters cnt) +static inline void rxe_counter_inc(struct rxe_dev *rxe, enum rxe_counters index) { - rxe->stats_counters[cnt]++; + atomic64_inc(&rxe->stats_counters[index]); } static inline struct rxe_dev *to_rdev(struct ib_device *dev)
Current rxe device counters are not thread safe. When multiple QPs are used, they can be racy. Make them thread safe by making it atomic64. Fixes: 0b1e5b99a48b ("IB/rxe: Add port protocol stats") Signed-off-by: Parav Pandit <parav@mellanox.com> --- drivers/infiniband/sw/rxe/rxe_hw_counters.c | 2 +- drivers/infiniband/sw/rxe/rxe_verbs.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)