On Mon, Jun 07, 2021 at 09:14:11AM -0300, Jason Gunthorpe wrote:

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?  

The write side of the counter is being updated from concurrent kernel
threads without locking, so this is an atomic because the write side
needs atomic_add().

So the atomic write forces a lock :(

Making them a naked u64 will cause significant corruption on the write
side, and packet counters that are not accurate after quiescence are
not very useful things.

How "accurate" do these have to be?

And have you all tried them?

I'm pushing back here as I see a lot of atomics used for debugging
statistics for no good reason all over the place.  Especially when
userspace just does not care.

thanks,

greg k-h