Re: [PATCH] cleanup sched_yield (sys)call nesting.

On Wed, 2009-11-18 at 22:34 +0100, Thomas Gleixner wrote:

On Wed, 18 Nov 2009, Sven-Thorsten Dietrich wrote:

quoted

On Wed, 2009-11-18 at 21:56 +0100, Thomas Gleixner wrote:

quoted

On Wed, 18 Nov 2009, Sven-Thorsten Dietrich wrote:

quoted

On Wed, 2009-11-18 at 17:52 +0100, Jean Delvare wrote:

quoted

On Wed, 18 Nov 2009 17:28:53 +0100, Leon Woestenberg wrote:

quoted

On Wed, Nov 18, 2009 at 2:05 AM, Alan Cox [off-list ref] wrote:

quoted

Our timers are very efficient and some day we will need to make jiffies a
function and stop the timer ticking for best performance. At that point
timers are probably the most efficient way to do much of this.

The problem with I2C bitbanged is the stringent timing, we need a way
to have fine-grained sleeping
mixed with real-time tasks in order to make this work.

FWIW, the problem that was initially reported has nothing to do with
this. i2c-algo-bit used mdelay() during transactions, not yield().
yield() is used only in once place, _between_ transactions attempts.
There are no strict timing constraints there.

I agree that dropping out sched_yield entirely should maybe start by
deprecating / flagging as a warning in sched_rt.c.

Errm, that's unrelated to sched_rt.c. 

yield() in the kernel in general is needs to be deprecated.
 

quoted

This is just a minimal cleanup I stumbled across while looking at it -
to get away from the uglyness of calling into the syscall interface from
inside the Kernel.

And why exactly is that ugly ?

Calling from a function returning void into a non-void function and then
ignoring the return code ?

 
Care to read what I wrote further down ?

quoted

quoted

Which is completely irrelevant because the return code is always 0. 

We are trying to get rid of __sched_yield calls from-inside-the-Kernel,
but sys_sched_yield() from user-space will remain.

This patch breaks out the in-Kernel interface for the yield()
functionality and deprecates it explicitly.

The sys_sched_yield() variety, however is not deprecated.

The objective is to deprecate *only* the in-kernel calls to
sched_yield(), in hopes of reducing new calls to sched_yield() being
added.

Eventually, when the in-Kernel calls are gone, the __sched_yield() would
be removed, and the first 2 hunks would essentially be reverted, leaving
only the user-space caller sys_sched_yield. 

For the time being we add 2 lines and 2 braces of bulk, in hopes that
elsewhere this eliminates more __sched_yield() calls being added while
we work to eliminate the ones that exist already.

In regards to the return code, maybe we can talk about returning an
error when an RT task calls sys_sched_yield(). But that is another
topic.

Thanks,

Sven



Subject: Deprecate in-Kernel use of __sched_yield()
From: Sven-Thorsten Dietrich <redacted>

Break out the syscall interface from the deprecated in-kernel
sched_yield() interface that is to be removed.

Acked-by: Sven-Thorsten Dietrich <redacted>

---
 kernel/sched.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Index: linux-2.6.31-openSUSE-11.2/kernel/sched.c
===================================================================

--- linux-2.6.31-openSUSE-11.2.orig/kernel/sched.c
+++ linux-2.6.31-openSUSE-11.2/kernel/sched.c

@@ -6566,12 +6566,12 @@ SYSCALL_DEFINE3(sched_getaffinity, pid_t
 }
 
 /**
- * sys_sched_yield - yield the current processor to other threads.
+ * do_sched_yield - yield the current processor to other threads.
  *
  * This function yields the current CPU to other tasks. If there are no
  * other threads running on this CPU then this function will return.
  */
-SYSCALL_DEFINE0(sched_yield)
+static inline void do_sched_yield(void) 
 {
 	struct rq *rq = this_rq_lock();
 

@@ -6588,7 +6588,11 @@ SYSCALL_DEFINE0(sched_yield)
 	preempt_enable_no_resched();
 
 	schedule();
+}
 
+SYSCALL_DEFINE0(sched_yield)
+{
+	do_sched_yield();
 	return 0;
 }
 

@@ -6670,10 +6674,10 @@ EXPORT_SYMBOL(cond_resched_softirq);
  * This is a shortcut for kernel-space yielding - it marks the
  * thread runnable and calls sys_sched_yield().
  */
-void __sched yield(void)
+void __sched __deprecated yield(void)
 {
 	set_current_state(TASK_RUNNING);
-	sys_sched_yield();
+	do_sched_yield();
 }
 EXPORT_SYMBOL(yield);