RE: RFC: better timer interface

[Thomas Gleixner]

On Tue, 23 May 2017, David Laight wrote:
> From: Thomas Gleixner
> > Sent: 23 May 2017 12:59
> > On Tue, 23 May 2017, David Laight wrote:
> > 
> > > From: Thomas Gleixner
> > > > Sent: 21 May 2017 19:15
> > > ...
> > > > > timer_start(timer, ms, abs)
> > > >
> > > > I'm not even sure, whether we need absolute timer wheel timers at
> > > > all, because most use cases are relative to now.
> > >
> > > Posix requires absolute timers for some userspace calls
> > > (annoying because the code often wants relative).
> > 
> > Posix is completely irrelevant here. These timers are purely kernel
> > internal.
> 
> Somehow pthread_cond_timedwait() has to be implemented.
> Doing so without kernel timers that use absolute 'wall clock' time is tricky.

Oh well. The timer wheel timers are NOT used to implement any posix
interface. That's all handled by hrtimers and they are not debated here.

So still nothing to see here.

Thanks,

tglx

RE: RFC: better timer interface

[Thomas Gleixner]

On Tue, 23 May 2017, David Laight wrote:
> From: Thomas Gleixner
> > Sent: 23 May 2017 12:59
> > On Tue, 23 May 2017, David Laight wrote:
> > 
> > > From: Thomas Gleixner
> > > > Sent: 21 May 2017 19:15
> > > ...
> > > > > timer_start(timer, ms, abs)
> > > >
> > > > I'm not even sure, whether we need absolute timer wheel timers at
> > > > all, because most use cases are relative to now.
> > >
> > > Posix requires absolute timers for some userspace calls
> > > (annoying because the code often wants relative).
> > 
> > Posix is completely irrelevant here. These timers are purely kernel
> > internal.
> 
> Somehow pthread_cond_timedwait() has to be implemented.
> Doing so without kernel timers that use absolute 'wall clock' time is tricky.

Oh well. The timer wheel timers are NOT used to implement any posix
interface. That's all handled by hrtimers and they are not debated here.

So still nothing to see here.

Thanks,

tglx

RE: RFC: better timer interface

[David Laight]

From: Thomas Gleixner
> Sent: 23 May 2017 12:59
> On Tue, 23 May 2017, David Laight wrote:
> 
> > From: Thomas Gleixner
> > > Sent: 21 May 2017 19:15
> > ...
> > > > timer_start(timer, ms, abs)
> > >
> > > I'm not even sure, whether we need absolute timer wheel timers at
> > > all, because most use cases are relative to now.
> >
> > Posix requires absolute timers for some userspace calls
> > (annoying because the code often wants relative).
> 
> Posix is completely irrelevant here. These timers are purely kernel
> internal.

Somehow pthread_cond_timedwait() has to be implemented.
Doing so without kernel timers that use absolute 'wall clock' time is tricky.

David

RE: RFC: better timer interface

[David Laight]

From: Thomas Gleixner
> Sent: 23 May 2017 12:59
> On Tue, 23 May 2017, David Laight wrote:
> 
> > From: Thomas Gleixner
> > > Sent: 21 May 2017 19:15
> > ...
> > > > timer_start(timer, ms, abs)
> > >
> > > I'm not even sure, whether we need absolute timer wheel timers at
> > > all, because most use cases are relative to now.
> >
> > Posix requires absolute timers for some userspace calls
> > (annoying because the code often wants relative).
> 
> Posix is completely irrelevant here. These timers are purely kernel
> internal.

Somehow pthread_cond_timedwait() has to be implemented.
Doing so without kernel timers that use absolute 'wall clock' time is tricky.

David

RE: RFC: better timer interface

[Thomas Gleixner]

On Tue, 23 May 2017, David Laight wrote:

> From: Thomas Gleixner
> > Sent: 21 May 2017 19:15
> ...
> > > timer_start(timer, ms, abs)
> > 
> > I'm not even sure, whether we need absolute timer wheel timers at
> > all, because most use cases are relative to now.
> 
> Posix requires absolute timers for some userspace calls
> (annoying because the code often wants relative).

Posix is completely irrelevant here. These timers are purely kernel
internal.

Thanks,

tglx

RE: RFC: better timer interface

[Thomas Gleixner]

On Tue, 23 May 2017, David Laight wrote:

> From: Thomas Gleixner
> > Sent: 21 May 2017 19:15
> ...
> > > timer_start(timer, ms, abs)
> > 
> > I'm not even sure, whether we need absolute timer wheel timers at
> > all, because most use cases are relative to now.
> 
> Posix requires absolute timers for some userspace calls
> (annoying because the code often wants relative).

Posix is completely irrelevant here. These timers are purely kernel
internal.

Thanks,

tglx

RE: RFC: better timer interface

[David Laight]

From: Thomas Gleixner
> Sent: 21 May 2017 19:15
...
> > timer_start(timer, ms, abs)
> 
> I'm not even sure, whether we need absolute timer wheel timers at
> all, because most use cases are relative to now.

Posix requires absolute timers for some userspace calls
(annoying because the code often wants relative).

OTOH how much conditional code is there for the 'abs' argument.
And is there any code that doesn't pass a constant?

Certainly worth a separate timer_start_abs(timer, wall_time)
function since you can't correctly map a wall_time timer
to a jiffies one.

David

RE: RFC: better timer interface

[David Laight]

From: Thomas Gleixner
> Sent: 21 May 2017 19:15
...
> > timer_start(timer, ms, abs)
> 
> I'm not even sure, whether we need absolute timer wheel timers at
> all, because most use cases are relative to now.

Posix requires absolute timers for some userspace calls
(annoying because the code often wants relative).

OTOH how much conditional code is there for the 'abs' argument.
And is there any code that doesn't pass a constant?

Certainly worth a separate timer_start_abs(timer, wall_time)
function since you can't correctly map a wall_time timer
to a jiffies one.

David

Re: RFC: better timer interface

[Thomas Gleixner]

On Mon, 22 May 2017, Arnd Bergmann wrote:
> On Sun, May 21, 2017 at 8:14 PM, Thomas Gleixner  wrote:
> > But it's easy enough to provide them. All we need for that is something
> > like
> >
> > unsigned long time_msec;
> >
> > which gets incremented every tick by the appropriate amount of
> > milliseconds.
> >
> > Having that would also allow to replace all the
> >
> >end = jiffies + msec_to_jiffies(xxx);
> >
> >while (time_before(jiffies, end))
> >  
> >
> > constructs with a milliseconds based machinery. So we can remove all
> > *_to_jiffies() interfaces over time.
> 
> A lot of those users could probably just ktime_get()/ktime_before() here,
> as they would by definition not be performance critical.

Right.

> I don't see a way to just tk->tkr_mono.base but with a ktime_get_coarse()
> we could just return the ktime_t of the last tick and not even need a seqlock
> on 64-bit architectures, or have to introduce a new API.

Yeah, that would be possible, but OTOH, for those loop thingies it probably
does not matter at all whether you have the overhead of ktime_get() or
not. We need to look at that stuff deeper.

Thanks,

tglx

Re: RFC: better timer interface

[Thomas Gleixner]

On Mon, 22 May 2017, Arnd Bergmann wrote:
> On Sun, May 21, 2017 at 8:14 PM, Thomas Gleixner  wrote:
> > But it's easy enough to provide them. All we need for that is something
> > like
> >
> > unsigned long time_msec;
> >
> > which gets incremented every tick by the appropriate amount of
> > milliseconds.
> >
> > Having that would also allow to replace all the
> >
> >end = jiffies + msec_to_jiffies(xxx);
> >
> >while (time_before(jiffies, end))
> >  
> >
> > constructs with a milliseconds based machinery. So we can remove all
> > *_to_jiffies() interfaces over time.
> 
> A lot of those users could probably just ktime_get()/ktime_before() here,
> as they would by definition not be performance critical.

Right.

> I don't see a way to just tk->tkr_mono.base but with a ktime_get_coarse()
> we could just return the ktime_t of the last tick and not even need a seqlock
> on 64-bit architectures, or have to introduce a new API.

Yeah, that would be possible, but OTOH, for those loop thingies it probably
does not matter at all whether you have the overhead of ktime_get() or
not. We need to look at that stuff deeper.

Thanks,

tglx

Re: RFC: better timer interface

[Thomas Gleixner]

On Mon, 22 May 2017, Arnd Bergmann wrote:
> On Sun, May 21, 2017 at 7:13 PM, Thomas Gleixner  wrote:
> I agree, one of the above is good enough, if we do the large-scale API
> replacement. Having both ms and sec variants would be for convenience
> to avoid having lots of open-coded  '*MSEC_PER_SEC) multiplications.

Right, but that _sec variant is just a wrapper around the _ms functionality.

> We still need at least three variants of timer_init, for statically 
> initialized
> timers, dynamic allocation and on-stack allocation, as before.

Again, that's variants. What I wanted to avoid is that mod_ restart_ start_
whatever distinction. It's pointless.

> For the 'abs' argument, I'd probably leave that out until we find code
> that actually needs it and that can't use hrtimer as easily.

> For timer_start_on(), that would be a replacement of add_timer_on(),
> which only has seven callers today:

