Re: [PATCHv2 for-next 13/19] RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight

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

 



On Wed, May 26, 2021 at 11:38 AM Jinpu Wang <jinpu.wang@xxxxxxxxx> wrote:
>
> On Wed, May 26, 2021 at 11:17 AM Haris Iqbal <haris.iqbal@xxxxxxxxx> wrote:
> >
> > On Tue, May 25, 2021 at 10:15 PM Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
> > >
> > > On Mon, May 17, 2021 at 11:18:37AM +0200, Gioh Kim wrote:
> > > > From: Md Haris Iqbal <haris.iqbal@xxxxxxxxxxxxxxx>
> > > >
> > > > ids_inflight is used to track the inflight IOs. But the use of atomic_t
> > > > variable can cause performance drops and can also become a performance
> > > > bottleneck.
> > > >
> > > > This commit replaces the use of atomic_t with a percpu_ref structure. The
> > > > advantage it offers is, it doesn't check if the reference has fallen to 0,
> > > > until the user explicitly signals it to; and that is done by the
> > > > percpu_ref_kill() function call. After that, the percpu_ref structure
> > > > behaves like an atomic_t and for every put call, checks whether the
> > > > reference has fallen to 0 or not.
> > > >
> > > > rtrs_srv_stats_rdma_to_str shows the count of ids_inflight as 0
> > > > for user-mode tools not to be confused.
> > > >
> > > > Fixes: 9cb837480424e ("RDMA/rtrs: server: main functionality")
> > > > Signed-off-by: Md Haris Iqbal <haris.iqbal@xxxxxxxxx>
> > > > Signed-off-by: Jack Wang <jinpu.wang@xxxxxxxxx>
> > > > Signed-off-by: Gioh Kim <gi-oh.kim@xxxxxxxxx>
> > > > ---
> > > >  drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c | 12 +++---
> > > >  drivers/infiniband/ulp/rtrs/rtrs-srv.c       | 43 +++++++++++++-------
> > > >  drivers/infiniband/ulp/rtrs/rtrs-srv.h       |  4 +-
> > > >  3 files changed, 35 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > > > index e102b1368d0c..df1d7d6b1884 100644
> > > > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
> > > > @@ -27,12 +27,10 @@ ssize_t rtrs_srv_stats_rdma_to_str(struct rtrs_srv_stats *stats,
> > > >                                   char *page, size_t len)
> > > >  {
> > > >       struct rtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> > > > -     struct rtrs_srv_sess *sess = stats->sess;
> > > >
> > > > -     return scnprintf(page, len, "%lld %lld %lld %lld %u\n",
> > > > -                      (s64)atomic64_read(&r->dir[READ].cnt),
> > > > -                      (s64)atomic64_read(&r->dir[READ].size_total),
> > > > -                      (s64)atomic64_read(&r->dir[WRITE].cnt),
> > > > -                      (s64)atomic64_read(&r->dir[WRITE].size_total),
> > > > -                      atomic_read(&sess->ids_inflight));
> > > > +     return sysfs_emit(page, "%lld %lld %lld %lldn\n",
> > > > +                       (s64)atomic64_read(&r->dir[READ].cnt),
> > > > +                       (s64)atomic64_read(&r->dir[READ].size_total),
> > > > +                       (s64)atomic64_read(&r->dir[WRITE].cnt),
> > > > +                       (s64)atomic64_read(&r->dir[WRITE].size_total));
> > > >  }
> > >
> > > This seems like an unrelated hunk
> >
> > Previous to the commit, ids_inflight was an atomic variable, and hence
> > was read and printed along with other rtrs_srv stats. With this
> > commit, since ids_inflight is changed to percpu_ref, we removed it
> > being printed in rtrs_srv stats. Its related to the commit.
> I think Jason meant the sysfs_emit conversion should be in a separate patch.

Okay. I missed that.

Thanks; will send the updated patches.

> >
> >
> > >
> > > Jason



[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