Message-ID: <5554AE2B.30201@linux.vnet.ibm.com>
Date: Thu, 14 May 2015 19:46:11 +0530
From: Shreyas B Prabhu <shreyas@linux.vnet.ibm.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0
MIME-Version: 1.0
To: Steven Rostedt <rostedt@goodmis.org>
CC: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
        preeti@linux.vnet.ibm.com, paulmck@linux.vnet.ibm.com
Subject: Re: [PATCH] tracing: Add comments explaining cpu online filter for
 trace events
References: <1431533263-5139-1-git-send-email-shreyas@linux.vnet.ibm.com> <20150513122147.4ead9f07@gandalf.local.home>
In-Reply-To: <20150513122147.4ead9f07@gandalf.local.home>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3036
Lines: 75


On Wednesday 13 May 2015 09:51 PM, Steven Rostedt wrote:
> On Wed, 13 May 2015 21:37:43 +0530
> "Shreyas B. Prabhu" <shreyas@linux.vnet.ibm.com> wrote:
> 
>> trace_mm_page_pcpu_drain, trace_kmem_cache_free, trace_mm_page_free
>> and trace_tlb_flush can be potentially called from an offlined cpu.
>> Since trace points use RCU and RCU should not be used from offlined
>> cpus, we have checks to filter out such calls. Add comments to explain
>> this.
>>
>> Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
>> ---
>> This applies on top of patches posted here:
>> https://lkml.org/lkml/2015/5/8/527
>>
>>  include/trace/events/kmem.h | 15 +++++++++++++++
>>  include/trace/events/tlb.h  |  5 +++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
>> index 6cd975f..9883f2f 100644
>> --- a/include/trace/events/kmem.h
>> +++ b/include/trace/events/kmem.h
>> @@ -146,6 +146,11 @@ DEFINE_EVENT_CONDITION(kmem_free, kmem_cache_free,
>>  
>>  	TP_ARGS(call_site, ptr),
>>  
>> +	/*
>> +	 * This trace can be potentially called from an offlined cpu.
>> +	 * Since trace points use RCU and RCU should not be used from
>> +	 * offline cpus, filter such calls out.
>> +	 */
>>  	TP_CONDITION(cpu_online(smp_processor_id()))
>>  );
>>  
> 
> Thanks for the comments, but can't these still be called with
> preemption enabled. What happens when CONFIG_DEBUG_PREEMPT is set and
> you enable these tracepoints. Wont it trigger a warning about
> smp_processor_id() being used in preemptible code?
> 
Yes. It does trigger "using smp_processor_id() in preemptible code"
warnings. But as you mentioned in the previous comments, we should be
safe even if the trace call happens from a preemptible section. Let me
play out the scenarios here again-

The task gets migrated after the smp_processor_id()
1. From an online cpu to another online cpu - No impact
2. From an online cpu to an offline cpu - Should never happen
3. From an offline cpu to an online cpu - IIUC, once a cpu has been
offlined it returns to cpu_idle_loop, discovers its offline and calls
arch_cpu_idle_dead. All this happens with preemption disabled. So this
scenario too should never happen.

So I don't see any downside to changing smp_processor_id() to
raw_smp_processor_id() which will suppress the warnings. If you agree
I'll send a patch doing this.

Another alternative which is perhaps worth considering is to change
__DO_TRACE itself to check for offline cpu, without a trace event
specifying the check. This will prevent any currently uncaught and any
future tracepoints from using RCU on offline cpus. But I guess it's
little extreme considering only a low fraction of tracepoints have
potential of being called from offline cpus.

Thanks,
Shreyas

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/