Re: [PATCH] [kgdb] Switch master cpu after gdb thread command forSMP(v2)

From: Jason Wessel
Date: Tue Oct 21 2008 - 14:36:11 EST




sonic zhang wrote:
> In blackfin SMP architecture, different core has its own L1 SRAM and MMR
> memory, which code running on the other core can't access. In current
kgdb
> impelemntation, cpus are represented by thread with minus prefix.
>
> If user run thread command in gdb to switch to the thread of the other cpu,
> kgdb should:
> 1. send IPI signal to master cpu
> 2. release the specific passive cpu waiting in IPI handler
> 3. exit kgdb exception loop on master cpu and trap into kgdb wait in
IPI handler
> 4. trap the released passive cpu into kgdb exception in IPI handler
>
> This patch is tested on bf561-ezkit board with SMP enabled.
>

It does look better this time around, but the patch does not apply to
the kgdb dev branch.

Hunk #10 FAILED at 1502.
Hunk #11 FAILED at 1518.
Hunk #12 FAILED at 1537.
Hunk #13 FAILED at 1550.
Hunk #14 succeeded at 1584 (offset -12 lines).
4 out of 14 hunks FAILED -- rejects in file kernel/kgdb.c


>
> Signed-off-by: Sonic Zhang <sonic.adi@xxxxxxxxx>
> ---
> include/linux/kgdb.h | 14 +++++++++++
> kernel/kgdb.c | 64
+++++++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 69 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> index cb8a3d6..4c3fd6c 100644
> --- a/include/linux/kgdb.h
> +++ b/include/linux/kgdb.h
> @@ -107,6 +107,7 @@ struct kgdb_bkpt {
> #endif
>
> #define KGDB_HW_BREAKPOINT 1
> +#define KGDB_THR_PROC_SWAP 2


I can fix this later, but it should have always been
a bit mask indicated by 0x1 0x2...


> --- a/kernel/kgdb.c
> +++ b/kernel/kgdb.c
> @@ -566,6 +566,7 @@ static void kgdb_wait(struct pt_regs *regs)
> {
> unsigned long flags;
> int cpu;
> + struct task_struct *thread;
>
> local_irq_save(flags);
> cpu = raw_smp_processor_id();
> @@ -578,6 +579,8 @@ static void kgdb_wait(struct pt_regs *regs)
> smp_wmb();
> atomic_set(&cpu_in_kgdb[cpu], 1);
>
> + kgdb_disable_hw_debug(regs);
> +


Good catch, this is a real arch independent defect,
which should be fixed.



> @@ -1223,7 +1253,13 @@ static int gdb_serial_stub(struct kgdb_state *ks)
> /* Clear the out buffer. */
> memset(remcom_out_buffer, 0, sizeof(remcom_out_buffer));
>
> - if (kgdb_connected) {
> + if (kgdb_contthread) {

I thik that check needs to be
if (arch_kgdb_ops.flags & KGDB_THR_PROC_SWAP &&
kgdb_contthread)

Although I am not quite certain, and ther was no way to
further look at it since your patch did not apply.

The use of contthread changed a bit in the 2.6.27, as
it was found to not be doing what it was intended.

See commit d7161a65341556bacb5e6654e133803f46f51063
for the details.


> + remcom_out_buffer[0] = 'O';
> + remcom_out_buffer[1] = 'K';
> + remcom_out_buffer[2] = 0;
> + put_packet(remcom_out_buffer);
> + kgdb_contthread = NULL;
> + } else if (kgdb_connected) {
> unsigned char thref[8];
> char *ptr;
>


Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/