> arch/x86/kernel/apic/x2apic_uv_x.c: add_timer_on(timer, cpu);
> drivers/tty/metag_da.c: add_timer_on(poll_timer, 0);
> drivers/tty/mips_ejtag_fdc.c:
> add_timer_on(>poll_timer, dev->cpu);
> drivers/tty/mips_ejtag_fdc.c:
> add_timer_on(>poll_timer, dev->cpu);
> kernel/time/clocksource.c:  add_timer_on(_timer, next_cpu);
> kernel/time/clocksource.c:  add_timer_on(_timer,
> cpumask_first(cpu_online_mask));
> kernel/workqueue.c: add_timer_on(timer, cpu);
> 
> If hrtimer isn't already a better interface for those, we can probably
> convert them all to the new API at once.

Some of them certainly can be converted. Though I'm starting to worry about
massive hrtimer usage, which can really become a burden because the expiry
functions run from hard interrupt context. We already have this horrible
construct of hrtimer -> tasklet -> real function, which is used to get the
hrtimer expiry out into softirq context.

We have similar issue in RT, where we need to move out everything which is
not hard irq safe to softirq context. I'm working on a solution there which
is less ugly than what we have now in RT. It's moving non irq safe timers
onto a different time base, e.g. MONOTONIC_SOFT, which merily wakes the
softirq when the first queued timer expires and let that one sort out the
expiry of those timers. It's WIP and should be available soon.

Thanks,

tglx

Re: RFC: better timer interface

[Thomas Gleixner]

On Mon, 22 May 2017, Arnd Bergmann wrote:
> On Sun, May 21, 2017 at 7:13 PM, Thomas Gleixner  wrote:
> I agree, one of the above is good enough, if we do the large-scale API
> replacement. Having both ms and sec variants would be for convenience
> to avoid having lots of open-coded  '*MSEC_PER_SEC) multiplications.

Right, but that _sec variant is just a wrapper around the _ms functionality.

> We still need at least three variants of timer_init, for statically 
> initialized
> timers, dynamic allocation and on-stack allocation, as before.

Again, that's variants. What I wanted to avoid is that mod_ restart_ start_
whatever distinction. It's pointless.

> For the 'abs' argument, I'd probably leave that out until we find code
> that actually needs it and that can't use hrtimer as easily.

> For timer_start_on(), that would be a replacement of add_timer_on(),
> which only has seven callers today:

> arch/x86/kernel/apic/x2apic_uv_x.c: add_timer_on(timer, cpu);
> drivers/tty/metag_da.c: add_timer_on(poll_timer, 0);
> drivers/tty/mips_ejtag_fdc.c:
> add_timer_on(>poll_timer, dev->cpu);
> drivers/tty/mips_ejtag_fdc.c:
> add_timer_on(>poll_timer, dev->cpu);
> kernel/time/clocksource.c:  add_timer_on(_timer, next_cpu);
> kernel/time/clocksource.c:  add_timer_on(_timer,
> cpumask_first(cpu_online_mask));
> kernel/workqueue.c: add_timer_on(timer, cpu);
> 
> If hrtimer isn't already a better interface for those, we can probably
> convert them all to the new API at once.

Some of them certainly can be converted. Though I'm starting to worry about
massive hrtimer usage, which can really become a burden because the expiry
functions run from hard interrupt context. We already have this horrible
construct of hrtimer -> tasklet -> real function, which is used to get the
hrtimer expiry out into softirq context.

We have similar issue in RT, where we need to move out everything which is
not hard irq safe to softirq context. I'm working on a solution there which
is less ugly than what we have now in RT. It's moving non irq safe timers
onto a different time base, e.g. MONOTONIC_SOFT, which merily wakes the
softirq when the first queued timer expires and let that one sort out the
expiry of those timers. It's WIP and should be available soon.

Thanks,

tglx

Re: RFC: better timer interface

[Arnd Bergmann]

On Sun, May 21, 2017 at 7:13 PM, Thomas Gleixner  wrote:
> On Tue, 16 May 2017, Arnd Bergmann wrote:
>> On Tue, May 16, 2017 at 5:51 PM, Christoph Hellwig  wrote:
>> > Yes, that sounds useful to me as well.  As you said it's an independent
>> > but somewhat related change.  I can add it to my series, but I'll
>> > need a suggestions for a good and short name.  That already was the
>> > hardest part for the setup side :)
>>
>> If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
>> has), we could use one of
>>
>> a) start_timer(struct timer_list *timer, unsigned long ms);
>> b) restart_timer(struct timer_list *timer, unsigned long ms);
>> c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
>> mod_timer_sec(struct timer_list *timer, unsigned long sec);
>
> Please make new functions prefixed with timer_ and get rid of that old
> interface completely. It's horrible.
>
> timer_init()
> timer_start(timer, ms, abs)
> timer_start_on(timer, ms, abs, cpu)
> timer_cancel(timer, sync)
>
> Is all what's required to make up a new milliseconds based interface.
>
> We really do not need all that mod/restart/ whatever variants. Where is the
> point of those?

