All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: Ben Woodard <woodard@redhat.com>,
	Neil Horman <nhorman@redhat.com>,
	kexec@lists.infradead.org, Andi Kleen <ak@suse.de>,
	linux-kernel@vger.kernel.org, hbabu@us.ibm.com,
	Andi Kleen <andi@firstfloor.org>,
	Yinghai Lu <yhlu.kernel@gmail.com>,
	hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Date: Wed, 12 Dec 2007 15:21:32 +0100	[thread overview]
Message-ID: <20071212142132.GC4889@bingen.suse.de> (raw)
In-Reply-To: <20071211205955.GF10999@hmsreliant.think-freely.org>

> +	htcfg = read_pci_config(num, slot, func, 0x68);
> +	if (htcfg & (1 << 18)) {	
> +		printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n");
> +		if ((htcfg & (1 << 17)) == 0) {
> +			printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n");
> +			printk(KERN_INFO "Note this is a bios bug, please contact your hw vendor\n");

I'm not convinced the message is correct. e.g. on a system with only a dual core not enabling
that is fine, but the extended IDs might be still set.

>  #endif /* CONFIG_ACPI */
>  
> -static void __init nvidia_bugs(void)
> +static void __init nvidia_bugs(int num, int slot, int func)
>  {
> +	static int fix_applied = 0;
> +
> +	if (fix_applied++)
> +		return;

This looks like the wrong place to do this. Better add a flag or something
in the structure. Dito others.

Also while not a problem here in general it's bad style to add potential
wrapping bugs like this. Never use ++ for flags.

-Andi

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Andi Kleen <ak@suse.de>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: Yinghai Lu <yhlu.kernel@gmail.com>,
	Ben Woodard <woodard@redhat.com>,
	Neil Horman <nhorman@redhat.com>,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	Andi Kleen <ak@suse.de>,
	hbabu@us.ibm.com, Andi Kleen <andi@firstfloor.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com
Subject: Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Date: Wed, 12 Dec 2007 15:21:32 +0100	[thread overview]
Message-ID: <20071212142132.GC4889@bingen.suse.de> (raw)
In-Reply-To: <20071211205955.GF10999@hmsreliant.think-freely.org>

> +	htcfg = read_pci_config(num, slot, func, 0x68);
> +	if (htcfg & (1 << 18)) {	
> +		printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n");
> +		if ((htcfg & (1 << 17)) == 0) {
> +			printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n");
> +			printk(KERN_INFO "Note this is a bios bug, please contact your hw vendor\n");

I'm not convinced the message is correct. e.g. on a system with only a dual core not enabling
that is fine, but the extended IDs might be still set.

>  #endif /* CONFIG_ACPI */
>  
> -static void __init nvidia_bugs(void)
> +static void __init nvidia_bugs(int num, int slot, int func)
>  {
> +	static int fix_applied = 0;
> +
> +	if (fix_applied++)
> +		return;

This looks like the wrong place to do this. Better add a flag or something
in the structure. Dito others.

Also while not a problem here in general it's bad style to add potential
wrapping bugs like this. Never use ++ for flags.

-Andi

  parent reply	other threads:[~2007-12-12 14:21 UTC|newest]

Thread overview: 202+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-27  1:47 [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu Neil Horman
2007-11-27  1:47 ` Neil Horman
2007-11-27  4:12 ` Eric W. Biederman
2007-11-27  4:12   ` Eric W. Biederman
2007-11-27 13:13   ` Neil Horman
2007-11-27 13:13     ` Neil Horman
2007-11-27 13:28     ` Eric W. Biederman
2007-11-27 13:28       ` Eric W. Biederman
2007-11-27 13:45       ` Andi Kleen
2007-11-27 13:45         ` Andi Kleen
2007-11-27 14:28         ` Neil Horman
2007-11-27 14:28           ` Neil Horman
2007-11-27 14:43           ` Andi Kleen
2007-11-27 14:43             ` Andi Kleen
2007-11-27 14:48             ` Neil Horman
2007-11-27 14:48               ` Neil Horman
2007-11-27 15:24               ` Andi Kleen
2007-11-27 15:24                 ` Andi Kleen
2007-11-27 15:30               ` Eric W. Biederman
2007-11-27 15:30                 ` Eric W. Biederman
2007-11-27 16:45                 ` Neil Horman
2007-11-27 16:45                   ` Neil Horman
2007-11-27 20:50                 ` Ben Woodard
2007-11-27 20:50                   ` Ben Woodard
2007-11-27 21:05                   ` Neil Horman
2007-11-27 21:05                     ` Neil Horman
2007-11-27 22:38                     ` Eric W. Biederman
2007-11-27 22:38                       ` Eric W. Biederman
2007-11-27 23:15                       ` Ben Woodard
2007-11-27 23:15                         ` Ben Woodard
2007-11-28  0:15                         ` Eric W. Biederman
2007-11-28  0:15                           ` Eric W. Biederman
2007-11-27 23:40                       ` Neil Horman
2007-11-27 23:40                         ` Neil Horman
2007-11-28  0:43                         ` Eric W. Biederman
2007-11-28  0:43                           ` Eric W. Biederman
2007-11-28 15:54                     ` Neil Horman
2007-11-28 15:54                       ` Neil Horman
2007-11-27 14:56         ` Eric W. Biederman
2007-11-27 14:56           ` Eric W. Biederman
2007-11-27 15:34           ` Neil Horman
2007-11-27 15:34             ` Neil Horman
2007-11-27 18:41           ` Ben Woodard
2007-11-27 18:41             ` Ben Woodard
2007-11-27 19:42             ` Neil Horman
2007-11-27 19:42               ` Neil Horman
2007-11-27 20:00               ` Vivek Goyal
2007-11-27 20:00                 ` Vivek Goyal
2007-11-27 20:52                 ` Neil Horman
2007-11-27 20:52                   ` Neil Horman
2007-11-27 22:24                 ` Andi Kleen
2007-11-27 22:24                   ` Andi Kleen
2007-11-27 23:24                   ` Ben Woodard
2007-11-27 23:24                     ` Ben Woodard
2007-11-27 23:56                     ` Andi Kleen
2007-11-27 23:56                       ` Andi Kleen
2007-11-28 15:36                     ` Vivek Goyal
2007-11-28 15:36                       ` Vivek Goyal
2007-11-28 16:02                       ` Neil Horman
2007-11-28 16:02                         ` Neil Horman
2007-11-28 17:36                         ` Eric W. Biederman
2007-11-28 17:36                           ` Eric W. Biederman
2007-11-28 18:16                           ` Neil Horman
2007-11-28 18:16                             ` Neil Horman
2007-11-28 19:05                         ` Vivek Goyal
2007-11-28 19:05                           ` Vivek Goyal
2007-11-28 19:42                           ` Eric W. Biederman
2007-11-28 19:42                             ` Eric W. Biederman
2007-11-28 21:09                             ` Neil Horman
2007-11-28 21:09                               ` Neil Horman
2007-11-28 23:27                               ` Eric W. Biederman
2007-11-28 23:27                                 ` Eric W. Biederman
2007-11-30  2:16                             ` Ben Woodard
2007-11-30  2:16                               ` Ben Woodard
2007-11-30  2:54                               ` Eric W. Biederman
2007-11-30  2:54                                 ` Eric W. Biederman
2007-11-30  8:59                                 ` Yinghai Lu
2007-11-30  8:59                                   ` Yinghai Lu
2007-11-30 14:35                                   ` Vivek Goyal
2007-11-30 14:35                                     ` Vivek Goyal
2007-11-30 14:32                                 ` Neil Horman
2007-11-30 14:32                                   ` Neil Horman
2007-11-30  2:12                           ` Ben Woodard
2007-11-30  2:12                             ` Ben Woodard
2007-11-30 14:42                             ` Vivek Goyal
2007-11-30 14:42                               ` Vivek Goyal
2007-11-30 14:51                               ` Neil Horman
2007-11-30 14:51                                 ` Neil Horman
2007-12-06 21:39                                 ` Neil Horman
2007-12-06 21:39                                   ` Neil Horman
2007-12-06 22:11                                   ` Vivek Goyal
2007-12-06 22:11                                     ` Vivek Goyal
2007-12-07  0:10                                     ` Neil Horman
2007-12-07  0:10                                       ` Neil Horman
2007-12-07 14:39                                       ` Vivek Goyal
2007-12-07 14:39                                         ` Vivek Goyal
2007-12-07 14:53                                         ` Neil Horman
2007-12-07 14:53                                           ` Neil Horman
2007-12-07 15:16                                           ` Vivek Goyal
2007-12-07 15:16                                             ` Vivek Goyal
2007-12-07 15:53                                             ` Neil Horman
2007-12-07 15:53                                               ` Neil Horman
2007-12-07 18:46                                               ` Eric W. Biederman
2007-12-07 18:46                                                 ` Eric W. Biederman
2007-12-07  0:33                                     ` Eric W. Biederman
2007-12-07  0:33                                       ` Eric W. Biederman
2007-12-07  2:04                                       ` Neil Horman
2007-12-07  2:04                                         ` Neil Horman
2007-12-07  8:50                                       ` Yinghai Lu
2007-12-07  8:50                                         ` Yinghai Lu
2007-12-07  9:22                                         ` Yinghai Lu
2007-12-07  9:22                                           ` Yinghai Lu
2007-12-07 14:21                                           ` Neil Horman
2007-12-07 14:21                                             ` Neil Horman
2007-12-07 17:58                                             ` Neil Horman
2007-12-07 17:58                                               ` Neil Horman
2007-12-07 19:19                                               ` yhlu
2007-12-07 19:19                                                 ` yhlu
2007-12-07 20:13                                                 ` Neil Horman
2007-12-07 20:13                                                   ` Neil Horman
2007-12-10 15:39                                               ` Neil Horman
2007-12-10 15:39                                                 ` Neil Horman
2007-12-10 16:20                                                 ` Vivek Goyal
2007-12-10 16:20                                                   ` Vivek Goyal
2007-12-11  1:17                                                 ` Eric W. Biederman
2007-12-11  1:17                                                   ` Eric W. Biederman
2007-12-11  1:08                                               ` Eric W. Biederman
2007-12-11  1:08                                                 ` Eric W. Biederman
2007-12-11  3:43                                                 ` Neil Horman
2007-12-11  3:43                                                   ` Neil Horman
2007-12-11  4:48                                                   ` Eric W. Biederman
2007-12-11  4:48                                                     ` Eric W. Biederman
2007-12-11  6:31                                                     ` Yinghai Lu
2007-12-11  6:31                                                       ` Yinghai Lu
2007-12-11 14:39                                                     ` Neil Horman
2007-12-11 14:39                                                       ` Neil Horman
2007-12-11 15:29                                                       ` Eric W. Biederman
2007-12-11 15:29                                                         ` Eric W. Biederman
2007-12-11 18:00                                                         ` Yinghai Lu
2007-12-11 18:00                                                           ` Yinghai Lu
2007-12-11 18:29                                                           ` Neil Horman
2007-12-11 18:29                                                             ` Neil Horman
2007-12-11 18:45                                                             ` Yinghai Lu
2007-12-11 18:45                                                               ` Yinghai Lu
2007-12-11 18:22                                                         ` Neil Horman
2007-12-11 18:22                                                           ` Neil Horman
2007-12-11 18:46                                                           ` Eric W. Biederman
2007-12-11 18:46                                                             ` Eric W. Biederman
2007-12-11 19:24                                                             ` Neil Horman
2007-12-11 19:24                                                               ` Neil Horman
2007-12-11 19:51                                                               ` Yinghai Lu
2007-12-11 19:51                                                                 ` Yinghai Lu
2007-12-11 20:59                                                                 ` Neil Horman
2007-12-11 20:59                                                                   ` Neil Horman
2007-12-12  0:16                                                                   ` Ben Woodard
2007-12-12  0:16                                                                     ` Ben Woodard
2007-12-12  0:52                                                                     ` Neil Horman
2007-12-12  0:52                                                                       ` Neil Horman
2007-12-12  1:07                                                                       ` Yinghai Lu
2007-12-12  1:07                                                                         ` Yinghai Lu
2007-12-12  8:43                                                                   ` [PATCH] k8: Enable legacy irqs with extended cpu ids Eric W. Biederman
2007-12-12  8:43                                                                     ` Eric W. Biederman
2007-12-12 14:21                                                                   ` Andi Kleen [this message]
2007-12-12 14:21                                                                     ` [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu Andi Kleen
2007-12-12 15:55                                                                     ` Neil Horman
2007-12-12 15:55                                                                       ` Neil Horman
2007-12-12 16:07                                                                       ` Andi Kleen
2007-12-12 16:07                                                                         ` Andi Kleen
2007-12-12 19:43                                                                         ` Eric W. Biederman
2007-12-12 19:43                                                                           ` Eric W. Biederman
2007-12-12 20:22                                                                           ` Neil Horman
2007-12-12 20:22                                                                             ` Neil Horman
2007-12-12 21:32                                                                             ` Eric W. Biederman
2007-12-12 21:32                                                                               ` Eric W. Biederman
2007-12-13 14:39                                                                               ` Neil Horman
2007-12-13 14:39                                                                                 ` Neil Horman
2007-12-13 15:16                                                                                 ` Andi Kleen
2007-12-13 15:16                                                                                   ` Andi Kleen
2007-12-13 15:32                                                                                   ` Neil Horman
2007-12-13 15:32                                                                                     ` Neil Horman
2007-12-17 11:38                                                                                     ` Neil Horman
2007-12-17 11:38                                                                                       ` Neil Horman
2007-12-18  0:13                                                                                   ` Eric W. Biederman
2007-12-18  0:13                                                                                     ` Eric W. Biederman
2007-12-17 15:16                                                                                 ` Ingo Molnar
2007-12-17 15:16                                                                                   ` Ingo Molnar
2007-12-17 15:47                                                                                   ` Neil Horman
2007-12-17 15:47                                                                                     ` Neil Horman
2007-12-07 18:36                                             ` Eric W. Biederman
2007-12-07 18:36                                               ` Eric W. Biederman
2007-12-07 18:48                                               ` Neil Horman
2007-12-07 18:48                                                 ` Neil Horman
2007-12-10 19:39                                                 ` Ben Woodard
2007-12-10 20:22                                                   ` Neil Horman
2007-11-27 13:53       ` Neil Horman
2007-11-27 13:53         ` Neil Horman
2007-11-27 10:55 ` Andi Kleen
2007-11-27 10:55   ` Andi Kleen
2007-11-27 11:19   ` Eric W. Biederman
2007-11-27 11:19     ` Eric W. Biederman
2007-11-27 13:28   ` Neil Horman
2007-11-27 13:28     ` Neil Horman

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=20071212142132.GC4889@bingen.suse.de \
    --to=ak@suse.de \
    --cc=andi@firstfloor.org \
    --cc=ebiederm@xmission.com \
    --cc=hbabu@us.ibm.com \
    --cc=hpa@zytor.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=nhorman@redhat.com \
    --cc=nhorman@tuxdriver.com \
    --cc=tglx@linutronix.de \
    --cc=woodard@redhat.com \
    --cc=yhlu.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.