Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-16 Thread Peter Seebach
On Fri, 13 Jul 2012 22:00:10 -0700 Linus Torvalds wrote: > (*) Technically, "&(x)[0]" is actually a really confused way of saying > "(x+0)" while making sure that "x" was a valid pointer. But wait, there's more! Should someone some day try to use an implementation with a fairly ferocious

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-16 Thread Peter Seebach
On Fri, 13 Jul 2012 22:00:10 -0700 Linus Torvalds torva...@linux-foundation.org wrote: (*) Technically, (x)[0] is actually a really confused way of saying (x+0) while making sure that x was a valid pointer. But wait, there's more! Should someone some day try to use an implementation with a

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-13 Thread Tejun Heo
Hey, Linus. On Fri, Jul 13, 2012 at 10:00:10PM -0700, Linus Torvalds wrote: > On Fri, Jul 13, 2012 at 9:44 PM, Tejun Heo wrote: > > > > nr_running is atomic_t (*nr_running)[2]. Ignoring the pointer to > > array part, it's just returning the address of N'th element of the > > array. ARRAY + N

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-13 Thread Linus Torvalds
On Fri, Jul 13, 2012 at 9:44 PM, Tejun Heo wrote: > > nr_running is atomic_t (*nr_running)[2]. Ignoring the pointer to > array part, it's just returning the address of N'th element of the > array. ARRAY + N == [N]. None of this matters one whit. You did "&(x)[0]". That's insane. It's crazy.

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-13 Thread Tejun Heo
Hello, Linus. On Fri, Jul 13, 2012 at 09:27:03PM -0700, Linus Torvalds wrote: > Seeing code like this > > + return &(*nr_running)[0]; > > just makes me go "WTF?" I was going WTF too. This was the smallest fix and I wanted to make it minimal because there's another stack of patches on

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-13 Thread Linus Torvalds
Seeing code like this + return &(*nr_running)[0]; just makes me go "WTF?" Why are you taking the address of something you just dereferenced (the "& [0]" part). And you actually do that *twice*, except the inner one is more complicated. When you assign nr_runing, you take the address of

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-13 Thread Tejun Heo
>From 8a0597bf9939d50039d4a6f446db51cf920daaad Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Fri, 13 Jul 2012 20:50:50 -0700 Introduce NR_WORKER_POOLS and for_each_worker_pool() and convert code paths which need to manipulate all pools in a gcwq to use them. NR_WORKER_POOLS is currently one and

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-13 Thread Tejun Heo
From 8a0597bf9939d50039d4a6f446db51cf920daaad Mon Sep 17 00:00:00 2001 From: Tejun Heo t...@kernel.org Date: Fri, 13 Jul 2012 20:50:50 -0700 Introduce NR_WORKER_POOLS and for_each_worker_pool() and convert code paths which need to manipulate all pools in a gcwq to use them. NR_WORKER_POOLS is

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-13 Thread Linus Torvalds
Seeing code like this + return (*nr_running)[0]; just makes me go WTF? Why are you taking the address of something you just dereferenced (the [0] part). And you actually do that *twice*, except the inner one is more complicated. When you assign nr_runing, you take the address of it, so

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-13 Thread Tejun Heo
Hello, Linus. On Fri, Jul 13, 2012 at 09:27:03PM -0700, Linus Torvalds wrote: Seeing code like this + return (*nr_running)[0]; just makes me go WTF? I was going WTF too. This was the smallest fix and I wanted to make it minimal because there's another stack of patches on top of it.

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-13 Thread Linus Torvalds
On Fri, Jul 13, 2012 at 9:44 PM, Tejun Heo t...@kernel.org wrote: nr_running is atomic_t (*nr_running)[2]. Ignoring the pointer to array part, it's just returning the address of N'th element of the array. ARRAY + N == ARRAY[N]. None of this matters one whit. You did (x)[0]. That's insane.

Re: [PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-13 Thread Tejun Heo
Hey, Linus. On Fri, Jul 13, 2012 at 10:00:10PM -0700, Linus Torvalds wrote: On Fri, Jul 13, 2012 at 9:44 PM, Tejun Heo t...@kernel.org wrote: nr_running is atomic_t (*nr_running)[2]. Ignoring the pointer to array part, it's just returning the address of N'th element of the array. ARRAY

[PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-09 Thread Tejun Heo
Introduce NR_WORKER_POOLS and for_each_worker_pool() and convert code paths which need to manipulate all pools in a gcwq to use them. NR_WORKER_POOLS is currently one and for_each_worker_pool() iterates over only @gcwq->pool. Note that nr_running is per-pool property and converted to an array

[PATCH 5/6] workqueue: introduce NR_WORKER_POOLS and for_each_worker_pool()

2012-07-09 Thread Tejun Heo
Introduce NR_WORKER_POOLS and for_each_worker_pool() and convert code paths which need to manipulate all pools in a gcwq to use them. NR_WORKER_POOLS is currently one and for_each_worker_pool() iterates over only @gcwq-pool. Note that nr_running is per-pool property and converted to an array with