This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: Review patches of user space kprobe
- From: Prasanna S Panchamukhi <prasanna at in dot ibm dot com>
- To: "Zhang, Yanmin" <yanmin dot zhang at intel dot com>
- Cc: systemtap at sources dot redhat dot com, "Keshavamurthy, Anil S" <anil dot s dot keshavamurthy at intel dot com>, "Mao, Bibo" <bibo dot mao at intel dot com>
- Date: Thu, 5 Jan 2006 16:39:57 +0530
- Subject: Re: Review patches of user space kprobe
- References: <8126E4F969BA254AB43EA03C59F44E840447BD45@pdsmsx404>
- Reply-to: prasanna at in dot ibm dot com
> >>+ struct page *page;
> >>+ struct uprobe_module *m;
> >>+ struct uprobe *up = NULL;
> >>+ struct hlist_node *node;
> >>+
> >>+ m = get_module_by_inode(file->f_dentry->d_inode);
> There is a race condition between unregister_userspace_probe and here.
> If a thread jumps to the beginning of function up_readpages while
> another thread calls unregister_userspace_probe to delete the um, the
> first thread might return error.
Some locking/semaphore can be used to avoid this.
> >>+ lock_page(up->page);
> The first patch doesn't do lock_page before calling insert_probe_page.
> Why does this patch do so? It's inconsistent.
>
Next patch release will take care of this.
> >>+ int kprobe_page_mapped = 0;
> >>+ struct hlist_node *node;
> >>+
> >>+ m = get_module_by_inode(file->f_dentry->d_inode);
> The same race condition like above function.
>
Some locking/semaphore can be used to avoid this.
> PAGE_CACHE_SHIFT,
> >>+ page->index <<
> PAGE_CACHE_SHIFT)) {
> >>+ up->page = page;
> >>+ if (!map_uprobe_page(up,
> kprobe_page_mapped)) {
> >>+ lock_page(up->page);
> Same inconsistent issue.
>
>
> >>+ kprobe_page_mapped = 1;
> >>+ retval = insert_probe_page(up);
> >>+ }
> >>+ }
> >>+ }
> >>+ if (kprobe_page_mapped) {
> The logic here is incorrect. If there are many uprobes at the same page,
> up just points to the last one. How about others?
>
readpage() routine reads one page at a time. we map the page one time and walk the
probes list for this inode, insert all the probes within this page and then unmap it.
Thanks
Prasanna
--
Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Email: prasanna@in.ibm.com
Ph: 91-80-25044636