public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fix cpu iterator on empty bitmask
@ 2004-04-27 23:52 Rusty Russell
  2004-04-28  0:05 ` William Lee Irwin III
  0 siblings, 1 reply; 6+ messages in thread
From: Rusty Russell @ 2004-04-27 23:52 UTC (permalink / raw)
  To: Joel Schopp, Anton Blanchard, Andrew Morton; +Cc: lkml - Kernel Mailing List

Name: Fix cpumask iterator over empty cpu set
Status: Trivial

Can't use _ffs() without first checking for zero, and if bits beyond
NR_CPUS set it'll give bogus results.  Use find_first_bit

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26180-linux-2.6.6-rc2-bk5/include/asm-generic/cpumask_arith.h .26180-linux-2.6.6-rc2-bk5.updated/include/asm-generic/cpumask_arith.h
--- .26180-linux-2.6.6-rc2-bk5/include/asm-generic/cpumask_arith.h	2004-01-10 13:59:33.000000000 +1100
+++ .26180-linux-2.6.6-rc2-bk5.updated/include/asm-generic/cpumask_arith.h	2004-04-28 09:50:23.000000000 +1000
@@ -43,7 +43,7 @@
 #define cpus_promote(map)		({ map; })
 #define cpumask_of_cpu(cpu)		({ ((cpumask_t)1) << (cpu); })
 
-#define first_cpu(map)			__ffs(map)
+#define first_cpu(map)			find_first_bit(&(map), NR_CPUS)
 #define next_cpu(cpu, map)		find_next_bit(&(map), NR_CPUS, cpu + 1)
 
 #endif /* __ASM_GENERIC_CPUMASK_ARITH_H */
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix cpu iterator on empty bitmask
  2004-04-27 23:52 [PATCH] Fix cpu iterator on empty bitmask Rusty Russell
@ 2004-04-28  0:05 ` William Lee Irwin III
  2004-04-28  1:22   ` Rusty Russell
  0 siblings, 1 reply; 6+ messages in thread
From: William Lee Irwin III @ 2004-04-28  0:05 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Joel Schopp, Anton Blanchard, Andrew Morton,
	lkml - Kernel Mailing List

On Wed, Apr 28, 2004 at 09:52:53AM +1000, Rusty Russell wrote:
> Name: Fix cpumask iterator over empty cpu set
> Status: Trivial
> Can't use _ffs() without first checking for zero, and if bits beyond
> NR_CPUS set it'll give bogus results.  Use find_first_bit

I sent in something equivalent to this along with a number of other
fixes (cpus_shift_right() leaking junk bits in in and cpus_weight()
and cpus_empty() and cpus_equal() and the like not ignoring tails) and
got a NAK since it clashes with something Paul Jackson's doing.


-- wli

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix cpu iterator on empty bitmask
  2004-04-28  0:05 ` William Lee Irwin III
@ 2004-04-28  1:22   ` Rusty Russell
  2004-04-28  1:31     ` Paul Jackson
  0 siblings, 1 reply; 6+ messages in thread
From: Rusty Russell @ 2004-04-28  1:22 UTC (permalink / raw)
  To: William Lee Irwin III
  Cc: Andrew Morton, lkml - Kernel Mailing List, Paul Jackson,
	Linus Torvalds

On Wed, 2004-04-28 at 10:05, William Lee Irwin III wrote:
> On Wed, Apr 28, 2004 at 09:52:53AM +1000, Rusty Russell wrote:
> > Name: Fix cpumask iterator over empty cpu set
> > Status: Trivial
> > Can't use _ffs() without first checking for zero, and if bits beyond
> > NR_CPUS set it'll give bogus results.  Use find_first_bit
> 
> I sent in something equivalent to this along with a number of other
> fixes (cpus_shift_right() leaking junk bits in in and cpus_weight()
> and cpus_empty() and cpus_equal() and the like not ignoring tails) and
> got a NAK since it clashes with something Paul Jackson's doing.

Agreed, I'm pretty sure Paul's work doesn't make this mistake, but this
is a trivial patch for a real big which is causing oopses today.

Linus, please apply...
Rusty.

Name: Fix cpumask iterator over empty cpu set
Status: Trivial

Can't use _ffs() without first checking for zero, and if bits beyond
NR_CPUS set it'll give bogus results.  Use find_first_bit

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26180-linux-2.6.6-rc2-bk5/include/asm-generic/cpumask_arith.h .26180-linux-2.6.6-rc2-bk5.updated/include/asm-generic/cpumask_arith.h
--- .26180-linux-2.6.6-rc2-bk5/include/asm-generic/cpumask_arith.h	2004-01-10 13:59:33.000000000 +1100
+++ .26180-linux-2.6.6-rc2-bk5.updated/include/asm-generic/cpumask_arith.h	2004-04-28 09:50:23.000000000 +1000
@@ -43,7 +43,7 @@
 #define cpus_promote(map)		({ map; })
 #define cpumask_of_cpu(cpu)		({ ((cpumask_t)1) << (cpu); })
 
-#define first_cpu(map)			__ffs(map)
+#define first_cpu(map)			find_first_bit(&(map), NR_CPUS)
 #define next_cpu(cpu, map)		find_next_bit(&(map), NR_CPUS, cpu + 1)
 
 #endif /* __ASM_GENERIC_CPUMASK_ARITH_H */