I agree, one of the above is good enough, if we do the large-scale API
replacement. Having both ms and sec variants would be for convenience
to avoid having lots of open-coded  '*MSEC_PER_SEC) multiplications.

We still need at least three variants of timer_init, for statically initialized
timers, dynamic allocation and on-stack allocation, as before.

For the 'abs' argument, I'd probably leave that out until we find code
that actually needs it and that can't use hrtimer as easily.

For timer_start_on(), that would be a replacement of add_timer_on(),
which only has seven callers today:

arch/x86/kernel/apic/x2apic_uv_x.c: add_timer_on(timer, cpu);
drivers/tty/metag_da.c: add_timer_on(poll_timer, 0);
drivers/tty/mips_ejtag_fdc.c:
add_timer_on(>poll_timer, dev->cpu);
drivers/tty/mips_ejtag_fdc.c:
add_timer_on(>poll_timer, dev->cpu);
kernel/time/clocksource.c:  add_timer_on(_timer, next_cpu);
kernel/time/clocksource.c:  add_timer_on(_timer,
cpumask_first(cpu_online_mask));
kernel/workqueue.c: add_timer_on(timer, cpu);

If hrtimer isn't already a better interface for those, we can probably
convert them all to the new API at once.

   Arnd

Re: RFC: better timer interface

[Arnd Bergmann]

On Sun, May 21, 2017 at 7:13 PM, Thomas Gleixner  wrote:
> On Tue, 16 May 2017, Arnd Bergmann wrote:
>> On Tue, May 16, 2017 at 5:51 PM, Christoph Hellwig  wrote:
>> > Yes, that sounds useful to me as well.  As you said it's an independent
>> > but somewhat related change.  I can add it to my series, but I'll
>> > need a suggestions for a good and short name.  That already was the
>> > hardest part for the setup side :)
>>
>> If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
>> has), we could use one of
>>
>> a) start_timer(struct timer_list *timer, unsigned long ms);
>> b) restart_timer(struct timer_list *timer, unsigned long ms);
>> c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
>> mod_timer_sec(struct timer_list *timer, unsigned long sec);
>
> Please make new functions prefixed with timer_ and get rid of that old
> interface completely. It's horrible.
>
> timer_init()
> timer_start(timer, ms, abs)
> timer_start_on(timer, ms, abs, cpu)
> timer_cancel(timer, sync)
>
> Is all what's required to make up a new milliseconds based interface.
>
> We really do not need all that mod/restart/ whatever variants. Where is the
> point of those?

I agree, one of the above is good enough, if we do the large-scale API
replacement. Having both ms and sec variants would be for convenience
to avoid having lots of open-coded  '*MSEC_PER_SEC) multiplications.

We still need at least three variants of timer_init, for statically initialized
timers, dynamic allocation and on-stack allocation, as before.

For the 'abs' argument, I'd probably leave that out until we find code
that actually needs it and that can't use hrtimer as easily.

For timer_start_on(), that would be a replacement of add_timer_on(),
which only has seven callers today:

arch/x86/kernel/apic/x2apic_uv_x.c: add_timer_on(timer, cpu);
drivers/tty/metag_da.c: add_timer_on(poll_timer, 0);
drivers/tty/mips_ejtag_fdc.c:
add_timer_on(>poll_timer, dev->cpu);
drivers/tty/mips_ejtag_fdc.c:
add_timer_on(>poll_timer, dev->cpu);
kernel/time/clocksource.c:  add_timer_on(_timer, next_cpu);
kernel/time/clocksource.c:  add_timer_on(_timer,
cpumask_first(cpu_online_mask));
kernel/workqueue.c: add_timer_on(timer, cpu);

If hrtimer isn't already a better interface for those, we can probably
convert them all to the new API at once.

   Arnd

Re: RFC: better timer interface

[Arnd Bergmann]

On Sun, May 21, 2017 at 8:14 PM, Thomas Gleixner  wrote:
> On Sun, 21 May 2017, Thomas Gleixner wrote:
>> On Tue, 16 May 2017, Arnd Bergmann wrote:
>> > On Tue, May 16, 2017 at 5:51 PM, Christoph Hellwig  wrote:
>> > > Yes, that sounds useful to me as well.  As you said it's an independent
>> > > but somewhat related change.  I can add it to my series, but I'll
>> > > need a suggestions for a good and short name.  That already was the
>> > > hardest part for the setup side :)
>> >
>> > If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
>> > has), we could use one of
>> >
>> > a) start_timer(struct timer_list *timer, unsigned long ms);
>> > b) restart_timer(struct timer_list *timer, unsigned long ms);
>> > c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
>> > mod_timer_sec(struct timer_list *timer, unsigned long sec);
>>
>> Please make new functions prefixed with timer_ and get rid of that old
>> interface completely. It's horrible.
>>
>> timer_init()
>> timer_start(timer, ms, abs)
>
> I'm not even sure, whether we need absolute timer wheel timers at
> all, because most use cases are relative to now.
>
> But it's easy enough to provide them. All we need for that is something
> like
>
> unsigned long time_msec;
>
> which gets incremented every tick by the appropriate amount of
> milliseconds.
>
> Having that would also allow to replace all the
>
>end = jiffies + msec_to_jiffies(xxx);
>
>while (time_before(jiffies, end))
>  
>
> constructs with a milliseconds based machinery. So we can remove all
> *_to_jiffies() interfaces over time.

