From: KOSAKI Motohiro <kosaki.motohiro@jp•fujitsu.com>
To: Ingo Molnar <mingo@elte•hu>
Cc: kosaki.motohiro@jp•fujitsu.com,
Andrew Morton <akpm@linux-foundation•org>,
Yinghai Lu <yhlu.kernel@gmail•com>,
LKML <linux-kernel@vger•kernel.org>,
linux-next <linux-next@vger•kernel.org>
Subject: Re: [mmotm and linux-next][PATCH] irq: enclose irq_desc_lock_class in CONFIG_LOCKDEP
Date: Wed, 17 Dec 2008 19:40:33 +0900 (JST) [thread overview]
Message-ID: <20081217193815.F674.KOSAKI.MOTOHIRO@jp.fujitsu.com> (raw)
In-Reply-To: <2f11576a0812160458y1c376878vc5fc7f3d208aa344@mail.gmail.com>
> >> or, following #ifdef ?
> >>
> >> #if defined(CONFIG_SPARSE_IRQ) || defined(CONFIG_TRACE_IRQFLAGS)
> >>
> >> /*
> >> * lockdep: we want to handle all irq_desc locks as a single lock-class:
> >> */
> >> static struct lock_class_key irq_desc_lock_class;
> >
> > instead of increasing the #ifdef jungle, how about removing some? For
> > example is this distinction:
> >
> >> > #ifndef CONFIG_SPARSE_IRQ
> >
> > really needed? We should use symmetric lock class annotations, regardless
> > of how irq_desc[] is laid out.
>
> it seems make much sense. I'll test your idea tommorow.
Ingo, you are right. I confirmed your idea works well.
I tested following ten pattern.
o handle.c can compile without any warnings?
SPARSE_IRQ TRACE_IRQ LOCKDEP
------------------------------------------
n n n
Y n n
n Y n
n n Y
Y Y n
N Y Y
Y n Y
Y Y Y
o builded kernel works well? (tested on x86_64)
SPARSE_IRQ TRACE_IRQ LOCKDEP
------------------------------------------
n n n
Y Y Y
==
Subject: [PATCH] irq: remove unnecessary ifdef
commit 08678b0841267c1d00d771fe01548d86043d065e introduced
irq_desc_lock_class variable.
But it is used only if CONFIG_SPARSE_IRQ=Y or CONFIG_TRACE_IRQFLAGS=Y.
otherwise, following warnings happend.
CC kernel/irq/handle.o
kernel/irq/handle.c:26: warning: 'irq_desc_lock_class' defined but not used
Actually, current early_init_irq_lock_class has a bit strange and messy ifdef.
In addition, it is not valueable.
1. this function protected by !CONFIG_SPARSE_IRQ. but it is not necessary.
if CONFIG_SPARSE_IRQ=Y, desc of all irq number are initialized by NULL at first.
then this function calling is safe.
2. this function protected by CONFIG_TRACE_IRQFLAGS too. but it is not necessary too.
because lockdep_set_class() doesn't have bad side effect even if CONFIG_TRACE_IRQFLAGS=n,
This patch bloat kernel size a bit on CONFIG_TRACE_IRQFLAGS=n and CONFIG_SPARSE_IRQ=Y.
that's ok. early_init_irq_lock_class() is not fastpatch at all.
To avoid ifdef messy is important than few byte diet.
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp•fujitsu.com>
CC: Yinghai Lu <yhlu.kernel@gmail•com>
CC: Ingo Molnar <mingo@elte•hu>
---
include/linux/lockdep.h | 2 +-
kernel/irq/handle.c | 5 -----
2 files changed, 1 insertion(+), 6 deletions(-)
Index: b/kernel/irq/handle.c
===================================================================
--- a/kernel/irq/handle.c 2008-12-17 10:45:56.000000000 +0900
+++ b/kernel/irq/handle.c 2008-12-17 17:26:42.000000000 +0900
@@ -417,11 +417,8 @@ out:
}
#endif
-
-#ifdef CONFIG_TRACE_IRQFLAGS
void early_init_irq_lock_class(void)
{
-#ifndef CONFIG_SPARSE_IRQ
struct irq_desc *desc;
int i;
@@ -431,9 +428,7 @@ void early_init_irq_lock_class(void)
lockdep_set_class(&desc->lock, &irq_desc_lock_class);
}
-#endif
}
-#endif
#ifdef CONFIG_SPARSE_IRQ
unsigned int kstat_irqs_cpu(unsigned int irq, int cpu)
Index: b/include/linux/lockdep.h
===================================================================
--- a/include/linux/lockdep.h 2008-12-17 10:46:26.000000000 +0900
+++ b/include/linux/lockdep.h 2008-12-17 17:27:18.000000000 +0900
@@ -407,7 +407,7 @@ do { \
#endif /* CONFIG_LOCKDEP */
-#if defined(CONFIG_TRACE_IRQFLAGS) && defined(CONFIG_GENERIC_HARDIRQS)
+#ifdef CONFIG_GENERIC_HARDIRQS
extern void early_init_irq_lock_class(void);
#else
static inline void early_init_irq_lock_class(void)
next prev parent reply other threads:[~2008-12-17 10:40 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-16 8:08 [mmotm and linux-next][PATCH] irq: enclose irq_desc_lock_class in CONFIG_LOCKDEP KOSAKI Motohiro
2008-12-16 8:34 ` Andrew Morton
2008-12-16 10:18 ` Ingo Molnar
2008-12-16 10:23 ` Ingo Molnar
2008-12-16 11:15 ` KOSAKI Motohiro
2008-12-16 11:20 ` Ingo Molnar
2008-12-16 11:45 ` KOSAKI Motohiro
2008-12-16 12:18 ` Ingo Molnar
2008-12-16 12:58 ` KOSAKI Motohiro
2008-12-17 10:40 ` KOSAKI Motohiro [this message]
2008-12-18 13:36 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20081217193815.F674.KOSAKI.MOTOHIRO@jp.fujitsu.com \
--to=kosaki.motohiro@jp$(echo .)fujitsu.com \
--cc=akpm@linux-foundation$(echo .)org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-next@vger$(echo .)kernel.org \
--cc=mingo@elte$(echo .)hu \
--cc=yhlu.kernel@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox