* [PATCH] sched/core: Export pelt_thermal_tp @ 2021-10-28 11:50 Qais Yousef 2021-10-28 16:00 ` Christoph Hellwig 2022-01-28 7:40 ` [tip: sched/core] " tip-bot2 for Qais Yousef 0 siblings, 2 replies; 8+ messages in thread From: Qais Yousef @ 2021-10-28 11:50 UTC (permalink / raw) To: Peter Zijlstra (Intel), Ingo Molnar Cc: Dietmar Eggemann, Vincent Guittot, Thara Gopinath, linux-kernel, Qais Yousef We can't use this tracepoint in modules without having the symbol exported first, fix that. Fixes: 765047932f15 ("sched/pelt: Add support to track thermal pressure") Signed-off-by: Qais Yousef <qais.yousef@arm.com> --- kernel/sched/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index f2611b9cf503..2f7a1d8f5f17 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -36,6 +36,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_rt_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_dl_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_irq_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_se_tp); +EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_thermal_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(sched_cpu_capacity_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(sched_overutilized_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(sched_util_est_cfs_tp); -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/core: Export pelt_thermal_tp 2021-10-28 11:50 [PATCH] sched/core: Export pelt_thermal_tp Qais Yousef @ 2021-10-28 16:00 ` Christoph Hellwig 2021-10-28 16:18 ` Peter Zijlstra 2022-01-28 7:40 ` [tip: sched/core] " tip-bot2 for Qais Yousef 1 sibling, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2021-10-28 16:00 UTC (permalink / raw) To: Qais Yousef Cc: Peter Zijlstra (Intel), Ingo Molnar, Dietmar Eggemann, Vincent Guittot, Thara Gopinath, linux-kernel On Thu, Oct 28, 2021 at 12:50:05PM +0100, Qais Yousef wrote: > We can't use this tracepoint in modules without having the symbol > exported first, fix that. Which modules is using this? In linux-next there does not seems to be any user outside of kernel/sched/pelt.c. > @@ -36,6 +36,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_rt_tp); > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_dl_tp); > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_irq_tp); > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_se_tp); > +EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_thermal_tp); > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_cpu_capacity_tp); > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_overutilized_tp); > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_util_est_cfs_tp); ... and while we're at it, all these exports are unused and should be deleted as well. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/core: Export pelt_thermal_tp 2021-10-28 16:00 ` Christoph Hellwig @ 2021-10-28 16:18 ` Peter Zijlstra 2021-10-28 16:22 ` Christoph Hellwig 0 siblings, 1 reply; 8+ messages in thread From: Peter Zijlstra @ 2021-10-28 16:18 UTC (permalink / raw) To: Christoph Hellwig Cc: Qais Yousef, Ingo Molnar, Dietmar Eggemann, Vincent Guittot, Thara Gopinath, linux-kernel On Thu, Oct 28, 2021 at 09:00:56AM -0700, Christoph Hellwig wrote: > On Thu, Oct 28, 2021 at 12:50:05PM +0100, Qais Yousef wrote: > > We can't use this tracepoint in modules without having the symbol > > exported first, fix that. > > Which modules is using this? In linux-next there does not seems to be > any user outside of kernel/sched/pelt.c. > > > @@ -36,6 +36,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_rt_tp); > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_dl_tp); > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_irq_tp); > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_se_tp); > > +EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_thermal_tp); > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_cpu_capacity_tp); > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_overutilized_tp); > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_util_est_cfs_tp); > > ... and while we're at it, all these exports are unused and should > be deleted as well. This is my concession wrt tracepoints. Actual tracepoints are ABI, exports are in-kernel interfaces and are explicitly not ABI. This way people can use an external module to get at the tracepoint data without having in-tree tracepoints. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/core: Export pelt_thermal_tp 2021-10-28 16:18 ` Peter Zijlstra @ 2021-10-28 16:22 ` Christoph Hellwig 2021-10-28 16:48 ` Phil Auld 2021-10-28 16:50 ` Peter Zijlstra 0 siblings, 2 replies; 8+ messages in thread From: Christoph Hellwig @ 2021-10-28 16:22 UTC (permalink / raw) To: Peter Zijlstra Cc: Christoph Hellwig, Qais Yousef, Ingo Molnar, Dietmar Eggemann, Vincent Guittot, Thara Gopinath, linux-kernel On Thu, Oct 28, 2021 at 06:18:55PM +0200, Peter Zijlstra wrote: > > > @@ -36,6 +36,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_rt_tp); > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_dl_tp); > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_irq_tp); > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_se_tp); > > > +EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_thermal_tp); > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_cpu_capacity_tp); > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_overutilized_tp); > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_util_est_cfs_tp); > > > > ... and while we're at it, all these exports are unused and should > > be deleted as well. > > This is my concession wrt tracepoints. Actual tracepoints are ABI, > exports are in-kernel interfaces and are explicitly not ABI. > > This way people can use an external module to get at the tracepoint data > without having in-tree tracepoints. All of this makes no sense at all. These are entirely dead exports. If you remove them nothing else changes. Note taht the tracepoints do have in-kernel callers, so if people thing of them as an ABI you've got your ABI already with or without the exports. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/core: Export pelt_thermal_tp 2021-10-28 16:22 ` Christoph Hellwig @ 2021-10-28 16:48 ` Phil Auld 2021-10-28 16:50 ` Peter Zijlstra 1 sibling, 0 replies; 8+ messages in thread From: Phil Auld @ 2021-10-28 16:48 UTC (permalink / raw) To: Christoph Hellwig Cc: Peter Zijlstra, Qais Yousef, Ingo Molnar, Dietmar Eggemann, Vincent Guittot, Thara Gopinath, linux-kernel On Thu, Oct 28, 2021 at 09:22:05AM -0700 Christoph Hellwig wrote: > On Thu, Oct 28, 2021 at 06:18:55PM +0200, Peter Zijlstra wrote: > > > > @@ -36,6 +36,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_rt_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_dl_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_irq_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_se_tp); > > > > +EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_thermal_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_cpu_capacity_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_overutilized_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_util_est_cfs_tp); > > > > > > ... and while we're at it, all these exports are unused and should > > > be deleted as well. > > > > This is my concession wrt tracepoints. Actual tracepoints are ABI, > > exports are in-kernel interfaces and are explicitly not ABI. > > > > This way people can use an external module to get at the tracepoint data > > without having in-tree tracepoints. > > All of this makes no sense at all. These are entirely dead exports. > If you remove them nothing else changes. Note taht the tracepoints > do have in-kernel callers, so if people thing of them as an ABI you've > got your ABI already with or without the exports. > Full blown trace _events_ create an ABI. These trace points are not ABI. But by exporting them they are accesible to little helper modules which can turn them into trace events which can then by used by trace-cmd and ftrace etc. That way we can have the tracepoints at the interesting spots in the code but still have control over them with respect to changes. See https://github.com/auldp/tracepoints-helpers for an example. Cheers, Phil -- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/core: Export pelt_thermal_tp 2021-10-28 16:22 ` Christoph Hellwig 2021-10-28 16:48 ` Phil Auld @ 2021-10-28 16:50 ` Peter Zijlstra 2021-11-25 16:56 ` Qais Yousef 1 sibling, 1 reply; 8+ messages in thread From: Peter Zijlstra @ 2021-10-28 16:50 UTC (permalink / raw) To: Christoph Hellwig Cc: Qais Yousef, Ingo Molnar, Dietmar Eggemann, Vincent Guittot, Thara Gopinath, linux-kernel On Thu, Oct 28, 2021 at 09:22:05AM -0700, Christoph Hellwig wrote: > On Thu, Oct 28, 2021 at 06:18:55PM +0200, Peter Zijlstra wrote: > > > > @@ -36,6 +36,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_rt_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_dl_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_irq_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_se_tp); > > > > +EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_thermal_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_cpu_capacity_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_overutilized_tp); > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_util_est_cfs_tp); > > > > > > ... and while we're at it, all these exports are unused and should > > > be deleted as well. > > > > This is my concession wrt tracepoints. Actual tracepoints are ABI, > > exports are in-kernel interfaces and are explicitly not ABI. > > > > This way people can use an external module to get at the tracepoint data > > without having in-tree tracepoints. > > All of this makes no sense at all. These are entirely dead exports. > If you remove them nothing else changes. Note taht the tracepoints > do have in-kernel callers, so if people thing of them as an ABI you've > got your ABI already with or without the exports. These are not normal traceevents, these are tracepoints, the distinction is that these things do not show up in tracefs and there is no userspace visible representation of them. No userspace gives no ABI. All they provide is the in-code hook and in-kernel registration interface. These EXPORTS export that registration interface, such that an out-of-tree module can make use of them. And yes, unused exports are iffy, out-of-tree modules are iffy, but in this case I made an exception since ABI contraints are worse. We very clearly state there is no such thing is kabi, so breaking any user of these exports if fair game. Breaking users of userspace trace-events gets kernel patches reverted (been there, done that, never want to ever be there again). People want to trace this stuff, but I *REALLY* do not want to commit to ABI, this is the middle-ground that sucks least :/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sched/core: Export pelt_thermal_tp 2021-10-28 16:50 ` Peter Zijlstra @ 2021-11-25 16:56 ` Qais Yousef 0 siblings, 0 replies; 8+ messages in thread From: Qais Yousef @ 2021-11-25 16:56 UTC (permalink / raw) To: Peter Zijlstra Cc: Christoph Hellwig, Ingo Molnar, Dietmar Eggemann, Vincent Guittot, Thara Gopinath, linux-kernel Hi Peter On 10/28/21 18:50, Peter Zijlstra wrote: > On Thu, Oct 28, 2021 at 09:22:05AM -0700, Christoph Hellwig wrote: > > On Thu, Oct 28, 2021 at 06:18:55PM +0200, Peter Zijlstra wrote: > > > > > @@ -36,6 +36,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_rt_tp); > > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_dl_tp); > > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_irq_tp); > > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_se_tp); > > > > > +EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_thermal_tp); > > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_cpu_capacity_tp); > > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_overutilized_tp); > > > > > EXPORT_TRACEPOINT_SYMBOL_GPL(sched_util_est_cfs_tp); > > > > > > > > ... and while we're at it, all these exports are unused and should > > > > be deleted as well. > > > > > > This is my concession wrt tracepoints. Actual tracepoints are ABI, > > > exports are in-kernel interfaces and are explicitly not ABI. > > > > > > This way people can use an external module to get at the tracepoint data > > > without having in-tree tracepoints. > > > > All of this makes no sense at all. These are entirely dead exports. > > If you remove them nothing else changes. Note taht the tracepoints > > do have in-kernel callers, so if people thing of them as an ABI you've > > got your ABI already with or without the exports. > > These are not normal traceevents, these are tracepoints, the distinction > is that these things do not show up in tracefs and there is no userspace > visible representation of them. No userspace gives no ABI. > > All they provide is the in-code hook and in-kernel registration > interface. These EXPORTS export that registration interface, such that > an out-of-tree module can make use of them. > > And yes, unused exports are iffy, out-of-tree modules are iffy, but in > this case I made an exception since ABI contraints are worse. We very > clearly state there is no such thing is kabi, so breaking any user of > these exports if fair game. > > Breaking users of userspace trace-events gets kernel patches reverted > (been there, done that, never want to ever be there again). > > People want to trace this stuff, but I *REALLY* do not want to commit to > ABI, this is the middle-ground that sucks least :/ AFAICS this wasn't picked up. Should I tweak the commit message to make things clearer? Thanks! -- Qais Yousef ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip: sched/core] sched/core: Export pelt_thermal_tp 2021-10-28 11:50 [PATCH] sched/core: Export pelt_thermal_tp Qais Yousef 2021-10-28 16:00 ` Christoph Hellwig @ 2022-01-28 7:40 ` tip-bot2 for Qais Yousef 1 sibling, 0 replies; 8+ messages in thread From: tip-bot2 for Qais Yousef @ 2022-01-28 7:40 UTC (permalink / raw) To: linux-tip-commits; +Cc: Qais Yousef, Peter Zijlstra (Intel), x86, linux-kernel The following commit has been merged into the sched/core branch of tip: Commit-ID: 77cf151b7bbdfa3577b3c3f3a5e267a6c60a263b Gitweb: https://git.kernel.org/tip/77cf151b7bbdfa3577b3c3f3a5e267a6c60a263b Author: Qais Yousef <qais.yousef@arm.com> AuthorDate: Thu, 28 Oct 2021 12:50:05 +01:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Thu, 27 Jan 2022 12:57:18 +01:00 sched/core: Export pelt_thermal_tp We can't use this tracepoint in modules without having the symbol exported first, fix that. Fixes: 765047932f15 ("sched/pelt: Add support to track thermal pressure") Signed-off-by: Qais Yousef <qais.yousef@arm.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20211028115005.873539-1-qais.yousef@arm.com --- kernel/sched/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 2e4ae00..1d863d7 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -36,6 +36,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_rt_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_dl_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_irq_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_se_tp); +EXPORT_TRACEPOINT_SYMBOL_GPL(pelt_thermal_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(sched_cpu_capacity_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(sched_overutilized_tp); EXPORT_TRACEPOINT_SYMBOL_GPL(sched_util_est_cfs_tp); ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-01-28 7:40 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-28 11:50 [PATCH] sched/core: Export pelt_thermal_tp Qais Yousef 2021-10-28 16:00 ` Christoph Hellwig 2021-10-28 16:18 ` Peter Zijlstra 2021-10-28 16:22 ` Christoph Hellwig 2021-10-28 16:48 ` Phil Auld 2021-10-28 16:50 ` Peter Zijlstra 2021-11-25 16:56 ` Qais Yousef 2022-01-28 7:40 ` [tip: sched/core] " tip-bot2 for Qais Yousef
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).