Re: [PATCH rdma-next v1 10/15] RDMA/cm: Use an attribute_group on the ib_port_attribute intead of kobj's

[Greg KH]

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:
> > On Mon, Jun 07, 2021 at 11:17:35AM +0300, Leon Romanovsky wrote:
> > > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > > 
> > > 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