This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[SYSTEMTAP/PATCH v2 2/6] stp: rt: replace __stp_tf_map_lock rd/wr lock with stp style raw lock


noticed that in rt mode - read and write_lock lead to several bug_on :

[ 1033.544821]  ffff880035463d38 ffffffff815fcaed ffff880035463d90 ffffffff816057f7
[ 1033.544826]  ffff880035463fd8 0000000000017e80 0000000000017e80 ffffffff810be5fb
[ 1033.544826] Call Trace:
[ 1033.544830]  [<ffffffff81602329>] dump_stack+0x4e/0x7a
[ 1033.544834]  [<ffffffff815fcaed>] __schedule_bug+0x9f/0xad
[ 1033.544838]  [<ffffffff816057f7>] __schedule+0x627/0x6a0
[ 1033.544842]  [<ffffffff810be5fb>] ? task_blocks_on_rt_mutex+0x19b/0x220
[ 1033.544846]  [<ffffffff816058a0>] schedule+0x30/0xa0
[ 1033.544850]  [<ffffffff8160727d>] rt_spin_lock_slowlock+0xbd/0x1f0
[ 1033.544856]  [<ffffffff81607df5>] __rt_spin_lock+0x25/0x30
[ 1033.544858]  [<ffffffff816080b0>] rt_read_lock+0x30/0x40
[ 1033.544861]  [<ffffffff816080ce>] rt_read_lock_irqsave+0xe/0x20
[ 1033.544867]  [<ffffffffa08a9b89>] __stp_tf_get_map_entry+0x19/0xc0 [stap_e40dcb2c46d7c0a2fb8d70ba343e393a_15235]
[ 1033.544873]  [<ffffffffa08afedd>] __stp_utrace_task_finder_target_syscall_exit+0x5d/0x350 [stap_e40dcb2c46d7c0a2fb8d70ba343e393a_15235]
[ 1033.544889]  [<ffffffffa08a91c5>] utrace_report_syscall_exit+0xc5/0x110 [stap_e40dcb2c46d7c0a2fb8d70ba343e393a_15235]
[ 1033.544893]  [<ffffffff81023ce0>] syscall_trace_leave+0x100/0x130
[ 1033.544896]  [<ffffffff8161090b>] int_check_syscall_exit_work+0x34/0x3d

Replacing rd/wr lock with raw lock supress these bug_on. However more suitable
fix is to replace read/write lock in general with RCU style lock, followed by
use of rcu variant hlist api but current module desing need to go through some
real desing chnages in order of adding and freeing entry to/from free list,
which i choose work upon as separate patch[todo.]

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
 runtime/linux/task_finder_map.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/runtime/linux/task_finder_map.c b/runtime/linux/task_finder_map.c
index d515e9f..010460c 100644
--- a/runtime/linux/task_finder_map.c
+++ b/runtime/linux/task_finder_map.c
@@ -1,6 +1,7 @@
 #include <linux/list.h>
 #include <linux/jhash.h>
-#include <linux/spinlock.h>
+
+#include "stp_helper_lock.h"
 
 // When handling mmap()/munmap()/mprotect() syscall tracing to notice
 // memory map changes, we need to cache syscall entry parameter values
@@ -13,7 +14,7 @@
 // contents in interrupt context (which should only ever call 
 // stap_find_map_map_info for getting stored info). So we might
 // want to look into that if this seems a bottleneck.
-static DEFINE_RWLOCK(__stp_tf_map_lock);
+static STP_DEFINE_RWLOCK(__stp_tf_map_lock);
 
 #define __STP_TF_HASH_BITS 4
 #define __STP_TF_TABLE_SIZE (1 << __STP_TF_HASH_BITS)
@@ -51,11 +52,11 @@ __stp_tf_map_initialize(void)
 	struct hlist_head *head = &__stp_tf_map_free_list[0];
 
 	unsigned long flags;