-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix cpu iterator on empty bitmask
  2004-04-28  1:22   ` Rusty Russell
@ 2004-04-28  1:31     ` Paul Jackson
  2004-04-28  1:57       ` William Lee Irwin III
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Jackson @ 2004-04-28  1:31 UTC (permalink / raw)
  To: Rusty Russell; +Cc: wli, akpm, linux-kernel, torvalds

Rusty wrote:
> Agreed, I'm pretty sure Paul's work doesn't make this mistake, but this
> is a trivial patch for a real big which is causing oopses today.
>
> Linus, please apply...

agreed

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix cpu iterator on empty bitmask
  2004-04-28  1:31     ` Paul Jackson
@ 2004-04-28  1:57       ` William Lee Irwin III
  2004-04-28  2:18         ` Paul Jackson
  0 siblings, 1 reply; 6+ messages in thread
From: William Lee Irwin III @ 2004-04-28  1:57 UTC (permalink / raw)
  To: Paul Jackson; +Cc: Rusty Russell, akpm, linux-kernel, torvalds

Rusty wrote:
>> Agreed, I'm pretty sure Paul's work doesn't make this mistake, but this
>> is a trivial patch for a real big which is causing oopses today.
>> Linus, please apply...

On Tue, Apr 27, 2004 at 06:31:35PM -0700, Paul Jackson wrote:
> agreed

Eh? Why are you now suddenly changing your tune from when I wrote a fix
for this and others just 10 days ago? The remainder of the missing fixes
below.


-- wli


Index: wli-2.6.5-mm6/include/asm-generic/cpumask_arith.h
===================================================================
--- wli-2.6.5-mm6.orig/include/asm-generic/cpumask_arith.h	2004-04-03 19:37:37.000000000 -0800
+++ wli-2.6.5-mm6/include/asm-generic/cpumask_arith.h	2004-04-17 19:11:53.000000000 -0700
@@ -15,17 +15,17 @@
 #define cpus_or(dst,src1,src2)		do { dst = (src1) | (src2); } while (0)
 #define cpus_clear(map)			do { map = 0; } while (0)
 #define cpus_complement(map)		do { map = ~(map); } while (0)
-#define cpus_equal(map1, map2)		((map1) == (map2))
-#define cpus_empty(map)			((map) == 0)
+#define cpus_equal(map1, map2)		(!(((map1) ^ (map2)) & CPU_MASK_ALL))
+#define cpus_empty(map)			(!((map) & CPU_MASK_ALL))
 #define cpus_addr(map)			(&(map))
 
 #if BITS_PER_LONG == 32
-#define cpus_weight(map)		hweight32(map)
+#define cpus_weight(map)		hweight32((map) & CPU_MASK_ALL)
 #elif BITS_PER_LONG == 64
-#define cpus_weight(map)		hweight64(map)
+#define cpus_weight(map)		hweight64((map) & CPU_MASK_ALL)
 #endif
 
-#define cpus_shift_right(dst, src, n)	do { dst = (src) >> (n); } while (0)
+#define cpus_shift_right(dst, src, n)	do { dst = ((src) >> (n)) & CPU_MASK_ALL; } while (0)
 #define cpus_shift_left(dst, src, n)	do { dst = (src) << (n); } while (0)
 
 #define any_online_cpu(map)			\
@@ -39,11 +39,15 @@
 #define CPU_MASK_NONE	((cpumask_t)0)
 
 /* only ever use this for things that are _never_ used on large boxen */
-#define cpus_coerce(map)		((unsigned long)(map))
+#define cpus_coerce(map)		((unsigned long)(map) & CPU_MASK_ALL)
 #define cpus_promote(map)		({ map; })
 #define cpumask_of_cpu(cpu)		({ ((cpumask_t)1) << (cpu); })
 
-#define first_cpu(map)			__ffs(map)
 #define next_cpu(cpu, map)		find_next_bit(&(map), NR_CPUS, cpu + 1)
+#define first_cpu(map)						\
+({								\
+	cpumask_t __first_cpu_map__ = (map) & CPU_MASK_ALL;	\
+	__first_cpu_map__ ? __ffs(__first_cpu_map__) : NR_CPUS;	\
+})
 
 #endif /* __ASM_GENERIC_CPUMASK_ARITH_H */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix cpu iterator on empty bitmask
  2004-04-28  1:57       ` William Lee Irwin III
@ 2004-04-28  2:18         ` Paul Jackson
  0 siblings, 0 replies; 6+ messages in thread
From: Paul Jackson @ 2004-04-28  2:18 UTC (permalink / raw)
  To: William Lee Irwin III; +Cc: rusty, akpm, linux-kernel, torvalds

> Eh? Why are you now suddenly changing your tune from when I wrote a fix

Rusty said that one fix was causing oopses.  And he caught me in a
weak moment - so I didn't challenge him on that assertion.

Yes I would prefer to limit changes in this code to what's actually
breaking other code here and now.

As soon as I can get the latest 2.6.6-rc2-mm2 to build various
arch's and merge with this stuff, I will be ready to request
Andrew's attention once again.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2004-04-28  2:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-04-27 23:52 [PATCH] Fix cpu iterator on empty bitmask Rusty Russell
2004-04-28  0:05 ` William Lee Irwin III
2004-04-28  1:22   ` Rusty Russell
2004-04-28  1:31     ` Paul Jackson
2004-04-28  1:57       ` William Lee Irwin III
2004-04-28  2:18         ` Paul Jackson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox