RE: [3/3] Userspace probes prototype-take2

["Zhang, Yanmin" <yanmin.zhang@intel.com>]

>>-----Original Message-----
>>From: systemtap-owner@sourceware.org [mailto:systemtap-owner@sourceware.org] On Behalf Of Prasanna S Panchamukhi
>>Sent: 2006年2月20日 13:38
>>To: Zhang, Yanmin
>>Cc: systemtap@sources.redhat.com
>>Subject: Re: [3/3] Userspace probes prototype-take2
>>
>>Yanmin,
>>
>>
>>Please see my comments inline below.
>>
>>Thanks
>>Prasanna
>>> >>
>>> >>2. This patch works only with PREEMPT config option disabled, to work
>>> >>in PREEMPT enabled condition handlers must be re-written and must
>>> >>be seperated out from kernel probes allowing preemption.
>>> One of my old comments is an external device interrupt might happen when cpu is single-stepping the original instruction, then the
>>task might be switched to another cpu. If we disable irq when exiting to user space to single step the instruction, kernel might switch
>>the task off just on the exit kernel path. 1) uprobe_page; 2) kprobe_ctlblk, These 2 resources shouldn't be pre cpu, or we need get
>>another approach. How could you resolve the task switch issue?
>>
>>My new design does not use the kprobe handlers and per cpu kprobe data
>>structures itself  so that task switch issue will be resolved.
>>We register a separte set of uprobe handlers and use uprobe data structure.
>>Also now we will be handling uprobes serially and synchronize using some lock/mutex, but later on we have scale it up for better performance.
>>
>>> >>+static int __kprobes copy_insn_on_new_page(struct uprobe *uprobe ,
>>> >>+			struct pt_regs *regs, struct vm_area_struct *vma)
>>> >>+{
>>> >>+	unsigned long addr, *vaddr, stack_addr = regs->esp;
>>> >>+	int size = MAX_INSN_SIZE * sizeof(kprobe_opcode_t);
>>> >>+	struct uprobe_page *upage;
>>> >>+	struct page *page;
>>> >>+	pte_t *pte;
>>> >>+
>>> >>+
>>> >>+	if (vma->vm_flags & VM_GROWSDOWN) {
>>> >>+		if (((stack_addr - sizeof(long long))) < (vma->vm_start + size))
>>> >>+			return -ENOMEM;
>>> >>+
>>> >>+		addr = vma->vm_start;
>>> >>+	} else if (vma->vm_flags & VM_GROWSUP) {
>>> >>+		if ((vma->vm_end - size) < (stack_addr + sizeof(long long)))
>>> >>+			return -ENOMEM;
>>> >>+
>>> >>+		addr = vma->vm_end - size;
>>> >>+	} else
>>> >>+		return -EFAULT;
>>> >>+
>>> The multi-thread case is not resolved here. One of typical multi-thread model is that the all threads share the same vma and every
>>thread has 8-k stack.
>>
>>>If 2 threads trigger uprobe (although might be not the same uprobe) at the same time, one thread might erase single-step instruction
>>of another.
>>
>>Do these threads share the same stack pages?
[YM] No. They share the same vma. And copy_insm_onstack might happen to fail on the 2 threads at the same time.