RE: Patch [2/3] Userspace probes readpage hooks

RE: Patch [2/3] Userspace probes readpage hooks

[Zhang, Yanmin]

>>-----Original Message-----
>>From: systemtap-owner@sourceware.org [mailto:systemtap-owner@sourceware.org] On Behalf Of Prasanna S Panchamukhi
>>Sent: 2006年1月19日 22:40
>>To: systemtap@sources.redhat.com
>>Subject: Re: Patch [2/3] Userspace probes readpage hooks
>>
>>
>>This patch provides the feature to insert the probes on the pages
>>that are not present in the memory during registeration.
>>
>>
>>diff -puN kernel/kprobes.c~kprobes_userspace_probes_hook_readpage kernel/kprobes.c
>>--- linux-2.6.15/kernel/kprobes.c~kprobes_userspace_probes_hook_readpage	2006-01-19 19:37:48.000000000 +0530
>>+++ linux-2.6.15-prasanna/kernel/kprobes.c	2006-01-19 19:37:49.000000000 +0530
>>@@ -815,6 +815,133 @@ static struct uprobe_module __kprobes *g
>> 	return NULL;
>> }
>>
>>+static inline void insert_readpage_uprobe(struct page *page,
>>+	struct address_space *mapping, struct uprobe *uprobe)
>>+{
>>+	struct vm_area_struct *vma = NULL;
>>+
>>+	if (find_page_probe(uprobe->offset >> PAGE_CACHE_SHIFT,
>>+				page->index << PAGE_CACHE_SHIFT)) {
>>+		spin_lock(&mapping->i_mmap_lock);
>>+		vma = find_get_vma(uprobe, page, mapping);
>>+		map_uprobe_page(page, vma, uprobe, insert_kprobe_user);
>>+
>>+		flush_vma(mapping, page, uprobe);
>>+		spin_unlock(&mapping->i_mmap_lock);
>>+	}
>>+}
>>+
>>+/**
>>+ *  This function hooks the readpages() of all modules that have active probes
>>+ *  on them. The original readpages() is called for the given
>>+ *  inode/address_space to actually read the pages into the memory. Then all
>>+ *  probes that are specified on these pages are inserted.
>>+ */
>>+static int __kprobes uprobe_readpages(struct file *file,
>>+				struct address_space *mapping,
>>+				struct list_head *pages, unsigned nr_pages)
>>+{
>>+	int retval = 0;
>>+	struct page *page;
>>+	struct uprobe_module *umodule;
>>+	struct uprobe *uprobe = NULL;
>>+	struct hlist_node *node;
>>+
>>+	mutex_lock(&uprobe_mutex);
>>+
>>+	umodule = get_module_by_inode(file->f_dentry->d_inode);
>>+	if (!umodule) {
[YM] The race condition is still not resolved. 


>>+		printk("uprobe_readpages: we don't have a module \
>>+				associated with this file.. Aborting\n");
>>+		mutex_unlock(&uprobe_mutex);
>>+		return -EINVAL;
>>+	}
>>+
>>+	/* call original readpages() */
>>+	retval = umodule->ori_a_ops->readpages(file, mapping, pages, nr_pages);
>>+	if (retval < 0)
>>+		goto out;
>>+
>>+	/*
>>+	 * TODO: Walk through readpages page list and get
>>+	 * pages with probes instead of find_get_page().
>>+	 */
>>+	mutex_lock(&kprobe_mutex);
>>+	hlist_for_each_entry(uprobe, node, &umodule->ulist_head, ulist) {
>>+		page = find_get_page(mapping,
>>+				uprobe->offset >> PAGE_CACHE_SHIFT);
>>+		if (!page)
>>+			continue;
It's incorrect. Some pages might be in memory and the corresponding uprobe are already inserted on the pages, so the same uprobe might be inserted more once on the same page. You could just go through pages read in by this call.

>>+
>>+		insert_readpage_uprobe(page, mapping, uprobe);
>>+		page_cache_release(page);
>>+	}
>>+	mutex_unlock(&kprobe_mutex);
>>+
>>+out:
>>+	mutex_unlock(&uprobe_mutex);
>>+	return retval;
>>+}
>>+
>>+/**
>>+ *  This function hooks the readpage() of all modules that have active probes
>>+ *  on them. The original readpage() is called for the given inode/address_space
>>+ *  to actually read the pages into the memory. Then all probes that are
>>+ *  specified on this page are inserted.
>>+ */
>>+int __kprobes uprobe_readpage(struct file *file, struct page *page)
>>+{
>>+	int retval = 0;
>>+	struct uprobe_module *umodule;
>>+	struct uprobe *uprobe = NULL;
>>+	struct hlist_node *node;
>>+	struct address_space *mapping = file->f_dentry->d_inode->i_mapping;
>>+
>>+	mutex_lock(&uprobe_mutex);
>>+	umodule = get_module_by_inode(file->f_dentry->d_inode);
>>+	if (!umodule) {
[YM] Same race is not resolved.


>>+		printk("uprobe_readpages: we don't have a module \
>>+				associated with this file.. Aborting\n");
>>+		mutex_unlock(&uprobe_mutex);
>>+		return -EINVAL;
>>+	}
>>+
>>+	/* call original readpage() */
>>+	retval = umodule->ori_a_ops->readpage(file, page);
>>+	if (retval < 0)
>>+		goto out;
>>+
>>+	mutex_lock(&kprobe_mutex);
>>+	hlist_for_each_entry(uprobe, node, &umodule->ulist_head, ulist)
>>+		insert_readpage_uprobe(page, mapping, uprobe);
Do all uprobe of the uprobe_module have to be inserted to the page?


>>+	mutex_unlock(&kprobe_mutex);
>>+
>>+out:
>>+	mutex_unlock(&uprobe_mutex);
>>+
>>+	return retval;
>>+}
>>+
>>+/**
>>+ * Gets exclusive write access to the given inode to ensure that the file
>>+ * on which probes are currently applied does not change. Use the function,
>>+ * deny_write_access_to_inode() we added in fs/namei.c.
>>+ */