-	write_lock_irqsave(&__stp_tf_map_lock, flags);
+	stp_write_lock_irqsave(&__stp_tf_map_lock, flags);
 	for (i = 0; i < TASK_FINDER_MAP_ENTRY_ITEMS; i++) {
 		hlist_add_head(&__stp_tf_map_free_list_items[i].hlist, head);
 	}
-	write_unlock_irqrestore(&__stp_tf_map_lock, flags);
+	stp_write_unlock_irqrestore(&__stp_tf_map_lock, flags);
 }
 
 
@@ -109,15 +110,15 @@ __stp_tf_get_map_entry(struct task_struct *tsk)
 	struct __stp_tf_map_entry *entry;
 
 	unsigned long flags;
-	read_lock_irqsave(&__stp_tf_map_lock, flags);
+	stp_read_lock_irqsave(&__stp_tf_map_lock, flags);
 	head = &__stp_tf_map_table[__stp_tf_map_hash(tsk)];
 	stap_hlist_for_each_entry(entry, node, head, hlist) {
 		if (tsk->pid == entry->pid) {
-			read_unlock_irqrestore(&__stp_tf_map_lock, flags);
+			stp_read_unlock_irqrestore(&__stp_tf_map_lock, flags);
 			return entry;
 		}
 	}
-	read_unlock_irqrestore(&__stp_tf_map_lock, flags);
+	stp_read_unlock_irqrestore(&__stp_tf_map_lock, flags);
 	return NULL;
 }
 
@@ -133,14 +134,14 @@ __stp_tf_add_map(struct task_struct *tsk, long syscall_no, unsigned long arg0,
 	struct __stp_tf_map_entry *entry;
 	unsigned long flags;
 
-	write_lock_irqsave(&__stp_tf_map_lock, flags);
+	stp_write_lock_irqsave(&__stp_tf_map_lock, flags);
 	head = &__stp_tf_map_table[__stp_tf_map_hash(tsk)];
 	stap_hlist_for_each_entry(entry, node, head, hlist) {
 		// If we find an existing entry, just increment the
 		// usage count.
 		if (tsk->pid == entry->pid) {
 			entry->usage++;
-			write_unlock_irqrestore(&__stp_tf_map_lock, flags);
+			stp_write_unlock_irqrestore(&__stp_tf_map_lock, flags);
 			return 0;
 		}
 	}
@@ -148,7 +149,7 @@ __stp_tf_add_map(struct task_struct *tsk, long syscall_no, unsigned long arg0,
 	// Get an element from the free list.
 	entry = __stp_tf_map_get_free_entry();
 	if (!entry) {
-		write_unlock_irqrestore(&__stp_tf_map_lock, flags);
+		stp_write_unlock_irqrestore(&__stp_tf_map_lock, flags);
 		return -ENOMEM;
 	}
 	entry->usage = 1;
@@ -158,7 +159,7 @@ __stp_tf_add_map(struct task_struct *tsk, long syscall_no, unsigned long arg0,
 	entry->arg1 = arg1;
 	entry->arg2 = arg2;
 	hlist_add_head(&entry->hlist, head);
-	write_unlock_irqrestore(&__stp_tf_map_lock, flags);
+	stp_write_unlock_irqrestore(&__stp_tf_map_lock, flags);
 	return 0;
 }
 
@@ -174,7 +175,7 @@ __stp_tf_remove_map_entry(struct __stp_tf_map_entry *entry)
 
 	if (entry != NULL) {
 		unsigned long flags;
-		write_lock_irqsave(&__stp_tf_map_lock, flags);
+		stp_write_lock_irqsave(&__stp_tf_map_lock, flags);
 
 		// Decrement the usage count.
 		entry->usage--;
@@ -185,7 +186,7 @@ __stp_tf_remove_map_entry(struct __stp_tf_map_entry *entry)
 			hlist_del(&entry->hlist);
 			__stp_tf_map_put_free_entry(entry);
 		}
-		write_unlock_irqrestore(&__stp_tf_map_lock, flags);
+		stp_write_unlock_irqrestore(&__stp_tf_map_lock, flags);
 	}
 	return 0;
 }
-- 
1.8.3.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]