linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Let sparse check for shadowed variables
@ 2018-10-17 17:05 Sebastian Andrzej Siewior
  2018-10-17 17:05 ` [PATCH 1/3] kbuild: Add -Wshadow to sparse Sebastian Andrzej Siewior
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-10-17 17:05 UTC (permalink / raw)
  To: linux-kernel; +Cc: tglx, linux-kbuild

#1 enables checking for shadowed variables by sparse. Apparently this is
no longer default as of sparse 0.5.2.
#2 and #3 are just two fixes for what sparse reported. There are plenty
of others…

Sebastian



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] kbuild: Add -Wshadow to sparse
  2018-10-17 17:05 [PATCH 0/3] Let sparse check for shadowed variables Sebastian Andrzej Siewior
@ 2018-10-17 17:05 ` Sebastian Andrzej Siewior
  2018-10-17 19:25   ` Luc Van Oostenryck
  2018-10-17 17:05 ` [PATCH 2/3] x86/mcelog: Remove one mce_helper definition Sebastian Andrzej Siewior
  2018-10-17 17:05 ` [PATCH 3/3] kvm: don't redefine flags as something else Sebastian Andrzej Siewior
  2 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-10-17 17:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: tglx, linux-kbuild, Sebastian Andrzej Siewior, linux-sparse,
	Masahiro Yamada, Michal Marek

I remember that `sparse' used to warn about shadowed variables but it
seems not to do it by default anymore. It was useful.

Enable `-Wshadow' while invoking sparse.

Cc: linux-sparse@vger.kernel.org
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <michal.lkml@markovi.net>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index bf3786e4ffece..0160c6b5dd45d 100644
--- a/Makefile
+++ b/Makefile
@@ -390,7 +390,7 @@ PYTHON3		= python3
 CHECK		= sparse
 
 CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
-		  -Wbitwise -Wno-return-void -Wno-unknown-attribute $(CF)
+		  -Wbitwise -Wno-return-void -Wno-unknown-attribute -Wshadow $(CF)
 NOSTDINC_FLAGS  =
 CFLAGS_MODULE   =
 AFLAGS_MODULE   =
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] x86/mcelog: Remove one mce_helper definition
  2018-10-17 17:05 [PATCH 0/3] Let sparse check for shadowed variables Sebastian Andrzej Siewior
  2018-10-17 17:05 ` [PATCH 1/3] kbuild: Add -Wshadow to sparse Sebastian Andrzej Siewior
@ 2018-10-17 17:05 ` Sebastian Andrzej Siewior
  2018-10-17 17:10   ` Borislav Petkov
  2018-10-17 22:12   ` [tip:ras/core] " tip-bot for Sebastian Andrzej Siewior
  2018-10-17 17:05 ` [PATCH 3/3] kvm: don't redefine flags as something else Sebastian Andrzej Siewior
  2 siblings, 2 replies; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-10-17 17:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: tglx, linux-kbuild, Sebastian Andrzej Siewior, Tony Luck,
	Borislav Petkov, linux-edac, x86

