Re: [PATCH [RT] 11/14] optimize the !printk fastpath through the lock acquisition

2008-02-22 Thread Pavel Machek 
Hi!

 Decorate the printk path with an unlikely()
 
 Signed-off-by: Gregory Haskins [EMAIL PROTECTED]
 ---
 
  kernel/rtmutex.c |8 
  1 files changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
 index 122f143..ebdaa17 100644
 --- a/kernel/rtmutex.c
 +++ b/kernel/rtmutex.c
 @@ -660,12 +660,12 @@ rt_spin_lock_fastlock(struct rt_mutex *lock,
   void fastcall (*slowfn)(struct rt_mutex *lock))
  {
   /* Temporary HACK! */
 - if (!current-in_printk)
 - might_sleep();
 - else if (in_atomic() || irqs_disabled())
 + if (unlikely(current-in_printk)  (in_atomic() || irqs_disabled()))
   /* don't grab locks for printk in atomic */
   return;
  
 + might_sleep();

I think you changed the code here... you call might_sleep() in
different cases afaict.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line unsubscribe linux-rt-users in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH [RT] 11/14] optimize the !printk fastpath through the lock acquisition

2008-02-22 Thread Gregory Haskins 

Pavel Machek wrote:

Hi!


Decorate the printk path with an unlikely()

Signed-off-by: Gregory Haskins [EMAIL PROTECTED]
---

 kernel/rtmutex.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
index 122f143..ebdaa17 100644
--- a/kernel/rtmutex.c
+++ b/kernel/rtmutex.c
@@ -660,12 +660,12 @@ rt_spin_lock_fastlock(struct rt_mutex *lock,
void fastcall (*slowfn)(struct rt_mutex *lock))
 {
/* Temporary HACK! */
-   if (!current-in_printk)
-   might_sleep();
-   else if (in_atomic() || irqs_disabled())
+   if (unlikely(current-in_printk)  (in_atomic() || irqs_disabled()))
/* don't grab locks for printk in atomic */
return;
 
+	might_sleep();


I think you changed the code here... you call might_sleep() in
different cases afaict.


Agreed, but it's still correct afaict.  I added an extra might_sleep() 
to a path that really might sleep.  I should have mentioned that in the 
header.


In any case, its moot.  Andi indicated this patch is probably a no-op so 
I was considering dropping it on the v2 pass.


Regards,
-Greg



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

Re: [PATCH [RT] 11/14] optimize the !printk fastpath through the lock acquisition

2008-02-22 Thread Bill Huey (hui) 
On Fri, Feb 22, 2008 at 2:20 PM, Gregory Haskins
[EMAIL PROTECTED] wrote:
  Agreed, but it's still correct afaict.  I added an extra might_sleep()
  to a path that really might sleep.  I should have mentioned that in the
  header.

  In any case, its moot.  Andi indicated this patch is probably a no-op so
  I was considering dropping it on the v2 pass.

The might_sleep is annotation and well as a conditional preemption
point for the regular kernel. You might want to do a schedule check
there, but it's the wrong function if memory serves me correctly. It's
reserved for things that actually are design to sleep. The rt_spin*()
function are really a method of preserving BKL semantics across real
schedule() calls. You'd have to use something else instead for that
purpose like cond_reschedule() instead.

bill
-
To unsubscribe from this list: send the line unsubscribe linux-rt-users in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH [RT] 11/14] optimize the !printk fastpath through the lock acquisition

2008-02-21 Thread Andi Kleen 
On Thursday 21 February 2008 16:27:22 Gregory Haskins wrote:

 @@ -660,12 +660,12 @@ rt_spin_lock_fastlock(struct rt_mutex *lock,
   void fastcall (*slowfn)(struct rt_mutex *lock))
  {
   /* Temporary HACK! */
 - if (!current-in_printk)
 - might_sleep();
 - else if (in_atomic() || irqs_disabled())
 + if (unlikely(current-in_printk)  (in_atomic() || irqs_disabled()))

I have my doubts that gcc will honor unlikelies that don't affect
the complete condition of an if.

Also conditions guarding returns are by default predicted unlikely
anyways AFAIK. 

The patch is likely a nop.

-Andi
-
To unsubscribe from this list: send the line unsubscribe linux-rt-users in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html