All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative
@ 2006-10-11 12:57 ` Amol Lad
  0 siblings, 0 replies; 20+ messages in thread
From: Amol Lad @ 2006-10-11 12:45 UTC (permalink / raw)
  To: linux kernel; +Cc: kernel Janitors

In 2.6, the semantics of calling yield() changed from "sleep for a
bit" to "I really don't want to run for a while".  This matches POSIX
better, but there's a lot of drivers still using yield() when they mean
cond_resched(), schedule() or even schedule_timeout().

For this driver cond_resched() seems to be a better
alternative

Tested compile only

Signed-off-by: Amol Lad <amol@verismonetworks.com>
---
diff -uprN -X linux-2.6.19-rc1-orig/Documentation/dontdiff linux-2.6.19-rc1-orig/drivers/mmc/mmc.c linux-2.6.19-rc1/drivers/mmc/mmc.c
--- linux-2.6.19-rc1-orig/drivers/mmc/mmc.c	2006-10-05 14:00:46.000000000 +0530
+++ linux-2.6.19-rc1/drivers/mmc/mmc.c	2006-10-11 17:57:02.000000000 +0530
@@ -454,7 +454,7 @@ static void mmc_deselect_cards(struct mm
 static inline void mmc_delay(unsigned int ms)
 {
 	if (ms < HZ / 1000) {
-		yield();
+		cond_resched();
 		mdelay(ms);
 	} else {
 		msleep_interruptible (ms);



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a
@ 2006-10-11 12:57 ` Amol Lad
  0 siblings, 0 replies; 20+ messages in thread
From: Amol Lad @ 2006-10-11 12:57 UTC (permalink / raw)
  To: linux kernel; +Cc: kernel Janitors

In 2.6, the semantics of calling yield() changed from "sleep for a
bit" to "I really don't want to run for a while".  This matches POSIX
better, but there's a lot of drivers still using yield() when they mean
cond_resched(), schedule() or even schedule_timeout().

For this driver cond_resched() seems to be a better
alternative

Tested compile only