Commit 5de97c9f6d85f ("x86/mce: Factor out and deprecate the /dev/mcelog
driver") moved the old interface into one file including mce_helper
definition as static and "extern".

Cc: Tony Luck <tony.luck@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: linux-edac <linux-edac@vger.kernel.org>
Cc: x86@kernel.org
Fixes:  5de97c9f6d85f ("x86/mce: Factor out and deprecate the /dev/mcelog driver")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
index 97685a0c31751..27f394ac983f6 100644
--- a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
+++ b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
@@ -38,9 +38,6 @@ static struct mce_log_buffer mcelog = {
 
 static DECLARE_WAIT_QUEUE_HEAD(mce_chrdev_wait);
 
-/* User mode helper program triggered by machine check event */
-extern char			mce_helper[128];
-
 static int dev_mce_log(struct notifier_block *nb, unsigned long val,
 				void *data)
 {
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] kvm: don't redefine flags as something else
  2018-10-17 17:05 [PATCH 0/3] Let sparse check for shadowed variables Sebastian Andrzej Siewior
  2018-10-17 17:05 ` [PATCH 1/3] kbuild: Add -Wshadow to sparse Sebastian Andrzej Siewior
  2018-10-17 17:05 ` [PATCH 2/3] x86/mcelog: Remove one mce_helper definition Sebastian Andrzej Siewior
@ 2018-10-17 17:05 ` Sebastian Andrzej Siewior
  2018-10-17 17:06   ` Paolo Bonzini
  2 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-10-17 17:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: tglx, linux-kbuild, Sebastian Andrzej Siewior, Paolo Bonzini,
	Radim Krčmář,
	kvm

The function irqfd_wakeup() has flags defined as __poll_t and then it
has additional flags which is used for irqflags.

Redefine the inner flags variable as iflags so it does not shadow the
outer flags.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 virt/kvm/eventfd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index b20b751286fc6..d15a51622d53e 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -214,9 +214,9 @@ irqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void *key)
 
 	if (flags & EPOLLHUP) {
 		/* The eventfd is closing, detach from KVM */
-		unsigned long flags;
+		unsigned long iflags;
 
-		spin_lock_irqsave(&kvm->irqfds.lock, flags);
+		spin_lock_irqsave(&kvm->irqfds.lock, iflags);
 
 		/*
 		 * We must check if someone deactivated the irqfd before
@@ -230,7 +230,7 @@ irqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void *key)
 		if (irqfd_is_active(irqfd))
 			irqfd_deactivate(irqfd);
 
-		spin_unlock_irqrestore(&kvm->irqfds.lock, flags);
+		spin_unlock_irqrestore(&kvm->irqfds.lock, iflags);
 	}
 
 	return 0;
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] kvm: don't redefine flags as something else
  2018-10-17 17:05 ` [PATCH 3/3] kvm: don't redefine flags as something else Sebastian Andrzej Siewior
@ 2018-10-17 17:06   ` Paolo Bonzini
  2019-03-13  8:58     ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2018-10-17 17:06 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, linux-kernel
  Cc: tglx, linux-kbuild, Radim Krčmář, kvm

On 17/10/2018 19:05, Sebastian Andrzej Siewior wrote:
> The function irqfd_wakeup() has flags defined as __poll_t and then it
> has additional flags which is used for irqflags.
> 
> Redefine the inner flags variable as iflags so it does not shadow the
> outer flags.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  virt/kvm/eventfd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index b20b751286fc6..d15a51622d53e 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -214,9 +214,9 @@ irqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void *key)
>  
>  	if (flags & EPOLLHUP) {
>  		/* The eventfd is closing, detach from KVM */
> -		unsigned long flags;
> +		unsigned long iflags;
>  
> -		spin_lock_irqsave(&kvm->irqfds.lock, flags);
> +		spin_lock_irqsave(&kvm->irqfds.lock, iflags);
>  
>  		/*
>  		 * We must check if someone deactivated the irqfd before
> @@ -230,7 +230,7 @@ irqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void *key)
>  		if (irqfd_is_active(irqfd))
>  			irqfd_deactivate(irqfd);
>  
> -		spin_unlock_irqrestore(&kvm->irqfds.lock, flags);
> +		spin_unlock_irqrestore(&kvm->irqfds.lock, iflags);
>  	}
>  
>  	return 0;
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] x86/mcelog: Remove one mce_helper definition
  2018-10-17 17:05 ` [PATCH 2/3] x86/mcelog: Remove one mce_helper definition Sebastian Andrzej Siewior
@ 2018-10-17 17:10   ` Borislav Petkov
  2018-10-17 18:22     ` Thomas Gleixner
  2018-10-17 22:12   ` [tip:ras/core] " tip-bot for Sebastian Andrzej Siewior
  1 sibling, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2018-10-17 17:10 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-kernel, tglx, linux-kbuild, Tony Luck, linux-edac, x86

On Wed, Oct 17, 2018 at 07:05:53PM +0200, Sebastian Andrzej Siewior wrote:
> Commit 5de97c9f6d85f ("x86/mce: Factor out and deprecate the /dev/mcelog
> driver") moved the old interface into one file including mce_helper
> definition as static and "extern".
> 
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: linux-edac <linux-edac@vger.kernel.org>
> Cc: x86@kernel.org
> Fixes:  5de97c9f6d85f ("x86/mce: Factor out and deprecate the /dev/mcelog driver")

I'm not sure about the Fixes: tag - it'll trigger a flood of backporting
for no reason AFAICT.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] x86/mcelog: Remove one mce_helper definition
  2018-10-17 17:10   ` Borislav Petkov
@ 2018-10-17 18:22     ` Thomas Gleixner
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Gleixner @ 2018-10-17 18:22 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Sebastian Andrzej Siewior, linux-kernel, linux-kbuild, Tony Luck,
	linux-edac, x86

On Wed, 17 Oct 2018, Borislav Petkov wrote:

> On Wed, Oct 17, 2018 at 07:05:53PM +0200, Sebastian Andrzej Siewior wrote:
> > Commit 5de97c9f6d85f ("x86/mce: Factor out and deprecate the /dev/mcelog
> > driver") moved the old interface into one file including mce_helper
> > definition as static and "extern".
> > 
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Borislav Petkov <bp@suse.de>
> > Cc: linux-edac <linux-edac@vger.kernel.org>
> > Cc: x86@kernel.org
> > Fixes:  5de97c9f6d85f ("x86/mce: Factor out and deprecate the /dev/mcelog driver")
> 
> I'm not sure about the Fixes: tag - it'll trigger a flood of backporting
> for no reason AFAICT.

Not when there is no Cc: stable. If the stable folks pick up stuff
nevertheless, shrug.

The fixes tag is interesting even for stuff which does not go into stable
because you can extract it properly and look at relations between original
commit and fix.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] kbuild: Add -Wshadow to sparse
  2018-10-17 17:05 ` [PATCH 1/3] kbuild: Add -Wshadow to sparse Sebastian Andrzej Siewior
@ 2018-10-17 19:25   ` Luc Van Oostenryck
  2018-10-17 20:13     ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 12+ messages in thread
From: Luc Van Oostenryck @ 2018-10-17 19:25 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-kernel, tglx, linux-kbuild, linux-sparse, Masahiro Yamada,
	Michal Marek

On Wed, Oct 17, 2018 at 07:05:52PM +0200, Sebastian Andrzej Siewior wrote:
> I remember that `sparse' used to warn about shadowed variables but it
> seems not to do it by default anymore. It was useful.
> 
> Enable `-Wshadow' while invoking sparse.

Hi,

I made a quick test and I saw it would add a lot of noise because of
(valid) warnings coming from macros using a statement expression
redeclaring variables like '__u', 'tmp', ...

BTW, as far as I can see, sparse never had it enabled by default.

Kind regards,
-- Luc Van Oostenryck

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] kbuild: Add -Wshadow to sparse
  2018-10-17 19:25   ` Luc Van Oostenryck
@ 2018-10-17 20:13     ` Sebastian Andrzej Siewior
  0 siblings, 0 replies; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-10-17 20:13 UTC (permalink / raw)
  To: Luc Van Oostenryck
  Cc: linux-kernel, tglx, linux-kbuild, linux-sparse, Masahiro Yamada,
	Michal Marek

On 2018-10-17 21:25:02 [+0200], Luc Van Oostenryck wrote:
> Hi,
Hi,

> I made a quick test and I saw it would add a lot of noise because of
> (valid) warnings coming from macros using a statement expression
> redeclaring variables like '__u', 'tmp', ...
> 
> BTW, as far as I can see, sparse never had it enabled by default.

I must have seen those warnings with C=1 _years_ ago otherwise I
wouldn't know about that feature. Or it was enabled by the check process
and got disabled but I can't find evidence for it.

> Kind regards,
> -- Luc Van Oostenryck

Sebastian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip:ras/core] x86/mcelog: Remove one mce_helper definition
  2018-10-17 17:05 ` [PATCH 2/3] x86/mcelog: Remove one mce_helper definition Sebastian Andrzej Siewior
  2018-10-17 17:10   ` Borislav Petkov
@ 2018-10-17 22:12   ` tip-bot for Sebastian Andrzej Siewior
  1 sibling, 0 replies; 12+ messages in thread
From: tip-bot for Sebastian Andrzej Siewior @ 2018-10-17 22:12 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, linux-edac, hpa, bp, mingo, bigeasy, linux-kernel, x86,
	mingo, tony.luck

Commit-ID:  711f76a328cbe5b49164bb14bcb593fa52102051
Gitweb:     https://git.kernel.org/tip/711f76a328cbe5b49164bb14bcb593fa52102051
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Wed, 17 Oct 2018 19:05:53 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Thu, 18 Oct 2018 00:05:04 +0200

x86/mcelog: Remove one mce_helper definition

Commit

  5de97c9f6d85f ("x86/mce: Factor out and deprecate the /dev/mcelog driver")

moved the old interface into one file including mce_helper definition as
static and "extern". Remove one.

Fixes: 5de97c9f6d85f ("x86/mce: Factor out and deprecate the /dev/mcelog driver")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Tony Luck <tony.luck@intel.com>
CC: linux-edac <linux-edac@vger.kernel.org>
CC: x86-ml <x86@kernel.org>
Link: http://lkml.kernel.org/r/20181017170554.18841-3-bigeasy@linutronix.de
---
 arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
index 97685a0c3175..27f394ac983f 100644
--- a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
+++ b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
@@ -38,9 +38,6 @@ static struct mce_log_buffer mcelog = {
 
 static DECLARE_WAIT_QUEUE_HEAD(mce_chrdev_wait);
 
-/* User mode helper program triggered by machine check event */
-extern char			mce_helper[128];
-
 static int dev_mce_log(struct notifier_block *nb, unsigned long val,
 				void *data)
 {

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] kvm: don't redefine flags as something else
  2018-10-17 17:06   ` Paolo Bonzini
@ 2019-03-13  8:58     ` Sebastian Andrzej Siewior
  2019-03-13 11:06       ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2019-03-13  8:58 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: linux-kernel, tglx, linux-kbuild, Radim Krčmář, kvm

On 2018-10-17 19:06:33 [+0200], Paolo Bonzini wrote:
> On 17/10/2018 19:05, Sebastian Andrzej Siewior wrote:
> > The function irqfd_wakeup() has flags defined as __poll_t and then it
> > has additional flags which is used for irqflags.
> > 
> > Redefine the inner flags variable as iflags so it does not shadow the
> > outer flags.
> > 
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> > Cc: kvm@vger.kernel.org
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>

this touches only kvm and was acked by a kvm person. What did it miss to
get applied? :)

Sebastian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] kvm: don't redefine flags as something else
  2019-03-13  8:58     ` Sebastian Andrzej Siewior
@ 2019-03-13 11:06       ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2019-03-13 11:06 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-kernel, tglx, linux-kbuild, Radim Krčmář, kvm

On 13/03/19 09:58, Sebastian Andrzej Siewior wrote:
> On 2018-10-17 19:06:33 [+0200], Paolo Bonzini wrote:
>> On 17/10/2018 19:05, Sebastian Andrzej Siewior wrote:
>>> The function irqfd_wakeup() has flags defined as __poll_t and then it
>>> has additional flags which is used for irqflags.
>>>
>>> Redefine the inner flags variable as iflags so it does not shadow the
>>> outer flags.
>>>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: "Radim Krčmář" <rkrcmar@redhat.com>
>>> Cc: kvm@vger.kernel.org
>>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> this touches only kvm and was acked by a kvm person. What did it miss to
> get applied? :)

I was expecting it to be applied together with the other patches.  I can
queue it too.

Paolo

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-03-13 11:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-17 17:05 [PATCH 0/3] Let sparse check for shadowed variables Sebastian Andrzej Siewior
2018-10-17 17:05 ` [PATCH 1/3] kbuild: Add -Wshadow to sparse Sebastian Andrzej Siewior
2018-10-17 19:25   ` Luc Van Oostenryck
2018-10-17 20:13     ` Sebastian Andrzej Siewior
2018-10-17 17:05 ` [PATCH 2/3] x86/mcelog: Remove one mce_helper definition Sebastian Andrzej Siewior
2018-10-17 17:10   ` Borislav Petkov
2018-10-17 18:22     ` Thomas Gleixner
2018-10-17 22:12   ` [tip:ras/core] " tip-bot for Sebastian Andrzej Siewior
2018-10-17 17:05 ` [PATCH 3/3] kvm: don't redefine flags as something else Sebastian Andrzej Siewior
2018-10-17 17:06   ` Paolo Bonzini
2019-03-13  8:58     ` Sebastian Andrzej Siewior
2019-03-13 11:06       ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).