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

[Sven-Thorsten Dietrich <sven-IsH+rWyeNGyzjR9+/8zPv5owlv4uC7bZ@public.gmane.org>]

On Wed, 2009-11-18 at 22:34 +0100, Thomas Gleixner wrote:
> On Wed, 18 Nov 2009, Sven-Thorsten Dietrich wrote:
> 
> > On Wed, 2009-11-18 at 21:56 +0100, Thomas Gleixner wrote:
> > > On Wed, 18 Nov 2009, Sven-Thorsten Dietrich wrote:
> > > > On Wed, 2009-11-18 at 17:52 +0100, Jean Delvare wrote:
> > > > > On Wed, 18 Nov 2009 17:28:53 +0100, Leon Woestenberg wrote:
> > > > > > On Wed, Nov 18, 2009 at 2:05 AM, Alan Cox <alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org> wrote:
> > > > > > > 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.
> > >  
> > > > 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 ?
> 
> >> 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 <sdietrich-l3A5Bk7waGM@public.gmane.org>

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

Acked-by: Sven-Thorsten Dietrich <sdietrich-l3A5Bk7waGM@public.gmane.org>

---
 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);