On 11/16/2010 10:28 AM, Sergei Shtylyov wrote: > Hello. > > Jason Wessel wrote: > > >>>> Thanks for the bug report Frederic. >>>> The attached patch hopefully addresses the problem. >>>> >>> You forgot to attach it. :-) >>> > > >>>> Many thanks, >>>> Jason. >>>> > > >>> WBR, Sergei >>> > > >> I suspect it is the case that the mailing list consumed the post. >> > > >> Frederic already responded back confirming that the test results are good. >> > > >> Here is the patch in a form that the list will not eat it. >> > > >> --- >> > > > >> From: Jason Wessel <[email protected]> >> Subject: [PATCH] kgdb,x86: fix regression in detach handling >> Date: Mon Nov 15 08:07:35 CST 2010 >> > > >> The fix from ba773f7c510c0b252145933926c636c439889207 >> (x86,kgdb: Fix hw breakpoint regression) was not entirely complete. >> > > >> The kgdb_remove_all_hw_break() function also needs to call the >> hw_break_release_slot() or else a breakpoint can get activated again >> after the debugger has detached. >> > > >> The kgdb test suite exposes the behavior in the form of either a hang >> or repetitive failure. The kernel config that exposes the problem >> contains all of the following: >> > > >> CONFIG_DEBUG_RODATA=y >> CONFIG_KGDB_TESTS=y >> CONFIG_KGDB_TESTS_ON_BOOT=y >> CONFIG_KGDB_TESTS_BOOT_STRING="V1F100" >> > > >> Reported-by: Frederic Weisbecker <[email protected]> >> Tested-by: Frederic Weisbecker <[email protected]> >> Signed-off-by: Jason Wessel <[email protected]> >> > > >> --- >> arch/x86/kernel/kgdb.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> > > >> --- a/arch/x86/kernel/kgdb.c >> +++ b/arch/x86/kernel/kgdb.c >> @@ -315,14 +315,19 @@ static void kgdb_remove_all_hw_break(voi >> if (!breakinfo[i].enabled) >> continue; >> bp = *per_cpu_ptr(breakinfo[i].pev, cpu); >> - if (bp->attr.disabled == 1) >> + if (!bp->attr.disabled) { >> + arch_uninstall_hw_breakpoint(bp); >> + bp->attr.disabled = 1; >> continue; >> + } >> if (dbg_is_early) >> early_dr7 &= ~encode_dr7(i, breakinfo[i].len, >> breakinfo[i].type); >> else >> - arch_uninstall_hw_breakpoint(bp); >> - bp->attr.disabled = 1; >> + if (hw_break_release_slot(i)) >> + printk(KERN_ERR "KGDB: hw bpt failed %lx\n", >> > > Also, direcet use of printk() is discouraged. Why not use pr_err()? >
Not that pr_err() is a bad idea, but I could find no documented policy that suggests printk() is discouraged outside of drivers/*. If this is really true I would have expected by now someone posting bulk patches to perform updates to change the printk(). Every other place in the arch/x86/kernel/kgdb.c uses a printk(...) at the moment, so perhaps for now we'll stay with that convention and bulk convert at a later point if it really makes sense to do so. I am interested in understanding the precedents for printk(...) usage outside of device drivers, and should there be a standard I don't mind converting to use it. Cheers, Jason. ------------------------------------------------------------------------------ Beautiful is writing same markup. Internet Explorer 9 supports standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3. Spend less time writing and rewriting code and more time creating great experiences on the web. Be a part of the beta today http://p.sf.net/sfu/msIE9-sfdev2dev _______________________________________________ Kgdb-bugreport mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
