Re: Fwd: Re: [PATCH] IB/core: Make device counter infrastructure dynamic

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2585 bytes --]

On Thu, May 19, 2016 at 11:15:45PM -0400, Doug Ledford wrote:
> Reply-all didn't include the list for some reason :-/
> 
> 
> -------- Forwarded Message --------
> Subject: Re: [PATCH] IB/core: Make device counter infrastructure dynamic
> Date: Thu, 19 May 2016 18:13:22 -0400
> From: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Organization: Red Hat, Inc.
> To: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> CC: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>, Mark
> Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> On 05/19/2016 05:50 PM, Jason Gunthorpe wrote:
> > On Thu, May 19, 2016 at 05:35:13PM -0400, Doug Ledford wrote:
> > 
> >> +	return sprintf(buf, "%d msecs (rounded from jiffies), 0-10000 allowed "
> >> +		       "range\n", msecs);
> > 
> > sysfs files should not have usage commentary like that, just use the
> > %d
> 
> I had that originally, and I'll pull this out when the stuff is
> documented somewhere else (either in the code or Documentation).

There is Documentation/infiniband/sysfs.txt [1], which looks as a good
candidate for this.

[1] http://lxr.free-electrons.com/source/Documentation/infiniband/sysfs.txt

> 
> >> +char *names[] = {
> > 
> > static const char * const names[] = {
> > 
> > For Kees
> 
> Fixed (in all places)
> 
> >> +	[NR_COUNTERS] = NULL
> > 
> > Don't really need this, just use ARRAY_SIZE(names) in place of
> > NR_COUTNERS
> 
> The original code required a NULL terminated array.  Given that the attr
> array requires a NULL terminated array, and these are used to set that
> up, I'm not sure I mind burning an extra NULL for the semantic match,
> even though it's no longer necessary.

I disagree with your statement that removing BUILD_BUG_ON() checks is
a positive thing. In this case, we are talking about constant array
which is not going to be changed till new code will be added and compiled
again.

The questions which can be asked, do we need so much BUILD_BUG_ON checks?
Can we minimize the number of them?

> 
> > Trailing comments are not the kdoc standard for structure members,
> > IIRC.
> 
> That falls in the same category as the lifespan sysfs file.  I tend to
> document things in place while I'm writing, and then erase the
> documentation when the final version is complete and the real docs are
> in place.

In such case, it is better to send as RFC to present that this patch is
not in its final stage yet.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]