[PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()

Nam Cao posted 21 patches 4 weeks ago
[PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()
Posted by Nam Cao 4 weeks ago
There is a newly introduced function hrtimer_setup_on_stack(), which will
replace hrtimer_init_on_stack(). In addition to what
hrtimer_init_on_stack() does, this new function also sanity-checks and
initializes the callback function pointer.

Switch to use the new function.

Patch was created by using Coccinelle.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/sched/idle.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index d2f096bb274c..631e42802925 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -399,8 +399,8 @@ void play_idle_precise(u64 duration_ns, u64 latency_ns)
 	cpuidle_use_deepest_state(latency_ns);
 
 	it.done = 0;
-	hrtimer_init_on_stack(&it.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
-	it.timer.function = idle_inject_timer_fn;
+	hrtimer_setup_on_stack(&it.timer, idle_inject_timer_fn, CLOCK_MONOTONIC,
+			       HRTIMER_MODE_REL_HARD);
 	hrtimer_start(&it.timer, ns_to_ktime(duration_ns),
 		      HRTIMER_MODE_REL_PINNED_HARD);
 
-- 
2.39.5
Re: [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()
Posted by Peter Zijlstra 4 weeks ago
On Mon, Oct 28, 2024 at 08:29:37AM +0100, Nam Cao wrote:
> There is a newly introduced function hrtimer_setup_on_stack(), which will
> replace hrtimer_init_on_stack(). In addition to what
> hrtimer_init_on_stack() does, this new function also sanity-checks and
> initializes the callback function pointer.
> 
> Switch to use the new function.
> 
> Patch was created by using Coccinelle.
> 
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
> Cc: Peter Zijlstra <peterz@infradead.org>
> ---
>  kernel/sched/idle.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index d2f096bb274c..631e42802925 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -399,8 +399,8 @@ void play_idle_precise(u64 duration_ns, u64 latency_ns)
>  	cpuidle_use_deepest_state(latency_ns);
>  
>  	it.done = 0;
> -	hrtimer_init_on_stack(&it.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
> -	it.timer.function = idle_inject_timer_fn;
> +	hrtimer_setup_on_stack(&it.timer, idle_inject_timer_fn, CLOCK_MONOTONIC,
> +			       HRTIMER_MODE_REL_HARD);

WTF is hrtimer_setup_on_stack() ?

Do NOT send partial series. How the hell am I supposed to review things
if I don't even get to see the implementation of things,eh?
Re: [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()
Posted by Thomas Gleixner 3 weeks, 6 days ago
On Mon, Oct 28 2024 at 10:09, Peter Zijlstra wrote:
> On Mon, Oct 28, 2024 at 08:29:37AM +0100, Nam Cao wrote:
>> There is a newly introduced function hrtimer_setup_on_stack(), which will
>> replace hrtimer_init_on_stack(). In addition to what
>> hrtimer_init_on_stack() does, this new function also sanity-checks and
>> initializes the callback function pointer.
>> 
>> Switch to use the new function.
>> 
>> Patch was created by using Coccinelle.
>> 
>> Signed-off-by: Nam Cao <namcao@linutronix.de>
>> ---
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> ---
>>  kernel/sched/idle.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
>> index d2f096bb274c..631e42802925 100644
>> --- a/kernel/sched/idle.c
>> +++ b/kernel/sched/idle.c
>> @@ -399,8 +399,8 @@ void play_idle_precise(u64 duration_ns, u64 latency_ns)
>>  	cpuidle_use_deepest_state(latency_ns);
>>  
>>  	it.done = 0;
>> -	hrtimer_init_on_stack(&it.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
>> -	it.timer.function = idle_inject_timer_fn;
>> +	hrtimer_setup_on_stack(&it.timer, idle_inject_timer_fn, CLOCK_MONOTONIC,
>> +			       HRTIMER_MODE_REL_HARD);
>
> WTF is hrtimer_setup_on_stack() ?
>
> Do NOT send partial series. How the hell am I supposed to review things
> if I don't even get to see the implementation of things,eh?

Can you tone down a bit? This was an oversight and I did not notice when
going over it. The full thread is in your LKML inbox, so can you just
move on?

Thanks,

        tglx
Re: [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()
Posted by Peter Zijlstra 3 weeks, 6 days ago
On Mon, Oct 28, 2024 at 11:50:44AM +0100, Thomas Gleixner wrote:
> On Mon, Oct 28 2024 at 10:09, Peter Zijlstra wrote:
> > On Mon, Oct 28, 2024 at 08:29:37AM +0100, Nam Cao wrote:
> >> There is a newly introduced function hrtimer_setup_on_stack(), which will
> >> replace hrtimer_init_on_stack(). In addition to what
> >> hrtimer_init_on_stack() does, this new function also sanity-checks and
> >> initializes the callback function pointer.
> >> 
> >> Switch to use the new function.
> >> 
> >> Patch was created by using Coccinelle.
> >> 
> >> Signed-off-by: Nam Cao <namcao@linutronix.de>
> >> ---
> >> Cc: Peter Zijlstra <peterz@infradead.org>
> >> ---
> >>  kernel/sched/idle.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> >> index d2f096bb274c..631e42802925 100644
> >> --- a/kernel/sched/idle.c
> >> +++ b/kernel/sched/idle.c
> >> @@ -399,8 +399,8 @@ void play_idle_precise(u64 duration_ns, u64 latency_ns)
> >>  	cpuidle_use_deepest_state(latency_ns);
> >>  
> >>  	it.done = 0;
> >> -	hrtimer_init_on_stack(&it.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
> >> -	it.timer.function = idle_inject_timer_fn;
> >> +	hrtimer_setup_on_stack(&it.timer, idle_inject_timer_fn, CLOCK_MONOTONIC,
> >> +			       HRTIMER_MODE_REL_HARD);
> >
> > WTF is hrtimer_setup_on_stack() ?
> >
> > Do NOT send partial series. How the hell am I supposed to review things
> > if I don't even get to see the implementation of things,eh?
> 
> Can you tone down a bit? This was an oversight and I did not notice when
> going over it. The full thread is in your LKML inbox, so can you just
> move on?

*sigh*.. how am I supposed to know it's an over-sight? Some people are
actively pushing for this broken arse 'model' of posting.

Yes, I can dig out the remaining patches, but that's more work for me.
As you well know, I don't really need more work.

I suppose I'll see a new posting eventually or not, who knows.
Re: [PATCH 18/21] sched/idle: Switch to use hrtimer_setup_on_stack()
Posted by Thomas Gleixner 3 weeks, 6 days ago
On Mon, Oct 28 2024 at 11:58, Peter Zijlstra wrote:
> On Mon, Oct 28, 2024 at 11:50:44AM +0100, Thomas Gleixner wrote:
>> > Do NOT send partial series. How the hell am I supposed to review things
>> > if I don't even get to see the implementation of things,eh?
>> 
>> Can you tone down a bit? This was an oversight and I did not notice when
>> going over it. The full thread is in your LKML inbox, so can you just
>> move on?
>
> *sigh*.. how am I supposed to know it's an over-sight? Some people are
> actively pushing for this broken arse 'model' of posting.
>
> Yes, I can dig out the remaining patches, but that's more work for me.
> As you well know, I don't really need more work.

Nobody needs more work. But as we (as a community) can't agree on how to
post a 150+ patch series with a potential cc list of 200+ people and a
gazillion of mailing lists, there are only a few options left. And yes,
any model will annoy some people...

You at least got the cover letter, no?

I'm tired of posting a "Add new API/infrastructure" patch and then
finally 5 years later having the last newly added offender converted
over. I rather inflict the pain once, but obviously there is no way to
please everyone.

> I suppose I'll see a new posting eventually or not, who knows.

I'm happy to personally bounce you the pile if you insist or
alternatively annoy everyone with a resend of the full glory as well.

Not sure what that buys, but one thing is sure that we both wasted 10
times more time debating this nonsense than what it would have cost you
to look at the full mail thread which is in your LKML folder anyway.

Seriously?

Thanks,

        tglx