From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 51276 invoked by alias); 17 Nov 2015 07:52:53 -0000 Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org Received: (qmail 51265 invoked by uid 89); 17 Nov 2015 07:52:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.1 required=5.0 tests=AWL,BAYES_99,BAYES_999,RCVD_IN_DNSWL_LOW,SPF_PASS,UNWANTED_LANGUAGE_BODY autolearn=no version=3.3.2 X-HELO: mail-pa0-f53.google.com Received: from mail-pa0-f53.google.com (HELO mail-pa0-f53.google.com) (209.85.220.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 17 Nov 2015 07:52:47 +0000 Received: by padhx2 with SMTP id hx2so1174069pad.1 for ; Mon, 16 Nov 2015 23:52:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=0wzEsdGMqWFjUNO2YDLwtDbL2gGK97z+UdPwQP8O8g4=; b=CsPT/orcJ51cy1XY8pcM4WTQCsdOrZWWBmF4TzDPs2Ho4ypg+FSzUdqZNfpHWM6eja kuOEmBIlK9UNCc5p7NsUor1xsLshcAINr1c4CNa7pueJTFKv87OXcfR9t7NFpc7IBsKF FAZjQFMl8rRJRwfgFHZnSSWCyPYL+6hKpIaFAeswfCevL3/zBU+SLqlh+zKsYL2ZeLRo 9un3No1+4MD6YauVlddk7FCMiTQZ1GF2coDE26zbDeqHOTlFgaYT1H2Xa0nA544flZC7 v8LqHL40YsViMlzhFY83gyILrLDE6osdkUcMT/T7YlqKOZFT7lxy0tzVVSGAk4/OwodC 9vvQ== X-Gm-Message-State: ALoCoQn+GWp2Nmj+ezwOH6NtXZFfI2LP7yKBBiT/9P2Q9s07XVKda112VZenVTIjvOBWwz2pi7NG MIME-Version: 1.0 X-Received: by 10.68.186.194 with SMTP id fm2mr60602297pbc.5.1447746765552; Mon, 16 Nov 2015 23:52:45 -0800 (PST) Received: by 10.66.13.233 with HTTP; Mon, 16 Nov 2015 23:52:45 -0800 (PST) In-Reply-To: <564AD964.6080008@windriver.com> References: <1445499965-23777-1-git-send-email-yanjun.zhu@windriver.com> <562895D7.7000505@windriver.com> <564AD964.6080008@windriver.com> Date: Tue, 17 Nov 2015 07:52:00 -0000 Message-ID: Subject: Re: [PATCH 1/1] stp: rt: replace spin_lock with stp style lock and use STP_ALLOC_FLAGS From: Santosh Shukla To: yzhu1 Cc: systemtap@sourceware.org Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-q4/txt/msg00138.txt.bz2 Pl. send patch via email tools like mutt or get-send-email. Attaching patch is not a good approach and then I will review. thanks. On Tue, Nov 17, 2015 at 1:08 PM, yzhu1 wrote: > 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 >> >> 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 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: >>>> [] dump_stack+0x19/0x1b >>>> [] __might_sleep+0xef/0x160 >>>> [] rt_spin_lock+0x20/0x50 >>>> [] d_path+0x79/0x1a0 >>>> [] __stp_get_mm_path.constprop.79+0x49/0x90 >>>> [stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073] >>>> [] __stp_utrace_attach_match_tsk.isra.53+0x7b/0x1b0 >>>> [stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073] >>>> [] __stp_utrace_task_finder_report_exec+0x3c/0x50 >>>> [stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073] >>>> [] utrace_report_exec+0xb9/0x100 >>>> [stap_f5bb3e3c9b162aab5a51afc2375fe9cf_2073] >>>> [] search_binary_handler+0x332/0x380 >>>> [] do_execve_common.isra.24+0x55c/0x640 >>>> [] do_execve+0x18/0x20 >>>> [] SyS_execve+0x32/0x50 >>>> [] stub_execve+0x69/0xa0 >>>> >>>> Signed-off-by: Zhu Yanjun >>>> --- >>>> 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 >>>> +#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; >>>> >>> >