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

[yzhu1 <Yanjun.Zhu@windriver.com>]

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.

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;
>>>>
>>>
>
>