A lot of those users could probably just ktime_get()/ktime_before() here,
as they would by definition not be performance critical.

I don't see a way to just tk->tkr_mono.base but with a ktime_get_coarse()
we could just return the ktime_t of the last tick and not even need a seqlock
on 64-bit architectures, or have to introduce a new API.

Arnd

Re: RFC: better timer interface

[Arnd Bergmann]

On Sun, May 21, 2017 at 8:14 PM, Thomas Gleixner  wrote:
> On Sun, 21 May 2017, Thomas Gleixner wrote:
>> On Tue, 16 May 2017, Arnd Bergmann wrote:
>> > On Tue, May 16, 2017 at 5:51 PM, Christoph Hellwig  wrote:
>> > > Yes, that sounds useful to me as well.  As you said it's an independent
>> > > but somewhat related change.  I can add it to my series, but I'll
>> > > need a suggestions for a good and short name.  That already was the
>> > > hardest part for the setup side :)
>> >
>> > If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
>> > has), we could use one of
>> >
>> > a) start_timer(struct timer_list *timer, unsigned long ms);
>> > b) restart_timer(struct timer_list *timer, unsigned long ms);
>> > c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
>> > mod_timer_sec(struct timer_list *timer, unsigned long sec);
>>
>> Please make new functions prefixed with timer_ and get rid of that old
>> interface completely. It's horrible.
>>
>> timer_init()
>> timer_start(timer, ms, abs)
>
> I'm not even sure, whether we need absolute timer wheel timers at
> all, because most use cases are relative to now.
>
> But it's easy enough to provide them. All we need for that is something
> like
>
> unsigned long time_msec;
>
> which gets incremented every tick by the appropriate amount of
> milliseconds.
>
> Having that would also allow to replace all the
>
>end = jiffies + msec_to_jiffies(xxx);
>
>while (time_before(jiffies, end))
>  
>
> constructs with a milliseconds based machinery. So we can remove all
> *_to_jiffies() interfaces over time.

A lot of those users could probably just ktime_get()/ktime_before() here,
as they would by definition not be performance critical.

I don't see a way to just tk->tkr_mono.base but with a ktime_get_coarse()
we could just return the ktime_t of the last tick and not even need a seqlock
on 64-bit architectures, or have to introduce a new API.

Arnd

Re: RFC: better timer interface

[Thomas Gleixner]

On Sun, 21 May 2017, Thomas Gleixner wrote:
> On Tue, 16 May 2017, Arnd Bergmann wrote:
> > On Tue, May 16, 2017 at 5:51 PM, Christoph Hellwig  wrote:
> > > Yes, that sounds useful to me as well.  As you said it's an independent
> > > but somewhat related change.  I can add it to my series, but I'll
> > > need a suggestions for a good and short name.  That already was the
> > > hardest part for the setup side :)
> > 
> > If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
> > has), we could use one of
> > 
> > a) start_timer(struct timer_list *timer, unsigned long ms);
> > b) restart_timer(struct timer_list *timer, unsigned long ms);
> > c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
> > mod_timer_sec(struct timer_list *timer, unsigned long sec);
> 
> Please make new functions prefixed with timer_ and get rid of that old
> interface completely. It's horrible.
>  
> timer_init()
> timer_start(timer, ms, abs)

I'm not even sure, whether we need absolute timer wheel timers at
all, because most use cases are relative to now.

But it's easy enough to provide them. All we need for that is something
like

unsigned long time_msec;

which gets incremented every tick by the appropriate amount of
milliseconds.

Having that would also allow to replace all the

   end = jiffies + msec_to_jiffies(xxx);

   while (time_before(jiffies, end))
 

constructs with a milliseconds based machinery. So we can remove all
*_to_jiffies() interfaces over time.

Thanks,

tglx

Re: RFC: better timer interface

[Thomas Gleixner]

