IB core: 64 bit counter support V3

Create a new function to retrieve performance management data from the existing code in get_pma_counter().

Reviewed-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 drivers/infiniband/core/sysfs.c | 62 ++++++++++++++++++++++++++---------------
 1 file changed, 40 insertions(+), 22 deletions(-)

drivers/infiniband/core/sysfs.c +40 -22

index b1f37d4..acefe85 100644
@@ -317,21 +317,21 @@ struct port_table_attribute port_pma_attr_##_name = {			\
 	.index = (_offset) | ((_width) << 16) | ((_counter) << 24)	\
 }
 
-static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
-				char *buf)
+/*
+ * Get a Perfmgmt MAD block of data.
+ * Returns error code or the number of bytes retrieved.
+ */
+static int get_perf_mad(struct ib_device *dev, int port_num, int attr,
+		void *data, int offset, size_t size)
 {
-	struct port_table_attribute *tab_attr =
-		container_of(attr, struct port_table_attribute, attr);
-	int offset = tab_attr->index & 0xffff;
-	int width  = (tab_attr->index >> 16) & 0xff;
-	struct ib_mad *in_mad  = NULL;
-	struct ib_mad *out_mad = NULL;
+	struct ib_mad *in_mad;
+	struct ib_mad *out_mad;
 	size_t mad_size = sizeof(*out_mad);
 	u16 out_mad_pkey_index = 0;
 	ssize_t ret;
 
-	if (!p->ibdev->process_mad)
-		return sprintf(buf, "N/A (no PMA)\n");
+	if (!dev->process_mad)
+		return -ENOSYS;
 
 	in_mad  = kzalloc(sizeof *in_mad, GFP_KERNEL);
 	out_mad = kmalloc(sizeof *out_mad, GFP_KERNEL);

@@ -344,12 +344,12 @@ static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
 	in_mad->mad_hdr.mgmt_class    = IB_MGMT_CLASS_PERF_MGMT;
 	in_mad->mad_hdr.class_version = 1;
 	in_mad->mad_hdr.method        = IB_MGMT_METHOD_GET;
-	in_mad->mad_hdr.attr_id       = cpu_to_be16(0x12); /* PortCounters */
+	in_mad->mad_hdr.attr_id       = attr;
 
-	in_mad->data[41] = p->port_num;	/* PortSelect field */
+	in_mad->data[41] = port_num;	/* PortSelect field */
 
-	if ((p->ibdev->process_mad(p->ibdev, IB_MAD_IGNORE_MKEY,
-		 p->port_num, NULL, NULL,
+	if ((dev->process_mad(dev, IB_MAD_IGNORE_MKEY,
+		 port_num, NULL, NULL,
 		 (const struct ib_mad_hdr *)in_mad, mad_size,
 		 (struct ib_mad_hdr *)out_mad, &mad_size,
 		 &out_mad_pkey_index) &

@@ -358,31 +358,49 @@ static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
 		ret = -EINVAL;
 		goto out;
 	}
+	memcpy(data, out_mad->data + offset, size);
+	ret = size;
+out:
+	kfree(in_mad);
+	kfree(out_mad);
+	return ret;
+}
+
+static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
+				char *buf)
+{
+	struct port_table_attribute *tab_attr =
+		container_of(attr, struct port_table_attribute, attr);
+	int offset = tab_attr->index & 0xffff;
+	int width  = (tab_attr->index >> 16) & 0xff;
+	ssize_t ret;
+	u8 data[8];
+
+	ret = get_perf_mad(p->ibdev, p->port_num, cpu_to_be16(0x12), &data,
+			40 + offset / 8, sizeof(data));
+	if (ret < 0)
+		return sprintf(buf, "N/A (no PMA)\n");
 
 	switch (width) {
 	case 4:
-		ret = sprintf(buf, "%u\n", (out_mad->data[40 + offset / 8] >>
+		ret = sprintf(buf, "%u\n", (*data >>
 					    (4 - (offset % 8))) & 0xf);
 		break;
 	case 8:
-		ret = sprintf(buf, "%u\n", out_mad->data[40 + offset / 8]);
+		ret = sprintf(buf, "%u\n", *data);
 		break;
 	case 16:
 		ret = sprintf(buf, "%u\n",
-			      be16_to_cpup((__be16 *)(out_mad->data + 40 + offset / 8)));
+			      be16_to_cpup((__be16 *)data));
 		break;
 	case 32:
 		ret = sprintf(buf, "%u\n",
-			      be32_to_cpup((__be32 *)(out_mad->data + 40 + offset / 8)));
+			      be32_to_cpup((__be32 *)data));
 		break;
 	default:
 		ret = 0;
 	}
 
-out:
-	kfree(in_mad);
-	kfree(out_mad);
-
 	return ret;
 }
 

Signature · 1 line

2.5.0

Add the attr_id on port_table_attribute since we will have to add a different port_table_attribute for the extended attribute soon.

Reviewed-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 drivers/infiniband/core/sysfs.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/infiniband/core/sysfs.c +5 -2

index acefe85..34dcc23 100644
@@ -39,6 +39,7 @@
 #include <linux/string.h>
 
 #include <rdma/ib_mad.h>
+#include <rdma/ib_pma.h>
 
 struct ib_port {
 	struct kobject         kobj;

@@ -65,6 +66,7 @@ struct port_table_attribute {
 	struct port_attribute	attr;
 	char			name[8];
 	int			index;
+	int			attr_id;
 };
 
 static ssize_t port_attr_show(struct kobject *kobj,

@@ -314,7 +316,8 @@ static ssize_t show_port_pkey(struct ib_port *p, struct port_attribute *attr,
 #define PORT_PMA_ATTR(_name, _counter, _width, _offset)			\
 struct port_table_attribute port_pma_attr_##_name = {			\
 	.attr  = __ATTR(_name, S_IRUGO, show_pma_counter, NULL),	\
-	.index = (_offset) | ((_width) << 16) | ((_counter) << 24)	\
+	.index = (_offset) | ((_width) << 16) | ((_counter) << 24),	\
+	.attr_id = IB_PMA_PORT_COUNTERS ,				\
 }
 
 /*

@@ -376,7 +379,7 @@ static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
 	ssize_t ret;
 	u8 data[8];
 
-	ret = get_perf_mad(p->ibdev, p->port_num, cpu_to_be16(0x12), &data,
+	ret = get_perf_mad(p->ibdev, p->port_num, tab_attr->attr_id, &data,
 			40 + offset / 8, sizeof(data));
 	if (ret < 0)
 		return sprintf(buf, "N/A (no PMA)\n");

Signature · 1 line

2.5.0

V2->V3: Add check for NOIETF mode and create special table
for that case.

Check if the extended counters are available and if so create the proper extended and additional counters.

Reviewed-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 drivers/infiniband/core/sysfs.c | 104 +++++++++++++++++++++++++++++++++++++++-
 include/rdma/ib_pma.h           |   1 +
 2 files changed, 104 insertions(+), 1 deletion(-)

drivers/infiniband/core/sysfs.c +102 -0

index 34dcc23..b179fca 100644
@@ -320,6 +320,13 @@ struct port_table_attribute port_pma_attr_##_name = {			\
 	.attr_id = IB_PMA_PORT_COUNTERS ,				\
 }
 
+#define PORT_PMA_ATTR_EXT(_name, _width, _offset)			\
+struct port_table_attribute port_pma_attr_ext_##_name = {		\
+	.attr  = __ATTR(_name, S_IRUGO, show_pma_counter, NULL),	\
+	.index = (_offset) | ((_width) << 16),				\
+	.attr_id = IB_PMA_PORT_COUNTERS_EXT ,				\
+}
+
 /*
  * Get a Perfmgmt MAD block of data.
  * Returns error code or the number of bytes retrieved.

@@ -400,6 +407,11 @@ static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
 		ret = sprintf(buf, "%u\n",
 			      be32_to_cpup((__be32 *)data));
 		break;
+	case 64:
+		ret = sprintf(buf, "%llu\n",
+				be64_to_cpup((__be64 *)data));
+		break;
+
 	default:
 		ret = 0;
 	}

@@ -424,6 +436,18 @@ static PORT_PMA_ATTR(port_rcv_data		    , 13, 32, 224);
 static PORT_PMA_ATTR(port_xmit_packets		    , 14, 32, 256);
 static PORT_PMA_ATTR(port_rcv_packets		    , 15, 32, 288);
 
+/*
+ * Counters added by extended set
+ */
+static PORT_PMA_ATTR_EXT(port_xmit_data		    , 64,  64);
+static PORT_PMA_ATTR_EXT(port_rcv_data		    , 64, 128);
+static PORT_PMA_ATTR_EXT(port_xmit_packets	    , 64, 192);
+static PORT_PMA_ATTR_EXT(port_rcv_packets	    , 64, 256);
+static PORT_PMA_ATTR_EXT(unicast_xmit_packets	    , 64, 320);
+static PORT_PMA_ATTR_EXT(unicast_rcv_packets	    , 64, 384);
+static PORT_PMA_ATTR_EXT(multicast_xmit_packets	    , 64, 448);
+static PORT_PMA_ATTR_EXT(multicast_rcv_packets	    , 64, 512);
+
 static struct attribute *pma_attrs[] = {
 	&port_pma_attr_symbol_error.attr.attr,
 	&port_pma_attr_link_error_recovery.attr.attr,

@@ -444,11 +468,65 @@ static struct attribute *pma_attrs[] = {
 	NULL
 };
 
+static struct attribute *pma_attrs_ext[] = {
+	&port_pma_attr_symbol_error.attr.attr,
+	&port_pma_attr_link_error_recovery.attr.attr,
+	&port_pma_attr_link_downed.attr.attr,
+	&port_pma_attr_port_rcv_errors.attr.attr,
+	&port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+	&port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+	&port_pma_attr_port_xmit_discards.attr.attr,
+	&port_pma_attr_port_xmit_constraint_errors.attr.attr,
+	&port_pma_attr_port_rcv_constraint_errors.attr.attr,
+	&port_pma_attr_local_link_integrity_errors.attr.attr,
+	&port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+	&port_pma_attr_VL15_dropped.attr.attr,
+	&port_pma_attr_ext_port_xmit_data.attr.attr,
+	&port_pma_attr_ext_port_rcv_data.attr.attr,
+	&port_pma_attr_ext_port_xmit_packets.attr.attr,
+	&port_pma_attr_ext_port_rcv_packets.attr.attr,
+	&port_pma_attr_ext_unicast_rcv_packets.attr.attr,
+	&port_pma_attr_ext_unicast_xmit_packets.attr.attr,
+	&port_pma_attr_ext_multicast_rcv_packets.attr.attr,
+	&port_pma_attr_ext_multicast_xmit_packets.attr.attr,
+	NULL
+};
+
+static struct attribute *pma_attrs_noietf[] = {
+	&port_pma_attr_symbol_error.attr.attr,
+	&port_pma_attr_link_error_recovery.attr.attr,
+	&port_pma_attr_link_downed.attr.attr,
+	&port_pma_attr_port_rcv_errors.attr.attr,
+	&port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+	&port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+	&port_pma_attr_port_xmit_discards.attr.attr,
+	&port_pma_attr_port_xmit_constraint_errors.attr.attr,
+	&port_pma_attr_port_rcv_constraint_errors.attr.attr,
+	&port_pma_attr_local_link_integrity_errors.attr.attr,
+	&port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+	&port_pma_attr_VL15_dropped.attr.attr,
+	&port_pma_attr_ext_port_xmit_data.attr.attr,
+	&port_pma_attr_ext_port_rcv_data.attr.attr,
+	&port_pma_attr_ext_port_xmit_packets.attr.attr,
+	&port_pma_attr_ext_port_rcv_packets.attr.attr,
+	NULL
+};
+
 static struct attribute_group pma_group = {
 	.name  = "counters",
 	.attrs  = pma_attrs
 };
 
+static struct attribute_group pma_group_ext = {
+	.name  = "counters",
+	.attrs  = pma_attrs_ext
+};
+
+static struct attribute_group pma_group_noietf = {
+	.name  = "counters",
+	.attrs  = pma_attrs_noietf
+};
+
 static void ib_port_release(struct kobject *kobj)
 {
 	struct ib_port *p = container_of(kobj, struct ib_port, kobj);

@@ -521,6 +599,30 @@ err:
 	return NULL;
 }
 
+/*
+ * Figure out which counter table to use depending on
+ * the device capabilities.
+ */
+static struct attribute_group *get_counter_table(struct ib_device *dev)
+{
+	struct ib_class_port_info cpi;
+
+	if (get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO,
+				&cpi, 40, sizeof(cpi)) >= 0) {
+
+		if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH)
+			/* We have extended counters */
+			return &pma_group_ext;
+
+		if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
+			/* But not the IETF ones */
+			return &pma_group_noietf;
+	}
+
+	/* Fall back to normal counters */
+	return &pma_group;
+}
+
 static int add_port(struct ib_device *device, int port_num,
 		    int (*port_callback)(struct ib_device *,
 					 u8, struct kobject *))

drivers/infiniband/core/sysfs.c +0 -0

 	if (ret)
 		goto err_put;
 

include/rdma/ib_pma.h +1 -0

index a5889f1..2f8a65c 100644
@@ -42,6 +42,7 @@
  */
 #define IB_PMA_CLASS_CAP_ALLPORTSELECT  cpu_to_be16(1 << 8)
 #define IB_PMA_CLASS_CAP_EXT_WIDTH      cpu_to_be16(1 << 9)
+#define IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF cpu_to_be16(1 << 10)
 #define IB_PMA_CLASS_CAP_XMIT_WAIT      cpu_to_be16(1 << 12)
 
 #define IB_PMA_CLASS_PORT_INFO          cpu_to_be16(0x0001)

Signature · 1 line

2.5.0

Quoted 163 lines · Christoph Lameter

V2->V3: Add check for NOIETF mode and create special table
for that case.

Check if the extended counters are available and if so create the proper extended and additional counters.

Reviewed-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 drivers/infiniband/core/sysfs.c | 104 +++++++++++++++++++++++++++++++++++++++-
 include/rdma/ib_pma.h           |   1 +
 2 files changed, 104 insertions(+), 1 deletion(-)

drivers/infiniband/core/sysfs.c +102 -0

index 34dcc23..b179fca 100644
@@ -320,6 +320,13 @@ struct port_table_attribute port_pma_attr_##_name = {			\
 	.attr_id = IB_PMA_PORT_COUNTERS ,				\
 }
 
+#define PORT_PMA_ATTR_EXT(_name, _width, _offset)			\
+struct port_table_attribute port_pma_attr_ext_##_name = {		\
+	.attr  = __ATTR(_name, S_IRUGO, show_pma_counter, NULL),	\
+	.index = (_offset) | ((_width) << 16),				\
+	.attr_id = IB_PMA_PORT_COUNTERS_EXT ,				\
+}
+
 /*
  * Get a Perfmgmt MAD block of data.
  * Returns error code or the number of bytes retrieved.

@@ -400,6 +407,11 @@ static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
 		ret = sprintf(buf, "%u\n",
 			      be32_to_cpup((__be32 *)data));
 		break;
+	case 64:
+		ret = sprintf(buf, "%llu\n",
+				be64_to_cpup((__be64 *)data));
+		break;
+
 	default:
 		ret = 0;
 	}

@@ -424,6 +436,18 @@ static PORT_PMA_ATTR(port_rcv_data		    , 13, 32, 224);
 static PORT_PMA_ATTR(port_xmit_packets		    , 14, 32, 256);
 static PORT_PMA_ATTR(port_rcv_packets		    , 15, 32, 288);
 
+/*
+ * Counters added by extended set
+ */
+static PORT_PMA_ATTR_EXT(port_xmit_data		    , 64,  64);
+static PORT_PMA_ATTR_EXT(port_rcv_data		    , 64, 128);
+static PORT_PMA_ATTR_EXT(port_xmit_packets	    , 64, 192);
+static PORT_PMA_ATTR_EXT(port_rcv_packets	    , 64, 256);
+static PORT_PMA_ATTR_EXT(unicast_xmit_packets	    , 64, 320);
+static PORT_PMA_ATTR_EXT(unicast_rcv_packets	    , 64, 384);
+static PORT_PMA_ATTR_EXT(multicast_xmit_packets	    , 64, 448);
+static PORT_PMA_ATTR_EXT(multicast_rcv_packets	    , 64, 512);
+
 static struct attribute *pma_attrs[] = {
 	&port_pma_attr_symbol_error.attr.attr,
 	&port_pma_attr_link_error_recovery.attr.attr,

@@ -444,11 +468,65 @@ static struct attribute *pma_attrs[] = {
 	NULL
 };
 
+static struct attribute *pma_attrs_ext[] = {
+	&port_pma_attr_symbol_error.attr.attr,
+	&port_pma_attr_link_error_recovery.attr.attr,
+	&port_pma_attr_link_downed.attr.attr,
+	&port_pma_attr_port_rcv_errors.attr.attr,
+	&port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+	&port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+	&port_pma_attr_port_xmit_discards.attr.attr,
+	&port_pma_attr_port_xmit_constraint_errors.attr.attr,
+	&port_pma_attr_port_rcv_constraint_errors.attr.attr,
+	&port_pma_attr_local_link_integrity_errors.attr.attr,
+	&port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+	&port_pma_attr_VL15_dropped.attr.attr,
+	&port_pma_attr_ext_port_xmit_data.attr.attr,
+	&port_pma_attr_ext_port_rcv_data.attr.attr,
+	&port_pma_attr_ext_port_xmit_packets.attr.attr,
+	&port_pma_attr_ext_port_rcv_packets.attr.attr,
+	&port_pma_attr_ext_unicast_rcv_packets.attr.attr,
+	&port_pma_attr_ext_unicast_xmit_packets.attr.attr,
+	&port_pma_attr_ext_multicast_rcv_packets.attr.attr,
+	&port_pma_attr_ext_multicast_xmit_packets.attr.attr,
+	NULL
+};
+
+static struct attribute *pma_attrs_noietf[] = {
+	&port_pma_attr_symbol_error.attr.attr,
+	&port_pma_attr_link_error_recovery.attr.attr,
+	&port_pma_attr_link_downed.attr.attr,
+	&port_pma_attr_port_rcv_errors.attr.attr,
+	&port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+	&port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+	&port_pma_attr_port_xmit_discards.attr.attr,
+	&port_pma_attr_port_xmit_constraint_errors.attr.attr,
+	&port_pma_attr_port_rcv_constraint_errors.attr.attr,
+	&port_pma_attr_local_link_integrity_errors.attr.attr,
+	&port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+	&port_pma_attr_VL15_dropped.attr.attr,
+	&port_pma_attr_ext_port_xmit_data.attr.attr,
+	&port_pma_attr_ext_port_rcv_data.attr.attr,
+	&port_pma_attr_ext_port_xmit_packets.attr.attr,
+	&port_pma_attr_ext_port_rcv_packets.attr.attr,
+	NULL
+};
+
 static struct attribute_group pma_group = {
 	.name  = "counters",
 	.attrs  = pma_attrs
 };
 
+static struct attribute_group pma_group_ext = {
+	.name  = "counters",
+	.attrs  = pma_attrs_ext
+};
+
+static struct attribute_group pma_group_noietf = {
+	.name  = "counters",
+	.attrs  = pma_attrs_noietf
+};
+
 static void ib_port_release(struct kobject *kobj)
 {
 	struct ib_port *p = container_of(kobj, struct ib_port, kobj);

@@ -521,6 +599,30 @@ err:
 	return NULL;
 }
 
+/*
+ * Figure out which counter table to use depending on
+ * the device capabilities.
+ */
+static struct attribute_group *get_counter_table(struct ib_device *dev)
+{
+	struct ib_class_port_info cpi;
+
+	if (get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO,
+				&cpi, 40, sizeof(cpi)) >= 0) {
+
+		if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH)
+			/* We have extended counters */
+			return &pma_group_ext;
+
+		if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
+			/* But not the IETF ones */
+			return &pma_group_noietf;
+	}
+
+	/* Fall back to normal counters */
+	return &pma_group;
+}
+
 static int add_port(struct ib_device *device, int port_num,
 		    int (*port_callback)(struct ib_device *,
 					 u8, struct kobject *))

@@ -549,7 +651,7 @@ static int add_port(struct ib_device *device, int port_num,
 		return ret;
 	}
 

drivers/infiniband/core/sysfs.c static int add_port(struct ib_device *device, int port_num, · line 654

Quoted 2 lines

drivers/infiniband/core/sysfs.c +1 -1

-	ret = sysfs_create_group(&p->kobj, &pma_group);
+	ret = sysfs_create_group(&p->kobj, get_counter_table(device));

Don't we need to change all the sysfs_remove_groups to use get_counter_table as well?

Ira

Quoted 14 lines

drivers/infiniband/core/sysfs.c +0 -0

 	if (ret)
 		goto err_put;
 

include/rdma/ib_pma.h +1 -0

index a5889f1..2f8a65c 100644
@@ -42,6 +42,7 @@
  */
 #define IB_PMA_CLASS_CAP_ALLPORTSELECT  cpu_to_be16(1 << 8)
 #define IB_PMA_CLASS_CAP_EXT_WIDTH      cpu_to_be16(1 << 9)
+#define IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF cpu_to_be16(1 << 10)
 #define IB_PMA_CLASS_CAP_XMIT_WAIT      cpu_to_be16(1 << 12)
 

Quoted 5 lines

include/rdma/ib_pma.h +0 -0

 #define IB_PMA_CLASS_PORT_INFO          cpu_to_be16(0x0001)

Signature · 1 line

2.5.0

Quoted 2 lines

Don't we need to change all the sysfs_remove_groups to use get_counter_table as well?

Looks like it to me too. Good catch.

Quoted 4 lines · Hal Rosenstock

Quoted 2 lines

Don't we need to change all the sysfs_remove_groups to use get_counter_table as well?

Looks like it to me too. Good catch.

Fix follows:

From: Christoph Lameter <[email protected]>
Subject: Fix sysfs entry removal by storing the table format in pma_table

Store the table being used in the ib_port structure and use it when sysfs entries have to be removed.

Signed-off-by Christoph Lameter <[email protected]>

Index: linux/drivers/infiniband/core/sysfs.c
===================================================================

drivers/infiniband/core/sysfs.c +4 -2

@@ -47,6 +47,7 @@ struct ib_port {
 	struct attribute_group gid_group;
 	struct attribute_group pkey_group;
 	u8                     port_num;
+	struct attribute_group *pma_table;
 };
 
 struct port_attribute {

@@ -651,7 +652,8 @@ static int add_port(struct ib_device *de
 		return ret;
 	}
 
-	ret = sysfs_create_group(&p->kobj, get_counter_table(device));
+	p->pma_table = get_counter_table(device);
+	ret = sysfs_create_group(&p->kobj, p->pma_table);
 	if (ret)
 		goto err_put;
 

@@ -710,7 +712,7 @@ err_free_gid:
 	p->gid_group.attrs = NULL;
 
 err_remove_pma:
-	sysfs_remove_group(&p->kobj, &pma_group);
+	sysfs_remove_group(&p->kobj, p->pma_table);
 
 err_put:
 	kobject_put(&p->kobj);

Quoted 12 lines · Christoph Lameter

Quoted 4 lines · Hal Rosenstock

Quoted 2 lines

Don't we need to change all the sysfs_remove_groups to use get_counter_table as well?

Looks like it to me too. Good catch.

Fix follows:

From: Christoph Lameter <[email protected]>
Subject: Fix sysfs entry removal by storing the table format in pma_table

Quoted 4 lines

Store the table being used in the ib_port structure and use it when sysfs entries have to be removed.

Signed-off-by Christoph Lameter <[email protected]>

Reviewed-by Ira Weiny <[email protected]>

Quoted 31 lines

Index: linux/drivers/infiniband/core/sysfs.c
===================================================================

drivers/infiniband/core/sysfs.c +4 -2

@@ -47,6 +47,7 @@ struct ib_port {
 	struct attribute_group gid_group;
 	struct attribute_group pkey_group;
 	u8                     port_num;
+	struct attribute_group *pma_table;
 };
 
 struct port_attribute {

@@ -651,7 +652,8 @@ static int add_port(struct ib_device *de
 		return ret;
 	}
 
-	ret = sysfs_create_group(&p->kobj, get_counter_table(device));
+	p->pma_table = get_counter_table(device);
+	ret = sysfs_create_group(&p->kobj, p->pma_table);
 	if (ret)
 		goto err_put;
 

@@ -710,7 +712,7 @@ err_free_gid:
 	p->gid_group.attrs = NULL;
 
 err_remove_pma:
-	sysfs_remove_group(&p->kobj, &pma_group);
+	sysfs_remove_group(&p->kobj, p->pma_table);
 

drivers/infiniband/core/sysfs.c static void free_port_list_attributes(st · line 931

Quoted 11 lines

drivers/infiniband/core/sysfs.c +1 -1

 err_put:
 	kobject_put(&p->kobj);

@@ -923,7 +925,7 @@ static void free_port_list_attributes(st
 	list_for_each_entry_safe(p, t, &device->port_list, entry) {
 		struct ib_port *port = container_of(p, struct ib_port, kobj);
 		list_del(&p->entry);
-		sysfs_remove_group(p, &pma_group);
+		sysfs_remove_group(p, port->pma_table);
 		sysfs_remove_group(p, &port->pkey_group);
 		sysfs_remove_group(p, &port->gid_group);
 		kobject_put(p);

V2->V3
- Also add support for NOIETF counter mode where we have 64 bit counters but not the multicast/unicast counters.
- Add Reviewed-by's from Hal.

V1->V2
- Add detection of the capability for 64 bit counter support
- Lots of improvements as a result of suggestions by Hal Rosenstock.

Currently we only use 32 bits for the packet and byte counters. There have been extended countes available for some time but we have no support for those yet upstream. We keep having issues with 32 bit counters wrapping. Especially the byte counter can wrap frequently (as in multiple times per minute)

This patch adds 4 new counters (for full extended mode) and updates 4 32 bit counters to use the 64 bit sizes (for NOIETF and full extended mode) so that they no longer wrap.

Should the device not support 64 bit counters then only the original 32 bit counters will be visible.

This patchset can be pulled from my git repo on kernel.org

git pull git://git.kernel.org/pub/scm/linux/kernle/git/christoph/rdma.git counter_64bit

Thanks to Hal Rosenstock and Ira Weiny for reviewing this patchset.

V2->V3: Add check for NOIETF mode and create special table
for that case.

Check if the extended counters are available and if so create the proper extended and additional counters.

Reviewed-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 drivers/infiniband/core/sysfs.c | 104 +++++++++++++++++++++++++++++++++++++++-
 include/rdma/ib_pma.h           |   1 +
 2 files changed, 104 insertions(+), 1 deletion(-)

drivers/infiniband/core/sysfs.c +78 -0

index 34dcc23..b179fca 100644
@@ -320,6 +320,13 @@ struct port_table_attribute port_pma_attr_##_name = {                      \
        .attr_id = IB_PMA_PORT_COUNTERS ,                               \
 }
 
+#define PORT_PMA_ATTR_EXT(_name, _width, _offset)                      \
+struct port_table_attribute port_pma_attr_ext_##_name = {              \
+       .attr  = __ATTR(_name, S_IRUGO, show_pma_counter, NULL),        \
+       .index = (_offset) | ((_width) << 16),                          \
+       .attr_id = IB_PMA_PORT_COUNTERS_EXT ,                           \
+}
+
 /*
  * Get a Perfmgmt MAD block of data.
  * Returns error code or the number of bytes retrieved.

@@ -400,6 +407,11 @@ static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
                ret = sprintf(buf, "%u\n",
                              be32_to_cpup((__be32 *)data));
                break;
+       case 64:
+               ret = sprintf(buf, "%llu\n",
+                               be64_to_cpup((__be64 *)data));
+               break;
+
        default:
                ret = 0;
        }

@@ -424,6 +436,18 @@ static PORT_PMA_ATTR(port_rcv_data             , 13, 32, 224);
 static PORT_PMA_ATTR(port_xmit_packets             , 14, 32, 256);
 static PORT_PMA_ATTR(port_rcv_packets              , 15, 32, 288);
 
+/*
+ * Counters added by extended set
+ */
+static PORT_PMA_ATTR_EXT(port_xmit_data                    , 64,  64);
+static PORT_PMA_ATTR_EXT(port_rcv_data             , 64, 128);
+static PORT_PMA_ATTR_EXT(port_xmit_packets         , 64, 192);
+static PORT_PMA_ATTR_EXT(port_rcv_packets          , 64, 256);
+static PORT_PMA_ATTR_EXT(unicast_xmit_packets      , 64, 320);
+static PORT_PMA_ATTR_EXT(unicast_rcv_packets       , 64, 384);
+static PORT_PMA_ATTR_EXT(multicast_xmit_packets            , 64, 448);
+static PORT_PMA_ATTR_EXT(multicast_rcv_packets     , 64, 512);
+
 static struct attribute *pma_attrs[] = {
        &port_pma_attr_symbol_error.attr.attr,
        &port_pma_attr_link_error_recovery.attr.attr,

@@ -444,11 +468,65 @@ static struct attribute *pma_attrs[] = {
        NULL
 };
 
+static struct attribute *pma_attrs_ext[] = {
+       &port_pma_attr_symbol_error.attr.attr,
+       &port_pma_attr_link_error_recovery.attr.attr,
+       &port_pma_attr_link_downed.attr.attr,
+       &port_pma_attr_port_rcv_errors.attr.attr,
+       &port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+       &port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+       &port_pma_attr_port_xmit_discards.attr.attr,
+       &port_pma_attr_port_xmit_constraint_errors.attr.attr,
+       &port_pma_attr_port_rcv_constraint_errors.attr.attr,
+       &port_pma_attr_local_link_integrity_errors.attr.attr,
+       &port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+       &port_pma_attr_VL15_dropped.attr.attr,
+       &port_pma_attr_ext_port_xmit_data.attr.attr,
+       &port_pma_attr_ext_port_rcv_data.attr.attr,
+       &port_pma_attr_ext_port_xmit_packets.attr.attr,
+       &port_pma_attr_ext_port_rcv_packets.attr.attr,
+       &port_pma_attr_ext_unicast_rcv_packets.attr.attr,
+       &port_pma_attr_ext_unicast_xmit_packets.attr.attr,
+       &port_pma_attr_ext_multicast_rcv_packets.attr.attr,
+       &port_pma_attr_ext_multicast_xmit_packets.attr.attr,
+       NULL
+};
+
+static struct attribute *pma_attrs_noietf[] = {
+       &port_pma_attr_symbol_error.attr.attr,
+       &port_pma_attr_link_error_recovery.attr.attr,
+       &port_pma_attr_link_downed.attr.attr,
+       &port_pma_attr_port_rcv_errors.attr.attr,
+       &port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+       &port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+       &port_pma_attr_port_xmit_discards.attr.attr,
+       &port_pma_attr_port_xmit_constraint_errors.attr.attr,
+       &port_pma_attr_port_rcv_constraint_errors.attr.attr,
+       &port_pma_attr_local_link_integrity_errors.attr.attr,
+       &port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+       &port_pma_attr_VL15_dropped.attr.attr,
+       &port_pma_attr_ext_port_xmit_data.attr.attr,
+       &port_pma_attr_ext_port_rcv_data.attr.attr,
+       &port_pma_attr_ext_port_xmit_packets.attr.attr,
+       &port_pma_attr_ext_port_rcv_packets.attr.attr,
+       NULL
+};
+
 static struct attribute_group pma_group = {
        .name  = "counters",
        .attrs  = pma_attrs
 };
 
+static struct attribute_group pma_group_ext = {
+       .name  = "counters",
+       .attrs  = pma_attrs_ext
+};
+
+static struct attribute_group pma_group_noietf = {
+       .name  = "counters",
+       .attrs  = pma_attrs_noietf
+};
+
 static void ib_port_release(struct kobject *kobj)
 {
        struct ib_port *p = container_of(kobj, struct ib_port, kobj);

@@ -521,6 +599,30 @@ err:
        return NULL;
 }

drivers/infiniband/core/sysfs.c +16 -1

+                               &cpi, 40, sizeof(cpi)) >= 0) {
+
+               if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH)
+                       /* We have extended counters */
+                       return &pma_group_ext;
+
+               if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
+                       /* But not the IETF ones */
+                       return &pma_group_noietf;
+       }
+
+       /* Fall back to normal counters */
+       return &pma_group;
+}
+
 static int add_port(struct ib_device *device, int port_num,
                    int (*port_callback)(struct ib_device *,
                                         u8, struct kobject *))

@@ -549,7 +651,7 @@ static int add_port(struct ib_device *device, int port_num,
                return ret;
        }
 
-       ret = sysfs_create_group(&p->kobj, &pma_group);
+       ret = sysfs_create_group(&p->kobj, get_counter_table(device));
        if (ret)
                goto err_put;
 

include/rdma/ib_pma.h +1 -1

index a5889f1..2f8a65c 100644
@@ -42,6 +42,7 @@
  */
 #define IB_PMA_CLASS_CAP_ALLPORTSELECT  cpu_to_be16(1 << 8)
 #define IB_PMA_CLASS_CAP_EXT_WIDTH      cpu_to_be16(1 << 9)
+#define IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF cpu_to_be16(1 << 10)
 #define IB_PMA_CLASS_CAP_XMIT_WAIT      cpu_to_be16(1 << 12)
 
 #define IB_PMA_CLASS_PORT_INFO          cpu_to_be16(0x0001)
--

2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 132 lines · Christoph Lameter

V2->V3: Add check for NOIETF mode and create special table
for that case.

Check if the extended counters are available and if so create the proper extended and additional counters.

Reviewed-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

---
 drivers/infiniband/core/sysfs.c | 104 +++++++++++++++++++++++++++++++++++++++-
 include/rdma/ib_pma.h           |   1 +
 2 files changed, 104 insertions(+), 1 deletion(-)

drivers/infiniband/core/sysfs.c +78 -0

index 34dcc23..b179fca 100644
@@ -320,6 +320,13 @@ struct port_table_attribute port_pma_attr_##_name = {                      \
        .attr_id = IB_PMA_PORT_COUNTERS ,                               \
 }
 
+#define PORT_PMA_ATTR_EXT(_name, _width, _offset)                      \
+struct port_table_attribute port_pma_attr_ext_##_name = {              \
+       .attr  = __ATTR(_name, S_IRUGO, show_pma_counter, NULL),        \
+       .index = (_offset) | ((_width) << 16),                          \
+       .attr_id = IB_PMA_PORT_COUNTERS_EXT ,                           \
+}
+
 /*
  * Get a Perfmgmt MAD block of data.
  * Returns error code or the number of bytes retrieved.

@@ -400,6 +407,11 @@ static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
                ret = sprintf(buf, "%u\n",
                              be32_to_cpup((__be32 *)data));
                break;
+       case 64:
+               ret = sprintf(buf, "%llu\n",
+                               be64_to_cpup((__be64 *)data));
+               break;
+
        default:
                ret = 0;
        }

@@ -424,6 +436,18 @@ static PORT_PMA_ATTR(port_rcv_data             , 13, 32, 224);
 static PORT_PMA_ATTR(port_xmit_packets             , 14, 32, 256);
 static PORT_PMA_ATTR(port_rcv_packets              , 15, 32, 288);
 
+/*
+ * Counters added by extended set
+ */
+static PORT_PMA_ATTR_EXT(port_xmit_data                    , 64,  64);
+static PORT_PMA_ATTR_EXT(port_rcv_data             , 64, 128);
+static PORT_PMA_ATTR_EXT(port_xmit_packets         , 64, 192);
+static PORT_PMA_ATTR_EXT(port_rcv_packets          , 64, 256);
+static PORT_PMA_ATTR_EXT(unicast_xmit_packets      , 64, 320);
+static PORT_PMA_ATTR_EXT(unicast_rcv_packets       , 64, 384);
+static PORT_PMA_ATTR_EXT(multicast_xmit_packets            , 64, 448);
+static PORT_PMA_ATTR_EXT(multicast_rcv_packets     , 64, 512);
+
 static struct attribute *pma_attrs[] = {
        &port_pma_attr_symbol_error.attr.attr,
        &port_pma_attr_link_error_recovery.attr.attr,

@@ -444,11 +468,65 @@ static struct attribute *pma_attrs[] = {
        NULL
 };
 
+static struct attribute *pma_attrs_ext[] = {
+       &port_pma_attr_symbol_error.attr.attr,
+       &port_pma_attr_link_error_recovery.attr.attr,
+       &port_pma_attr_link_downed.attr.attr,
+       &port_pma_attr_port_rcv_errors.attr.attr,
+       &port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+       &port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+       &port_pma_attr_port_xmit_discards.attr.attr,
+       &port_pma_attr_port_xmit_constraint_errors.attr.attr,
+       &port_pma_attr_port_rcv_constraint_errors.attr.attr,
+       &port_pma_attr_local_link_integrity_errors.attr.attr,
+       &port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+       &port_pma_attr_VL15_dropped.attr.attr,
+       &port_pma_attr_ext_port_xmit_data.attr.attr,
+       &port_pma_attr_ext_port_rcv_data.attr.attr,
+       &port_pma_attr_ext_port_xmit_packets.attr.attr,
+       &port_pma_attr_ext_port_rcv_packets.attr.attr,
+       &port_pma_attr_ext_unicast_rcv_packets.attr.attr,
+       &port_pma_attr_ext_unicast_xmit_packets.attr.attr,
+       &port_pma_attr_ext_multicast_rcv_packets.attr.attr,
+       &port_pma_attr_ext_multicast_xmit_packets.attr.attr,
+       NULL
+};
+
+static struct attribute *pma_attrs_noietf[] = {
+       &port_pma_attr_symbol_error.attr.attr,
+       &port_pma_attr_link_error_recovery.attr.attr,
+       &port_pma_attr_link_downed.attr.attr,
+       &port_pma_attr_port_rcv_errors.attr.attr,
+       &port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+       &port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+       &port_pma_attr_port_xmit_discards.attr.attr,
+       &port_pma_attr_port_xmit_constraint_errors.attr.attr,
+       &port_pma_attr_port_rcv_constraint_errors.attr.attr,
+       &port_pma_attr_local_link_integrity_errors.attr.attr,
+       &port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+       &port_pma_attr_VL15_dropped.attr.attr,
+       &port_pma_attr_ext_port_xmit_data.attr.attr,
+       &port_pma_attr_ext_port_rcv_data.attr.attr,
+       &port_pma_attr_ext_port_xmit_packets.attr.attr,
+       &port_pma_attr_ext_port_rcv_packets.attr.attr,
+       NULL
+};
+
 static struct attribute_group pma_group = {
        .name  = "counters",
        .attrs  = pma_attrs
 };
 
+static struct attribute_group pma_group_ext = {
+       .name  = "counters",
+       .attrs  = pma_attrs_ext
+};
+
+static struct attribute_group pma_group_noietf = {
+       .name  = "counters",
+       .attrs  = pma_attrs_noietf
+};
+
 static void ib_port_release(struct kobject *kobj)
 {
        struct ib_port *p = container_of(kobj, struct ib_port, kobj);

@@ -521,6 +599,30 @@ err:
        return NULL;
 }
 

drivers/infiniband/core/sysfs.c err: · line 610

Quoted 9 lines

drivers/infiniband/core/sysfs.c +9 -0

+/*
+ * Figure out which counter table to use depending on
+ * the device capabilities.
+ */
+static struct attribute_group *get_counter_table(struct ib_device *dev)
+{
+       struct ib_class_port_info cpi;
+
+       if (get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO,

Why 0, need to pass port num.

See proposal Matan and myself sent.

Eran

Quoted 43 lines

drivers/infiniband/core/sysfs.c +16 -1

+                               &cpi, 40, sizeof(cpi)) >= 0) {
+
+               if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH)
+                       /* We have extended counters */
+                       return &pma_group_ext;
+
+               if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
+                       /* But not the IETF ones */
+                       return &pma_group_noietf;
+       }
+
+       /* Fall back to normal counters */
+       return &pma_group;
+}
+
 static int add_port(struct ib_device *device, int port_num,
                    int (*port_callback)(struct ib_device *,
                                         u8, struct kobject *))

