Re: [PATCH] locking: remove three unused Kconfig symbols

2013-04-08 Thread Ingo Molnar

* Paul Bolle  wrote:

> On Mon, 2013-04-08 at 17:16 +0200, Ingo Molnar wrote:
> > * Paul Bolle  wrote:
> > > The Kconfig symbols ARCH_INLINE_READ_UNLOCK_IRQ,
> > > ARCH_INLINE_SPIN_UNLOCK_IRQ, and ARCH_INLINE_WRITE_UNLOCK_IRQ were added
> > > in v2.6.33, but have never actually been used. It is safe to remove
> > > these.
> > > 
> > Erm, if you look at the code in question you'll see that they are unused I 
> > think 
> > due to a bug:
> > 
> >  config INLINE_READ_UNLOCK_IRQ
> > def_bool y
> > depends on !PREEMPT || ARCH_INLINE_READ_UNLOCK_BH
> > 
> > Shouldn't that depend on ARCH_INLINE_READ_UNLOCK_IRQ?
> > 
> > Similarly for the others.
> 
> Yes, that seems correct. I must admit that I didn't spot that
> possibility, but then again this Kconfig file is near unreadable (too
> much repetition for human readers).

Yeah, Kconfig isn't a very sophisticated language - it's the COBOL of config 
languages.

> Would you have any idea why this problem wasn't noticed in 16 (!) releases? 
> Fedora 17, which I did this patch on, has Kconfig symbolk PREEMPT not set. Is 
> it 
> perhaps commit to not enable that option?

AFAICS it's a latent bug: it can only be noticed if these switches are set to 
different values by an arch - but the defaults and s390 (the two main cases) 
never 
do that.

So it's not causing any runtime problems today - but should be fixed 
nevertheless, 
now that you noticed it! :-)

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] locking: remove three unused Kconfig symbols

2013-04-08 Thread Paul Bolle
On Mon, 2013-04-08 at 17:30 +0200, Paul Bolle wrote:
> Would you have any idea why this problem wasn't noticed in 16 (!)
> releases? Fedora 17, which I did this patch on, has Kconfig symbolk
> PREEMPT not set. Is it perhaps commit to not enable that option?

s/commit/common/ (and, obviously, s/symbolk/symbol/).


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] locking: remove three unused Kconfig symbols

2013-04-08 Thread Paul Bolle
On Mon, 2013-04-08 at 17:16 +0200, Ingo Molnar wrote:
> * Paul Bolle  wrote:
> > The Kconfig symbols ARCH_INLINE_READ_UNLOCK_IRQ,
> > ARCH_INLINE_SPIN_UNLOCK_IRQ, and ARCH_INLINE_WRITE_UNLOCK_IRQ were added
> > in v2.6.33, but have never actually been used. It is safe to remove
> > these.
> > 
> Erm, if you look at the code in question you'll see that they are unused I 
> think 
> due to a bug:
> 
>  config INLINE_READ_UNLOCK_IRQ
> def_bool y
> depends on !PREEMPT || ARCH_INLINE_READ_UNLOCK_BH
> 
> Shouldn't that depend on ARCH_INLINE_READ_UNLOCK_IRQ?
> 
> Similarly for the others.

Yes, that seems correct. I must admit that I didn't spot that
possibility, but then again this Kconfig file is near unreadable (too
much repetition for human readers).

Would you have any idea why this problem wasn't noticed in 16 (!)
releases? Fedora 17, which I did this patch on, has Kconfig symbolk
PREEMPT not set. Is it perhaps commit to not enable that option?


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] locking: remove three unused Kconfig symbols

2013-04-08 Thread Ingo Molnar

* Paul Bolle  wrote:

