[RFC PATCH 1/2] check safety in workqueue

[Masami Hiramatsu]

Hi, Andi

Andi Kleen wrote:
> [cc list from hell trimmed]
> 
> (Didn't do an in depth review. Just some things that jumped out on me)

Thank you.

>>+# jmp into this function from other functions.
>>+.global arch_tmpl_stub_entry
>>+arch_tmpl_stub_entry:
>>+	nop
>>+	subl $8, %esp	#skip segment registers.
>>+	pushf
>>+	subl $20, %esp	#skip segment registers.
> 
> 
> I don't know why you try to fake pt_regs. Seems useless.

I think we needs this pt_regs to access registers from the C program 
called by the djprobe.


>>+static void local_flush_icache(void * info) 
>>+{
>>+	cpuid_eax(0);
> 
> 
> cpuid_eax is not marked volatile, so gcc will likely optimize this away.

You are right.

> x86-64 has a sync_core() that works. But I'm not convinced it even makes
> any sense to have.

In my opinion, to port sync_core() function into i386/processor.h is 
better than it.

>>+#define ARCH_STUB_VAL_IDX ((long)&arch_tmpl_stub_val - (long)&arch_tmpl_stub_entry + 1)
>>+#define ARCH_STUB_CALL_IDX ((long)&arch_tmpl_stub_call - (long)&arch_tmpl_stub_entry + 1)
>>+#define ARCH_STUB_INST_IDX ((long)&arch_tmpl_stub_inst - (long)&arch_tmpl_stub_entry)
>>+#define ARCH_STUB_END_IDX ((long)&arch_tmpl_stub_end - (long)&arch_tmpl_stub_entry)
>>+#define ARCH_STUB_SIZE ((long)&arch_tmpl_stub_end - (long)&arch_tmpl_stub_entry + 5)
> 
> 
> You likely need RELOC_HIDEs here, otherwise the gcc optimizer might
> do unexpected things again.

I mistook declaring of the types of symbols. I should declare symbols 
like below. The size of kprobes_opecode_t is 1 byte.

extern kprobe_opcode_t arch_tmpl_stub_entry;
extern kprobe_opcode_t arch_tmpl_stub_val;
extern kprobe_opcode_t arch_tmpl_stub_call;
extern kprobe_opcode_t arch_tmpl_stub_inst;
extern kprobe_opcode_t arch_tmpl_stub_end;

So, I think we do not need RELOC_HIDEs any more.

-- 
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp