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]

Re: [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS


Hi, Shukla

I run systemtap's autotest. I think this patch can pass it.
So I send this patch to you.

Best Regards!
Zhu Yanjun

On 10/23/2015 12:04 PM, Santosh Shukla wrote:
Looks Ok to me.

But I like to know systemtap's autotest / regression test is fine or
not? Pl. run that regression test and let us know the feedback.

If regression passes then

Reviewed-by : Santosh Shukla <sshukla@mvista.com>

Next time pl. add me in to list incase you want me to review -rt aware
stap patches.

Thanks.

On Thu, Oct 22, 2015 at 1:22 PM, yzhu1 <Yanjun.Zhu@windriver.com> wrote:
Hi, Shukla

I am a beginner. To fix bug, I made this patch. Please comment it.

Thanks a lot.
Zhu Yanjun

On 10/22/2015 03:46 PM, Zhu Yanjun wrote:
-rt mode spin lock lead to __might_sleep calltrace.
Replacing spin lock with stp type raw lock and
changing STP_ALLOC_SLEEP_FLAGS to STP_ALLOC_FLAGS solves the problem.

Call Trace:
   [<ffffffff818d83f1>] dump_stack+0x19/0x1b
   [<ffffffff81070e3f>] __might_sleep+0xef/0x160
   [<ffffffff818de710>] rt_spin_lock+0x20/0x50
   [<ffffffff81178699>] d_path+0x79/0x1a0
   [<ffffffffa0047be9>] __stp_get_mm_path.constprop.79+0x49/0x90
[stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073]
   [<ffffffffa004d01b>] __stp_utrace_attach_match_tsk.isra.53+0x7b/0x1b0
[stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073]
   [<ffffffffa004d18c>] __stp_utrace_task_finder_report_exec+0x3c/0x50
[stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073]
   [<ffffffffa0047b59>] utrace_report_exec+0xb9/0x100
[stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073]
   [<ffffffff811674b2>] search_binary_handler+0x332/0x380
   [<ffffffff81168d0c>] do_execve_common.isra.24+0x55c/0x640
   [<ffffffff81168e08>] do_execve+0x18/0x20
   [<ffffffff81169082>] SyS_execve+0x32/0x50
   [<ffffffff818e6979>] stub_execve+0x69/0xa0

Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
---
   runtime/linux/alloc.c |   27 ++++++++++++++++-----------
   1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/runtime/linux/alloc.c b/runtime/linux/alloc.c
index 7e7e5ce..914cf2b 100644
--- a/runtime/linux/alloc.c
+++ b/runtime/linux/alloc.c
@@ -12,6 +12,7 @@
   #define _STAPLINUX_ALLOC_C_
     #include <linux/percpu.h>
+#include "stp_helper_lock.h"
     static int _stp_allocated_net_memory = 0;
   /* Default, and should be "safe" from anywhere. */
@@ -19,7 +20,11 @@ static int _stp_allocated_net_memory = 0;
                          & ~__GFP_WAIT)
   /* May only be used in context that can sleep. __GFP_NORETRY is to
      suppress the oom-killer from kicking in. */
+#ifndef CONFIG_PREEMPT_RT_FULL
   #define STP_ALLOC_SLEEP_FLAGS (GFP_KERNEL | __GFP_NORETRY)
+#else
+#define STP_ALLOC_SLEEP_FLAGS STP_ALLOC_FLAGS
+#endif
     /* #define DEBUG_MEM */
   /*
@@ -40,7 +45,7 @@ static int _stp_allocated_net_memory = 0;
   static int _stp_allocated_memory = 0;
     #ifdef DEBUG_MEM
-static DEFINE_SPINLOCK(_stp_mem_lock);
+static STP_DEFINE_SPINLOCK(_stp_mem_lock);
     #define MEM_MAGIC 0xc11cf77f
   #define MEM_FENCE_SIZE 32
@@ -108,9 +113,9 @@ static void *_stp_mem_debug_setup(void *addr, size_t
size, enum _stp_memtype typ
         m->type = type;
         m->len = size;
         m->addr = addr;
-       spin_lock(&_stp_mem_lock);
+       stp_spin_lock(&_stp_mem_lock);
         list_add(p, &_stp_mem_list);
-       spin_unlock(&_stp_mem_lock);
+       stp_spin_unlock(&_stp_mem_lock);
         return addr;
   }
   @@ -122,9 +127,9 @@ static void _stp_mem_debug_percpu(struct
_stp_mem_entry *m, void *addr, size_t s
         m->type = MEM_PERCPU;
         m->len = size;
         m->addr = addr;
-       spin_lock(&_stp_mem_lock);
+       stp_spin_lock(&_stp_mem_lock);
         list_add(p, &_stp_mem_list);
-       spin_unlock(&_stp_mem_lock);
+       stp_spin_unlock(&_stp_mem_lock);
   }
     static void _stp_mem_debug_free(void *addr, enum _stp_memtype type)
@@ -133,7 +138,7 @@ static void _stp_mem_debug_free(void *addr, enum
_stp_memtype type)
         struct list_head *p, *tmp;
         struct _stp_mem_entry *m = NULL;
   -     spin_lock(&_stp_mem_lock);
+       stp_spin_lock(&_stp_mem_lock);
         list_for_each_safe(p, tmp, &_stp_mem_list) {
                 m = list_entry(p, struct _stp_mem_entry, list);
                 if (m->addr == addr) {
@@ -142,7 +147,7 @@ static void _stp_mem_debug_free(void *addr, enum
_stp_memtype type)
                         break;
                 }
         }
-       spin_unlock(&_stp_mem_lock);
+       stp_spin_unlock(&_stp_mem_lock);
         if (!found) {
                 printk("SYSTEMTAP ERROR: Free of unallocated memory %p
type=%s\n",
                        addr, _stp_malloc_types[type].free);
@@ -184,7 +189,7 @@ static void _stp_mem_debug_validate(void *addr)
         struct list_head *p, *tmp;
         struct _stp_mem_entry *m = NULL;
   -     spin_lock(&_stp_mem_lock);
+       stp_spin_lock(&_stp_mem_lock);
         list_for_each_safe(p, tmp, &_stp_mem_list) {
                 m = list_entry(p, struct _stp_mem_entry, list);
                 if (m->addr == addr) {
@@ -192,7 +197,7 @@ static void _stp_mem_debug_validate(void *addr)
                         break;
                 }
         }
-       spin_unlock(&_stp_mem_lock);
+       stp_spin_unlock(&_stp_mem_lock);
         if (!found) {
                 printk("SYSTEMTAP ERROR: Couldn't validate memory %p\n",
                        addr);
@@ -551,7 +556,7 @@ static void _stp_mem_debug_done(void)
         struct list_head *p, *tmp;
         struct _stp_mem_entry *m;
   -     spin_lock(&_stp_mem_lock);
+       stp_spin_lock(&_stp_mem_lock);
         list_for_each_safe(p, tmp, &_stp_mem_list) {
                 m = list_entry(p, struct _stp_mem_entry, list);
                 list_del(p);
@@ -583,7 +588,7 @@ static void _stp_mem_debug_done(void)
                 }
         }
   done:
-       spin_unlock(&_stp_mem_lock);
+       stp_spin_unlock(&_stp_mem_lock);
         return;



>From b75fb2f91b8daa18c5927b1254a6b11de133203e Mon Sep 17 00:00:00 2001
From: Zhu Yanjun <yanjun.zhu@windriver.com>
Date: Thu, 22 Oct 2015 15:41:57 +0800
Subject: [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use
 STP_ALLOC_FLAGS

-rt mode spin lock lead to __might_sleep calltrace.
Replacing spin lock with stp type raw lock and
changing STP_ALLOC_SLEEP_FLAGS to STP_ALLOC_FLAGS solves the problem.

Call Trace:
 [<ffffffff818d83f1>] dump_stack+0x19/0x1b
 [<ffffffff81070e3f>] __might_sleep+0xef/0x160
 [<ffffffff818de710>] rt_spin_lock+0x20/0x50
 [<ffffffff81178699>] d_path+0x79/0x1a0
 [<ffffffffa0047be9>] __stp_get_mm_path.constprop.79+0x49/0x90 [stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073]
 [<ffffffffa004d01b>] __stp_utrace_attach_match_tsk.isra.53+0x7b/0x1b0 [stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073]
 [<ffffffffa004d18c>] __stp_utrace_task_finder_report_exec+0x3c/0x50 [stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073]
 [<ffffffffa0047b59>] utrace_report_exec+0xb9/0x100 [stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073]
 [<ffffffff811674b2>] search_binary_handler+0x332/0x380
 [<ffffffff81168d0c>] do_execve_common.isra.24+0x55c/0x640
 [<ffffffff81168e08>] do_execve+0x18/0x20
 [<ffffffff81169082>] SyS_execve+0x32/0x50
 [<ffffffff818e6979>] stub_execve+0x69/0xa0

Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com>
Reviewed-by : Santosh Shukla <sshukla@mvista.com>
---
 runtime/linux/alloc.c |   27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/runtime/linux/alloc.c b/runtime/linux/alloc.c
index 7e7e5ce..914cf2b 100644
--- a/runtime/linux/alloc.c
+++ b/runtime/linux/alloc.c
@@ -12,6 +12,7 @@
 #define _STAPLINUX_ALLOC_C_
 
 #include <linux/percpu.h>
+#include "stp_helper_lock.h"
 
 static int _stp_allocated_net_memory = 0;
 /* Default, and should be "safe" from anywhere. */
