From: Ming Lei <tom.leiming@gmail.com> To: Thomas Renninger <trenn@suse.de> Cc: rostedt@goodmis.org, fweisbec@gmail.com, jean.pihet@newoldbits.com, mingo@redhat.com, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, Len Brown <lenb@kernel.org> Subject: Re: [PATCH 3/3] trace points: power: remove 'cpu_id' from trace_cpu_idle Date: Sat, 20 Aug 2011 10:40:09 +0800 [thread overview] Message-ID: <CACVXFVPeFyg7YoAsY63Mpy9HVR1y3ibg9R8HnUfLVe=hKf3ApA@mail.gmail.com> (raw) In-Reply-To: <201108192231.02665.trenn@suse.de> Hi, 2011/8/20 Thomas Renninger <trenn@suse.de>: > On Friday, August 19, 2011 05:04:04 PM tom.leiming@gmail.com wrote: >> From: Ming Lei <tom.leiming@gmail.com> >> >> This patch removes the 'cpu_id' parameter of the cpu_idle >> trace point, based on the ideas below: >> >> - the cpu_id which is passed to trace point is always the current >> cpu > Are you sure this will always be true? It is sure at least now, the only place to pass 'dev->cpu' is inside cpuidle_idle_call, but the cpuidle_device is got from __this_cpu_read(cpuidle_devices), so it should be true for such case. > >> - the current cpu info has been included into the trace result >> already >> - smp_processor_id() can't be used safely in preemptible context. > > The cpuid has been added to idle events on purpose for example to be > able to pass C-state dependencies. > 2 cores may only enter a deeper sleep state if the 2nd core enters it > as well. > There may be architectures where you could trigger a sleep state on > a foreign cpu. > > This may not be used now, but if the kernel does want to use it, you do > not want to adjust a dozen userspace apps. > > Not sure how to quickly solve the: > "smp_processor_id() can't be used safely in preemptible context." > problem, though. > A convention could be declared that if -1 is passed, it's the same cpu > entering the sleep state as the running one. This will probably > break userspace apps as well... > > Best would be to clean up x86 and let idle routines always be entered > from cpuidle subsystem which knows the cpu id already and eliminate > all idle functions in arch/x86/kernel/process.c. > > Thomas > > PS: I do not care about patch 1 and 2 as these events are ARM specific > afaik. But I vote against this one. > thanks, -- Ming Lei
WARNING: multiple messages have this Message-ID (diff)
From: Ming Lei <tom.leiming@gmail.com> To: Thomas Renninger <trenn@suse.de> Cc: rostedt@goodmis.org, fweisbec@gmail.com, jean.pihet@newoldbits.com, mingo@redhat.com, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, Len Brown <lenb@kernel.org> Subject: Re: [PATCH 3/3] trace points: power: remove 'cpu_id' from trace_cpu_idle Date: Sat, 20 Aug 2011 10:40:09 +0800 [thread overview] Message-ID: <CACVXFVPeFyg7YoAsY63Mpy9HVR1y3ibg9R8HnUfLVe=hKf3ApA@mail.gmail.com> (raw) In-Reply-To: <201108192231.02665.trenn@suse.de> Hi, 2011/8/20 Thomas Renninger <trenn@suse.de>: > On Friday, August 19, 2011 05:04:04 PM tom.leiming@gmail.com wrote: >> From: Ming Lei <tom.leiming@gmail.com> >> >> This patch removes the 'cpu_id' parameter of the cpu_idle >> trace point, based on the ideas below: >> >> - the cpu_id which is passed to trace point is always the current >> cpu > Are you sure this will always be true? It is sure at least now, the only place to pass 'dev->cpu' is inside cpuidle_idle_call, but the cpuidle_device is got from __this_cpu_read(cpuidle_devices), so it should be true for such case. > >> - the current cpu info has been included into the trace result >> already >> - smp_processor_id() can't be used safely in preemptible context. > > The cpuid has been added to idle events on purpose for example to be > able to pass C-state dependencies. > 2 cores may only enter a deeper sleep state if the 2nd core enters it > as well. > There may be architectures where you could trigger a sleep state on > a foreign cpu. > > This may not be used now, but if the kernel does want to use it, you do > not want to adjust a dozen userspace apps. > > Not sure how to quickly solve the: > "smp_processor_id() can't be used safely in preemptible context." > problem, though. > A convention could be declared that if -1 is passed, it's the same cpu > entering the sleep state as the running one. This will probably > break userspace apps as well... > > Best would be to clean up x86 and let idle routines always be entered > from cpuidle subsystem which knows the cpu id already and eliminate > all idle functions in arch/x86/kernel/process.c. > > Thomas > > PS: I do not care about patch 1 and 2 as these events are ARM specific > afaik. But I vote against this one. > thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2011-08-20 2:40 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-08-19 15:04 [PATCH 1/3] trace points: power: remove 'cpu_id' from trace_clock_* tom.leiming 2011-08-19 15:04 ` [PATCH 2/3] trace points: power: remove 'cpu_id' from trace_power_domain_target tom.leiming 2011-08-19 15:04 ` [PATCH 3/3] trace points: power: remove 'cpu_id' from trace_cpu_idle tom.leiming 2011-08-19 20:31 ` Thomas Renninger 2011-08-20 2:40 ` Ming Lei [this message] 2011-08-20 2:40 ` Ming Lei 2011-08-22 8:27 ` Thomas Renninger 2011-09-02 7:26 ` Jean Pihet 2011-09-02 7:38 ` Ming Lei 2011-08-19 15:14 ` [PATCH 1/3] trace points: power: remove 'cpu_id' from trace_clock_* Steven Rostedt 2011-08-19 15:39 ` Ming Lei 2011-08-19 16:16 ` Arjan van de Ven 2011-08-20 2:44 ` Ming Lei 2011-08-20 2:44 ` Ming Lei 2011-08-20 15:59 ` Arjan van de Ven
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CACVXFVPeFyg7YoAsY63Mpy9HVR1y3ibg9R8HnUfLVe=hKf3ApA@mail.gmail.com' \ --to=tom.leiming@gmail.com \ --cc=fweisbec@gmail.com \ --cc=jean.pihet@newoldbits.com \ --cc=lenb@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-omap@vger.kernel.org \ --cc=mingo@redhat.com \ --cc=rostedt@goodmis.org \ --cc=trenn@suse.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.