* Re: Fwd: Re: [PATCH] IB/core: Make device counter infrastructure dynamic
[not found] ` <490a5365-ce89-f4a5-3547-32c37757da1d-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2016-05-20 3:17 ` Doug Ledford
2016-05-20 11:53 ` Leon Romanovsky
1 sibling, 0 replies; 3+ messages in thread
From: Doug Ledford @ 2016-05-20 3:17 UTC (permalink / raw)
To: linux-rdma
[-- Attachment #1: Type: text/plain, Size: 795 bytes --]
On 05/19/2016 11:15 PM, Doug Ledford wrote:
> On 05/19/2016 05:50 PM, Jason Gunthorpe wrote:
>> On Thu, May 19, 2016 at 05:35:13PM -0400, Doug Ledford wrote:
>>> + [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.
As a second issue, this is what allowed us to remove half of the
BUILD_BUG_ON statements. So, removing it means putting those back in.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: Fwd: Re: [PATCH] IB/core: Make device counter infrastructure dynamic
[not found] ` <490a5365-ce89-f4a5-3547-32c37757da1d-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-05-20 3:17 ` Doug Ledford
@ 2016-05-20 11:53 ` Leon Romanovsky
1 sibling, 0 replies; 3+ messages in thread
From: Leon Romanovsky @ 2016-05-20 11:53 UTC (permalink / raw)
To: Doug Ledford; +Cc: linux-rdma
[-- 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 --]
^ permalink raw reply [flat|nested] 3+ messages in thread