linux-kernel - Re: [PATCH] Make for_each_cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 11 May 2008 16:01:12 -0600
From:	Matthew Wilcox <matthew@....cx>
To:	Alexander van Heukelum <heukelum@...tmail.fm>
Cc:	Alexander van Heukelum <heukelum@...lshack.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Paul Jackson <pj@....com>, Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-arch <linux-arch@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Make for_each_cpu_mask a bit smaller

On Sun, May 11, 2008 at 06:19:39PM +0200, Alexander van Heukelum wrote:
> 
> On Sun, 11 May 2008 09:24:40 -0600, "Matthew Wilcox" <matthew@....cx>
> said:
> > On Sun, May 11, 2008 at 03:50:39PM +0200, Alexander van Heukelum wrote:
> > >  #if NR_CPUS > 1
> > > -#define for_each_cpu_mask(cpu, mask)		\
> > > -	for ((cpu) = first_cpu(mask);		\
> > > -		(cpu) < NR_CPUS;		\
> > > -		(cpu) = next_cpu((cpu), (mask)))
> > > +#define for_each_cpu_mask(cpu, mask)				\
> > > +	for ((cpu) = 0;						\
> > > +		(cpu) = find_next_cpu_mask((cpu), &(mask)),	\
> > > +		(cpu) < NR_CPUS; (cpu)++)
> > 
> > For anyone else having similar cognitive dissonance while reading this
> > thinking "But won't the first call to find_next_cpu_mask return a number
> > > 0", the answer is "no, find_next_bit returns the next set bit that's
> > >= the number passed in, which is why we need both the cpu++ and
> > find_next_cpu_mask".  
> 
> That's how it works, indeed.
> 
> > > +int find_next_cpu_mask(int n, const cpumask_t *srcp)
> > > +{
> > > +	return find_next_bit(srcp->bits, NR_CPUS, n);
> > > +}
> > > +EXPORT_SYMBOL(find_next_cpu_mask);
> > 
> > Maybe a better name for this function would help.  I can't think of a
> > good one right now though.
> 
> I can't think of a better name, and there is find_next_bit of which
> find_next_cpu_mask is just a wrapper. I think the name is good enough.

How about doing it this way?

#define for_each_cpu_mask(cpu, mask)				\
	for ((cpu) = -1;					\
	     (cpu) < NR_CPUS;					\
	     (cpu) = find_next_cpu_mask((cpu), &(mask)))

int find_next_cpu_mask(int n, const cpumask_t *srcp)
{
	return find_next_bit(srcp->bits, NR_CPUS, ++n);
}

That actually behaves the way I'd expect a function called
'find_next_cpu_mask' to work.  It also abuses the 'for' condtion
less and might take a little less text space.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/