[UPDATE PATCH] Add schedule_timeout_{interruptible,uninterruptible}_msecs() interfaces

2005-07-23 Thread Nishanth Aravamudan
On 23.07.2005 [12:30:00 +1000], Andrew Morton wrote:
> Nishanth Aravamudan <[EMAIL PROTECTED]> wrote:
> >
> > 
> > +/*
> > + * schedule_timeout_msecs - sleep until timeout
> > + * @timeout_msecs: timeout value in milliseconds
> > + *
> > + * A human-time (but otherwise identical) alternative to
> > + * schedule_timeout() The state, therefore, *does* need to be set before
> > + * calling this function, but this function should *never* be called
> > + * directly. Use the nice wrappers, schedule_{interruptible,
> > + * uninterruptible}_timeout_msecs().
> > + *
> > + * See the comment for schedule_timeout() for details.
> > + */
> > +inline unsigned int __sched schedule_timeout_msecs(unsigned int 
> > timeout_msecs)
> > +{
> 
> Making this inline will add more kernel text.Can't we uninline it?

Uninlined in the latest version (see below).

> I'm surprised that this function is not implemented as a call to the
> existing schedule_timeout()?

Well, I was considering doing that, but I tried to make this new
interface a little more sane. I don't think we need to worry about
signedness of the parameter any more (and thus we don't need to consider
returning a negative value). Calling schedule_timeout() underneath
schedule_timeout_msecs() will require us to do those conditional checks;
I guess this isn't criticial in a sleeping path.

Also, I would need to make some modifications (not critical, really), so
that MAX_SCHEDULE_TIMEOUT_MSECS would map 1:1 to MAX_SCHEDULE_TIMEOUT
(which is in jiffies).

But I'm happy to change this, also done below -- does that map better to
what you were thinking?

> > +   init_timer();
> > +   timer.expires = expire_jifs;
> > +   timer.data = (unsigned long) current;
> > +   timer.function = process_timeout;
> 
> hm, if we add the needed typecast to TIMER_INITIALIZER() the above could be
> 
>   timer = TIMER_INITIALIZER(process_timeout, expire_jifs,
>   (unsigned long)current);

Done as well below.

Thanks,
Nish

Description: Add millisecond wrapper for schedule_timeout(),
schedule_timeout_msecs(). Add wrappers for interruptible/uninterruptible
schedule_timeout_msecs() callers.

Signed-off-by: Nishanth Aravamudan <[EMAIL PROTECTED]>

---

 include/linux/sched.h |7 
 kernel/timer.c|   75 ++
 2 files changed, 81 insertions(+), 1 deletion(-)

diff -urpN 2.6.13-rc3/include/linux/sched.h 
2.6.13-rc3-new_interfaces/include/linux/sched.h
--- 2.6.13-rc3/include/linux/sched.h2005-07-13 15:52:14.0 -0700
+++ 2.6.13-rc3-new_interfaces/include/linux/sched.h 2005-07-22 
18:06:36.0 -0700
@@ -181,8 +181,13 @@ extern void scheduler_tick(void);
 /* Is this address in the __sched functions? */
 extern int in_sched_functions(unsigned long addr);
 
-#defineMAX_SCHEDULE_TIMEOUTLONG_MAX
+#defineMAX_SCHEDULE_TIMEOUTLONG_MAX
+#defineMAX_SCHEDULE_TIMEOUT_MSECS  UINT_MAX
 extern signed long FASTCALL(schedule_timeout(signed long timeout));
+extern unsigned int schedule_timeout_msecs_interruptible
+   (unsigned int timeout_msecs);
+extern unsigned int schedule_timeout_msecs_uninterruptible
+   (unsigned int timeout_msecs);
 asmlinkage void schedule(void);
 
 struct namespace;
diff -urpN 2.6.13-rc3/kernel/timer.c 2.6.13-rc3-new_interfaces/kernel/timer.c
--- 2.6.13-rc3/kernel/timer.c   2005-07-13 15:52:14.0 -0700
+++ 2.6.13-rc3-new_interfaces/kernel/timer.c2005-07-23 09:20:51.0 
-0700
@@ -1153,6 +1153,81 @@ fastcall signed long __sched schedule_ti
 
 EXPORT_SYMBOL(schedule_timeout);
 
+/*
+ * schedule_timeout_msecs - sleep until timeout
+ * @timeout_msecs: timeout value in milliseconds
+ *
+ * A human-time (but otherwise identical) alternative to
+ * schedule_timeout() The state, therefore, *does* need to be set before
+ * calling this function, but this function should *never* be called
+ * directly. Use the nice wrappers, schedule_{interruptible,
+ * uninterruptible}_timeout_msecs().
+ *
+ * See the comment for schedule_timeout() for details.
+ */
+unsigned int __sched schedule_timeout_msecs(unsigned int timeout_msecs)
+{
+   unsigned long expire_jifs;
+
+   if (timeout_msecs == MAX_SCHEDULE_TIMEOUT_MSECS) {
+   expire_jifs = MAX_SCHEDULE_TIMEOUT;
+   } else {
+   /*
+* msecs_to_jiffies() is a unit conversion, which truncates
+* (rounds down), so we need to add 1.
+*/
+   expire_jifs = msecs_to_jiffies(timeout_msecs) + 1;
+   }
+
+   expire_jifs = schedule_timeout(expire_jifs);
+
+   /*
+* don't need to add 1 here, even though there is truncation,
+* because we will add 1 if/when the value is sent back in
+*/
+   return jiffies_to_msecs(expire_jifs);
+}
+
+/**
+ * schedule_timeout_msecs_interruptible - sleep 

[UPDATE PATCH] Add schedule_timeout_{interruptible,uninterruptible}_msecs() interfaces

2005-07-23 Thread Nishanth Aravamudan
On 23.07.2005 [12:30:00 +1000], Andrew Morton wrote:
 Nishanth Aravamudan [EMAIL PROTECTED] wrote:
 
  
  +/*
  + * schedule_timeout_msecs - sleep until timeout
  + * @timeout_msecs: timeout value in milliseconds
  + *
  + * A human-time (but otherwise identical) alternative to
  + * schedule_timeout() The state, therefore, *does* need to be set before
  + * calling this function, but this function should *never* be called
  + * directly. Use the nice wrappers, schedule_{interruptible,
  + * uninterruptible}_timeout_msecs().
  + *
  + * See the comment for schedule_timeout() for details.
  + */
  +inline unsigned int __sched schedule_timeout_msecs(unsigned int 
  timeout_msecs)
  +{
 
 Making this inline will add more kernel text.Can't we uninline it?

Uninlined in the latest version (see below).

 I'm surprised that this function is not implemented as a call to the
 existing schedule_timeout()?

Well, I was considering doing that, but I tried to make this new
interface a little more sane. I don't think we need to worry about
signedness of the parameter any more (and thus we don't need to consider
returning a negative value). Calling schedule_timeout() underneath
schedule_timeout_msecs() will require us to do those conditional checks;
I guess this isn't criticial in a sleeping path.

Also, I would need to make some modifications (not critical, really), so
that MAX_SCHEDULE_TIMEOUT_MSECS would map 1:1 to MAX_SCHEDULE_TIMEOUT
(which is in jiffies).

But I'm happy to change this, also done below -- does that map better to
what you were thinking?

  +   init_timer(timer);
  +   timer.expires = expire_jifs;
  +   timer.data = (unsigned long) current;
  +   timer.function = process_timeout;
 
 hm, if we add the needed typecast to TIMER_INITIALIZER() the above could be
 
   timer = TIMER_INITIALIZER(process_timeout, expire_jifs,
   (unsigned long)current);

Done as well below.

Thanks,
Nish

Description: Add millisecond wrapper for schedule_timeout(),
schedule_timeout_msecs(). Add wrappers for interruptible/uninterruptible
schedule_timeout_msecs() callers.

Signed-off-by: Nishanth Aravamudan [EMAIL PROTECTED]

---

 include/linux/sched.h |7 
 kernel/timer.c|   75 ++
 2 files changed, 81 insertions(+), 1 deletion(-)

diff -urpN 2.6.13-rc3/include/linux/sched.h 
2.6.13-rc3-new_interfaces/include/linux/sched.h
--- 2.6.13-rc3/include/linux/sched.h2005-07-13 15:52:14.0 -0700
+++ 2.6.13-rc3-new_interfaces/include/linux/sched.h 2005-07-22 
18:06:36.0 -0700
@@ -181,8 +181,13 @@ extern void scheduler_tick(void);
 /* Is this address in the __sched functions? */
 extern int in_sched_functions(unsigned long addr);
 
-#defineMAX_SCHEDULE_TIMEOUTLONG_MAX
+#defineMAX_SCHEDULE_TIMEOUTLONG_MAX
+#defineMAX_SCHEDULE_TIMEOUT_MSECS  UINT_MAX
 extern signed long FASTCALL(schedule_timeout(signed long timeout));
+extern unsigned int schedule_timeout_msecs_interruptible
+   (unsigned int timeout_msecs);
+extern unsigned int schedule_timeout_msecs_uninterruptible
+   (unsigned int timeout_msecs);
 asmlinkage void schedule(void);
 
 struct namespace;
diff -urpN 2.6.13-rc3/kernel/timer.c 2.6.13-rc3-new_interfaces/kernel/timer.c
--- 2.6.13-rc3/kernel/timer.c   2005-07-13 15:52:14.0 -0700
+++ 2.6.13-rc3-new_interfaces/kernel/timer.c2005-07-23 09:20:51.0 
-0700
@@ -1153,6 +1153,81 @@ fastcall signed long __sched schedule_ti
 
 EXPORT_SYMBOL(schedule_timeout);
 
+/*
+ * schedule_timeout_msecs - sleep until timeout
+ * @timeout_msecs: timeout value in milliseconds
+ *
+ * A human-time (but otherwise identical) alternative to
+ * schedule_timeout() The state, therefore, *does* need to be set before
+ * calling this function, but this function should *never* be called
+ * directly. Use the nice wrappers, schedule_{interruptible,
+ * uninterruptible}_timeout_msecs().
+ *
+ * See the comment for schedule_timeout() for details.
+ */
+unsigned int __sched schedule_timeout_msecs(unsigned int timeout_msecs)
+{
+   unsigned long expire_jifs;
+
+   if (timeout_msecs == MAX_SCHEDULE_TIMEOUT_MSECS) {
+   expire_jifs = MAX_SCHEDULE_TIMEOUT;
+   } else {
+   /*
+* msecs_to_jiffies() is a unit conversion, which truncates
+* (rounds down), so we need to add 1.
+*/
+   expire_jifs = msecs_to_jiffies(timeout_msecs) + 1;
+   }
+
+   expire_jifs = schedule_timeout(expire_jifs);
+
+   /*
+* don't need to add 1 here, even though there is truncation,
+* because we will add 1 if/when the value is sent back in
+*/
+   return jiffies_to_msecs(expire_jifs);
+}
+
+/**
+ * schedule_timeout_msecs_interruptible - sleep until timeout,
+ *   

Re: [UPDATE PATCH] Add schedule_timeout_{interruptible,uninterruptible}_msecs() interfaces

2005-07-22 Thread Andrew Morton
Nishanth Aravamudan <[EMAIL PROTECTED]> wrote:
>
> 
> +/*
> + * schedule_timeout_msecs - sleep until timeout
> + * @timeout_msecs: timeout value in milliseconds
> + *
> + * A human-time (but otherwise identical) alternative to
> + * schedule_timeout() The state, therefore, *does* need to be set before
> + * calling this function, but this function should *never* be called
> + * directly. Use the nice wrappers, schedule_{interruptible,
> + * uninterruptible}_timeout_msecs().
> + *
> + * See the comment for schedule_timeout() for details.
> + */
> +inline unsigned int __sched schedule_timeout_msecs(unsigned int 
> timeout_msecs)
> +{

Making this inline will add more kernel text.Can't we uninline it?

I'm surprised that this function is not implemented as a call to the
existing schedule_timeout()?

> + init_timer();
> + timer.expires = expire_jifs;
> + timer.data = (unsigned long) current;
> + timer.function = process_timeout;

hm, if we add the needed typecast to TIMER_INITIALIZER() the above could be

timer = TIMER_INITIALIZER(process_timeout, expire_jifs,
(unsigned long)current);


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[UPDATE PATCH] Add schedule_timeout_{interruptible,uninterruptible}_msecs() interfaces

2005-07-22 Thread Nishanth Aravamudan
On 22.07.2005 [20:31:55 -0400], Arjan van de Ven wrote:
> 
> > How does something like this look? If this looks ok, I'll send out
> > bunches of patches to add users of the new interfaces.
> 
> I'd drop the FASTCALL stuff... nowadays with regparm that's automatic
> and the cost of register-vs-stack isn't too big anyway
> 
> 
> Also I'd rather not add the non-msec ones... either you're raw and use
> HZ, or you are "cooked" and use the msec variant.. I dont' see the point
> of adding an "in the middle" one. (Yes this means that several users
> need to be transformed to msecs but... I consider that progress ;)

Thanks for the feedback! Does this look better? I will be more than
happy to convert those users to msecs which should, btw :) I've been
waiting to add this interface so that the ones that couldn't use
msleep{,_interruptible}() because of wait-queues, can also be changed to
use milliseconds.

Hrm, I also noticed I typo'd the externs in sched.h. Fixed in the new
patch.

Description: Add schedule_timeout_msecs() and add wrappers for
interruptible/uninterruptible schedule_timeout_msecs() callers.

Signed-off-by: Nishanth Aravamudan <[EMAIL PROTECTED]>

---

 include/linux/sched.h |7 +++
 kernel/timer.c|   94 ++
 2 files changed, 100 insertions(+), 1 deletion(-)

diff -urpN 2.6.13-rc3/include/linux/sched.h 
2.6.13-rc3-new_interfaces/include/linux/sched.h
--- 2.6.13-rc3/include/linux/sched.h2005-07-13 15:52:14.0 -0700
+++ 2.6.13-rc3-new_interfaces/include/linux/sched.h 2005-07-22 
18:06:36.0 -0700
@@ -181,8 +181,13 @@ extern void scheduler_tick(void);
 /* Is this address in the __sched functions? */
 extern int in_sched_functions(unsigned long addr);
 
-#defineMAX_SCHEDULE_TIMEOUTLONG_MAX
+#defineMAX_SCHEDULE_TIMEOUTLONG_MAX
+#defineMAX_SCHEDULE_TIMEOUT_MSECS  UINT_MAX
 extern signed long FASTCALL(schedule_timeout(signed long timeout));
+extern unsigned int schedule_timeout_msecs_interruptible
+   (unsigned int timeout_msecs);
+extern unsigned int schedule_timeout_msecs_uninterruptible
+   (unsigned int timeout_msecs);
 asmlinkage void schedule(void);
 
 struct namespace;
diff -urpN 2.6.13-rc3/kernel/timer.c 2.6.13-rc3-new_interfaces/kernel/timer.c
--- 2.6.13-rc3/kernel/timer.c   2005-07-13 15:52:14.0 -0700
+++ 2.6.13-rc3-new_interfaces/kernel/timer.c2005-07-22 18:00:46.0 
-0700
@@ -1153,6 +1153,100 @@ fastcall signed long __sched schedule_ti
 
 EXPORT_SYMBOL(schedule_timeout);
 
+/*
+ * schedule_timeout_msecs - sleep until timeout
+ * @timeout_msecs: timeout value in milliseconds
+ *
+ * A human-time (but otherwise identical) alternative to
+ * schedule_timeout() The state, therefore, *does* need to be set before
+ * calling this function, but this function should *never* be called
+ * directly. Use the nice wrappers, schedule_{interruptible,
+ * uninterruptible}_timeout_msecs().
+ *
+ * See the comment for schedule_timeout() for details.
+ */
+inline unsigned int __sched schedule_timeout_msecs(unsigned int timeout_msecs)
+{
+   struct timer_list timer;
+   unsigned long expire_jifs;
+   signed long remaining_jifs;
+
+   if (timeout_msecs == MAX_SCHEDULE_TIMEOUT_MSECS) {
+   schedule();
+   goto out;
+   }
+
+   /*
+* msecs_to_jiffies() is a unit conversion, which truncates
+* (rounds down), so we need to add 1.
+*/
+   expire_jifs = jiffies + msecs_to_jiffies(timeout_msecs) + 1;
+
+   init_timer();
+   timer.expires = expire_jifs;
+   timer.data = (unsigned long) current;
+   timer.function = process_timeout;
+
+   add_timer();
+   schedule();
+   del_singleshot_timer_sync();
+
+   remaining_jifs = expire_jifs - jiffies;
+   /* if we have woken up *before* we have requested */
+   if (remaining_jifs > 0)
+   /*
+* don't need to add 1 here, even though there is
+* truncation, because we will add 1 if/when the value
+* is sent back in
+*/
+   timeout_msecs = jiffies_to_msecs(remaining_jifs);
+   else
+   timeout_msecs = 0;
+
+ out:
+   return timeout_msecs;
+}
+
+/**
+ * schedule_timeout_msecs_interruptible - sleep until timeout,
+ *  wait-queue event, or signal
+ * @timeout_msecs: timeout value in milliseconds
+ *
+ * A nice wrapper for the common
+ * set_current_state()/schedule_timeout_msecs() usage.  The state,
+ * therefore, does *not* need to be set before calling this function.
+ *
+ * See the comment for schedule_timeout() for details.
+ */
+unsigned int __sched schedule_timeout_msecs_interruptible
+   (unsigned int timeout_msecs)
+{
+   

[UPDATE PATCH] Add schedule_timeout_{interruptible,uninterruptible}_msecs() interfaces

2005-07-22 Thread Nishanth Aravamudan
On 22.07.2005 [20:31:55 -0400], Arjan van de Ven wrote:
 
  How does something like this look? If this looks ok, I'll send out
  bunches of patches to add users of the new interfaces.
 
 I'd drop the FASTCALL stuff... nowadays with regparm that's automatic
 and the cost of register-vs-stack isn't too big anyway
 
 
 Also I'd rather not add the non-msec ones... either you're raw and use
 HZ, or you are cooked and use the msec variant.. I dont' see the point
 of adding an in the middle one. (Yes this means that several users
 need to be transformed to msecs but... I consider that progress ;)

Thanks for the feedback! Does this look better? I will be more than
happy to convert those users to msecs which should, btw :) I've been
waiting to add this interface so that the ones that couldn't use
msleep{,_interruptible}() because of wait-queues, can also be changed to
use milliseconds.

Hrm, I also noticed I typo'd the externs in sched.h. Fixed in the new
patch.

Description: Add schedule_timeout_msecs() and add wrappers for
interruptible/uninterruptible schedule_timeout_msecs() callers.

Signed-off-by: Nishanth Aravamudan [EMAIL PROTECTED]

---

 include/linux/sched.h |7 +++
 kernel/timer.c|   94 ++
 2 files changed, 100 insertions(+), 1 deletion(-)

diff -urpN 2.6.13-rc3/include/linux/sched.h 
2.6.13-rc3-new_interfaces/include/linux/sched.h
--- 2.6.13-rc3/include/linux/sched.h2005-07-13 15:52:14.0 -0700
+++ 2.6.13-rc3-new_interfaces/include/linux/sched.h 2005-07-22 
18:06:36.0 -0700
@@ -181,8 +181,13 @@ extern void scheduler_tick(void);
 /* Is this address in the __sched functions? */
 extern int in_sched_functions(unsigned long addr);
 
-#defineMAX_SCHEDULE_TIMEOUTLONG_MAX
+#defineMAX_SCHEDULE_TIMEOUTLONG_MAX
+#defineMAX_SCHEDULE_TIMEOUT_MSECS  UINT_MAX
 extern signed long FASTCALL(schedule_timeout(signed long timeout));
+extern unsigned int schedule_timeout_msecs_interruptible
+   (unsigned int timeout_msecs);
+extern unsigned int schedule_timeout_msecs_uninterruptible
+   (unsigned int timeout_msecs);
 asmlinkage void schedule(void);
 
 struct namespace;
diff -urpN 2.6.13-rc3/kernel/timer.c 2.6.13-rc3-new_interfaces/kernel/timer.c
--- 2.6.13-rc3/kernel/timer.c   2005-07-13 15:52:14.0 -0700
+++ 2.6.13-rc3-new_interfaces/kernel/timer.c2005-07-22 18:00:46.0 
-0700
@@ -1153,6 +1153,100 @@ fastcall signed long __sched schedule_ti
 
 EXPORT_SYMBOL(schedule_timeout);
 
+/*
+ * schedule_timeout_msecs - sleep until timeout
+ * @timeout_msecs: timeout value in milliseconds
+ *
+ * A human-time (but otherwise identical) alternative to
+ * schedule_timeout() The state, therefore, *does* need to be set before
+ * calling this function, but this function should *never* be called
+ * directly. Use the nice wrappers, schedule_{interruptible,
+ * uninterruptible}_timeout_msecs().
+ *
+ * See the comment for schedule_timeout() for details.
+ */
+inline unsigned int __sched schedule_timeout_msecs(unsigned int timeout_msecs)
+{
+   struct timer_list timer;
+   unsigned long expire_jifs;
+   signed long remaining_jifs;
+
+   if (timeout_msecs == MAX_SCHEDULE_TIMEOUT_MSECS) {
+   schedule();
+   goto out;
+   }
+
+   /*
+* msecs_to_jiffies() is a unit conversion, which truncates
+* (rounds down), so we need to add 1.
+*/
+   expire_jifs = jiffies + msecs_to_jiffies(timeout_msecs) + 1;
+
+   init_timer(timer);
+   timer.expires = expire_jifs;
+   timer.data = (unsigned long) current;
+   timer.function = process_timeout;
+
+   add_timer(timer);
+   schedule();
+   del_singleshot_timer_sync(timer);
+
+   remaining_jifs = expire_jifs - jiffies;
+   /* if we have woken up *before* we have requested */
+   if (remaining_jifs  0)
+   /*
+* don't need to add 1 here, even though there is
+* truncation, because we will add 1 if/when the value
+* is sent back in
+*/
+   timeout_msecs = jiffies_to_msecs(remaining_jifs);
+   else
+   timeout_msecs = 0;
+
+ out:
+   return timeout_msecs;
+}
+
+/**
+ * schedule_timeout_msecs_interruptible - sleep until timeout,
+ *  wait-queue event, or signal
+ * @timeout_msecs: timeout value in milliseconds
+ *
+ * A nice wrapper for the common
+ * set_current_state()/schedule_timeout_msecs() usage.  The state,
+ * therefore, does *not* need to be set before calling this function.
+ *
+ * See the comment for schedule_timeout() for details.
+ */
+unsigned int __sched schedule_timeout_msecs_interruptible
+   (unsigned int timeout_msecs)
+{
+   

Re: [UPDATE PATCH] Add schedule_timeout_{interruptible,uninterruptible}_msecs() interfaces

2005-07-22 Thread Andrew Morton
Nishanth Aravamudan [EMAIL PROTECTED] wrote:

 
 +/*
 + * schedule_timeout_msecs - sleep until timeout
 + * @timeout_msecs: timeout value in milliseconds
 + *
 + * A human-time (but otherwise identical) alternative to
 + * schedule_timeout() The state, therefore, *does* need to be set before
 + * calling this function, but this function should *never* be called
 + * directly. Use the nice wrappers, schedule_{interruptible,
 + * uninterruptible}_timeout_msecs().
 + *
 + * See the comment for schedule_timeout() for details.
 + */
 +inline unsigned int __sched schedule_timeout_msecs(unsigned int 
 timeout_msecs)
 +{

Making this inline will add more kernel text.Can't we uninline it?

I'm surprised that this function is not implemented as a call to the
existing schedule_timeout()?

 + init_timer(timer);
 + timer.expires = expire_jifs;
 + timer.data = (unsigned long) current;
 + timer.function = process_timeout;

hm, if we add the needed typecast to TIMER_INITIALIZER() the above could be

timer = TIMER_INITIALIZER(process_timeout, expire_jifs,
(unsigned long)current);


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/