On Sun, 21 May 2017, Thomas Gleixner wrote:
> On Tue, 16 May 2017, Arnd Bergmann wrote:
> > On Tue, May 16, 2017 at 5:51 PM, Christoph Hellwig  wrote:
> > > Yes, that sounds useful to me as well.  As you said it's an independent
> > > but somewhat related change.  I can add it to my series, but I'll
> > > need a suggestions for a good and short name.  That already was the
> > > hardest part for the setup side :)
> > 
> > If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
> > has), we could use one of
> > 
> > a) start_timer(struct timer_list *timer, unsigned long ms);
> > b) restart_timer(struct timer_list *timer, unsigned long ms);
> > c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
> > mod_timer_sec(struct timer_list *timer, unsigned long sec);
> 
> Please make new functions prefixed with timer_ and get rid of that old
> interface completely. It's horrible.
>  
> timer_init()
> timer_start(timer, ms, abs)

I'm not even sure, whether we need absolute timer wheel timers at
all, because most use cases are relative to now.

But it's easy enough to provide them. All we need for that is something
like

unsigned long time_msec;

which gets incremented every tick by the appropriate amount of
milliseconds.

Having that would also allow to replace all the

   end = jiffies + msec_to_jiffies(xxx);

   while (time_before(jiffies, end))
 

constructs with a milliseconds based machinery. So we can remove all
*_to_jiffies() interfaces over time.

Thanks,

tglx

Re: RFC: better timer interface

[Thomas Gleixner]

On Tue, 16 May 2017, Arnd Bergmann wrote:
> On Tue, May 16, 2017 at 5:51 PM, Christoph Hellwig  wrote:
> > Yes, that sounds useful to me as well.  As you said it's an independent
> > but somewhat related change.  I can add it to my series, but I'll
> > need a suggestions for a good and short name.  That already was the
> > hardest part for the setup side :)
> 
> If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
> has), we could use one of
> 
> a) start_timer(struct timer_list *timer, unsigned long ms);
> b) restart_timer(struct timer_list *timer, unsigned long ms);
> c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
> mod_timer_sec(struct timer_list *timer, unsigned long sec);

Please make new functions prefixed with timer_ and get rid of that old
interface completely. It's horrible.
 
timer_init()
timer_start(timer, ms, abs)
timer_start_on(timer, ms, abs, cpu)
timer_cancel(timer, sync)

Is all what's required to make up a new milliseconds based interface.

We really do not need all that mod/restart/ whatever variants. Where is the
point of those?

Thanks,

tglx

Re: RFC: better timer interface

[Thomas Gleixner]

On Tue, 16 May 2017, Arnd Bergmann wrote:
> On Tue, May 16, 2017 at 5:51 PM, Christoph Hellwig  wrote:
> > Yes, that sounds useful to me as well.  As you said it's an independent
> > but somewhat related change.  I can add it to my series, but I'll
> > need a suggestions for a good and short name.  That already was the
> > hardest part for the setup side :)
> 
> If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
> has), we could use one of
> 
> a) start_timer(struct timer_list *timer, unsigned long ms);
> b) restart_timer(struct timer_list *timer, unsigned long ms);
> c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
> mod_timer_sec(struct timer_list *timer, unsigned long sec);

Please make new functions prefixed with timer_ and get rid of that old
interface completely. It's horrible.
 
timer_init()
timer_start(timer, ms, abs)
timer_start_on(timer, ms, abs, cpu)
timer_cancel(timer, sync)

Is all what's required to make up a new milliseconds based interface.

We really do not need all that mod/restart/ whatever variants. Where is the
point of those?

Thanks,

tglx

Re: RFC: better timer interface

[Christoph Hellwig]

On Tue, May 16, 2017 at 10:26:39PM +0200, Arnd Bergmann wrote:
> If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
> has), we could use one of
> 
> a) start_timer(struct timer_list *timer, unsigned long ms);
> b) restart_timer(struct timer_list *timer, unsigned long ms);
> c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
> mod_timer_sec(struct timer_list *timer, unsigned long sec);
> 
> The first is slightly shorter but conflicts with three files that use
> the same name for a local function name. The third one fits
> well with the existing interfaces and provides both millisecond
> and second versions, I'd probably go with that.

Yeah, I'd take c) as well.  I'll give it a spin.

> We could consider even passing a default interval as another
> argument to prepare_timer(), and using that in add_timer(),
> but that would in those cases that have a constant interval
> (maybe about half of the users from) and would be a bit surprising
> to readers that are only familiar with the existing interfaces.

That seems rather ugly to me.

> One final option would be a larger-scale replacement of
> the API by mirroring the hrtimer style where possible while
> staying compatible with the existing calls, e.g. timer_prepare(),
> timer_add_expires(), timer_start(), ...

I'd chose timer_* for an entirely new API, but at this point this
seems a bit too much churn to me.

Re: RFC: better timer interface

[Christoph Hellwig]

On Tue, May 16, 2017 at 10:26:39PM +0200, Arnd Bergmann wrote:
> If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
> has), we could use one of
> 
> a) start_timer(struct timer_list *timer, unsigned long ms);
> b) restart_timer(struct timer_list *timer, unsigned long ms);
> c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
> mod_timer_sec(struct timer_list *timer, unsigned long sec);
> 
> The first is slightly shorter but conflicts with three files that use
> the same name for a local function name. The third one fits
> well with the existing interfaces and provides both millisecond
> and second versions, I'd probably go with that.

Yeah, I'd take c) as well.  I'll give it a spin.

> We could consider even passing a default interval as another
> argument to prepare_timer(), and using that in add_timer(),
> but that would in those cases that have a constant interval
> (maybe about half of the users from) and would be a bit surprising
> to readers that are only familiar with the existing interfaces.

That seems rather ugly to me.

> One final option would be a larger-scale replacement of
> the API by mirroring the hrtimer style where possible while
> staying compatible with the existing calls, e.g. timer_prepare(),
> timer_add_expires(), timer_start(), ...

I'd chose timer_* for an entirely new API, but at this point this
seems a bit too much churn to me.

Re: RFC: better timer interface

[Arnd Bergmann]

On Tue, May 16, 2017 at 5:51 PM, Christoph Hellwig  wrote:
> On Tue, May 16, 2017 at 05:45:07PM +0200, Arnd Bergmann wrote:
>> This looks really nice, but what is the long-term plan for the interface?
>> Do you expect that we will eventually change all 700+ users of timer_list
>> to the new type, or do we keep both variants around indefinitely to avoid
>> having to do mass-conversions?
>
> I think we should eventually move everyone over, but it might take
> some time.

Ok.

>> If we are going to touch them all in the end, we might want to think
>> about other changes that could be useful here. The main one I have
>> in mind would be moving away from 'jiffies + timeout' as the interface,
>> and instead passing a relative number of milliseconds (or seconds)
>> into a mod_timer() variant. This is what most drivers want anyway,
>> and if we have both changes (callback argument and expiration
>> time) in place, we modernize the API one driver at a time with both
>> changes at once.
>
> Yes, that sounds useful to me as well.  As you said it's an independent
> but somewhat related change.  I can add it to my series, but I'll
> need a suggestions for a good and short name.  That already was the
> hardest part for the setup side :)

If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
has), we could use one of

a) start_timer(struct timer_list *timer, unsigned long ms);
b) restart_timer(struct timer_list *timer, unsigned long ms);
c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
mod_timer_sec(struct timer_list *timer, unsigned long sec);

The first is slightly shorter but conflicts with three files that use
the same name for a local function name. The third one fits
well with the existing interfaces and provides both millisecond
and second versions, I'd probably go with that.

We could consider even passing a default interval as another
argument to prepare_timer(), and using that in add_timer(),
but that would in those cases that have a constant interval
(maybe about half of the users from) and would be a bit surprising
to readers that are only familiar with the existing interfaces.

One final option would be a larger-scale replacement of
the API by mirroring the hrtimer style where possible while
staying compatible with the existing calls, e.g. timer_prepare(),
timer_add_expires(), timer_start(), ...

   Arnd

Re: RFC: better timer interface

[Arnd Bergmann]

On Tue, May 16, 2017 at 5:51 PM, Christoph Hellwig  wrote:
> On Tue, May 16, 2017 at 05:45:07PM +0200, Arnd Bergmann wrote:
>> This looks really nice, but what is the long-term plan for the interface?
>> Do you expect that we will eventually change all 700+ users of timer_list
>> to the new type, or do we keep both variants around indefinitely to avoid
>> having to do mass-conversions?
>
> I think we should eventually move everyone over, but it might take
> some time.

Ok.

>> If we are going to touch them all in the end, we might want to think
>> about other changes that could be useful here. The main one I have
>> in mind would be moving away from 'jiffies + timeout' as the interface,
>> and instead passing a relative number of milliseconds (or seconds)
>> into a mod_timer() variant. This is what most drivers want anyway,
>> and if we have both changes (callback argument and expiration
>> time) in place, we modernize the API one driver at a time with both
>> changes at once.
>
> Yes, that sounds useful to me as well.  As you said it's an independent
> but somewhat related change.  I can add it to my series, but I'll
> need a suggestions for a good and short name.  That already was the
> hardest part for the setup side :)

If we keep the unusual *_timer() naming (rather than timer_*() as hrtimer
has), we could use one of

a) start_timer(struct timer_list *timer, unsigned long ms);
b) restart_timer(struct timer_list *timer, unsigned long ms);
c) mod_timer_ms(struct timer_list *timer, unsigned long ms);
mod_timer_sec(struct timer_list *timer, unsigned long sec);

