Sergei Shtylyov wrote:
> Since it doesn't make sense both to set kgdb_may_fault flag before setting up
> the jump buffer and to resotre the preempt count if a fault hasn't happened,
> get rid of kgdb_[un]set_may_fault() introducing the inline wrapper around
> kgdb_fault_setjmp() call to enable the kernel faults and save/restore the
> preempt count if the preemption is enabled (removing kgdb_fault_preempt_count
> global variable at the same time) and directly clearing kgdb_may_fault on the
> normal, non-faulted exit path from the memory manipulation functions...
>
> Also, while at it, do some more cleanups:
>
> - add newlines after block declarations;
>
> - get rid of unneeded braces;
>
I liked the unneeded braces. I know it's not popular
in the Linux community. We has some very good
programmers back at Siemens in Germany that
advocated using unneeded braces and I think I
bought into their point of view. Just looks much
easier to read for me now that I'm use to it.
Sigh.
> - clean up grammar/style in comments...
>
> Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
>
> ---
> Just remembered that I intended to add newlines after block decrations. :-)
> The patch is against the linux_2_6_21_uprev branch as usual...
>
> kernel/kgdb.c | 120
> +++++++++++++++++++++++++++++++---------------------------
> 1 files changed, 65 insertions(+), 55 deletions(-)
>
> Index: linux-2.6/kernel/kgdb.c
> ===================================================================
> --- linux-2.6.orig/kernel/kgdb.c
> +++ linux-2.6/kernel/kgdb.c
> @@ -10,6 +10,7 @@
> * Copyright (C) 2004-2006 Tom Rini <[EMAIL PROTECTED]>
> * Copyright (C) 2004-2006 LinSysSoft Technologies Pvt. Ltd.
> * Copyright (C) 2005-2007 Wind River Systems, Inc.
> + * Copyright (C) 2007 MontaVista Software, Inc.
> *
> * Contributors at various stages not listed above:
> * Jason Wessel ( [EMAIL PROTECTED] )
> @@ -65,12 +66,9 @@ extern int pid_max;
> int kgdb_initialized;
> /* Is a host GDB connected to us? */
> int kgdb_connected;
> -/* Could we be about to try and access a bad memory location? If so we
> - * also need to flag this has happend. */
> +/* Could we be about to try and access a bad memory location?
> + * If so we also need to flag this has happened. */
> int kgdb_may_fault;
> -#ifdef CONFIG_PREEMPT
> -static int kgdb_fault_preempt_count;
> -#endif
>
> /* All the KGDB handlers are installed */
> int kgdb_from_module_registered = 0;
> @@ -196,6 +194,7 @@ static void get_packet(char *buffer)
> unsigned char xmitcsum;
> int count;
> char ch;
> +
> if (!kgdb_io_ops.read_char)
> return;
> do {
> @@ -235,20 +234,6 @@ static void get_packet(char *buffer)
> } while (checksum != xmitcsum);
> }
>
> -static void kgdb_set_may_fault(void) {
> - kgdb_may_fault = 1;
> -#ifdef CONFIG_PREEMPT
> - kgdb_fault_preempt_count = preempt_count();
> -#endif
> -}
> -
> -static void kgdb_unset_may_fault(void) {
> - kgdb_may_fault = 0;
> -#ifdef CONFIG_PREEMPT
> - preempt_count() = kgdb_fault_preempt_count;
> -#endif
> -}
> -
> /*
> * Send the packet in buffer.
> * Check for gdb connection if asked for.
> @@ -303,21 +288,48 @@ static void put_packet(char *buffer)
> }
>
> /*
> - * convert the memory pointed to by mem into hex, placing result in buf
> - * return a pointer to the last char put in buf (null). May return an error.
> + * Wrap kgdb_fault_setjmp() call to enable the kernel faults and save/restore
> + * the state before/after a fault has happened.
> + * Note that it *must* be inline to work correctly.
> + */
> +static inline int fault_setjmp(void)
> +{
> +#ifdef CONFIG_PREEMPT
> + int count = preempt_count();
> +#endif
> +
> + /*
> + * kgdb_fault_setjmp() returns 0 after the jump buffer has been setup,
> + * and non-zero when an expected kernel fault has happened.
> + */
> + if (kgdb_fault_setjmp(kgdb_fault_jmp_regs) == 0) {
> + kgdb_may_fault = 1;
> + return 0;
> + } else {
> +#ifdef CONFIG_PREEMPT
> + preempt_count() = count;
> +#endif
> + kgdb_may_fault = 0;
> + return 1;
> + }
> +}
> +
> +/*
> + * Convert the memory pointed to by mem into hex, placing result in buf.
> + * Return a pointer to the last char put in buf (null). May return an error.
> */
> char *kgdb_mem2hex(char *mem, char *buf, int count)
> {
> - kgdb_set_may_fault();
> - if ((kgdb_fault_setjmp(kgdb_fault_jmp_regs)) != 0) {
> - kgdb_unset_may_fault();
> + if (fault_setjmp() != 0)
> return ERR_PTR(-EINVAL);
> - }
> - /* Accessing some registers in a single load instruction is
> +
> + /*
> + * Accessing some registers in a single load instruction is
> * required to avoid bad side effects for some I/O registers.
> */
> if ((count == 2) && (((long)mem & 1) == 0)) {
> unsigned short tmp_s = *(unsigned short *)mem;
> +
> mem += 2;
> #ifdef __BIG_ENDIAN
> *buf++ = hexchars[(tmp_s >> 12) & 0xf];
> @@ -332,6 +344,7 @@ char *kgdb_mem2hex(char *mem, char *buf,
> #endif
> } else if ((count == 4) && (((long)mem & 3) == 0)) {
> unsigned long tmp_l = *(unsigned int *)mem;
> +
> mem += 4;
> #ifdef __BIG_ENDIAN
> *buf++ = hexchars[(tmp_l >> 28) & 0xf];
> @@ -355,6 +368,7 @@ char *kgdb_mem2hex(char *mem, char *buf,
> #ifdef CONFIG_64BIT
> } else if ((count == 8) && (((long)mem & 7) == 0)) {
> unsigned long long tmp_ll = *(unsigned long long *)mem;
> +
> mem += 8;
> #ifdef __BIG_ENDIAN
> *buf++ = hexchars[(tmp_ll >> 60) & 0xf];
> @@ -392,14 +406,14 @@ char *kgdb_mem2hex(char *mem, char *buf,
> *buf++ = hexchars[(tmp_ll >> 56) & 0xf];
> #endif
> #endif
> - } else {
> + } else
> while (count-- > 0) {
> unsigned char ch = *mem++;
> +
> *buf++ = hexchars[ch >> 4];
> *buf++ = hexchars[ch & 0xf];
> }
> - }
> - kgdb_unset_may_fault();
> + kgdb_may_fault = 0;
> *buf = 0;
> return (buf);
> }
> @@ -411,35 +425,32 @@ char *kgdb_mem2hex(char *mem, char *buf,
> */
> static char *kgdb_ebin2mem(char *buf, char *mem, int count)
> {
> - kgdb_set_may_fault();
> - if ((kgdb_fault_setjmp(kgdb_fault_jmp_regs)) != 0) {
> - kgdb_unset_may_fault();
> + if (fault_setjmp() != 0)
> return ERR_PTR(-EINVAL);
> - }
> - for (; count > 0; count--, buf++) {
> +
> + for (; count > 0; count--, buf++)
> if (*buf == 0x7d)
> *mem++ = *(++buf) ^ 0x20;
> else
> *mem++ = *buf;
> - }
> - kgdb_unset_may_fault();
> +
> + kgdb_may_fault = 0;
> return mem;
> }
>
> /*
> - * convert the hex array pointed to by buf into binary to be placed in mem
> - * return a pointer to the character AFTER the last byte written
> + * Convert the hex array pointed to by buf into binary to be placed in mem.
> + * Return a pointer to the character AFTER the last byte written.
> * May return an error.
> */
> char *kgdb_hex2mem(char *buf, char *mem, int count)
> {
> - kgdb_set_may_fault();
> - if ((kgdb_fault_setjmp(kgdb_fault_jmp_regs)) != 0) {
> - kgdb_unset_may_fault();
> + if (fault_setjmp() != 0)
> return ERR_PTR(-EINVAL);
> - }
> +
> if ((count == 2) && (((long)mem & 1) == 0)) {
> unsigned short tmp_s = 0;
> +
> #ifdef __BIG_ENDIAN
> tmp_s |= hex(*buf++) << 12;
> tmp_s |= hex(*buf++) << 8;
> @@ -455,6 +466,7 @@ char *kgdb_hex2mem(char *buf, char *mem,
> mem += 2;
> } else if ((count == 4) && (((long)mem & 3) == 0)) {
> unsigned long tmp_l = 0;
> +
> #ifdef __BIG_ENDIAN
> tmp_l |= hex(*buf++) << 28;
> tmp_l |= hex(*buf++) << 24;
> @@ -478,13 +490,15 @@ char *kgdb_hex2mem(char *buf, char *mem,
> mem += 4;
> } else {
> int i;
> +
> for (i = 0; i < count; i++) {
> unsigned char ch = hex(*buf++) << 4;
> +
> ch |= hex(*buf++);
> *mem++ = ch;
> }
> }
> - kgdb_unset_may_fault();
> + kgdb_may_fault = 0;
> return (mem);
> }
>
> @@ -639,39 +653,35 @@ static void kgdb_wait(struct pt_regs *re
>
> int kgdb_get_mem(char *addr, unsigned char *buf, int count)
> {
> - kgdb_set_may_fault();
> - if ((kgdb_fault_setjmp(kgdb_fault_jmp_regs)) != 0) {
> - kgdb_unset_may_fault();
> + if (fault_setjmp() != 0)
> return -EINVAL;
> - }
> +
> while (count) {
> if ((unsigned long)addr < TASK_SIZE) {
> - kgdb_unset_may_fault();
> + kgdb_may_fault = 0;
> return -EINVAL;
> }
> *buf++ = *addr++;
> count--;
> }
> - kgdb_unset_may_fault();
> + kgdb_may_fault = 0;
> return 0;
> }
>
> int kgdb_set_mem(char *addr, unsigned char *buf, int count)
> {
> - kgdb_set_may_fault();
> - if ((kgdb_fault_setjmp(kgdb_fault_jmp_regs)) != 0) {
> - kgdb_unset_may_fault();
> + if (fault_setjmp() != 0)
> return -EINVAL;
> - }
> +
> while (count) {
> if ((unsigned long)addr < TASK_SIZE) {
> - kgdb_unset_may_fault();
> + kgdb_may_fault = 0;
> return -EINVAL;
> }
> *addr++ = *buf++;
> count--;
> }
> - kgdb_unset_may_fault();
> + kgdb_may_fault = 0;
> return 0;
> }
> int kgdb_activate_sw_breakpoints(void)
>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems? Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> _______________________________________________
> Kgdb-bugreport mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
>
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
Kgdb-bugreport mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