> The Kconfig symbols ARCH_INLINE_READ_UNLOCK_IRQ,
> ARCH_INLINE_SPIN_UNLOCK_IRQ, and ARCH_INLINE_WRITE_UNLOCK_IRQ were added
> in v2.6.33, but have never actually been used. It is safe to remove
> these.
> 
> Signed-off-by: Paul Bolle 
> ---
> Untested. These three symbols were added in commit
> 6beb000923882f6204ea2cfcd932e568e900803f ("locking: Make inlining
> decision Kconfig based").
> 
>  arch/s390/Kconfig|  3 ---
>  kernel/Kconfig.locks | 10 --
>  2 files changed, 13 deletions(-)
> 
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index eb8fb62..8c544d2 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -70,7 +70,6 @@ config S390
>   select ARCH_INLINE_READ_TRYLOCK
>   select ARCH_INLINE_READ_UNLOCK
>   select ARCH_INLINE_READ_UNLOCK_BH
> - select ARCH_INLINE_READ_UNLOCK_IRQ
>   select ARCH_INLINE_READ_UNLOCK_IRQRESTORE
>   select ARCH_INLINE_SPIN_LOCK
>   select ARCH_INLINE_SPIN_LOCK_BH
> @@ -80,7 +79,6 @@ config S390
>   select ARCH_INLINE_SPIN_TRYLOCK_BH
>   select ARCH_INLINE_SPIN_UNLOCK
>   select ARCH_INLINE_SPIN_UNLOCK_BH
> - select ARCH_INLINE_SPIN_UNLOCK_IRQ
>   select ARCH_INLINE_SPIN_UNLOCK_IRQRESTORE
>   select ARCH_INLINE_WRITE_LOCK
>   select ARCH_INLINE_WRITE_LOCK_BH
> @@ -89,7 +87,6 @@ config S390
>   select ARCH_INLINE_WRITE_TRYLOCK
>   select ARCH_INLINE_WRITE_UNLOCK
>   select ARCH_INLINE_WRITE_UNLOCK_BH
> - select ARCH_INLINE_WRITE_UNLOCK_IRQ
>   select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
>   select ARCH_SAVE_PAGE_KEYS if HIBERNATION
>   select ARCH_WANT_IPC_PARSE_VERSION
> diff --git a/kernel/Kconfig.locks b/kernel/Kconfig.locks
> index 44511d1..c438507 100644
> --- a/kernel/Kconfig.locks
> +++ b/kernel/Kconfig.locks
> @@ -25,9 +25,6 @@ config ARCH_INLINE_SPIN_UNLOCK
>  config ARCH_INLINE_SPIN_UNLOCK_BH
>   bool
>  
> -config ARCH_INLINE_SPIN_UNLOCK_IRQ
> - bool
> -
>  config ARCH_INLINE_SPIN_UNLOCK_IRQRESTORE
>   bool
>  
> @@ -53,13 +50,9 @@ config ARCH_INLINE_READ_UNLOCK
>  config ARCH_INLINE_READ_UNLOCK_BH
>   bool
>  
> -config ARCH_INLINE_READ_UNLOCK_IRQ
> - bool
> -
>  config ARCH_INLINE_READ_UNLOCK_IRQRESTORE
>   bool
>  
> -
>  config ARCH_INLINE_WRITE_TRYLOCK
>   bool
>  
> @@ -81,9 +74,6 @@ config ARCH_INLINE_WRITE_UNLOCK
>  config ARCH_INLINE_WRITE_UNLOCK_BH
>   bool
>  
> -config ARCH_INLINE_WRITE_UNLOCK_IRQ
> - bool
> -
>  config ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
>   bool

Erm, if you look at the code in question you'll see that they are unused I 
think 
due to a bug:

 config INLINE_READ_UNLOCK_IRQ
def_bool y
depends on !PREEMPT || ARCH_INLINE_READ_UNLOCK_BH

Shouldn't that depend on ARCH_INLINE_READ_UNLOCK_IRQ?

Similarly for the others.

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] locking: remove three unused Kconfig symbols

2013-04-08 Thread Paul Bolle
The Kconfig symbols ARCH_INLINE_READ_UNLOCK_IRQ,
ARCH_INLINE_SPIN_UNLOCK_IRQ, and ARCH_INLINE_WRITE_UNLOCK_IRQ were added
in v2.6.33, but have never actually been used. It is safe to remove
these.

Signed-off-by: Paul Bolle 
---
Untested. These three symbols were added in commit
6beb000923882f6204ea2cfcd932e568e900803f ("locking: Make inlining
decision Kconfig based").

 arch/s390/Kconfig|  3 ---
 kernel/Kconfig.locks | 10 --
 2 files changed, 13 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index eb8fb62..8c544d2 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -70,7 +70,6 @@ config S390
select ARCH_INLINE_READ_TRYLOCK
select ARCH_INLINE_READ_UNLOCK
select ARCH_INLINE_READ_UNLOCK_BH
-   select ARCH_INLINE_READ_UNLOCK_IRQ
select ARCH_INLINE_READ_UNLOCK_IRQRESTORE
select ARCH_INLINE_SPIN_LOCK
select ARCH_INLINE_SPIN_LOCK_BH
@@ -80,7 +79,6 @@ config S390
select ARCH_INLINE_SPIN_TRYLOCK_BH
select ARCH_INLINE_SPIN_UNLOCK
select ARCH_INLINE_SPIN_UNLOCK_BH
-   select ARCH_INLINE_SPIN_UNLOCK_IRQ
select ARCH_INLINE_SPIN_UNLOCK_IRQRESTORE
select ARCH_INLINE_WRITE_LOCK
select ARCH_INLINE_WRITE_LOCK_BH
@@ -89,7 +87,6 @@ config S390
select ARCH_INLINE_WRITE_TRYLOCK
select ARCH_INLINE_WRITE_UNLOCK
select ARCH_INLINE_WRITE_UNLOCK_BH
-   select ARCH_INLINE_WRITE_UNLOCK_IRQ
select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
select ARCH_SAVE_PAGE_KEYS if HIBERNATION
select ARCH_WANT_IPC_PARSE_VERSION
diff --git a/kernel/Kconfig.locks b/kernel/Kconfig.locks
index 44511d1..c438507 100644
--- a/kernel/Kconfig.locks
+++ b/kernel/Kconfig.locks
@@ -25,9 +25,6 @@ config ARCH_INLINE_SPIN_UNLOCK
 config ARCH_INLINE_SPIN_UNLOCK_BH
bool
 
-config ARCH_INLINE_SPIN_UNLOCK_IRQ
-   bool
-
 config ARCH_INLINE_SPIN_UNLOCK_IRQRESTORE
bool
 
@@ -53,13 +50,9 @@ config ARCH_INLINE_READ_UNLOCK
 config ARCH_INLINE_READ_UNLOCK_BH
bool
 
-config ARCH_INLINE_READ_UNLOCK_IRQ
-   bool
-
 config ARCH_INLINE_READ_UNLOCK_IRQRESTORE
bool
 
-
 config ARCH_INLINE_WRITE_TRYLOCK
bool
 
@@ -81,9 +74,6 @@ config ARCH_INLINE_WRITE_UNLOCK
 config ARCH_INLINE_WRITE_UNLOCK_BH
bool
 
-config ARCH_INLINE_WRITE_UNLOCK_IRQ
-   bool
-
 config ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
bool
 
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/