Signed-off-by: Amol Lad <amol@verismonetworks.com>
---
diff -uprN -X linux-2.6.19-rc1-orig/Documentation/dontdiff linux-2.6.19-rc1-orig/drivers/mmc/mmc.c linux-2.6.19-rc1/drivers/mmc/mmc.c
--- linux-2.6.19-rc1-orig/drivers/mmc/mmc.c	2006-10-05 14:00:46.000000000 +0530
+++ linux-2.6.19-rc1/drivers/mmc/mmc.c	2006-10-11 17:57:02.000000000 +0530
@@ -454,7 +454,7 @@ static void mmc_deselect_cards(struct mm
 static inline void mmc_delay(unsigned int ms)
 {
 	if (ms < HZ / 1000) {
-		yield();
+		cond_resched();
 		mdelay(ms);
 	} else {
 		msleep_interruptible (ms);


_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a
  2006-10-11 12:57 ` [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a Amol Lad
@ 2006-10-11 12:58   ` Arjan van de Ven
  -1 siblings, 0 replies; 20+ messages in thread
From: Arjan van de Ven @ 2006-10-11 12:58 UTC (permalink / raw)
  To: Amol Lad; +Cc: linux kernel, kernel Janitors

On Wed, 2006-10-11 at 18:15 +0530, Amol Lad wrote:
> In 2.6, the semantics of calling yield() changed from "sleep for a
> bit" to "I really don't want to run for a while".  This matches POSIX
> better, but there's a lot of drivers still using yield() when they mean
> cond_resched(), schedule() or even schedule_timeout().
> 
> For this driver cond_resched() seems to be a better
> alternative
> 

are you sure?

> Tested compile only
> 
> Signed-off-by: Amol Lad <amol@verismonetworks.com>
> ---
> diff -uprN -X linux-2.6.19-rc1-orig/Documentation/dontdiff linux-2.6.19-rc1-orig/drivers/mmc/mmc.c linux-2.6.19-rc1/drivers/mmc/mmc.c
> --- linux-2.6.19-rc1-orig/drivers/mmc/mmc.c	2006-10-05 14:00:46.000000000 +0530
> +++ linux-2.6.19-rc1/drivers/mmc/mmc.c	2006-10-11 17:57:02.000000000 +0530
> @@ -454,7 +454,7 @@ static void mmc_deselect_cards(struct mm
>  static inline void mmc_delay(unsigned int ms)
>  {
>  	if (ms < HZ / 1000) {
> -		yield();
> +		cond_resched();
>  		mdelay(ms);


this probably wants msleep(), especially with hrtimers comming up; there
the sleeps are always exact...


_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative
@ 2006-10-11 12:58   ` Arjan van de Ven
  0 siblings, 0 replies; 20+ messages in thread
From: Arjan van de Ven @ 2006-10-11 12:58 UTC (permalink / raw)
  To: Amol Lad; +Cc: linux kernel, kernel Janitors

On Wed, 2006-10-11 at 18:15 +0530, Amol Lad wrote:
> In 2.6, the semantics of calling yield() changed from "sleep for a
> bit" to "I really don't want to run for a while".  This matches POSIX
> better, but there's a lot of drivers still using yield() when they mean
> cond_resched(), schedule() or even schedule_timeout().
> 
> For this driver cond_resched() seems to be a better
> alternative
> 

are you sure?

> Tested compile only
> 
> Signed-off-by: Amol Lad <amol@verismonetworks.com>
> ---
> diff -uprN -X linux-2.6.19-rc1-orig/Documentation/dontdiff linux-2.6.19-rc1-orig/drivers/mmc/mmc.c linux-2.6.19-rc1/drivers/mmc/mmc.c
> --- linux-2.6.19-rc1-orig/drivers/mmc/mmc.c	2006-10-05 14:00:46.000000000 +0530
> +++ linux-2.6.19-rc1/drivers/mmc/mmc.c	2006-10-11 17:57:02.000000000 +0530
> @@ -454,7 +454,7 @@ static void mmc_deselect_cards(struct mm
>  static inline void mmc_delay(unsigned int ms)
>  {
>  	if (ms < HZ / 1000) {
> -		yield();
> +		cond_resched();
>  		mdelay(ms);


this probably wants msleep(), especially with hrtimers comming up; there
the sleeps are always exact...



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [KJ] most users of msleep_interruptible are broken
  2006-10-11 12:58   ` [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative Arjan van de Ven
@ 2006-10-11 14:16     ` Matthew Wilcox
  -1 siblings, 0 replies; 20+ messages in thread
From: Matthew Wilcox @ 2006-10-11 14:16 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Amol Lad, kernel Janitors, linux kernel

On Wed, Oct 11, 2006 at 02:58:11PM +0200, Arjan van de Ven wrote:
> > +++ linux-2.6.19-rc1/drivers/mmc/mmc.c	2006-10-11 17:57:02.000000000 +0530
> > @@ -454,7 +454,7 @@ static void mmc_deselect_cards(struct mm
> >  static inline void mmc_delay(unsigned int ms)
> >  {
> >  	if (ms < HZ / 1000) {
> > -		yield();
> > +		cond_resched();
> >  		mdelay(ms);
> 
> 
> this probably wants msleep(), especially with hrtimers comming up; there
> the sleeps are always exact...

They clearly don't care about exactness; they msleep_interruptible and
throw away the return value, so they don't know how long they slept
before they got a signal.

__must_check treatment for msleep_interruptible, anyone?  On the one hand,
that's 136 new warnings.  On the other hand, that's 136 places wheree
we may as well *delete the call* to msleep_interruptible.  Since it can
return immediately, the code must be prepared to deal with that ... right?
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 20+ messages in thread

* most users of msleep_interruptible are broken
@ 2006-10-11 14:16     ` Matthew Wilcox
  0 siblings, 0 replies; 20+ messages in thread
From: Matthew Wilcox @ 2006-10-11 14:16 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Amol Lad, kernel Janitors, linux kernel

On Wed, Oct 11, 2006 at 02:58:11PM +0200, Arjan van de Ven wrote:
> > +++ linux-2.6.19-rc1/drivers/mmc/mmc.c	2006-10-11 17:57:02.000000000 +0530
> > @@ -454,7 +454,7 @@ static void mmc_deselect_cards(struct mm
> >  static inline void mmc_delay(unsigned int ms)
> >  {
> >  	if (ms < HZ / 1000) {
> > -		yield();
> > +		cond_resched();
> >  		mdelay(ms);
> 
> 
> this probably wants msleep(), especially with hrtimers comming up; there
> the sleeps are always exact...

They clearly don't care about exactness; they msleep_interruptible and
throw away the return value, so they don't know how long they slept
before they got a signal.

__must_check treatment for msleep_interruptible, anyone?  On the one hand,
that's 136 new warnings.  On the other hand, that's 136 places wheree
we may as well *delete the call* to msleep_interruptible.  Since it can
return immediately, the code must be prepared to deal with that ... right?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [KJ] most users of msleep_interruptible are broken
  2006-10-11 14:16     ` Matthew Wilcox
@ 2006-10-11 14:20       ` Pierre Ossman
  -1 siblings, 0 replies; 20+ messages in thread
From: Pierre Ossman @ 2006-10-11 14:20 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Arjan van de Ven, Amol Lad, kernel Janitors, linux kernel

Matthew Wilcox wrote:
> 
> They clearly don't care about exactness; they msleep_interruptible and
> throw away the return value, so they don't know how long they slept
> before they got a signal.
> 

It's broken then. That delay function should delay at least the amount
given.

Rgds
Pierre

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: most users of msleep_interruptible are broken
@ 2006-10-11 14:20       ` Pierre Ossman
  0 siblings, 0 replies; 20+ messages in thread
From: Pierre Ossman @ 2006-10-11 14:20 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Arjan van de Ven, Amol Lad, kernel Janitors, linux kernel

Matthew Wilcox wrote:
> 
> They clearly don't care about exactness; they msleep_interruptible and
> throw away the return value, so they don't know how long they slept
> before they got a signal.
> 

It's broken then. That delay function should delay at least the amount
given.

Rgds
Pierre


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [KJ] most users of msleep_interruptible are broken
  2006-10-11 14:20       ` Pierre Ossman
@ 2006-10-11 14:53         ` Arjan van de Ven
  -1 siblings, 0 replies; 20+ messages in thread
From: Arjan van de Ven @ 2006-10-11 14:53 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: Matthew Wilcox, Amol Lad, kernel Janitors, linux kernel

On Wed, 2006-10-11 at 16:20 +0200, Pierre Ossman wrote:
> Matthew Wilcox wrote:
> > 
> > They clearly don't care about exactness; they msleep_interruptible and
> > throw away the return value, so they don't know how long they slept
> > before they got a signal.
> > 
> 
> It's broken then. That delay function should delay at least the amount
> given.


if you want that don't use the _interruptible variant. It's that simple.


_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: most users of msleep_interruptible are broken
@ 2006-10-11 14:53         ` Arjan van de Ven
  0 siblings, 0 replies; 20+ messages in thread
From: Arjan van de Ven @ 2006-10-11 14:53 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: Matthew Wilcox, Amol Lad, kernel Janitors, linux kernel

On Wed, 2006-10-11 at 16:20 +0200, Pierre Ossman wrote:
> Matthew Wilcox wrote:
> > 
> > They clearly don't care about exactness; they msleep_interruptible and
> > throw away the return value, so they don't know how long they slept
> > before they got a signal.
> > 
> 
> It's broken then. That delay function should delay at least the amount
> given.


if you want that don't use the _interruptible variant. It's that simple.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [KJ] most users of msleep_interruptible are broken
  2006-10-11 14:53         ` Arjan van de Ven
@ 2006-10-11 15:07           ` Pierre Ossman
  -1 siblings, 0 replies; 20+ messages in thread
From: Pierre Ossman @ 2006-10-11 15:07 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Matthew Wilcox, Amol Lad, kernel Janitors, linux kernel

Arjan van de Ven wrote:
>
> if you want that don't use the _interruptible variant. It's that simple.
>
>   

Patches welcome ;)

I believe noone has fixed the if-condition yet as well...

Rgds
Pierre

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: most users of msleep_interruptible are broken
@ 2006-10-11 15:07           ` Pierre Ossman
  0 siblings, 0 replies; 20+ messages in thread
From: Pierre Ossman @ 2006-10-11 15:07 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Matthew Wilcox, Amol Lad, kernel Janitors, linux kernel

Arjan van de Ven wrote:
>
> if you want that don't use the _interruptible variant. It's that simple.
>
>   

Patches welcome ;)

I believe noone has fixed the if-condition yet as well...

Rgds
Pierre


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [KJ] most users of msleep_interruptible are broken
  2006-10-11 14:16     ` Matthew Wilcox
@ 2006-10-11 20:30       ` Pavel Machek
  -1 siblings, 0 replies; 20+ messages in thread
From: Pavel Machek @ 2006-10-11 20:30 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Arjan van de Ven, Amol Lad, kernel Janitors, linux kernel

Hi!

> > > +++ linux-2.6.19-rc1/drivers/mmc/mmc.c	2006-10-11 17:57:02.000000000 +0530
> > > @@ -454,7 +454,7 @@ static void mmc_deselect_cards(struct mm
> > >  static inline void mmc_delay(unsigned int ms)
> > >  {
> > >  	if (ms < HZ / 1000) {
> > > -		yield();
> > > +		cond_resched();
> > >  		mdelay(ms);
> > 
> > 
> > this probably wants msleep(), especially with hrtimers comming up; there
> > the sleeps are always exact...
> 
> They clearly don't care about exactness; they msleep_interruptible and
> throw away the return value, so they don't know how long they slept
> before they got a signal.
> 
> __must_check treatment for msleep_interruptible, anyone?  On the one hand,
> that's 136 new warnings.  On the other hand, that's 136 places wheree
> we may as well *delete the call* to msleep_interruptible.  Since it can
> return immediately, the code must be prepared to deal with that ... right?

Well, it must work, but it may busyloop instead of sleeping. This does
not look like must_check to me.
						Pavel

-- 
Thanks for all the (sleeping) penguins.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: most users of msleep_interruptible are broken
@ 2006-10-11 20:30       ` Pavel Machek
  0 siblings, 0 replies; 20+ messages in thread
From: Pavel Machek @ 2006-10-11 20:30 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Arjan van de Ven, Amol Lad, kernel Janitors, linux kernel

Hi!

> > > +++ linux-2.6.19-rc1/drivers/mmc/mmc.c	2006-10-11 17:57:02.000000000 +0530
> > > @@ -454,7 +454,7 @@ static void mmc_deselect_cards(struct mm
> > >  static inline void mmc_delay(unsigned int ms)
> > >  {
> > >  	if (ms < HZ / 1000) {
> > > -		yield();
> > > +		cond_resched();
> > >  		mdelay(ms);
> > 
> > 
> > this probably wants msleep(), especially with hrtimers comming up; there
> > the sleeps are always exact...
> 
> They clearly don't care about exactness; they msleep_interruptible and
> throw away the return value, so they don't know how long they slept
> before they got a signal.
> 
> __must_check treatment for msleep_interruptible, anyone?  On the one hand,
> that's 136 new warnings.  On the other hand, that's 136 places wheree
> we may as well *delete the call* to msleep_interruptible.  Since it can
> return immediately, the code must be prepared to deal with that ... right?

Well, it must work, but it may busyloop instead of sleeping. This does
not look like must_check to me.
						Pavel

-- 
Thanks for all the (sleeping) penguins.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a
  2006-10-11 12:58   ` [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative Arjan van de Ven
@ 2006-10-12  5:54     ` Nick Piggin
  -1 siblings, 0 replies; 20+ messages in thread
From: Nick Piggin @ 2006-10-12  5:54 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Amol Lad, linux kernel, kernel Janitors

Arjan van de Ven wrote:
> On Wed, 2006-10-11 at 18:15 +0530, Amol Lad wrote:
> 
>>In 2.6, the semantics of calling yield() changed from "sleep for a
>>bit" to "I really don't want to run for a while".  This matches POSIX
>>better, but there's a lot of drivers still using yield() when they mean
>>cond_resched(), schedule() or even schedule_timeout().
>>
>>For this driver cond_resched() seems to be a better
>>alternative
>>
> 
> 
> are you sure?
> 
> 
>>Tested compile only
>>
>>Signed-off-by: Amol Lad <amol@verismonetworks.com>
>>---
>>diff -uprN -X linux-2.6.19-rc1-orig/Documentation/dontdiff linux-2.6.19-rc1-orig/drivers/mmc/mmc.c linux-2.6.19-rc1/drivers/mmc/mmc.c
>>--- linux-2.6.19-rc1-orig/drivers/mmc/mmc.c	2006-10-05 14:00:46.000000000 +0530
>>+++ linux-2.6.19-rc1/drivers/mmc/mmc.c	2006-10-11 17:57:02.000000000 +0530
>>@@ -454,7 +454,7 @@ static void mmc_deselect_cards(struct mm
>> static inline void mmc_delay(unsigned int ms)
>> {
>> 	if (ms < HZ / 1000) {
>>-		yield();
>>+		cond_resched();
>> 		mdelay(ms);
> 
> 
> 
> this probably wants msleep(), especially with hrtimers comming up; there
> the sleeps are always exact...

The condition looks broken too. It should be
if (ms < 1000 / HZ) {...}

Shouldn't it?
-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative
@ 2006-10-12  5:54     ` Nick Piggin
  0 siblings, 0 replies; 20+ messages in thread
From: Nick Piggin @ 2006-10-12  5:54 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Amol Lad, linux kernel, kernel Janitors

Arjan van de Ven wrote:
> On Wed, 2006-10-11 at 18:15 +0530, Amol Lad wrote:
> 
>>In 2.6, the semantics of calling yield() changed from "sleep for a
>>bit" to "I really don't want to run for a while".  This matches POSIX
>>better, but there's a lot of drivers still using yield() when they mean
>>cond_resched(), schedule() or even schedule_timeout().
>>
>>For this driver cond_resched() seems to be a better
>>alternative
>>
> 
> 
> are you sure?
> 
> 
>>Tested compile only
>>
>>Signed-off-by: Amol Lad <amol@verismonetworks.com>
>>---
>>diff -uprN -X linux-2.6.19-rc1-orig/Documentation/dontdiff linux-2.6.19-rc1-orig/drivers/mmc/mmc.c linux-2.6.19-rc1/drivers/mmc/mmc.c
>>--- linux-2.6.19-rc1-orig/drivers/mmc/mmc.c	2006-10-05 14:00:46.000000000 +0530
>>+++ linux-2.6.19-rc1/drivers/mmc/mmc.c	2006-10-11 17:57:02.000000000 +0530
>>@@ -454,7 +454,7 @@ static void mmc_deselect_cards(struct mm
>> static inline void mmc_delay(unsigned int ms)
>> {
>> 	if (ms < HZ / 1000) {
>>-		yield();
>>+		cond_resched();
>> 		mdelay(ms);
> 
> 
> 
> this probably wants msleep(), especially with hrtimers comming up; there
> the sleeps are always exact...

The condition looks broken too. It should be
if (ms < 1000 / HZ) {...}

Shouldn't it?
-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a
  2006-10-12  5:54     ` [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative Nick Piggin
@ 2006-10-16  6:49       ` Pierre Ossman
  -1 siblings, 0 replies; 20+ messages in thread
From: Pierre Ossman @ 2006-10-16  6:49 UTC (permalink / raw)
  To: Nick Piggin; +Cc: Arjan van de Ven, Amol Lad, linux kernel, kernel Janitors

Nick Piggin wrote:
> 
> The condition looks broken too. It should be
> if (ms < 1000 / HZ) {...}
> 
> Shouldn't it?

Yup. I poked Russell about it ages ago, but he must have forgotten (and
admittedly, so have I). Since MMC is now on my table, I guess I should
put it on my todo list.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org
  OLPC, developer                     http://www.laptop.org
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative
@ 2006-10-16  6:49       ` Pierre Ossman
  0 siblings, 0 replies; 20+ messages in thread
From: Pierre Ossman @ 2006-10-16  6:49 UTC (permalink / raw)
  To: Nick Piggin; +Cc: Arjan van de Ven, Amol Lad, linux kernel, kernel Janitors

Nick Piggin wrote:
> 
> The condition looks broken too. It should be
> if (ms < 1000 / HZ) {...}
> 
> Shouldn't it?

Yup. I poked Russell about it ages ago, but he must have forgotten (and
admittedly, so have I). Since MMC is now on my table, I guess I should
put it on my todo list.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org
  OLPC, developer                     http://www.laptop.org

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a
  2006-10-11 12:57 ` [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a Amol Lad
@ 2006-10-22 20:23   ` Pierre Ossman
  -1 siblings, 0 replies; 20+ messages in thread
From: Pierre Ossman @ 2006-10-22 20:23 UTC (permalink / raw)
  To: Amol Lad; +Cc: linux kernel, kernel Janitors

Amol Lad wrote:
> In 2.6, the semantics of calling yield() changed from "sleep for a
> bit" to "I really don't want to run for a while".  This matches POSIX
> better, but there's a lot of drivers still using yield() when they mean
> cond_resched(), schedule() or even schedule_timeout().
> 
> For this driver cond_resched() seems to be a better
> alternative
> 

A version of this patch has been pushed towards Andrew. Thanks for
pointing it out.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative
@ 2006-10-22 20:23   ` Pierre Ossman
  0 siblings, 0 replies; 20+ messages in thread
From: Pierre Ossman @ 2006-10-22 20:23 UTC (permalink / raw)
  To: Amol Lad; +Cc: linux kernel, kernel Janitors

Amol Lad wrote:
> In 2.6, the semantics of calling yield() changed from "sleep for a
> bit" to "I really don't want to run for a while".  This matches POSIX
> better, but there's a lot of drivers still using yield() when they mean
> cond_resched(), schedule() or even schedule_timeout().
> 
> For this driver cond_resched() seems to be a better
> alternative
> 

A version of this patch has been pushed towards Andrew. Thanks for
pointing it out.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2006-10-22 20:24 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-11 12:45 [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative Amol Lad
2006-10-11 12:57 ` [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a Amol Lad
2006-10-11 12:58 ` Arjan van de Ven
2006-10-11 12:58   ` [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative Arjan van de Ven
2006-10-11 14:16   ` [KJ] most users of msleep_interruptible are broken Matthew Wilcox
2006-10-11 14:16     ` Matthew Wilcox
2006-10-11 14:20     ` [KJ] " Pierre Ossman
2006-10-11 14:20       ` Pierre Ossman
2006-10-11 14:53       ` [KJ] " Arjan van de Ven
2006-10-11 14:53         ` Arjan van de Ven
2006-10-11 15:07         ` [KJ] " Pierre Ossman
2006-10-11 15:07           ` Pierre Ossman
2006-10-11 20:30     ` [KJ] " Pavel Machek
2006-10-11 20:30       ` Pavel Machek
2006-10-12  5:54   ` [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a Nick Piggin
2006-10-12  5:54     ` [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative Nick Piggin
2006-10-16  6:49     ` [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a Pierre Ossman
2006-10-16  6:49       ` [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative Pierre Ossman
2006-10-22 20:23 ` [KJ] [PATCH] drivers/mmc/mmc.c: Replacing yield() with a Pierre Ossman
2006-10-22 20:23   ` [PATCH] drivers/mmc/mmc.c: Replacing yield() with a better alternative Pierre Ossman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.