| Message ID | 20151211182543.329283794@linux.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Dec 11, 2015 at 12:25:35PM -0600, Christoph Lameter wrote: > Display the additional 64 bit counters available through the extended > set and replace the existing 32 bit counters if there is a 64 bit > alternative available. > > Note: This requires universal support of extended counters in > the devices. If there are still devices around that do not > support extended counters then we will have to add some fallback > technique here. Looks like ocrdma will break here. I'm not sure about mthca. qib, mlx4 are fine. mlx5 should be as well I would think (I don't have that hardware.) hfi1 did not process these MADs previously as all the hardware counters are 64 bits. But with this patch series we would add it. ehca, amso1100, and ipath are all gone so they don't matter. I can whip up a patch for hfi1 and we have to wait for Doug to take over that driver anyway to make sure that the patch would apply. So I think you can ignore it. ocrdma seems like it could be a quick patch pre this one. Ira > > Signed-off-by: Christoph Lameter <cl@linux.com> > --- > drivers/infiniband/core/sysfs.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c > index 0083a4f..f7f2954 100644 > --- a/drivers/infiniband/core/sysfs.c > +++ b/drivers/infiniband/core/sysfs.c > @@ -406,10 +406,14 @@ static PORT_PMA_ATTR(port_rcv_constraint_errors , 8, 8, 136, IB_PMA_PORT_C > static PORT_PMA_ATTR(local_link_integrity_errors , 9, 4, 152, IB_PMA_PORT_COUNTERS); > static PORT_PMA_ATTR(excessive_buffer_overrun_errors, 10, 4, 156, IB_PMA_PORT_COUNTERS); > static PORT_PMA_ATTR(VL15_dropped , 11, 16, 176, IB_PMA_PORT_COUNTERS); > -static PORT_PMA_ATTR(port_xmit_data , 12, 32, 192, IB_PMA_PORT_COUNTERS); > -static PORT_PMA_ATTR(port_rcv_data , 13, 32, 224, IB_PMA_PORT_COUNTERS); > -static PORT_PMA_ATTR(port_xmit_packets , 14, 32, 256, IB_PMA_PORT_COUNTERS); > -static PORT_PMA_ATTR(port_rcv_packets , 15, 32, 288, IB_PMA_PORT_COUNTERS); > +static PORT_PMA_ATTR(port_xmit_data , 0, 64, 64, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(port_rcv_data , 0, 64, 128, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(port_xmit_packets , 0, 64, 192, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(port_rcv_packets , 0, 64, 256, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(unicast_xmit_packets , 0, 64, 320, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(unicast_rcv_packets , 0, 64, 384, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(multicast_xmit_packets , 0, 64, 448, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(multicast_rcv_packets , 0, 64, 512, IB_PMA_PORT_COUNTERS_EXT); > > static struct attribute *pma_attrs[] = { > &port_pma_attr_symbol_error.attr.attr, > @@ -428,6 +432,10 @@ 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_unicast_rcv_packets.attr.attr, > + &port_pma_attr_unicast_xmit_packets.attr.attr, > + &port_pma_attr_multicast_rcv_packets.attr.attr, > + &port_pma_attr_multicast_xmit_packets.attr.attr, > NULL > }; > > -- > 2.5.0 > > > -- > 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 -- 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
> qib, mlx4 are fine. mlx5 should be as well I would think (I don't have that > hardware.) I have no specifics to add, but I keep running into systems, even today, where the 64 bit counters don't work. The MAD might be there, but several counters are wired to 0. Not sure exactly which HW though. Mellanox should really confirm this for their hardware matrix. FWIW, I also hate the sysfs counters that reflect the PMA, these would be much better are free running, wrapping, non-resetting counters unrelated to the PMA. Something that doesn't zero after the SM samples it. Sounds like qib, hif, rdmavt can trivially fix this, and should, IMHO. Jason -- 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, Dec 11, 2015 at 05:00:47PM -0700, Jason Gunthorpe wrote: > > FWIW, I also hate the sysfs counters that reflect the PMA, these would > be much better are free running, wrapping, non-resetting counters > unrelated to the PMA. Something that doesn't zero after the SM samples > it. Sounds like qib, hif, rdmavt can trivially fix this, and should, IMHO. To be fair with a 64bit counter these are not going to get reset very often. Furthermore, I don't think we can change the behavior now. Frankly for IB/OPA one should be using the central PM. Ira -- 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, Dec 11, 2015 at 07:23:13PM -0500, ira.weiny wrote: > On Fri, Dec 11, 2015 at 05:00:47PM -0700, Jason Gunthorpe wrote: > > > > FWIW, I also hate the sysfs counters that reflect the PMA, these would > > be much better are free running, wrapping, non-resetting counters > > unrelated to the PMA. Something that doesn't zero after the SM samples > > it. Sounds like qib, hif, rdmavt can trivially fix this, and should, IMHO. > > To be fair with a 64bit counter these are not going to get reset very often. It resets when ever the SM sends a reset packet, so 'whenever' > Furthermore, I don't think we can change the behavior now. Sure we can, the restting is really a bug, to the point that nothing can actually use the sysfs counters reliably. Jason -- 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, Dec 11, 2015 at 05:47:15PM -0700, Jason Gunthorpe wrote: > On Fri, Dec 11, 2015 at 07:23:13PM -0500, ira.weiny wrote: > > On Fri, Dec 11, 2015 at 05:00:47PM -0700, Jason Gunthorpe wrote: > > > > > > FWIW, I also hate the sysfs counters that reflect the PMA, these would > > > be much better are free running, wrapping, non-resetting counters > > > unrelated to the PMA. Something that doesn't zero after the SM samples > > > it. Sounds like qib, hif, rdmavt can trivially fix this, and should, IMHO. > > > > To be fair with a 64bit counter these are not going to get reset very often. > > It resets when ever the SM sends a reset packet, so 'whenever' It's been a while since I have looked at OpenSMs PM but a 64 bit counter does not get reset very often. For OPA with 64bit counters the same is true. > > > Furthermore, I don't think we can change the behavior now. > > Sure we can, the restting is really a bug, to the point that nothing > can actually use the sysfs counters reliably. Well... That would be changing the behavior that everyone is expecting. I know your argument; these are useless and no one is using them. But that is not the point. They were there as a check to see the PMA counters directly. Any change now is breaking the existing ABI. Don't get me wrong. I'm not against keeping running counters for devices. But, I would rather see a more flexible interface. Something like netlink perhaps. And I will again mention that both IB and OPA have PMs which keep running counters. For now Christophs series is a welcome improvement. Ira -- 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, Dec 11, 2015 at 8:25 PM, Christoph Lameter <cl@linux.com> wrote: > Display the additional 64 bit counters available through the extended > set and replace the existing 32 bit counters if there is a 64 bit > alternative available. > > Note: This requires universal support of extended counters in > the devices. If there are still devices around that do not > support extended counters then we will have to add some fallback > technique here. > > Signed-off-by: Christoph Lameter <cl@linux.com> > --- > drivers/infiniband/core/sysfs.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c > index 0083a4f..f7f2954 100644 > --- a/drivers/infiniband/core/sysfs.c > +++ b/drivers/infiniband/core/sysfs.c > @@ -406,10 +406,14 @@ static PORT_PMA_ATTR(port_rcv_constraint_errors , 8, 8, 136, IB_PMA_PORT_C > static PORT_PMA_ATTR(local_link_integrity_errors , 9, 4, 152, IB_PMA_PORT_COUNTERS); > static PORT_PMA_ATTR(excessive_buffer_overrun_errors, 10, 4, 156, IB_PMA_PORT_COUNTERS); > static PORT_PMA_ATTR(VL15_dropped , 11, 16, 176, IB_PMA_PORT_COUNTERS); > -static PORT_PMA_ATTR(port_xmit_data , 12, 32, 192, IB_PMA_PORT_COUNTERS); > -static PORT_PMA_ATTR(port_rcv_data , 13, 32, 224, IB_PMA_PORT_COUNTERS); > -static PORT_PMA_ATTR(port_xmit_packets , 14, 32, 256, IB_PMA_PORT_COUNTERS); > -static PORT_PMA_ATTR(port_rcv_packets , 15, 32, 288, IB_PMA_PORT_COUNTERS); > +static PORT_PMA_ATTR(port_xmit_data , 0, 64, 64, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(port_rcv_data , 0, 64, 128, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(port_xmit_packets , 0, 64, 192, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(port_rcv_packets , 0, 64, 256, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(unicast_xmit_packets , 0, 64, 320, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(unicast_rcv_packets , 0, 64, 384, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(multicast_xmit_packets , 0, 64, 448, IB_PMA_PORT_COUNTERS_EXT); > +static PORT_PMA_ATTR(multicast_rcv_packets , 0, 64, 512, IB_PMA_PORT_COUNTERS_EXT); > Why do we use 0 as the counter argument for all EXT counters? > static struct attribute *pma_attrs[] = { > &port_pma_attr_symbol_error.attr.attr, > @@ -428,6 +432,10 @@ 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_unicast_rcv_packets.attr.attr, > + &port_pma_attr_unicast_xmit_packets.attr.attr, > + &port_pma_attr_multicast_rcv_packets.attr.attr, > + &port_pma_attr_multicast_xmit_packets.attr.attr, > NULL > }; > > -- > 2.5.0 > > > -- > 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 -- 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, 14 Dec 2015, Matan Barak wrote: > > +static PORT_PMA_ATTR(unicast_rcv_packets , 0, 64, 384, IB_PMA_PORT_COUNTERS_EXT); > > +static PORT_PMA_ATTR(multicast_xmit_packets , 0, 64, 448, IB_PMA_PORT_COUNTERS_EXT); > > +static PORT_PMA_ATTR(multicast_rcv_packets , 0, 64, 512, IB_PMA_PORT_COUNTERS_EXT); > > > > Why do we use 0 as the counter argument for all EXT counters? No idea what the counter is doing. Saw another EXT counter implementation use 0 so I thought that was fine. -- 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, Dec 14, 2015 at 4:55 PM, Christoph Lameter <cl@linux.com> wrote: > On Mon, 14 Dec 2015, Matan Barak wrote: > >> > +static PORT_PMA_ATTR(unicast_rcv_packets , 0, 64, 384, IB_PMA_PORT_COUNTERS_EXT); >> > +static PORT_PMA_ATTR(multicast_xmit_packets , 0, 64, 448, IB_PMA_PORT_COUNTERS_EXT); >> > +static PORT_PMA_ATTR(multicast_rcv_packets , 0, 64, 512, IB_PMA_PORT_COUNTERS_EXT); >> > >> >> Why do we use 0 as the counter argument for all EXT counters? > > No idea what the counter is doing. Saw another EXT counter implementation > use 0 so I thought that was fine. It seems like a counter index, but I might be wrong though. If it is, don't we want to preserve the existing non-EXT schema for the new counters too? -- 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, 14 Dec 2015, Matan Barak wrote: > > No idea what the counter is doing. Saw another EXT counter implementation > > use 0 so I thought that was fine. > > It seems like a counter index, but I might be wrong though. If it is, > don't we want to preserve the existing non-EXT schema for the new > counters too? I do not see any use of that field so I am not sure what to put in there. Could it be obsolete? -- 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 12/11/2015 7:00 PM, Jason Gunthorpe wrote: >> qib, mlx4 are fine. mlx5 should be as well I would think (I don't have that >> hardware.) I'm not 100% sure but I don't think that mthca supports the PortCountersExtended attribute. > I have no specifics to add, but I keep running into systems, even > today, where the 64 bit counters don't work. The MAD might be there, > but several counters are wired to 0. I have seen this too :-( > Not sure exactly which HW though. > > Mellanox should really confirm this for their hardware matrix. I am trying to get definitive answer to this. -- 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
Hello all, On Sat, Dec 12, 2015 at 5:26 AM, ira.weiny <ira.weiny@intel.com> wrote: > On Fri, Dec 11, 2015 at 12:25:35PM -0600, Christoph Lameter wrote: >> Display the additional 64 bit counters available through the extended >> set and replace the existing 32 bit counters if there is a 64 bit >> alternative available. >> >> Note: This requires universal support of extended counters in >> the devices. If there are still devices around that do not >> support extended counters then we will have to add some fallback >> technique here. > > Looks like ocrdma will break here. Yes, today we report 32 bit counters and to support this change a simple patch is needed to replace those cpu_to_be32() with cpu_to_be64(). Internally we already have 64bit countes. > > I'm not sure about mthca. > > qib, mlx4 are fine. mlx5 should be as well I would think (I don't have that > hardware.) > > hfi1 did not process these MADs previously as all the hardware counters are 64 > bits. But with this patch series we would add it. > > ehca, amso1100, and ipath are all gone so they don't matter. > > I can whip up a patch for hfi1 and we have to wait for Doug to take over that > driver anyway to make sure that the patch would apply. So I think you can > ignore it. > > ocrdma seems like it could be a quick patch pre this one. > > Ira > >> >> Signed-off-by: Christoph Lameter <cl@linux.com> >> --- >> drivers/infiniband/core/sysfs.c | 16 ++++++++++++---- >> 1 file changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c >> index 0083a4f..f7f2954 100644 >> --- a/drivers/infiniband/core/sysfs.c >> +++ b/drivers/infiniband/core/sysfs.c >> @@ -406,10 +406,14 @@ static PORT_PMA_ATTR(port_rcv_constraint_errors , 8, 8, 136, IB_PMA_PORT_C >> static PORT_PMA_ATTR(local_link_integrity_errors , 9, 4, 152, IB_PMA_PORT_COUNTERS); >> static PORT_PMA_ATTR(excessive_buffer_overrun_errors, 10, 4, 156, IB_PMA_PORT_COUNTERS); >> static PORT_PMA_ATTR(VL15_dropped , 11, 16, 176, IB_PMA_PORT_COUNTERS); >> -static PORT_PMA_ATTR(port_xmit_data , 12, 32, 192, IB_PMA_PORT_COUNTERS); >> -static PORT_PMA_ATTR(port_rcv_data , 13, 32, 224, IB_PMA_PORT_COUNTERS); >> -static PORT_PMA_ATTR(port_xmit_packets , 14, 32, 256, IB_PMA_PORT_COUNTERS); >> -static PORT_PMA_ATTR(port_rcv_packets , 15, 32, 288, IB_PMA_PORT_COUNTERS); >> +static PORT_PMA_ATTR(port_xmit_data , 0, 64, 64, IB_PMA_PORT_COUNTERS_EXT); >> +static PORT_PMA_ATTR(port_rcv_data , 0, 64, 128, IB_PMA_PORT_COUNTERS_EXT); >> +static PORT_PMA_ATTR(port_xmit_packets , 0, 64, 192, IB_PMA_PORT_COUNTERS_EXT); >> +static PORT_PMA_ATTR(port_rcv_packets , 0, 64, 256, IB_PMA_PORT_COUNTERS_EXT); >> +static PORT_PMA_ATTR(unicast_xmit_packets , 0, 64, 320, IB_PMA_PORT_COUNTERS_EXT); >> +static PORT_PMA_ATTR(unicast_rcv_packets , 0, 64, 384, IB_PMA_PORT_COUNTERS_EXT); >> +static PORT_PMA_ATTR(multicast_xmit_packets , 0, 64, 448, IB_PMA_PORT_COUNTERS_EXT); >> +static PORT_PMA_ATTR(multicast_rcv_packets , 0, 64, 512, IB_PMA_PORT_COUNTERS_EXT); >> >> static struct attribute *pma_attrs[] = { >> &port_pma_attr_symbol_error.attr.attr, >> @@ -428,6 +432,10 @@ 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_unicast_rcv_packets.attr.attr, >> + &port_pma_attr_unicast_xmit_packets.attr.attr, >> + &port_pma_attr_multicast_rcv_packets.attr.attr, >> + &port_pma_attr_multicast_xmit_packets.attr.attr, >> NULL >> }; >> >> -- >> 2.5.0 >> >> >> -- >> 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 > -- > 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 -- 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, 14 Dec 2015, Hal Rosenstock wrote: > > Mellanox should really confirm this for their hardware matrix. > > I am trying to get definitive answer to this. I was told today on a conf call with a couple of Mellanox employees that extended counters are always available. -- 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 Tue, Dec 15, 2015 at 01:51:35PM -0600, Christoph Lameter wrote: > On Mon, 14 Dec 2015, Hal Rosenstock wrote: > > > > Mellanox should really confirm this for their hardware matrix. > > > > I am trying to get definitive answer to this. > > I was told today on a conf call with a couple of Mellanox employees that > extended counters are always available. .. and every field is valid? I would agree, from my observation, that the two main byte counters are always available.. But not necessarily the others. Jason -- 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 12/15/2015 2:55 PM, Jason Gunthorpe wrote: > On Tue, Dec 15, 2015 at 01:51:35PM -0600, Christoph Lameter wrote: >> On Mon, 14 Dec 2015, Hal Rosenstock wrote: >> >>>> Mellanox should really confirm this for their hardware matrix. >>> >>> I am trying to get definitive answer to this. >> >> I was told today on a conf call with a couple of Mellanox employees that >> extended counters are always available. > > .. and every field is valid? > > I would agree, from my observation, that the two main byte counters > are always available. The extended packet counts work but I thought there was a PMA with one of the extended byte counts wired to 0. > But not necessarily the others. The unicast/multicast extended counters are not always supported - depends on setting of PerfMgt ClassPortInfo CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10). -- Hal > Jason -- 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 Tue, Dec 15, 2015 at 03:01:03PM -0500, Hal Rosenstock wrote: > > I would agree, from my observation, that the two main byte counters > > are always available. > > The extended packet counts work but I thought there was a PMA with one > of the extended byte counts wired to 0. Can't remember > > But not necessarily the others. > > The unicast/multicast extended counters are not always supported - > depends on setting of PerfMgt ClassPortInfo > CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10). Yes.. certainly this proposed patch needs to account for that and continue to use the 32 bit ones in that case. Jason -- 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 12/15/2015 4:20 PM, Jason Gunthorpe wrote: >> The unicast/multicast extended counters are not always supported - >> > depends on setting of PerfMgt ClassPortInfo >> > CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10). > Yes.. certainly this proposed patch needs to account for that and > continue to use the 32 bit ones in that case. There are no 32 bit equivalents of those 4 "IETF" counters ([uni multi]cast [xmit rcv] pkts). When not supported, perhaps it is best not to populate these counters in sysfs so one can discern between counter not supported and 0 value. I'm still working on definitive mthca answer but think the attribute is not supported there. Does anyone out there have an mthca setup where they can try this ? -- 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 12/15/2015 04:42 PM, Hal Rosenstock wrote: > On 12/15/2015 4:20 PM, Jason Gunthorpe wrote: >>> The unicast/multicast extended counters are not always supported - >>>> depends on setting of PerfMgt ClassPortInfo >>>> CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10). > >> Yes.. certainly this proposed patch needs to account for that and >> continue to use the 32 bit ones in that case. > > There are no 32 bit equivalents of those 4 "IETF" counters ([uni > multi]cast [xmit rcv] pkts). > > When not supported, perhaps it is best not to populate these counters in > sysfs so one can discern between counter not supported and 0 value. > > I'm still working on definitive mthca answer but think the attribute is > not supported there. Does anyone out there have an mthca setup where > they can try this ? Yes.
On Tue, 15 Dec 2015, Doug Ledford wrote: > On 12/15/2015 04:42 PM, Hal Rosenstock wrote: > > On 12/15/2015 4:20 PM, Jason Gunthorpe wrote: > >>> The unicast/multicast extended counters are not always supported - > >>>> depends on setting of PerfMgt ClassPortInfo > >>>> CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10). > > > >> Yes.. certainly this proposed patch needs to account for that and > >> continue to use the 32 bit ones in that case. > > > > There are no 32 bit equivalents of those 4 "IETF" counters ([uni > > multi]cast [xmit rcv] pkts). > > > > When not supported, perhaps it is best not to populate these counters in > > sysfs so one can discern between counter not supported and 0 value. > > > > I'm still working on definitive mthca answer but think the attribute is > > not supported there. Does anyone out there have an mthca setup where > > they can try this ? > > Yes. We can return ENOSYS for the counters not supported. Or simply not create the sysfs files when the device is instantiated as well as fall back to the 32 bit counters on instantiation for those devices not supporting the extended set. -- 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 12/15/2015 04:46 PM, Doug Ledford wrote: > On 12/15/2015 04:42 PM, Hal Rosenstock wrote: >> On 12/15/2015 4:20 PM, Jason Gunthorpe wrote: >>>> The unicast/multicast extended counters are not always supported - >>>>> depends on setting of PerfMgt ClassPortInfo >>>>> CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10). >> >>> Yes.. certainly this proposed patch needs to account for that and >>> continue to use the 32 bit ones in that case. >> >> There are no 32 bit equivalents of those 4 "IETF" counters ([uni >> multi]cast [xmit rcv] pkts). >> >> When not supported, perhaps it is best not to populate these counters in >> sysfs so one can discern between counter not supported and 0 value. >> >> I'm still working on definitive mthca answer but think the attribute is >> not supported there. Does anyone out there have an mthca setup where >> they can try this ? > > Yes. > > From my mthca machine: [root@rdma-dev-04 ~]$ lspci | grep Mellanox 01:00.0 InfiniBand: Mellanox Technologies MT25208 InfiniHost III Ex (Tavor compatibility mode) (rev 20) [root@rdma-dev-04 ~]$ perfquery # Port counters: Lid 36 port 1 (CapMask: 0x00) PortSelect:......................1 CounterSelect:...................0x0000 SymbolErrorCounter:..............0 LinkErrorRecoveryCounter:........0 LinkDownedCounter:...............0 PortRcvErrors:...................0 PortRcvRemotePhysicalErrors:.....0 PortRcvSwitchRelayErrors:........0 PortXmitDiscards:................0 PortXmitConstraintErrors:........0 PortRcvConstraintErrors:.........0 CounterSelect2:..................0x00 LocalLinkIntegrityErrors:........0 ExcessiveBufferOverrunErrors:....0 VL15Dropped:.....................1 PortXmitData:....................2470620192 PortRcvData:.....................2401094563 PortXmitPkts:....................6363544 PortRcvPkts:.....................6321251 [root@rdma-dev-04 ~]$ perfquery -x ibwarn: [29831] dump_perfcounters: PerfMgt ClassPortInfo 0x0; No extended counter support indicated perfquery: iberror: failed: perfextquery So, no extended counters on this device.
On Mon, Dec 14, 2015 at 6:06 PM, Christoph Lameter <cl@linux.com> wrote: > On Mon, 14 Dec 2015, Matan Barak wrote: > >> > No idea what the counter is doing. Saw another EXT counter implementation >> > use 0 so I thought that was fine. >> >> It seems like a counter index, but I might be wrong though. If it is, >> don't we want to preserve the existing non-EXT schema for the new >> counters too? > > I do not see any use of that field so I am not sure what to put in there. > Could it be obsolete? > I don't see any usage to that field either, but I think 0 is a bit misleading. So maybe it's more appropriate to delete this field. -- 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 12/20/2015 5:10 AM, Matan Barak wrote: > On Mon, Dec 14, 2015 at 6:06 PM, Christoph Lameter <cl@linux.com> wrote: >> On Mon, 14 Dec 2015, Matan Barak wrote: >> >>>> No idea what the counter is doing. Saw another EXT counter implementation >>>> use 0 so I thought that was fine. >>> >>> It seems like a counter index, but I might be wrong though. If it is, >>> don't we want to preserve the existing non-EXT schema for the new >>> counters too? >> >> I do not see any use of that field so I am not sure what to put in there. >> Could it be obsolete? >> > > I don't see any usage to that field either, but I think 0 is a bit misleading. > So maybe it's more appropriate to delete this field. counter number is gone in Christoph's latest patches in terms of the new port extended counters but still present for the original 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
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c index 0083a4f..f7f2954 100644 --- a/drivers/infiniband/core/sysfs.c +++ b/drivers/infiniband/core/sysfs.c @@ -406,10 +406,14 @@ static PORT_PMA_ATTR(port_rcv_constraint_errors , 8, 8, 136, IB_PMA_PORT_C static PORT_PMA_ATTR(local_link_integrity_errors , 9, 4, 152, IB_PMA_PORT_COUNTERS); static PORT_PMA_ATTR(excessive_buffer_overrun_errors, 10, 4, 156, IB_PMA_PORT_COUNTERS); static PORT_PMA_ATTR(VL15_dropped , 11, 16, 176, IB_PMA_PORT_COUNTERS); -static PORT_PMA_ATTR(port_xmit_data , 12, 32, 192, IB_PMA_PORT_COUNTERS); -static PORT_PMA_ATTR(port_rcv_data , 13, 32, 224, IB_PMA_PORT_COUNTERS); -static PORT_PMA_ATTR(port_xmit_packets , 14, 32, 256, IB_PMA_PORT_COUNTERS); -static PORT_PMA_ATTR(port_rcv_packets , 15, 32, 288, IB_PMA_PORT_COUNTERS); +static PORT_PMA_ATTR(port_xmit_data , 0, 64, 64, IB_PMA_PORT_COUNTERS_EXT); +static PORT_PMA_ATTR(port_rcv_data , 0, 64, 128, IB_PMA_PORT_COUNTERS_EXT); +static PORT_PMA_ATTR(port_xmit_packets , 0, 64, 192, IB_PMA_PORT_COUNTERS_EXT); +static PORT_PMA_ATTR(port_rcv_packets , 0, 64, 256, IB_PMA_PORT_COUNTERS_EXT); +static PORT_PMA_ATTR(unicast_xmit_packets , 0, 64, 320, IB_PMA_PORT_COUNTERS_EXT); +static PORT_PMA_ATTR(unicast_rcv_packets , 0, 64, 384, IB_PMA_PORT_COUNTERS_EXT); +static PORT_PMA_ATTR(multicast_xmit_packets , 0, 64, 448, IB_PMA_PORT_COUNTERS_EXT); +static PORT_PMA_ATTR(multicast_rcv_packets , 0, 64, 512, IB_PMA_PORT_COUNTERS_EXT); static struct attribute *pma_attrs[] = { &port_pma_attr_symbol_error.attr.attr, @@ -428,6 +432,10 @@ 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_unicast_rcv_packets.attr.attr, + &port_pma_attr_unicast_xmit_packets.attr.attr, + &port_pma_attr_multicast_rcv_packets.attr.attr, + &port_pma_attr_multicast_xmit_packets.attr.attr, NULL };
Display the additional 64 bit counters available through the extended set and replace the existing 32 bit counters if there is a 64 bit alternative available. Note: This requires universal support of extended counters in the devices. If there are still devices around that do not support extended counters then we will have to add some fallback technique here. Signed-off-by: Christoph Lameter <cl@linux.com> --- drivers/infiniband/core/sysfs.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)