Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2012-01-04 Thread Or Gerlitz
On Tue, Dec 20, 2011 Jason Gunthorpe jguntho...@obsidianresearch.com wrote: On Tue, Dec 20, 2011 Or Gerlitz wrote: Jason Gunthorpe jguntho...@obsidianresearch.com wrote: The netdev counters are all the same size and there is some other way to discover what the size is. I'd like to see that

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-12-22 Thread Or Gerlitz
On 11/8/2011 2:54 AM, Roland Dreier wrote: Let's make sure we learn from our mistakes. Let's say we create a new ext_counters directory. What should the format of those files be? Should they be assumed to be 64-bit quantities? Do we want to allow some way of indicating the number of bits (ie

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-12-20 Thread Or Gerlitz
Ira Weiny wrote: Jason Gunthorpejguntho...@obsidianresearch.com wrote: How do you feel about having counters_ext appear in ethernet mode and disappear in IB mode? That might get complicated with ports which can be either mode. Ira (reviving this thread), At the ib core level, the link layer

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-12-20 Thread Or Gerlitz
Ira Weiny wrote: Jason Gunthorpejguntho...@obsidianresearch.com wrote: How do you feel about having counters_ext appear in ethernet mode and disappear in IB mode? That might get complicated with ports which can be either mode. Ira (reviving this thread), At the ib core level, the link layer

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-12-20 Thread Or Gerlitz
On 11/8/2011 3:09 AM, Jason Gunthorpe wrote: Roland Dreier wrote: Let's make sure we learn from our mistakes. Let's say we create a new ext_counters directory. What should the format of those files be? Should they be assumed to be 64-bit quantities? Do we want to allow some way of

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-12-20 Thread Jason Gunthorpe
On Tue, Dec 20, 2011 at 02:03:31PM +0200, Or Gerlitz wrote: That is a good idea. Let's require counters_ext to be sane: 1 Hex quantity of unspecified size 2 Prefixed with 0x and leading zeros to fill out to size and allow userspace discovery of size 3 Size must be a multiple of 4

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-12-20 Thread Or Gerlitz
Jason Gunthorpe jguntho...@obsidianresearch.com wrote: The netdev counters are all the same size and there is some other way to discover what the size is. I'd like to see that for IB counters too, but it is probably infeasible. So if we have counters that are not the same size as netdev

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-12-20 Thread Jason Gunthorpe
On Tue, Dec 20, 2011 at 09:40:09PM +0200, Or Gerlitz wrote: Jason Gunthorpe jguntho...@obsidianresearch.com wrote: The netdev counters are all the same size and there is some other way to discover what the size is. I'd like to see that for IB counters too, but it is probably infeasible. So

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-12-20 Thread Or Gerlitz
Jason Gunthorpe jguntho...@obsidianresearch.com wrote: We're talking now only on the IB extended counters who are all 64 bits netdev counters are 32 bit or 64 bit, depending on how the kernel was compiled. I think indicating the size explicitly, or always being 64 bit (and extending all the

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-08 Thread Ira Weiny
On Mon, 7 Nov 2011 17:09:52 -0800 Jason Gunthorpe jguntho...@obsidianresearch.com wrote: On Mon, Nov 07, 2011 at 04:54:42PM -0800, Roland Dreier wrote: On Wed, Nov 2, 2011 at 10:16 AM, Or Gerlitz ogerl...@mellanox.com wrote: I suggest we go that least bad way along the lines of your

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-07 Thread Roland Dreier
On Wed, Nov 2, 2011 at 10:16 AM, Or Gerlitz ogerl...@mellanox.com wrote: I suggest we go that least bad way along the lines of your comment. If/when on some future point something constructive can be formed from Jason's observations, changes will follow, agree? Let's make sure we learn from

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-07 Thread Jason Gunthorpe
On Mon, Nov 07, 2011 at 04:54:42PM -0800, Roland Dreier wrote: On Wed, Nov 2, 2011 at 10:16 AM, Or Gerlitz ogerl...@mellanox.com wrote: I suggest we go that least bad way along the lines of your comment. If/when on some future point something constructive can be formed from Jason's

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-02 Thread Or Gerlitz
On 11/2/2011 12:46 AM, Ira Weiny wrote: What do you mean for both? Ira The sysfs PMA counters are functional for both IB and IBoE, the latter uses a HW counter allocated per device/port for which all the QPs created on that port are reporting their rx/tx bytes/packets, see commits

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-02 Thread Or Gerlitz
On 11/1/2011 11:58 PM, Jason Gunthorpe wrote: maybe you should patch to un-export them until things can be fixed sanely... Jason, I don't think we want to work this way, bug are either opened or attempted to be fixed or fixed. You can't just come out of the the blue and remove

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-02 Thread Or Gerlitz
On 11/1/2011 11:42 PM, Roland Dreier wrote: The least bad way forward does seem like it is probably the separate new directory thing Hi Roland, I suggest we go that least bad way along the lines of your comment. If/when on some future point something constructive can be formed from Jason's

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Or Gerlitz
On Mon, Oct 31, 2011 at 9:38 PM, Roland Dreier rol...@kernel.org wrote: Sorry for the late review here Oh yes... BTW this is patch 4/5, I don't see patches 1,2,3 on your for-next tree/branch @ kernel.org, have you accepted them? Sorry for the late review here, but does it seem like the best

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Jason Gunthorpe
On Tue, Nov 01, 2011 at 08:40:12AM +0200, Or Gerlitz wrote: Today, e.g in some IBoE perf monitoring scripts we wrote, the distinction is done by if (the ext counter directory exists) then go and read the counters from there, else read from the non extended counters directory. With the change

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Or Gerlitz
Jason Gunthorpe jguntho...@obsidianresearch.com wrote: Is there any reason to expose the 32 and 64 bit version of the same counter? That seems needless. Emit the largest version available and prepend 0's to fill out to the available width so that userspace can know the counter size.

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Jason Gunthorpe
On Tue, Nov 01, 2011 at 07:23:52PM +0200, Or Gerlitz wrote: Jason Gunthorpe jguntho...@obsidianresearch.com wrote: Is there any reason to expose the 32 and 64 bit version of the same counter? That seems needless. Emit the largest version available and prepend 0's to fill out to the

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Or Gerlitz
Jason Gunthorpe jguntho...@obsidianresearch.com wrote: Guys (Roland, Jason), I'm open to any comments, any time, for any patch, but for a patch which was posted weeks ago it's pretty unfair to have your comments coming only eight days after the merge window has been opened, lets try to come

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Jason Gunthorpe
I don't see a problem with having a sysfs counter file being extended to return a 64 bit number.. I think that is within the purvue of acceptable changes. Shame the counter wasn't exported as hex though - makes it harder to signal if it is 32 or 64 bit. if I understand you right, we

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Roland Dreier
On Tue, Nov 1, 2011 at 11:37 AM, Jason Gunthorpe jguntho...@obsidianresearch.com wrote: Whats the problem here? If a 64 bit counter is available then export it as 64 bit otherwise keep exporting something smaller. I agree zero padding non-hex numbers isn't ideal. Export as hex? I agree that

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Or Gerlitz
On Tue, Nov 1, 2011 at 11:42 PM, Roland Dreier rol...@kernel.org wrote: The least bad way forward does seem like it is probably the separate new directory thing. I agree Or. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Or Gerlitz
Jason Gunthorpe jguntho...@obsidianresearch.com wrote: I don't mean the 32 bit counters are useless, I mean exposing PMA counters that saturate and can be randomly reset by external agents through sysfs is useless. You can't make any kind of data collection based on such a system. Ideally

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Roland Dreier
On Tue, Nov 1, 2011 at 11:14 AM, Or Gerlitz or.gerl...@gmail.com wrote: Guys (Roland, Jason), I'm open to any comments, any time, for any patch, but for a patch which was posted weeks ago it's pretty unfair to have your comments coming only eight days after the merge window has been opened,

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Jason Gunthorpe
On Tue, Nov 01, 2011 at 02:42:33PM -0700, Roland Dreier wrote: I agree that it definitely is more appealing, if we have a 64-bit version of a counter, that we should just export that counter where we used to export the 32-bit version. I think this falls under the 'undocumented, beware' API

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Jason Gunthorpe
On Tue, Nov 01, 2011 at 11:46:08PM +0200, Or Gerlitz wrote: In the same vien adding saturating but non-resettable PMA-esque counters for IBoE seems pretty hackish to me.. Though I agree it is not terribly relevant for 64 bit counters. To put things in place, the IB stack PMA counters

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Jason Gunthorpe
On Tue, Nov 01, 2011 at 03:03:58PM -0700, Ira Weiny wrote: And again, this is a useless interface in IB. Why do you mean by this? A counter that is randomly reset by an external IB performance manager is not useful for collecting local statistical information. A counter that saturates

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Ira Weiny
On Tue, 1 Nov 2011 15:11:35 -0700 Jason Gunthorpe jguntho...@obsidianresearch.com wrote: On Tue, Nov 01, 2011 at 03:03:58PM -0700, Ira Weiny wrote: And again, this is a useless interface in IB. Why do you mean by this? A counter that is randomly reset by an external IB performance

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Or Gerlitz
On Tue, Nov 1, 2011 at 11:49 PM, Roland Dreier rol...@kernel.org wrote: There's no obligation to merge something just because you posted it before the merge window, and in fact Linus's complaint at the kernel summit is always that sub-maintainers don't say no enough. And let's be honest in

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Or Gerlitz
Jason Gunthorpe jguntho...@obsidianresearch.com wrote: Why have a sysfs counter at all when you can just ask the PMA and get exactly the same data? The HW/FW PMA agent isn't supported for IBoE only for IB, the counters are for both Or. -- To unsubscribe from this list: send the line

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Ira Weiny
On Tue, 1 Nov 2011 15:34:46 -0700 Or Gerlitz or.gerl...@gmail.com wrote: Jason Gunthorpe jguntho...@obsidianresearch.com wrote: Why have a sysfs counter at all when you can just ask the PMA and get exactly the same data? The HW/FW PMA agent isn't supported for IBoE only for IB, the

RE: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-11-01 Thread Hefty, Sean
Let's not get into fairness here... I'm trying to make progress on my backlog but there are patches that for better or worse have been around for a year or more. Along these lines, is there any news on when patchwork might be available again? I've been trying to help review some of the

Re: [PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-10-31 Thread Roland Dreier
On Mon, Oct 10, 2011 at 1:56 AM, Or Gerlitz ogerl...@mellanox.com wrote: +static struct attribute_group pma_ext_group = { +       .name  = counters_ext, +       .attrs  = pma_attrs_ext +}; Sorry for the late review here, but does it seem like the best approach to have a separate counters_ext

[PATCH 4/5] ib/core: add support for extended performance counters in sysfs

2011-10-10 Thread Or Gerlitz
issue a PMA classportinfo query towards the HW driver, and if extended counters are supported, expose new sysfs entries which allow to read them. Signed-off-by: Vu Pham v...@mellanox.com Signed-off-by: Or Gerlitz ogerl...@mellanox.com --- --- drivers/infiniband/core/sysfs.c | 100