diff mbox series

[v4,11/25] ibtrs: server: statistics functions

Message ID 20190620150337.7847-12-jinpuwang@gmail.com (mailing list archive)
State New, archived
Headers show
Series InfiniBand Transport (IBTRS) and Network Block Device (IBNBD) | expand

Commit Message

Jinpu Wang June 20, 2019, 3:03 p.m. UTC 
From: Roman Pen <roman.penyaev@profitbricks.com>

This introduces set of functions used on server side to account
statistics of RDMA data sent/received.

Signed-off-by: Danil Kipnis <danil.kipnis@cloud.ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 .../infiniband/ulp/ibtrs/ibtrs-srv-stats.c    | 103 ++++++++++++++++++
 1 file changed, 103 insertions(+)
 create mode 100644 drivers/infiniband/ulp/ibtrs/ibtrs-srv-stats.c

Comments

Bart Van Assche Sept. 23, 2019, 11:56 p.m. UTC | #1 
On 6/20/19 8:03 AM, Jack Wang wrote:
> +ssize_t ibtrs_srv_stats_rdma_to_str(struct ibtrs_srv_stats *stats,
> +				    char *page, size_t len)
> +{
> +	struct ibtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> +	struct ibtrs_srv_sess *sess;
> +
> +	sess = container_of(stats, typeof(*sess), stats);
> +
> +	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));
> +}

Does this follow the sysfs one-value-per-file rule? See also 
Documentation/filesystems/sysfs.txt.

Thanks,

Bart.
Jinpu Wang Oct. 2, 2019, 3:15 p.m. UTC | #2 
On Tue, Sep 24, 2019 at 1:56 AM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 6/20/19 8:03 AM, Jack Wang wrote:
> > +ssize_t ibtrs_srv_stats_rdma_to_str(struct ibtrs_srv_stats *stats,
> > +                                 char *page, size_t len)
> > +{
> > +     struct ibtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> > +     struct ibtrs_srv_sess *sess;
> > +
> > +     sess = container_of(stats, typeof(*sess), stats);
> > +
> > +     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));
> > +}
>
> Does this follow the sysfs one-value-per-file rule? See also
> Documentation/filesystems/sysfs.txt.
>
> Thanks,
>
> Bart.
It looks overkill to create one file for each value to me, and there
are enough stats in sysfs contain multiple values.

Thanks
Jinpu
Leon Romanovsky Oct. 2, 2019, 3:42 p.m. UTC | #3 
On Wed, Oct 02, 2019 at 05:15:10PM +0200, Jinpu Wang wrote:
> On Tue, Sep 24, 2019 at 1:56 AM Bart Van Assche <bvanassche@acm.org> wrote:
> >
> > On 6/20/19 8:03 AM, Jack Wang wrote:
> > > +ssize_t ibtrs_srv_stats_rdma_to_str(struct ibtrs_srv_stats *stats,
> > > +                                 char *page, size_t len)
> > > +{
> > > +     struct ibtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> > > +     struct ibtrs_srv_sess *sess;
> > > +
> > > +     sess = container_of(stats, typeof(*sess), stats);
> > > +
> > > +     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));
> > > +}
> >
> > Does this follow the sysfs one-value-per-file rule? See also
> > Documentation/filesystems/sysfs.txt.
> >
> > Thanks,
> >
> > Bart.
> It looks overkill to create one file for each value to me, and there
> are enough stats in sysfs contain multiple values.

Not for statistics.

Thanks

>
> Thanks
> Jinpu
Jinpu Wang Oct. 2, 2019, 3:45 p.m. UTC | #4 
On Wed, Oct 2, 2019 at 5:42 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Wed, Oct 02, 2019 at 05:15:10PM +0200, Jinpu Wang wrote:
> > On Tue, Sep 24, 2019 at 1:56 AM Bart Van Assche <bvanassche@acm.org> wrote:
> > >
> > > On 6/20/19 8:03 AM, Jack Wang wrote:
> > > > +ssize_t ibtrs_srv_stats_rdma_to_str(struct ibtrs_srv_stats *stats,
> > > > +                                 char *page, size_t len)
> > > > +{
> > > > +     struct ibtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> > > > +     struct ibtrs_srv_sess *sess;
> > > > +
> > > > +     sess = container_of(stats, typeof(*sess), stats);
> > > > +
> > > > +     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));
> > > > +}
> > >
> > > Does this follow the sysfs one-value-per-file rule? See also
> > > Documentation/filesystems/sysfs.txt.
> > >
> > > Thanks,
> > >
> > > Bart.
> > It looks overkill to create one file for each value to me, and there
> > are enough stats in sysfs contain multiple values.
>
> Not for statistics.
2 examples:
cat /sys/block/nvme0n1/inflight
       0        0
cat /sys/block/nvme0n1/stat
 1267566       53 85396638   927624  4790532  3076340 198306930
19413605        0  2459788 17013620    74392        0 397606816
6864

