This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS
- From: Santosh Shukla <sshukla at mvista dot com>
- To: yzhu1 <Yanjun dot Zhu at windriver dot com>
- Cc: systemtap at sourceware dot org
- Date: Mon, 26 Oct 2015 14:13:27 +0530
- Subject: Re: [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS
- Authentication-results: sourceware.org; auth=none
- References: <1445499965-23777-1-git-send-email-yanjun dot zhu at windriver dot com> <562895D7 dot 7000505 at windriver dot com> <CAAyOgsZjHnMmrD=0n9r8pmD3K8T2cJ5Ufy5NFWbZHRfm9QZifg at mail dot gmail dot com> <562DD054 dot 1000604 at windriver dot com> <562DE378 dot 90701 at windriver dot com>
On Mon, Oct 26, 2015 at 1:55 PM, yzhu1 <Yanjun.Zhu@windriver.com> wrote:
> On 10/26/2015 03:03 PM, yzhu1 wrote:
>>
>> Hi, Santosh
>>
>> Thanks for your review.
>>
>> Do you mean that I do systemtap's autotest / regression test?
>>
>> If so, would you like to let me know how to make
>> systemtap's autotest / regression test ? I am glad to make such tests.
>
> Hi, Santosh
>
> Sorry, I mean that the backtrace will not appear and the whole systemtap can
> work well after this patch is applied to systemtap.
> That is, the whole systemtap can pass our autotest / regression tests.
>
> The tester and I can confirm the above.
>
> And I do not know systemtap's autotest / regression tests in systemtap
> software. If the systemtap's autotest / regression tests exists in systemtap
> software,
> please let me know how to make this systemtap's autotest / regression test.
>
Refer this [1], I guess it runs all the existing testsuites and make
sure your patches not breaking other modules etc..
[1] https://sourceware.org/systemtap/wiki/TestSuites
>
> Thanks a lot.
> Zhu Yanjun
>
>>
>> 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;
>>>>>
>>>>
>>
>>
>