Prev: [PATCH 3/6] direct-io: do not merge logically non-contiguous requests
Next: [PATCH] cfq-iosched: Revert the logic of deep queues
From: Andrew Morton on 19 May 2010 16:30
On Mon, 17 May 2010 08:57:38 +0900
OGAWA Hirofumi <hirofumi(a)mail.parknet.co.jp> wrote:

> ratelimit_state initialization of printk_ratelimited() seems
> broken. This fixes it by using DEFINE_RATELIMIT_STATE() to initialize
> spinlock properly.
>
> Signed-off-by: OGAWA Hirofumi <hirofumi(a)mail.parknet.co.jp>
> ---
>
> include/linux/kernel.h | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff -puN include/linux/kernel.h~printk_ratelimited-fix include/linux/kernel.h
> --- linux-2.6/include/linux/kernel.h~printk_ratelimited-fix 2010-05-17 03:37:33.000000000 +0900
> +++ linux-2.6-hirofumi/include/linux/kernel.h 2010-05-17 03:37:33.000000000 +0900
> @@ -420,14 +420,13 @@ static inline char *pack_hex_byte(char *
> * no local ratelimit_state used in the !PRINTK case
> */
> #ifdef CONFIG_PRINTK
> -#define printk_ratelimited(fmt, ...) ({ \
> - static struct ratelimit_state _rs = { \
> - .interval = DEFAULT_RATELIMIT_INTERVAL, \
> - .burst = DEFAULT_RATELIMIT_BURST, \
> - }; \
> - \
> - if (__ratelimit(&_rs)) \
> - printk(fmt, ##__VA_ARGS__); \
> +#define printk_ratelimited(fmt, ...) ({ \
> + static DEFINE_RATELIMIT_STATE(_rs, \
> + DEFAULT_RATELIMIT_INTERVAL, \
> + DEFAULT_RATELIMIT_BURST); \
> + \
> + if (__ratelimit(&_rs)) \
> + printk(fmt, ##__VA_ARGS__); \
> })

hm, yes, that spinlock will get the all-zeroes pattern.

It's been like this since December 2009. I'm a bit surprised that none
of our spinlock-debugging goodies picked this up. All the
CONFIG_DEBUG_SPINLOCK spinlock fields end up zeroed out also.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: OGAWA Hirofumi on 19 May 2010 16:50
Andrew Morton <akpm(a)linux-foundation.org> writes:

>> #ifdef CONFIG_PRINTK
>> -#define printk_ratelimited(fmt, ...) ({ \
>> - static struct ratelimit_state _rs = { \
>> - .interval = DEFAULT_RATELIMIT_INTERVAL, \
>> - .burst = DEFAULT_RATELIMIT_BURST, \
>> - }; \
>> - \
>> - if (__ratelimit(&_rs)) \
>> - printk(fmt, ##__VA_ARGS__); \
>> +#define printk_ratelimited(fmt, ...) ({ \
>> + static DEFINE_RATELIMIT_STATE(_rs, \
>> + DEFAULT_RATELIMIT_INTERVAL, \
>> + DEFAULT_RATELIMIT_BURST); \
>> + \
>> + if (__ratelimit(&_rs)) \
>> + printk(fmt, ##__VA_ARGS__); \
>> })
>
> hm, yes, that spinlock will get the all-zeroes pattern.
>
> It's been like this since December 2009. I'm a bit surprised that none
> of our spinlock-debugging goodies picked this up. All the
> CONFIG_DEBUG_SPINLOCK spinlock fields end up zeroed out also.

The reason that dynamic analysis didn't pick up is simple - nobody is
using this for now :)

Thanks.
--
OGAWA Hirofumi <hirofumi(a)mail.parknet.co.jp>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: [PATCH 3/6] direct-io: do not merge logically non-contiguous requests
Next: [PATCH] cfq-iosched: Revert the logic of deep queues