| Message ID | alpine.DEB.2.20.1607081026550.5092@east.gentwo.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 7/8/2016 11:27 AM, Christoph Lameter wrote: > Add the missing port_xmit_wait counter. This counter is displayed through > some tools like perfquery but is not available via sysfs. > > For the PORT_PMA_ATTR macro the _counter field is set to zero > allowing us to specify the offset directly like with PORT_PMA_ATTR_EXT What does the value of the _counter field have to do with specifying the offset directly ? > See also the earlier work in 2008 by Vladimir Skolovsky > > https://www.mail-archive.com/general@lists.openfabrics.org/msg20313.html > > Signed-off-by: Vladimir Sokolvsky <vlad@mellanox.com> > Signed-off-by: Christoph Lameter <cl@linux.com> > --- > > V1->V2 > - Tested. Found that we missed modifying on table. > > V2->V3 > - Change description, topic etc. > > Index: rh73alpha/drivers/infiniband/core/sysfs.c > =================================================================== > --- rh73alpha.orig/drivers/infiniband/core/sysfs.c > +++ rh73alpha/drivers/infiniband/core/sysfs.c > @@ -530,6 +530,7 @@ static PORT_PMA_ATTR(port_xmit_data > 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); > +static PORT_PMA_ATTR(port_xmit_wait , 0, 32, 320); Although it doesn't matter now as it's not used, wouldn't the _counter be better as 16 rather than 0 ? If it were to be used (in the future), it indicates the corresponding number to the counter. In the case of set support, it would be translated to a bit in PortCounters CounterSelect or CounterSelect2 (if option to set port xmit wait counter is supported). An alternative is to remove _counter which could be done as either part of this or as a follow on change. -- Hal -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 8 Jul 2016, Hal Rosenstock wrote: > What does the value of the _counter field have to do with specifying the > offset directly ? Its used to calculate the offset. See the macro definition. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/8/2016 3:43 PM, Christoph Lameter wrote: > On Fri, 8 Jul 2016, Hal Rosenstock wrote: > >> What does the value of the _counter field have to do with specifying the >> offset directly ? > > Its used to calculate the offset. See the macro definition. Isn't it used to calculate the index not the offset (and that part of index is not used anywhere AFAIT) ? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 8 Jul 2016, Hal Rosenstock wrote: > > Its used to calculate the offset. See the macro definition. > > Isn't it used to calculate the index not the offset (and that part of > index is not used anywhere AFAIT) ? _width, _offset and _counter are all just used for the calculation of "index" which is the final value that is stored in the port table. Ok we can change that to index to be accurate? #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), \ .attr_id = IB_PMA_PORT_COUNTERS , \ } -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/10/2016 8:15 PM, Christoph Lameter wrote: > On Fri, 8 Jul 2016, Hal Rosenstock wrote: > >>> Its used to calculate the offset. See the macro definition. >> >> Isn't it used to calculate the index not the offset (and that part of >> index is not used anywhere AFAIT) ? > > _width, _offset and _counter are all just used for the calculation of > "index" which is the final value that is stored in the port table. > > Ok we can change that to index to be accurate? > > > #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), \ > .attr_id = IB_PMA_PORT_COUNTERS , \ > } I'm not sure I'm following what you mean. Are you proposing to change _counter to _index ? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 11 Jul 2016, Hal Rosenstock wrote: > > I'm not sure I'm following what you mean. Are you proposing to change > _counter to _index ? > No I am proposing to change the description to refer to index instead. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/11/2016 8:12 PM, Christoph Lameter wrote: > On Mon, 11 Jul 2016, Hal Rosenstock wrote: >> >> I'm not sure I'm following what you mean. Are you proposing to change >> _counter to _index ? >> > No I am proposing to change the description to refer to index instead. I see now what you mean: For the PORT_PMA_ATTR macro the _counter field is set to zero allowing us to specify the index like with PORT_PMA_ATTR_EXT The elimination of the unused _counter field from PORT_PMA_ATTR can be done subsequent to this change. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/8/2016 11:27 AM, Christoph Lameter wrote: > Add the missing port_xmit_wait counter. This counter is displayed through > some tools like perfquery but is not available via sysfs. > > For the PORT_PMA_ATTR macro the _counter field is set to zero > allowing us to specify the offset directly like with PORT_PMA_ATTR_EXT > > See also the earlier work in 2008 by Vladimir Skolovsky > > https://www.mail-archive.com/general@lists.openfabrics.org/msg20313.html > > Signed-off-by: Vladimir Sokolvsky <vlad@mellanox.com> > Signed-off-by: Christoph Lameter <cl@linux.com> Thanks, applied.
Index: rh73alpha/drivers/infiniband/core/sysfs.c =================================================================== --- rh73alpha.orig/drivers/infiniband/core/sysfs.c +++ rh73alpha/drivers/infiniband/core/sysfs.c @@ -530,6 +530,7 @@ static PORT_PMA_ATTR(port_xmit_data 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); +static PORT_PMA_ATTR(port_xmit_wait , 0, 32, 320); /* * Counters added by extended set @@ -560,6 +561,7 @@ static struct attribute *pma_attrs[] = { &port_pma_attr_port_rcv_data.attr.attr, &port_pma_attr_port_xmit_packets.attr.attr, &port_pma_attr_port_rcv_packets.attr.attr, + &port_pma_attr_port_xmit_wait.attr.attr, NULL }; @@ -579,6 +581,7 @@ static struct attribute *pma_attrs_ext[] &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_port_xmit_wait.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, @@ -604,6 +607,7 @@ static struct attribute *pma_attrs_noiet &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_port_xmit_wait.attr.attr, NULL };