diff mbox series

[v6,11/25] rtrs: server: statistics functions

Message ID 20191230102942.18395-12-jinpuwang@gmail.com (mailing list archive)
State New, archived
Headers show
Series RTRS (former IBTRS) rdma transport library and RNBD (former IBNBD) rdma network block device | expand

Commit Message

Jinpu Wang Dec. 30, 2019, 10:29 a.m. UTC 
From: Jack Wang <jinpu.wang@cloud.ionos.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>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c | 91 ++++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100644 drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c

Comments

Bart Van Assche Jan. 2, 2020, 10:02 p.m. UTC | #1 
On 12/30/19 2:29 AM, Jack Wang wrote:
> +int rtrs_srv_reset_rdma_stats(struct rtrs_srv_stats *stats, bool enable)
> +{
> +	if (enable) {
> +		struct rtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> +
> +		memset(r, 0, sizeof(*r));
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}

I think the traditional kernel coding style is "if (!enable) return ...".

> +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;
> +
> +	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?

> +int rtrs_srv_stats_wc_completion_to_str(struct rtrs_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));
> +}

Same comment here.

Thanks,

Bart.
Jinpu Wang Jan. 8, 2020, 12:55 p.m. UTC | #2 
On Thu, Jan 2, 2020 at 11:02 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 12/30/19 2:29 AM, Jack Wang wrote:
> > +int rtrs_srv_reset_rdma_stats(struct rtrs_srv_stats *stats, bool enable)
> > +{
> > +     if (enable) {
> > +             struct rtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
> > +
> > +             memset(r, 0, sizeof(*r));
> > +             return 0;
> > +     }
> > +
> > +     return -EINVAL;
> > +}
>
> I think the traditional kernel coding style is "if (!enable) return ...".
This can be changed.
>
> > +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;
> > +
> > +     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?
We have user space tools already depend on it.
On the other side one-value-per-file rule is never really enforced,
see https://lwn.net/Articles/378884/
I think it doesn't really make sense for the use case.
>
> > +int rtrs_srv_stats_wc_completion_to_str(struct rtrs_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));
> > +}
>
> Same comment here.
See comment above.
>
> Thanks,
>
> Bart.
Thanks Bart
diff mbox series

Patch

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
new file mode 100644
index 000000000000..515f7088db71
--- /dev/null
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
@@ -0,0 +1,91 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * InfiniBand Transport Layer
+ *
+ * Copyright (c) 2014 - 2018 ProfitBricks GmbH. All rights reserved.
+ *
+ * Copyright (c) 2018 - 2019 1&1 IONOS Cloud GmbH. All rights reserved.
+ *
+ * Copyright (c) 2019 1&1 IONOS SE. All rights reserved.
+ */
+#undef pr_fmt
+#define pr_fmt(fmt) KBUILD_MODNAME " L" __stringify(__LINE__) ": " fmt
+
+#include "rtrs-srv.h"
+
+void rtrs_srv_update_rdma_stats(struct rtrs_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 rtrs_srv_update_wc_stats(struct rtrs_srv_stats *s)
+{
+	atomic64_inc(&s->wc_comp.calls);
+	atomic64_inc(&s->wc_comp.total_wc_cnt);
+}
+
+int rtrs_srv_reset_rdma_stats(struct rtrs_srv_stats *stats, bool enable)
+{
+	if (enable) {
+		struct rtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
+
+		memset(r, 0, sizeof(*r));
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+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;
+
+	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 rtrs_srv_reset_wc_completion_stats(struct rtrs_srv_stats *stats,
+					bool enable)
+{
+	if (enable) {
+		memset(&stats->wc_comp, 0, sizeof(stats->wc_comp));
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+int rtrs_srv_stats_wc_completion_to_str(struct rtrs_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 rtrs_srv_reset_all_help(struct rtrs_srv_stats *stats,
+				 char *page, size_t len)
+{
+	return scnprintf(page, PAGE_SIZE, "echo 1 to reset all statistics\n");
+}
+
+int rtrs_srv_reset_all_stats(struct rtrs_srv_stats *stats, bool enable)
+{
+	if (enable) {
+		rtrs_srv_reset_wc_completion_stats(stats, enable);
+		rtrs_srv_reset_rdma_stats(stats, enable);
+		return 0;
+	}
+
+	return -EINVAL;
+}