On Mon, Jun 07, 2021 at 01:22:26PM +0200, Greg KH wrote:

On Mon, Jun 07, 2021 at 01:51:36PM +0300, Leon Romanovsky wrote:

quoted

On Mon, Jun 07, 2021 at 12:25:03PM +0200, Greg KH wrote:

quoted

On Mon, Jun 07, 2021 at 11:17:35AM +0300, Leon Romanovsky wrote:

quoted

From: Jason Gunthorpe <jgg@nvidia.com>

This code is trying to attach a list of counters grouped into 4 groups to
the ib_port sysfs. Instead of creating a bunch of kobjects simply express
everything naturally as an ib_port_attribute and add a single
attribute_groups list.

Remove all the naked kobject manipulations.

Much nicer.

But why do you need your counters to be atomic in the first place?  What
are they counting that requires this?  Given that they are just a
statistic for userspace, making them be a u64 should work just the same,
right?

The statistic counters are per-port, while the cm.c flows run in
asynchronically in parallel for every CM connection.

We need atomic variable to ensure that "write to u64" is not
interrupted.

On what system is "write to u64" interruptable? 

On 32 bits, and yes, we have a customer who still uses such system.

As these are per-port, do multiple threads try to increment these at
the same time?  

Yes, CM connection can be seen as thread. Bottom line everything in parallel.

And even if they do, what happens if one is 'dropped' somehow because of this?

Probably nothing, we increment the statistics only.

It's just a userspace statistic counter, what relies on this being
exact?

In kernel nothing, but I have no idea what userspace does with these counters.

thanks,

greg k-h