Thread View: gmane.linux.kernel
6 messages
6 total messages
Started by Oleg Nesterov
Sun, 18 Sep 2005 17:51
[PATCH] introduce setup_timer() helper
Author: Oleg Nesterov
Date: Sun, 18 Sep 2005 17:51
Date: Sun, 18 Sep 2005 17:51
40 lines
1331 bytes
1331 bytes
Every user of init_timer() also needs to initialize ->function and
->data fields. This patch adds a simple setup_timer() helper for that.
The schedule_timeout() is patched as an example of usage.
Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
--- 2.6.14-rc1/include/linux/timer.h~4_SETUP 2005-09-17 18:57:30.000000000 +0400
+++ 2.6.14-rc1/include/linux/timer.h 2005-09-18 20:55:15.000000000 +0400
@@ -38,6 +38,15 @@ extern struct timer_base_s __init_timer_
void fastcall init_timer(struct timer_list * timer);
+static inline void setup_timer(struct timer_list * timer,
+ void (*function)(unsigned long),
+ unsigned long data)
+{
+ timer->function = function;
+ timer->data = data;
+ init_timer(timer);
+}
+
/***
* timer_pending - is a timer pending?
* @timer: the timer in question
--- 2.6.14-rc1/kernel/timer.c~4_SETUP 2005-09-17 18:57:30.000000000 +0400
+++ 2.6.14-rc1/kernel/timer.c 2005-09-18 20:59:43.000000000 +0400
@@ -1137,12 +1137,8 @@ fastcall signed long __sched schedule_ti
expire = timeout + jiffies;
- init_timer(&timer);
- timer.expires = expire;
- timer.data = (unsigned long) current;
- timer.function = process_timeout;
-
- add_timer(&timer);
+ setup_timer(&timer, process_timeout, (unsigned long)current);
+ __mod_timer(&timer, expire);
schedule();
del_singleshot_timer_sync(&timer);
Re: [PATCH] introduce setup_timer() helper
Author: Arjan van de Ven
Date: Sun, 18 Sep 2005 11:12
Date: Sun, 18 Sep 2005 11:12
29 lines
673 bytes
673 bytes
--=-BmeL1qHC7K3/U3gQjNdl
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable
> unsigned long data)
> +{
> + timer->function = function;
> + timer->data = data;
> + init_timer(timer);
> +}
are you sure you want to do this in this order???
I'd expect the init_timer to be first...
--=-BmeL1qHC7K3/U3gQjNdl
Content-Type: application/pgp-signature; name=signature.asc
Content-Description: This is a digitally signed message part
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
iD8DBQBDLYPxpv2rCoFn+CIRAl0LAJ9vKYUNxD1bHlRQtQufGAQ/4btZQwCfQKtl
aQs39LwJyzOpN7eSVar68SY=NG7S
-----END PGP SIGNATURE-----
--=-BmeL1qHC7K3/U3gQjNdl--
Re: [PATCH] introduce setup_timer() helper
Author: Andrew Morton
Date: Sun, 18 Sep 2005 13:06
Date: Sun, 18 Sep 2005 13:06
9 lines
321 bytes
321 bytes
Oleg Nesterov <oleg@tv-sign.ru> wrote: > > I think this can save a couple of cpu cycles. The init_timer() > is not inline, gcc can't reorder exprx() and init_timer() calls. > > Ok, I do not want to persist very much, I can resend this patch. > > Andrew, should I? Try both, see which one generates the shorter code?
Re: [PATCH] introduce setup_timer() helper
Author: Arjan van de Ven
Date: Sun, 18 Sep 2005 17:43
Date: Sun, 18 Sep 2005 17:43
28 lines
1012 bytes
1012 bytes
On Sun, Sep 18, 2005 at 07:51:20PM +0400, Oleg Nesterov wrote:
> Arjan van de Ven wrote:
> >
> > > unsigned long data)
> > > +{
> > > + timer->function = function;
> > > + timer->data = data;
> > > + init_timer(timer);
> > > +}
> >
> > are you sure you want to do this in this order???
> > I'd expect the init_timer to be first...
>
> I think it does not matter from correctness point of view.
right now.. it probably doesn't.
However I think conceptually, touching a timer before init_timer() is just
wrong. For one... it would prevent init_timer() from being able to use
memset() on the timer. Which it doesn't today but it's the kind of thing
that you don't want to prevent happening in the future.
> setup_timer(timer, expr1(), expr2())
>
> it is better to initialize ->func and ->data first, otherwise
> the compiler should save the results from expr{1,2}, then call
> init_timer(), then copy these results to *timer.
I don't see how that is different....
Re: [PATCH] introduce setup_timer() helper
Author: Oleg Nesterov
Date: Sun, 18 Sep 2005 19:51
Date: Sun, 18 Sep 2005 19:51
22 lines
587 bytes
587 bytes
Arjan van de Ven wrote:
>
> > unsigned long data)
> > +{
> > + timer->function = function;
> > + timer->data = data;
> > + init_timer(timer);
> > +}
>
> are you sure you want to do this in this order???
> I'd expect the init_timer to be first...
I think it does not matter from correctness point of view.
But if we have:
setup_timer(timer, expr1(), expr2())
it is better to initialize ->func and ->data first, otherwise
the compiler should save the results from expr{1,2}, then call
init_timer(), then copy these results to *timer.
Oleg.
Re: [PATCH] introduce setup_timer() helper
Author: Oleg Nesterov
Date: Sun, 18 Sep 2005 20:22
Date: Sun, 18 Sep 2005 20:22
37 lines
1270 bytes
1270 bytes
Arjan van de Ven wrote:
>
> On Sun, Sep 18, 2005 at 07:51:20PM +0400, Oleg Nesterov wrote:
> >
> > I think it does not matter from correctness point of view.
>
> right now.. it probably doesn't.
> However I think conceptually, touching a timer before init_timer() is just
> wrong.
It is indeed wrong outside timer.{h,c}, but setup_timer() is a
part of timers subsystem, so I hope it's ok.
> For one... it would prevent init_timer() from being able to use
> memset() on the timer. Which it doesn't today but it's the kind of thing
> that you don't want to prevent happening in the future.
Yes, in that case we will have to change setup_timer(). But I
doubt this will happen. init_timer() only needs to set timer's
->base, and to ensure the timer is not pending.
>
> > setup_timer(timer, expr1(), expr2())
> >
> > it is better to initialize ->func and ->data first, otherwise
> > the compiler should save the results from expr{1,2}, then call
> > init_timer(), then copy these results to *timer.
>
> I don't see how that is different....
I think this can save a couple of cpu cycles. The init_timer()
is not inline, gcc can't reorder exprx() and init_timer() calls.
Ok, I do not want to persist very much, I can resend this patch.
Andrew, should I?
Oleg.
Thread Navigation
This is a paginated view of messages in the thread with full content displayed inline.
Messages are displayed in chronological order, with the original post highlighted in green.
Use pagination controls to navigate through all messages in large threads.
Back to All Threads