Re: Patch [2/3] Userspace probes readpage hooks

[Prasanna S Panchamukhi]

Yanmin,

> >>+ */
> >>+static int __kprobes uprobe_readpages(struct file *file,
> >>+				struct address_space *mapping,
> >>+				struct list_head *pages, unsigned nr_pages)
> >>+{
> >>+	int retval = 0;
> >>+	struct page *page;
> >>+	struct uprobe_module *umodule;
> >>+	struct uprobe *uprobe = NULL;
> >>+	struct hlist_node *node;
> >>+
> >>+	mutex_lock(&uprobe_mutex);
> >>+
> >>+	umodule = get_module_by_inode(file->f_dentry->d_inode);
> >>+	if (!umodule) {
> [YM] The race condition is still not resolved. 

Could you please elaborate the race condition?
> >>+		if (!page)
> >>+			continue;
> It's incorrect. Some pages might be in memory and the corresponding uprobe are already inserted on the pages, so the same uprobe might be inserted more once on the same page. You could just go through pages read in by this call.
> 

yes, may be we can check for valid opcode and if there is a valid opcode
then the probe is already inserted and no need to insert it again.
> >>+		goto out;
> >>+
> >>+	mutex_lock(&kprobe_mutex);
> >>+	hlist_for_each_entry(uprobe, node, &umodule->ulist_head, ulist)
> >>+		insert_readpage_uprobe(page, mapping, uprobe);
> Do all uprobe of the uprobe_module have to be inserted to the page?

yes, we need to check if there is a appropriate probe within page and
then insert them if they are not inserted. 


Thanks
Prasanna

-- 

Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Email: prasanna@in.ibm.com
Ph: 91-80-51776329

Re: Patch [2/3] Userspace probes readpage hooks

[Prasanna S Panchamukhi]

On Thu, Feb 02, 2006 at 03:26:29PM +0800, Zhang, Yanmin wrote:
> >>-----Original Message-----
> >>From: systemtap-owner@sourceware.org [mailto:systemtap-owner@sourceware.org] On Behalf Of Prasanna S Panchamukhi
> >>Sent: 2006年1月27日 21:13
> >>To: Zhang, Yanmin
> >>Cc: systemtap@sources.redhat.com; Keshavamurthy, Anil S; Mao, Bibo
> >>Subject: Re: Patch [2/3] Userspace probes readpage hooks
> >>
> >>Yanmin,
> >>
> >>> >>+ */
> >>> >>+static int __kprobes uprobe_readpages(struct file *file,
> >>> >>+				struct address_space *mapping,
> >>> >>+				struct list_head *pages, unsigned nr_pages)
> >>> >>+{
> >>> >>+	int retval = 0;
> >>> >>+	struct page *page;
> >>> >>+	struct uprobe_module *umodule;
> >>> >>+	struct uprobe *uprobe = NULL;
> >>> >>+	struct hlist_node *node;
> >>> >>+
> >>> >>+	mutex_lock(&uprobe_mutex);
> >>> >>+
> >>> >>+	umodule = get_module_by_inode(file->f_dentry->d_inode);
> >>> >>+	if (!umodule) {
> >>> [YM] The race condition is still not resolved.
> >>
> >>Could you please elaborate the race condition?
> [YM] Scenario: There are 2 threads. The 1st is calling uprobe_readpages (from readpages path), while the 2nd one is unregistering the uprobe and uprobe_module. So before the 1st one is executing the mutex_lock, the 2nd one might already remove the uprobe_module, then, uprobe_readpages returns failure. Actually, the 1st should reexecutes the original readpages proc instead of returning failure when it couldn't find right uprobe_module.

Yes, you are right about the race. Could you please tell us how to figure out
the readpage() routine, given that the uprobe_module no longer exists?

Thanks
Prasanna
-- 
Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Email: prasanna@in.ibm.com
Ph: 91-80-51776329

Re: Patch [2/3] Userspace probes readpage hooks

[Prasanna S Panchamukhi]

Yanmin,

> >>> >>+	mutex_lock(&uprobe_mutex);
> >>> >>+
> >>> >>+	umodule = get_module_by_inode(file->f_dentry->d_inode);
> >>> >>+	if (!umodule) {
> >>> [YM] The race condition is still not resolved.
> >>
> >>Could you please elaborate the race condition?
> [YM] Scenario: There are 2 threads. The 1st is calling uprobe_readpages (from readpages path), while the 2nd one is unregistering the uprobe and uprobe_module. So before the 1st one is executing the mutex_lock, the 2nd one might already remove the uprobe_module, then, uprobe_readpages returns failure. Actually, the 1st should reexecutes the original readpages proc instead of returning failure when it couldn't find right uprobe_module.

Yes, this will be taken care in next patch release.

Thanks
Prasanna
-- 
Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Email: prasanna@in.ibm.com
Ph: 91-80-51776329

RE: Patch [2/3] Userspace probes readpage hooks

[Zhang, Yanmin]

>>-----Original Message-----
>>From: systemtap-owner@sourceware.org [mailto:systemtap-owner@sourceware.org] On Behalf Of Prasanna S Panchamukhi
>>Sent: 2006年1月27日 21:13
>>To: Zhang, Yanmin
>>Cc: systemtap@sources.redhat.com; Keshavamurthy, Anil S; Mao, Bibo
>>Subject: Re: Patch [2/3] Userspace probes readpage hooks
>>
>>Yanmin,
>>
>>> >>+ */
>>> >>+static int __kprobes uprobe_readpages(struct file *file,
>>> >>+				struct address_space *mapping,
>>> >>+				struct list_head *pages, unsigned nr_pages)
>>> >>+{
>>> >>+	int retval = 0;
>>> >>+	struct page *page;
>>> >>+	struct uprobe_module *umodule;
>>> >>+	struct uprobe *uprobe = NULL;
>>> >>+	struct hlist_node *node;
>>> >>+
>>> >>+	mutex_lock(&uprobe_mutex);
>>> >>+
>>> >>+	umodule = get_module_by_inode(file->f_dentry->d_inode);
>>> >>+	if (!umodule) {
>>> [YM] The race condition is still not resolved.
>>
>>Could you please elaborate the race condition?
[YM] Scenario: There are 2 threads. The 1st is calling uprobe_readpages (from readpages path), while the 2nd one is unregistering the uprobe and uprobe_module. So before the 1st one is executing the mutex_lock, the 2nd one might already remove the uprobe_module, then, uprobe_readpages returns failure. Actually, the 1st should reexecutes the original readpages proc instead of returning failure when it couldn't find right uprobe_module.

Re: Patch [2/3] Userspace probes readpage hooks

[Prasanna S Panchamukhi]

This patch provides the feature to insert the probes on the pages
that are not present in the memory during registeration.

To add readpage and readpages() hooks, two new elements
are added to uprobe_module object.
struct address_space_operations *ori_a_ops;
struct address_space_operations user_a_ops;

User space probe also allows the probes to be inserted within the
pages even that are not present in the memory at the time of
registration. This is done by adding hooks to readpage and readpages
routines. During registration, the address space operation object is
modified by replacing with userspace probes's specific readpage and
readpages routines. So that when the pages are readinto the memory
through the readpage and readpages address space operations, the
probes can be automatically inserted into those pages. These user
space readpage and readpages routines internally call the original
readpage() and readpages() routines and then check if the probes are
to be added to these pages and then insert the probes on these pages.
Overhead of adding these hooks are limited to the application on
which probes are inserted.

While unregiteration, care should be taken to replace the readpage and
readpages() hooks by original routines if no probes exists on that
application.

Signed-of-by: Prasanna S Panchamukhi <prasanna@in.ibm.com>


 fs/namei.c            |   11 ++-
 include/linux/namei.h |    1 
 kernel/kprobes.c      |  141 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 150 insertions(+), 3 deletions(-)

diff -puN kernel/kprobes.c~kprobes_userspace_probes_hook_readpage kernel/kprobes.c
--- linux-2.6.15/kernel/kprobes.c~kprobes_userspace_probes_hook_readpage	2006-01-19 19:37:48.000000000 +0530
+++ linux-2.6.15-prasanna/kernel/kprobes.c	2006-01-19 19:37:49.000000000 +0530
@@ -815,6 +815,133 @@ static struct uprobe_module __kprobes *g
 	return NULL;
 }
 
+static inline void insert_readpage_uprobe(struct page *page,
+	struct address_space *mapping, struct uprobe *uprobe)
+{
+	struct vm_area_struct *vma = NULL;
+
+	if (find_page_probe(uprobe->offset >> PAGE_CACHE_SHIFT,
+				page->index << PAGE_CACHE_SHIFT)) {
+		spin_lock(&mapping->i_mmap_lock);
+		vma = find_get_vma(uprobe, page, mapping);
+		map_uprobe_page(page, vma, uprobe, insert_kprobe_user);
+
+		flush_vma(mapping, page, uprobe);
+		spin_unlock(&mapping->i_mmap_lock);
+	}
+}
+
+/**
+ *  This function hooks the readpages() of all modules that have active probes
+ *  on them. The original readpages() is called for the given
+ *  inode/address_space to actually read the pages into the memory. Then all
+ *  probes that are specified on these pages are inserted.
+ */
+static int __kprobes uprobe_readpages(struct file *file,
+				struct address_space *mapping,
+				struct list_head *pages, unsigned nr_pages)
+{
+	int retval = 0;
+	struct page *page;
+	struct uprobe_module *umodule;
+	struct uprobe *uprobe = NULL;
+	struct hlist_node *node;
+
+	mutex_lock(&uprobe_mutex);
+
+	umodule = get_module_by_inode(file->f_dentry->d_inode);
+	if (!umodule) {
+		printk("uprobe_readpages: we don't have a module \
+				associated with this file.. Aborting\n");
+		mutex_unlock(&uprobe_mutex);
+		return -EINVAL;
+	}
+
+	/* call original readpages() */
+	retval = umodule->ori_a_ops->readpages(file, mapping, pages, nr_pages);
+	if (retval < 0)
+		goto out;
+
+	/*
+	 * TODO: Walk through readpages page list and get
+	 * pages with probes instead of find_get_page().
+	 */
+	mutex_lock(&kprobe_mutex);
+	hlist_for_each_entry(uprobe, node, &umodule->ulist_head, ulist) {
+		page = find_get_page(mapping,
+				uprobe->offset >> PAGE_CACHE_SHIFT);
+		if (!page)
+			continue;
+
+		insert_readpage_uprobe(page, mapping, uprobe);
+		page_cache_release(page);
+	}
+	mutex_unlock(&kprobe_mutex);
+
+out:
+	mutex_unlock(&uprobe_mutex);
+	return retval;
+}
+
+/**
+ *  This function hooks the readpage() of all modules that have active probes
+ *  on them. The original readpage() is called for the given inode/address_space
+ *  to actually read the pages into the memory. Then all probes that are
+ *  specified on this page are inserted.
+ */
+int __kprobes uprobe_readpage(struct file *file, struct page *page)
+{
+	int retval = 0;
+	struct uprobe_module *umodule;
+	struct uprobe *uprobe = NULL;
+	struct hlist_node *node;
+	struct address_space *mapping = file->f_dentry->d_inode->i_mapping;
+
+	mutex_lock(&uprobe_mutex);
+	umodule = get_module_by_inode(file->f_dentry->d_inode);
+	if (!umodule) {
+		printk("uprobe_readpages: we don't have a module \
+				associated with this file.. Aborting\n");
+		mutex_unlock(&uprobe_mutex);
+		return -EINVAL;
+	}
+
+	/* call original readpage() */
+	retval = umodule->ori_a_ops->readpage(file, page);
+	if (retval < 0)
+		goto out;
+
+	mutex_lock(&kprobe_mutex);
+	hlist_for_each_entry(uprobe, node, &umodule->ulist_head, ulist)
+		insert_readpage_uprobe(page, mapping, uprobe);
+	mutex_unlock(&kprobe_mutex);
+
+out:
+	mutex_unlock(&uprobe_mutex);
+
+	return retval;
+}
+
+/**
+ * Gets exclusive write access to the given inode to ensure that the file
+ * on which probes are currently applied does not change. Use the function,
+ * deny_write_access_to_inode() we added in fs/namei.c.
+ */
+static inline int ex_write_lock(struct inode *inode)
+{
+	return deny_write_access_to_inode(inode);
+}
+
+/**
+ * Called when removing user space probes to release the write lock on the
+ * inode.
+ */
+static inline int ex_write_unlock(struct inode *inode)
+{
+	atomic_inc(&inode->i_writecount);
+	return 0;
+}
+
 /**
  * Get the inode operations. This function leaves with the dentry held
  * and taking with the inode writelock held to ensure that the file on
@@ -828,11 +955,21 @@ static int __kprobes get_inode_ops(struc
 	struct address_space *as;
 	int error = 0;
 
+	error = ex_write_lock(umodule->nd.dentry->d_inode);
+	if (error)
+		return error;
+
 	INIT_HLIST_HEAD(&umodule->ulist_head);
 	hlist_add_head(&uprobe->ulist, &umodule->ulist_head);
 	list_add(&umodule->mlist, &uprobe_module_list);
 	as = umodule->nd.dentry->d_inode->i_mapping;
 
+	umodule->ori_a_ops = as->a_ops;
+	umodule->user_a_ops = *as->a_ops;
+	umodule->user_a_ops.readpage = uprobe_readpage;
+	umodule->user_a_ops.readpages = uprobe_readpages;
+	as->a_ops = &umodule->user_a_ops;
+
 	return error;
 }
 
@@ -863,6 +1000,7 @@ int __kprobes remove_kprobe_user(struct 
 	int cleanup_p;
 
 	p = &uprobe->kp;
+	 /*TODO: change it to spin lock, we enter here holding i_mmap_lock */
 	mutex_lock(&kprobe_mutex);
 	old_p = get_kprobe_user(uprobe->inode, uprobe->offset);
 	if (unlikely(!old_p)) {
@@ -946,6 +1084,9 @@ void __kprobes unregister_uprobe(struct 
 	hlist_del(&uprobe->ulist);
 	if (hlist_empty(&umodule->ulist_head)) {
 		list_del(&umodule->mlist);
+		umodule->nd.dentry->d_inode->i_mapping->a_ops =
+							umodule->ori_a_ops;
+		ex_write_unlock(uprobe->inode);
 		path_release(&umodule->nd);
 		kfree(umodule);
 	}
diff -puN fs/namei.c~kprobes_userspace_probes_hook_readpage fs/namei.c
--- linux-2.6.15/fs/namei.c~kprobes_userspace_probes_hook_readpage	2006-01-19 19:37:49.000000000 +0530
+++ linux-2.6.15-prasanna/fs/namei.c	2006-01-19 19:37:49.000000000 +0530
@@ -322,10 +322,8 @@ int get_write_access(struct inode * inod
 	return 0;
 }
 
-int deny_write_access(struct file * file)
+int deny_write_access_to_inode(struct inode *inode)
 {
-	struct inode *inode = file->f_dentry->d_inode;
-
 	spin_lock(&inode->i_lock);
 	if (atomic_read(&inode->i_writecount) > 0) {
 		spin_unlock(&inode->i_lock);
@@ -337,6 +335,13 @@ int deny_write_access(struct file * file
 	return 0;
 }
 
+int deny_write_access(struct file * file)
+{
+	struct inode *inode = file->f_dentry->d_inode;
+
+	return deny_write_access_to_inode(inode);
+}
+
 void path_release(struct nameidata *nd)
 {
 	dput(nd->dentry);
diff -puN include/linux/namei.h~kprobes_userspace_probes_hook_readpage include/linux/namei.h
--- linux-2.6.15/include/linux/namei.h~kprobes_userspace_probes_hook_readpage	2006-01-19 19:37:49.000000000 +0530
+++ linux-2.6.15-prasanna/include/linux/namei.h	2006-01-19 19:37:49.000000000 +0530
@@ -82,6 +82,7 @@ extern int follow_up(struct vfsmount **,
 
 extern struct dentry *lock_rename(struct dentry *, struct dentry *);
 extern void unlock_rename(struct dentry *, struct dentry *);
+extern int deny_write_access_to_inode(struct inode *inode);
 
 static inline void nd_set_link(struct nameidata *nd, char *path)
 {

_
-- 
Thanks & Regards
Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Email: prasanna@in.ibm.com
Ph: 91-80-25044636