The first is slightly shorter but conflicts with three files that use
the same name for a local function name. The third one fits
well with the existing interfaces and provides both millisecond
and second versions, I'd probably go with that.

We could consider even passing a default interval as another
argument to prepare_timer(), and using that in add_timer(),
but that would in those cases that have a constant interval
(maybe about half of the users from) and would be a bit surprising
to readers that are only familiar with the existing interfaces.

One final option would be a larger-scale replacement of
the API by mirroring the hrtimer style where possible while
staying compatible with the existing calls, e.g. timer_prepare(),
timer_add_expires(), timer_start(), ...

   Arnd

Re: RFC: better timer interface

[Christoph Hellwig]

On Tue, May 16, 2017 at 05:45:07PM +0200, Arnd Bergmann wrote:
> This looks really nice, but what is the long-term plan for the interface?
> Do you expect that we will eventually change all 700+ users of timer_list
> to the new type, or do we keep both variants around indefinitely to avoid
> having to do mass-conversions?

I think we should eventually move everyone over, but it might take
some time.

> If we are going to touch them all in the end, we might want to think
> about other changes that could be useful here. The main one I have
> in mind would be moving away from 'jiffies + timeout' as the interface,
> and instead passing a relative number of milliseconds (or seconds)
> into a mod_timer() variant. This is what most drivers want anyway,
> and if we have both changes (callback argument and expiration
> time) in place, we modernize the API one driver at a time with both
> changes at once.

Yes, that sounds useful to me as well.  As you said it's an independent
but somewhat related change.  I can add it to my series, but I'll
need a suggestions for a good and short name.  That already was the
hardest part for the setup side :)

Re: RFC: better timer interface

[Christoph Hellwig]

On Tue, May 16, 2017 at 05:45:07PM +0200, Arnd Bergmann wrote:
> This looks really nice, but what is the long-term plan for the interface?
> Do you expect that we will eventually change all 700+ users of timer_list
> to the new type, or do we keep both variants around indefinitely to avoid
> having to do mass-conversions?

I think we should eventually move everyone over, but it might take
some time.

> If we are going to touch them all in the end, we might want to think
> about other changes that could be useful here. The main one I have
> in mind would be moving away from 'jiffies + timeout' as the interface,
> and instead passing a relative number of milliseconds (or seconds)
> into a mod_timer() variant. This is what most drivers want anyway,
> and if we have both changes (callback argument and expiration
> time) in place, we modernize the API one driver at a time with both
> changes at once.

Yes, that sounds useful to me as well.  As you said it's an independent
but somewhat related change.  I can add it to my series, but I'll
need a suggestions for a good and short name.  That already was the
hardest part for the setup side :)

Re: RFC: better timer interface

[Arnd Bergmann]

On Tue, May 16, 2017 at 1:48 PM, Christoph Hellwig  wrote:
> Hi all,
>
> this series attempts to provide a "modern" timer interface where the
> callback gets the timer_list structure as an argument so that it
> can use container_of instead of having to cast to/from unsigned long
> all the time (or even worse use function pointer casts, we have quite
> a few of those as well).

This looks really nice, but what is the long-term plan for the interface?
Do you expect that we will eventually change all 700+ users of timer_list
to the new type, or do we keep both variants around indefinitely to avoid
having to do mass-conversions?

If we are going to touch them all in the end, we might want to think
about other changes that could be useful here. The main one I have
in mind would be moving away from 'jiffies + timeout' as the interface,
and instead passing a relative number of milliseconds (or seconds)
into a mod_timer() variant. This is what most drivers want anyway,
and if we have both changes (callback argument and expiration
time) in place, we modernize the API one driver at a time with both
changes at once.

  Arnd

Re: RFC: better timer interface

[Arnd Bergmann]

On Tue, May 16, 2017 at 1:48 PM, Christoph Hellwig  wrote:
> Hi all,
>
> this series attempts to provide a "modern" timer interface where the
> callback gets the timer_list structure as an argument so that it
> can use container_of instead of having to cast to/from unsigned long
> all the time (or even worse use function pointer casts, we have quite
> a few of those as well).

This looks really nice, but what is the long-term plan for the interface?
Do you expect that we will eventually change all 700+ users of timer_list
to the new type, or do we keep both variants around indefinitely to avoid
having to do mass-conversions?

If we are going to touch them all in the end, we might want to think
about other changes that could be useful here. The main one I have
in mind would be moving away from 'jiffies + timeout' as the interface,
and instead passing a relative number of milliseconds (or seconds)
into a mod_timer() variant. This is what most drivers want anyway,
and if we have both changes (callback argument and expiration
time) in place, we modernize the API one driver at a time with both
changes at once.

  Arnd