[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090511082258.GC5636@elte.hu>
Date: Mon, 11 May 2009 10:22:58 +0200
From: Ingo Molnar <mingo@...e.hu>
To: Yinghai Lu <yinghai@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>,
"H. Peter Anvin" <hpa@...or.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Jesse Barnes <jbarnes@...tuousgeek.org>,
Len Brown <lenb@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linux-pci@...r.kernel.org,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH 3/7] x86: fix alloc_mptable
* Yinghai Lu <yinghai@...nel.org> wrote:
> fix the condition checking.
>
> [ Impact: make alloc_mptable working ]
>
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
>
> ---
> arch/x86/kernel/mpparse.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> Index: linux-2.6/arch/x86/kernel/mpparse.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/mpparse.c
> +++ linux-2.6/arch/x86/kernel/mpparse.c
> @@ -873,21 +873,24 @@ inline void __init check_irq_src(struct
> static int check_slot(unsigned long mpc_new_phys, unsigned long mpc_new_length,
> int count)
> {
> + int ret = 0;
> +
> if (!mpc_new_phys) {
> - pr_info("No spare slots, try to append...take your risk, "
> + pr_warning("No spare slots, try to append...take your risk, "
> "new mpc_length %x\n", count);
> } else {
> - if (count <= mpc_new_length)
> + if (count <= mpc_new_length) {
> pr_info("No spare slots, try to append..., "
> "new mpc_length %x\n", count);
> - else {
> + ret = 1;
> + } else {
> pr_err("mpc_new_length %lx is too small\n",
> mpc_new_length);
> - return -1;
> + ret = -1;
> }
> }
>
> - return 0;
> + return ret;
> }
>
> static int __init replace_intsrc_all(struct mpc_table *mpc,
> @@ -946,7 +949,7 @@ static int __init replace_intsrc_all(st
> } else {
> struct mpc_intsrc *m = (struct mpc_intsrc *)mpt;
> count += sizeof(struct mpc_intsrc);
> - if (!check_slot(mpc_new_phys, mpc_new_length, count))
> + if (check_slot(mpc_new_phys, mpc_new_length, count) < 0)
> goto out;
> assign_to_mpc_intsrc(&mp_irqs[i], m);
> mpc->length = count;
hm, i modified this to the code attached below instead. Things like:
> - pr_info("No spare slots, try to append...take your risk, "
> + pr_warning("No spare slots, try to append...take your risk, "
> "new mpc_length %x\n", count);
are not acceptable at all. If there's _anything_ wrong with code
like this, if we run out of a static pool of slots, we dont try to
hack our way out of it... Instead we inform the user and bail out
ASAP!
A predictable, well working way out of a resource shortage is _way_
more important than trying to clinge to some functionality and
hoping that it will be all fine ...
So ... please describe under which conditions we can run out of
slots, and what the options are in that situation.
Ingo
------------------------------>
Subject: x86: fix alloc_mptable()
From: Yinghai Lu <yinghai@...nel.org>
Date: Wed, 06 May 2009 10:07:07 -0700
Fix the conditions when we stop updating the mptable due to
running out of slots.
[ Impact: fix memory corruption / non-working update_mptable boot parameter ]
Signed-off-by: Yinghai Lu <yinghai@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Jesse Barnes <jbarnes@...tuousgeek.org>
Cc: Len Brown <lenb@...nel.org>
LKML-Reference: <4A01C3BB.1000609@...nel.org>
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
arch/x86/kernel/mpparse.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
Index: tip/arch/x86/kernel/mpparse.c
===================================================================
--- tip.orig/arch/x86/kernel/mpparse.c
+++ tip/arch/x86/kernel/mpparse.c
@@ -870,24 +870,17 @@ static
inline void __init check_irq_src(struct mpc_intsrc *m, int *nr_m_spare) {}
#endif /* CONFIG_X86_IO_APIC */
-static int check_slot(unsigned long mpc_new_phys, unsigned long mpc_new_length,
- int count)
+static int
+check_slot(unsigned long mpc_new_phys, unsigned long mpc_new_length, int count)
{
- if (!mpc_new_phys) {
- pr_info("No spare slots, try to append...take your risk, "
- "new mpc_length %x\n", count);
- } else {
- if (count <= mpc_new_length)
- pr_info("No spare slots, try to append..., "
- "new mpc_length %x\n", count);
- else {
- pr_err("mpc_new_length %lx is too small\n",
- mpc_new_length);
- return -1;
- }
+ int ret = 0;
+
+ if (!mpc_new_phys || count <= mpc_new_length) {
+ WARN(1, "update_mptable: No spare slots (length: %x)\n", count);
+ return -1;
}
- return 0;
+ return ret;
}
static int __init replace_intsrc_all(struct mpc_table *mpc,
@@ -946,7 +939,7 @@ static int __init replace_intsrc_all(st
} else {
struct mpc_intsrc *m = (struct mpc_intsrc *)mpt;
count += sizeof(struct mpc_intsrc);
- if (!check_slot(mpc_new_phys, mpc_new_length, count))
+ if (check_slot(mpc_new_phys, mpc_new_length, count) < 0)
goto out;
assign_to_mpc_intsrc(&mp_irqs[i], m);
mpc->length = count;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists