* [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS
2015-11-17 8:09 stp: rt: replace spin_lock with stp style lock and use yzhu1
@ 2015-11-17 8:09 ` yzhu1
2015-11-17 16:45 ` David Smith
0 siblings, 1 reply; 6+ messages in thread
From: yzhu1 @ 2015-11-17 8:09 UTC (permalink / raw)
To: sshukla, systemtap
From: Zhu Yanjun <yanjun.zhu@windriver.com>
-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
^ permalink raw reply [flat|nested] 6+ messages in thread
* stp: rt: replace spin_lock with stp style lock and use
@ 2015-11-17 8:09 yzhu1
2015-11-17 8:09 ` [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS yzhu1
0 siblings, 1 reply; 6+ messages in thread
From: yzhu1 @ 2015-11-17 8:09 UTC (permalink / raw)
To: sshukla, systemtap
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS
2015-11-17 8:09 ` [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS yzhu1
@ 2015-11-17 16:45 ` David Smith
2015-11-18 8:14 ` yzhu1
0 siblings, 1 reply; 6+ messages in thread
From: David Smith @ 2015-11-17 16:45 UTC (permalink / raw)
To: yzhu1, sshukla, systemtap
On 11/17/2015 02:09 AM, yzhu1 wrote:
> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>
> -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.
I have the same comment I had before. The raw lock changes are fine, but
I still don't believe the STP_ALLOC_SLEEP_FLAGS change is correct, at
least not without some further explanation. In the call trace below,
STP_ALLOC_SLEEP_FLAGS wasn't used from what I can tell.
Did you get a chance to try running systemtap with your raw lock changes
and the added might_sleep() call patch I sent you?
> 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
--
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS
2015-11-17 16:45 ` David Smith
@ 2015-11-18 8:14 ` yzhu1
2015-11-18 17:14 ` David Smith
0 siblings, 1 reply; 6+ messages in thread
From: yzhu1 @ 2015-11-18 8:14 UTC (permalink / raw)
To: David Smith, sshukla, systemtap
Hi, David
Thanks for your comments.
I can not reproduce this problem. And my user can reproduce this problem
ocassionly.
And I requested him to help me to make tests about your changes. But he
refused.
So I made auto tests in systemtap after this patch is applied. And I can
not find any
regressions about this patch.
Best Regards!
Zhu Yanjun
On 11/18/2015 12:45 AM, David Smith wrote:
> On 11/17/2015 02:09 AM, yzhu1 wrote:
>> From: Zhu Yanjun <yanjun.zhu@windriver.com>
>>
>> -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.
> I have the same comment I had before. The raw lock changes are fine, but
> I still don't believe the STP_ALLOC_SLEEP_FLAGS change is correct, at
> least not without some further explanation. In the call trace below,
> STP_ALLOC_SLEEP_FLAGS wasn't used from what I can tell.
>
> Did you get a chance to try running systemtap with your raw lock changes
> and the added might_sleep() call patch I sent you?
>
>> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS
2015-11-18 8:14 ` yzhu1
@ 2015-11-18 17:14 ` David Smith
2015-11-19 7:53 ` yzhu1
0 siblings, 1 reply; 6+ messages in thread
From: David Smith @ 2015-11-18 17:14 UTC (permalink / raw)
To: yzhu1, sshukla, systemtap
On 11/18/2015 02:14 AM, yzhu1 wrote:
> Hi, David
>
> Thanks for your comments.
>
> I can not reproduce this problem. And my user can reproduce this problem
> ocassionly.
> And I requested him to help me to make tests about your changes. But he
> refused.
>
> So I made auto tests in systemtap after this patch is applied. And I can
> not find any
> regressions about this patch.
Most of the time you won't see any difference when using STP_ALLOC_FLAGS
in place of STP_ALLOC_SLEEP_FLAGS. You would see a difference if your
system is under memory pressure.
Let me refresh everyone's knowledge of the difference between
STP_ALLOC_FLAGS and STP_ALLOC_SLEEP_FLAGS. When using STP_ALLOC_FLAGS,
if memory isn't available, the allocation will fail. When using
STP_ALLOC_SLEEP_FLAGS, the kernel will try to wait until memory is
available. Obviously, we can only use STP_ALLOC_SLEEP_FLAGS in certain
places in the code where sleeping is permitted.
If changing STP_ALLOC_SLEEP_FLAGS to STP_ALLOC_FLAGS makes a difference,
this really means that we're using STP_ALLOC_SLEEP_FLAGS in a place
where it isn't safe to sleep.
It could be that in the normal kernel it is safe to sleep in a spot
where we're using STP_ALLOC_SLEEP_FLAGS, but in the realtime kernel it
isn't safe to sleep at that same spot. I guess it also makes sense to
sleep as little as possible when running systemtap on the realtime kernel.
So, I'll withdraw my objection. As long as the switch from
STP_ALLOC_SLEEP_FLAGS to STP_ALLOC_FLAGS only affects the realtime
kernel, I'm OK with it.
--
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS
2015-11-18 17:14 ` David Smith
@ 2015-11-19 7:53 ` yzhu1
0 siblings, 0 replies; 6+ messages in thread
From: yzhu1 @ 2015-11-19 7:53 UTC (permalink / raw)
To: David Smith, sshukla, systemtap
On 11/19/2015 01:14 AM, David Smith wrote:
> On 11/18/2015 02:14 AM, yzhu1 wrote:
>> Hi, David
>>
>> Thanks for your comments.
>>
>> I can not reproduce this problem. And my user can reproduce this problem
>> ocassionly.
>> And I requested him to help me to make tests about your changes. But he
>> refused.
>>
>> So I made auto tests in systemtap after this patch is applied. And I can
>> not find any
>> regressions about this patch.
Hi, David
Thanks for your comments.
Zhu Yanjun
> Most of the time you won't see any difference when using STP_ALLOC_FLAGS
> in place of STP_ALLOC_SLEEP_FLAGS. You would see a difference if your
> system is under memory pressure.
>
> Let me refresh everyone's knowledge of the difference between
> STP_ALLOC_FLAGS and STP_ALLOC_SLEEP_FLAGS. When using STP_ALLOC_FLAGS,
> if memory isn't available, the allocation will fail. When using
> STP_ALLOC_SLEEP_FLAGS, the kernel will try to wait until memory is
> available. Obviously, we can only use STP_ALLOC_SLEEP_FLAGS in certain
> places in the code where sleeping is permitted.
>
> If changing STP_ALLOC_SLEEP_FLAGS to STP_ALLOC_FLAGS makes a difference,
> this really means that we're using STP_ALLOC_SLEEP_FLAGS in a place
> where it isn't safe to sleep.
>
> It could be that in the normal kernel it is safe to sleep in a spot
> where we're using STP_ALLOC_SLEEP_FLAGS, but in the realtime kernel it
> isn't safe to sleep at that same spot. I guess it also makes sense to
> sleep as little as possible when running systemtap on the realtime kernel.
>
> So, I'll withdraw my objection. As long as the switch from
> STP_ALLOC_SLEEP_FLAGS to STP_ALLOC_FLAGS only affects the realtime
> kernel, I'm OK with it.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-19 7:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-17 8:09 stp: rt: replace spin_lock with stp style lock and use yzhu1
2015-11-17 8:09 ` [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS yzhu1
2015-11-17 16:45 ` David Smith
2015-11-18 8:14 ` yzhu1
2015-11-18 17:14 ` David Smith
2015-11-19 7:53 ` yzhu1
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).