[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4F89E9A3.3020800@linux.vnet.ibm.com>
Date: Sun, 15 Apr 2012 02:48:27 +0530
From: "Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
To: Sasha Levin <levinsasha928@...il.com>
CC: torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
peterz@...radead.org, mingo@...e.hu, hpa@...or.com,
tglx@...utronix.de, linux-kernel@...r.kernel.org
Subject: Re: [RFC 0/3] Extend type checking macros
On 04/15/2012 03:44 AM, Sasha Levin wrote:
> Commit e3831ed ("sched: Fix incorrect usage of for_each_cpu_mask() in
> select_fallback_rq()") fixes a very non obvious bug in select_fallback_rq()
> which was caused by passing 'struct cpumask' instead of 'struct cpumask *'
> to a macro in include/linux/cpumask.h
>
Good heavens! I just found out that *each* *and* *every* *one* of the
existing 12 users of for_each_cpu_mask() are wrong!! Unbelievable!
> This bug was quite a pain to debug since it doesn't raise any warnings or
> erros during compilation, and the assumption of the kernel hackers who try
> to fix a bug is that if the compiler didn't complain, they passed the right
> types to functions.
>
> This series of patches adds some more type checking macros to the forgotten
> include/linux/typecheck.h, it modified for_each_cpu_mask() to use those
> macros to trigger a warning when needed (this is a nice demonstration of how
> the bug mentioned before would have been visible with these checks), and
> modifies min()/max() and friend to use these macros as well to show their
> value in reducing duplicate code and improving readability.
>
> Sasha Levin (3):
> typecheck: extend typecheck.h with more useful typechecking macros
> sched: add type checks to for_each_cpu_mask()
I think it would be better to correct and move the existing 12 users of
for_each_cpu_mask() to for_each_cpu() and simply get rid of the obsolete
for_each_cpu_mask() macro. After all, why do we need 2 macros that do the
exact same thing?
> kernel.h: use new typechecking macros in min()/max() and friends
>
> include/linux/cpumask.h | 4 ++-
> include/linux/kernel.h | 47 ++++++++++++++------------------------------
> include/linux/typecheck.h | 42 ++++++++++++++++++++++++++++++++-------
> 3 files changed, 52 insertions(+), 41 deletions(-)
>
Regards,
Srivatsa S. Bhat
--
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/
Powered by blists - more mailing lists