jason.wessel@windriver.com wrote:
quoted text
> --- /dev/null
> +++ b/include/asm-generic/kgdb.h
> @@ -0,0 +1,105 @@
> +/*
> + * include/asm-generic/kgdb.h
> + *
> + * This provides the assembly level information so that KGDB can provi=
de
quoted text
> + * a GDB that has been patched with enough information to know to stop=

quoted text
> + * trying to unwind the function.
> + *
> + * Author: Tom Rini <trini@kernel.crashing.org>
> + *
> + * 2005 (c) MontaVista Software, Inc.
> + * 2006 (c) Embedded Alley Solutions, Inc.
> + *
> + * This file is licensed under the terms of the GNU General Public Lic=
ense
quoted text
> + * version 2. This program is licensed "as is" without any warranty of=
 any
quoted text
> + * kind, whether express or implied.
> + */
> +
> +#ifndef __ASM_GENERIC_KGDB_H__
> +#define __ASM_GENERIC_KGDB_H__
> +
> +struct pt_regs;
> +
> +#ifdef CONFIG_X86

Hmm, I just realized that I didn't really looked at the arch interface
in details yet. Now it appears a bit inconsistent to me in its way to
tell "this arch supports that". Something in include/asm-generic that
starts with #ifdef CONFIG_X86 looks ugly, to begin with.

=2E..
quoted text
> +#ifdef CONFIG_KGDB_ARCH_HAS_SHADOW_INFO

Then we have this KGDB_ARCH_HAS thing here, the second way to control
arch-dependent stuff.

=2E..
quoted text
> --- /dev/null
> +++ b/include/linux/kgdb.h
=2E..
quoted text
> +struct kgdb_arch {
> +	unsigned char		gdb_bpt_instr[BREAK_INSTR_SIZE];
> +	unsigned long		flags;
> +	unsigned		shadowth;
> +
> +	int	(*set_breakpoint)(unsigned long, char *);
> +	int	(*remove_breakpoint)(unsigned long, char *);
> +	int	(*set_hw_breakpoint)(unsigned long, int, enum kgdb_bptype);
> +	int	(*remove_hw_breakpoint)(unsigned long, int, enum kgdb_bptype);
> +	void	(*remove_all_hw_break)(void);
> +	void	(*correct_hw_break)(void);
> +};

And this is the third way. Can we consolidate this?

=2E..
quoted text
> --- /dev/null
> +++ b/kernel/kgdb.c
=2E..
quoted text
> +/*
> + * Convert the memory pointed to by mem into hex, placing result in bu=
f.
quoted text
> + * Return a pointer to the last char put in buf (null). May return an =
error.
quoted text
> + */
> +char *kgdb_mem2hex(char *mem, char *buf, int count)

As Linus remarked, still pointer-based return values while only the
error code is used by the callers (there are more of such cases in the
core).

=2E..
quoted text
> --- a/kernel/softlockup.c
> +++ b/kernel/softlockup.c
=2E..
quoted text
> @@ -52,7 +55,6 @@ static unsigned long get_timestamp(int this_cpu)
>  void touch_softlockup_watchdog(void)
>  {
>  	int this_cpu =3D raw_smp_processor_id();
> -

Probably some relic from older changes which should be killed.

Jan