* Yinghai Lu <yinghai@xxxxxxxxxx> wrote: > fix the condition checking. > > [ Impact: make alloc_mptable working ] > > Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx> > > --- > 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@xxxxxxxxxx> 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@xxxxxxxxxx> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> Cc: Len Brown <lenb@xxxxxxxxxx> LKML-Reference: <4A01C3BB.1000609@xxxxxxxxxx> Signed-off-by: Ingo Molnar <mingo@xxxxxxx> --- 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-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html