Messages in this thread | | | Date | Thu, 6 Mar 2008 10:56:03 +0000 | From | Andy Whitcroft <> | Subject | Re: [patch 1/3] Relay Reset Consumed |
| |
On Wed, Mar 05, 2008 at 02:12:29PM -0800, Andrew Morton wrote: > On Mon, 03 Mar 2008 15:51:50 -0800 > "David J. Wilder" <dwilder@us.ibm.com> wrote: > > > This patch allows relay channels to be reset i.e. unconsumed. > > Basically allows a 'rewind' function for flight-recorder tracing. > > > > +void relay_reset_consumed(struct rchan *chan) > > +{ > > + unsigned int i; > > + struct rchan_buf *prev = NULL; > > + > > + if (!chan) > > + return; > > + > > + for (i = 0; i < NR_CPUS; i++) { > > Use of NR_CPUS is usually wrong. In this case it seems you should be using > for_each_possible_cpu(). > > Also the existing usage of NR_CPUS in relay_subbufs_consumed() should be > switched to using cpu_possible(). > > > New usage of NR_CPUS might be checkpatch-worthy, actually: > > akpm:/usr/src/25> grep -l '^+.*NR_CPUS' patches/*.patch > patches/ext4-mm-mballoc-core.patch > patches/git-kvm.patch > patches/git-perfmon.patch > patches/relay-reset-consumed.patch > patches/x86-andi-git-x86.patch > patches/x86-andi-smp-switch-optimize.patch > > that's a sample of 1852 patches. > > An appropriate warning would be "Usage of NR_CPUS is often wrong - should > you be using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), > etc?".
That seems reasonable. If we special case the definitions of NR_CPUS and the use of them to define arrays of things both of which appear reasonable,
#define NR_CPUS 10 int semid[NR_CPUS];
then it seems sensible to report this one:
WARNING: usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc #1: FILE: Z101.c:1: + for (i = 0; i < NR_CPUS; i++)
This will be in the next push to -mm.
-apw
| |