@@ -19,7 +20,11 @@ static int _stp_allocated_net_memory = 0;
 			 & ~__GFP_WAIT)
 /* May only be used in context that can sleep. __GFP_NORETRY is to
    suppress the oom-killer from kicking in. */
+#ifndef CONFIG_PREEMPT_RT_FULL
 #define STP_ALLOC_SLEEP_FLAGS (GFP_KERNEL | __GFP_NORETRY)
+#else
+#define STP_ALLOC_SLEEP_FLAGS STP_ALLOC_FLAGS
+#endif
 
 /* #define DEBUG_MEM */
 /*
@@ -40,7 +45,7 @@ static int _stp_allocated_net_memory = 0;
 static int _stp_allocated_memory = 0;
 
 #ifdef DEBUG_MEM
-static DEFINE_SPINLOCK(_stp_mem_lock);
+static STP_DEFINE_SPINLOCK(_stp_mem_lock);
 
 #define MEM_MAGIC 0xc11cf77f
 #define MEM_FENCE_SIZE 32
@@ -108,9 +113,9 @@ static void *_stp_mem_debug_setup(void *addr, size_t size, enum _stp_memtype typ
 	m->type = type;
 	m->len = size;
 	m->addr = addr;
-	spin_lock(&_stp_mem_lock);
+	stp_spin_lock(&_stp_mem_lock);
 	list_add(p, &_stp_mem_list); 
-	spin_unlock(&_stp_mem_lock);
+	stp_spin_unlock(&_stp_mem_lock);
 	return addr;
 }
 
@@ -122,9 +127,9 @@ static void _stp_mem_debug_percpu(struct _stp_mem_entry *m, void *addr, size_t s
 	m->type = MEM_PERCPU;
 	m->len = size;
 	m->addr = addr;
-	spin_lock(&_stp_mem_lock);
+	stp_spin_lock(&_stp_mem_lock);
 	list_add(p, &_stp_mem_list);
-	spin_unlock(&_stp_mem_lock);	
+	stp_spin_unlock(&_stp_mem_lock);	
 }
 
 static void _stp_mem_debug_free(void *addr, enum _stp_memtype type)
@@ -133,7 +138,7 @@ static void _stp_mem_debug_free(void *addr, enum _stp_memtype type)
 	struct list_head *p, *tmp;
 	struct _stp_mem_entry *m = NULL;
 
-	spin_lock(&_stp_mem_lock);
+	stp_spin_lock(&_stp_mem_lock);
 	list_for_each_safe(p, tmp, &_stp_mem_list) {
 		m = list_entry(p, struct _stp_mem_entry, list);
 		if (m->addr == addr) {
@@ -142,7 +147,7 @@ static void _stp_mem_debug_free(void *addr, enum _stp_memtype type)
 			break;
 		}
 	}
-	spin_unlock(&_stp_mem_lock);
+	stp_spin_unlock(&_stp_mem_lock);
 	if (!found) {
 		printk("SYSTEMTAP ERROR: Free of unallocated memory %p type=%s\n", 
 		       addr, _stp_malloc_types[type].free);
@@ -184,7 +189,7 @@ static void _stp_mem_debug_validate(void *addr)
 	struct list_head *p, *tmp;
 	struct _stp_mem_entry *m = NULL;
 
-	spin_lock(&_stp_mem_lock);
+	stp_spin_lock(&_stp_mem_lock);
 	list_for_each_safe(p, tmp, &_stp_mem_list) {
 		m = list_entry(p, struct _stp_mem_entry, list);
 		if (m->addr == addr) {
@@ -192,7 +197,7 @@ static void _stp_mem_debug_validate(void *addr)
 			break;
 		}
 	}
-	spin_unlock(&_stp_mem_lock);
+	stp_spin_unlock(&_stp_mem_lock);
 	if (!found) {
 		printk("SYSTEMTAP ERROR: Couldn't validate memory %p\n", 
 		       addr);
@@ -551,7 +556,7 @@ static void _stp_mem_debug_done(void)
 	struct list_head *p, *tmp;
 	struct _stp_mem_entry *m;
 
-	spin_lock(&_stp_mem_lock);
+	stp_spin_lock(&_stp_mem_lock);
 	list_for_each_safe(p, tmp, &_stp_mem_list) {
 		m = list_entry(p, struct _stp_mem_entry, list);
 		list_del(p);
@@ -583,7 +588,7 @@ static void _stp_mem_debug_done(void)
 		}
 	}
 done:
-	spin_unlock(&_stp_mem_lock);
+	stp_spin_unlock(&_stp_mem_lock);
 
 	return;
 
-- 
1.7.9.5


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