@@ -549,7 +651,7 @@ static int add_port(struct ib_device *device, int port_num,
                return ret;
        }
 
-       ret = sysfs_create_group(&p->kobj, &pma_group);
+       ret = sysfs_create_group(&p->kobj, get_counter_table(device));
        if (ret)
                goto err_put;
 

include/rdma/ib_pma.h +1 -1

index a5889f1..2f8a65c 100644
@@ -42,6 +42,7 @@
  */
 #define IB_PMA_CLASS_CAP_ALLPORTSELECT  cpu_to_be16(1 << 8)
 #define IB_PMA_CLASS_CAP_EXT_WIDTH      cpu_to_be16(1 << 9)
+#define IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF cpu_to_be16(1 << 10)
 #define IB_PMA_CLASS_CAP_XMIT_WAIT      cpu_to_be16(1 << 12)
 
 #define IB_PMA_CLASS_PORT_INFO          cpu_to_be16(0x0001)
--

2.5.0

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

On Mon, Dec 21, 2015 at 4:20 PM, Christoph Lameter <[email protected]> wrote:

drivers/infiniband/core/sysfs.c err: · line 610

Quoted 10 lines

drivers/infiniband/core/sysfs.c +9 -0

+/*
+ * Figure out which counter table to use depending on
+ * the device capabilities.
+ */
+static struct attribute_group *get_counter_table(struct ib_device *dev)
+{
+       struct ib_class_port_info cpi;
+
+       if (get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO,

Why 0, need to pass port num.
See proposal Matan and myself sent.