#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
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: [email protected]
Cc: Masahiro Yamada <[email protected]>
Cc: Michal Marek <[email protected]>
Cc: [email protected]
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
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
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 <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: linux-edac <[email protected]>
Cc: [email protected]
Fixes: 5de97c9f6d85f ("x86/mce: Factor out and deprecate the /dev/mcelog driver")
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
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
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 <[email protected]>
> Cc: "Radim Krčmář" <[email protected]>
> Cc: [email protected]
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
Cc: "Radim Krčmář" <[email protected]>
Cc: [email protected]
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
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
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 <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: linux-edac <[email protected]>
> Cc: [email protected]
> 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)
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 <[email protected]>
> > Cc: Borislav Petkov <[email protected]>
> > Cc: linux-edac <[email protected]>
> > Cc: [email protected]
> > 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
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
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
Commit-ID: 711f76a328cbe5b49164bb14bcb593fa52102051
Gitweb: https://git.kernel.org/tip/711f76a328cbe5b49164bb14bcb593fa52102051
Author: Sebastian Andrzej Siewior <[email protected]>
AuthorDate: Wed, 17 Oct 2018 19:05:53 +0200
Committer: Borislav Petkov <[email protected]>
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 <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
CC: "H. Peter Anvin" <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: Tony Luck <[email protected]>
CC: linux-edac <[email protected]>
CC: x86-ml <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
---
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)
{
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 <[email protected]>
> > Cc: "Radim Krčmář" <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> Acked-by: Paolo Bonzini <[email protected]>
this touches only kvm and was acked by a kvm person. What did it miss to
get applied? :)
Sebastian
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 <[email protected]>
>>> Cc: "Radim Krčmář" <[email protected]>
>>> Cc: [email protected]
>>> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
>> Acked-by: Paolo Bonzini <[email protected]>
>
> 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