Thanks
Leon Romanovsky Oct. 2, 2019, 4 p.m. UTC | #5 
On Wed, Oct 02, 2019 at 05:45:04PM +0200, Jinpu Wang wrote:
> On Wed, Oct 2, 2019 at 5:42 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Wed, Oct 02, 2019 at 05:15:10PM +0200, Jinpu Wang wrote:
> > > On Tue, Sep 24, 2019 at 1:56 AM Bart Van Assche <bvanassche@acm.org> wrote:
> > > >
> > > > On 6/20/19 8:03 AM, Jack Wang wrote:
> > > > > +ssize_t ibtrs_srv_stats_rdma_to_str(struct ibtrs_srv_stats *stats,
> > > > > +                                 char *page, size_t len)
> > > > > +{
> > > > > +     struct ibtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> > > > > +     struct ibtrs_srv_sess *sess;
> > > > > +
> > > > > +     sess = container_of(stats, typeof(*sess), stats);
> > > > > +
> > > > > +     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));
> > > > > +}
> > > >
> > > > Does this follow the sysfs one-value-per-file rule? See also
> > > > Documentation/filesystems/sysfs.txt.
> > > >
> > > > Thanks,
> > > >
> > > > Bart.
> > > It looks overkill to create one file for each value to me, and there
> > > are enough stats in sysfs contain multiple values.
> >
> > Not for statistics.
> 2 examples:
> cat /sys/block/nvme0n1/inflight
>        0        0
> cat /sys/block/nvme0n1/stat
>  1267566       53 85396638   927624  4790532  3076340 198306930
> 19413605        0  2459788 17013620    74392        0 397606816
> 6864

OMG, I feel sorry for users who now should go and read code to see what
column 3 in second row means.

We respect our users, please don't do like they did.

Thanks

>
> Thanks
diff mbox series

Patch

diff --git a/drivers/infiniband/ulp/ibtrs/ibtrs-srv-stats.c b/drivers/infiniband/ulp/ibtrs/ibtrs-srv-stats.c
new file mode 100644
index 000000000000..47f8d6d2d88d
--- /dev/null
+++ b/drivers/infiniband/ulp/ibtrs/ibtrs-srv-stats.c
@@ -0,0 +1,103 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * InfiniBand Transport Layer
+ *
+ * Copyright (c) 2014 - 2017 ProfitBricks GmbH. All rights reserved.
+ * Authors: Fabian Holler <mail@fholler.de>
+ *          Jack Wang <jinpu.wang@profitbricks.com>
+ *          Kleber Souza <kleber.souza@profitbricks.com>
+ *          Danil Kipnis <danil.kipnis@profitbricks.com>
+ *          Roman Penyaev <roman.penyaev@profitbricks.com>
+ *          Milind Dumbare <Milind.dumbare@gmail.com>
+ *
+ * Copyright (c) 2017 - 2018 ProfitBricks GmbH. All rights reserved.
+ * Authors: Danil Kipnis <danil.kipnis@profitbricks.com>
+ *          Roman Penyaev <roman.penyaev@profitbricks.com>
+ *
+ * Copyright (c) 2018 - 2019 1&1 IONOS Cloud GmbH. All rights reserved.
+ * Authors: Roman Penyaev <roman.penyaev@profitbricks.com>
+ *          Jack Wang <jinpu.wang@cloud.ionos.com>
+ *          Danil Kipnis <danil.kipnis@cloud.ionos.com>
+ */
+
+#undef pr_fmt
+#define pr_fmt(fmt) KBUILD_MODNAME " L" __stringify(__LINE__) ": " fmt
+
+#include "ibtrs-srv.h"
+
+void ibtrs_srv_update_rdma_stats(struct ibtrs_srv_stats *s,
+				 size_t size, int d)
+{
+	atomic64_inc(&s->rdma_stats.dir[d].cnt);
+	atomic64_add(size, &s->rdma_stats.dir[d].size_total);
+}
+
+void ibtrs_srv_update_wc_stats(struct ibtrs_srv_stats *s)
+{
+	atomic64_inc(&s->wc_comp.calls);
+	atomic64_inc(&s->wc_comp.total_wc_cnt);
+}
+
+int ibtrs_srv_reset_rdma_stats(struct ibtrs_srv_stats *stats, bool enable)
+{
+	if (enable) {
+		struct ibtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
+
+		memset(r, 0, sizeof(*r));
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+ssize_t ibtrs_srv_stats_rdma_to_str(struct ibtrs_srv_stats *stats,
+				    char *page, size_t len)
+{
+	struct ibtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
+	struct ibtrs_srv_sess *sess;
+
+	sess = container_of(stats, typeof(*sess), stats);
+
+	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));
+}
+
+int ibtrs_srv_reset_wc_completion_stats(struct ibtrs_srv_stats *stats,
+					bool enable)
+{
+	if (enable) {
+		memset(&stats->wc_comp, 0, sizeof(stats->wc_comp));
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+int ibtrs_srv_stats_wc_completion_to_str(struct ibtrs_srv_stats *stats,
+					 char *buf, size_t len)
+{
+	return snprintf(buf, len, "%lld %lld\n",
+			(s64)atomic64_read(&stats->wc_comp.total_wc_cnt),
+			(s64)atomic64_read(&stats->wc_comp.calls));
+}
+
+ssize_t ibtrs_srv_reset_all_help(struct ibtrs_srv_stats *stats,
+				 char *page, size_t len)
+{
+	return scnprintf(page, PAGE_SIZE, "echo 1 to reset all statistics\n");
+}
+
+int ibtrs_srv_reset_all_stats(struct ibtrs_srv_stats *stats, bool enable)
+{
+	if (enable) {
+		ibtrs_srv_reset_wc_completion_stats(stats, enable);
+		ibtrs_srv_reset_rdma_stats(stats, enable);
+		return 0;
+	}
+
+	return -EINVAL;
+}