Hi Ingo,

(please cc me again)

On Fri, Feb 29, 2008 at 02:34:33PM +0100, Ingo Molnar wrote:
> * Simon Huggins <huggie@earth.li> wrote:
> > [ Please Cc me on replies ]
> > I had a bug with sdhci which Pierre Ossman looked at for me.
> > In the end essentially the fix was to use HZ=1000 and nothing else. 
> > Pierre seemed to think that this was a bug in the scheduler.
> does the patch below help, even if you keep HZ=100? This doesnt look 
> like a scheduler issue, it's more of a timer/timing issue. Different HZ 
> means different msleep() results - and the mmc code does a loop of small 
> msleep delays.

Thanks for looking at it.

I did tests with 2.6.24.3 with HZ=1000 and HZ=100 and as expected the
latter didn't work.

> -------------->
> ---
>  drivers/mmc/core/core.h |    7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)

> Index: linux/drivers/mmc/core/core.h
> ===================================================================
> --- linux.orig/drivers/mmc/core/core.h
> +++ linux/drivers/mmc/core/core.h
> @@ -36,12 +36,7 @@ void mmc_set_timing(struct mmc_host *hos

>  static inline void mmc_delay(unsigned int ms)
>  {
> -	if (ms < 1000 / HZ) {
> -		cond_resched();
> -		mdelay(ms);
> -	} else {
> -		msleep(ms);
> -	}
> +	mdelay(ms);
>  }

>  void mmc_rescan(struct work_struct *work);

That doesn't work.  I did a test with HZ=100 and this patch.  I've
attached the log as patch1-log.

On Fri, Feb 29, 2008 at 03:00:24PM +0100, Ingo Molnar wrote:

> * Ingo Molnar <mingo@elte.hu> wrote:

> > alternatively, instead of applying the first patch, could you apply 
> > the patch below instead? This makes "msleep()" much more precise. Does 
> > this help in the HZ != 1000 case?

> ok, this had a build failure - use the one below instead.

That doesn't work either.  Again I did a test with HZ=100 and this
patch.  I've attached the log as patch2-log.

It had some fuzz but no rejects with 2.6.24.3.

> -------------->
> Subject: hrtimers: use hrtimers so that msleep() sleeps for the requested time
> From: Jonathan Corbet <corbet@lwn.net>

> The problem being addressed here is that the current msleep() will stop
> for a minimum of two jiffies, meaning that, on a HZ=100 system,
> msleep(1) delays for for about 20ms.  In a driver with one such delay
> for each of 150 or so register setting operations, the extra time adds
> up to a few seconds.

> This patch addresses the situation by using hrtimers.  On tickless
> systems with working timers, msleep(1) now sleeps for 1ms, even with
> HZ=100.

> Most comments last time were favorable.  The one dissenter was Roman,
> who worries about the overhead of using hrtimers for this operation; my
> understanding is that he would rather see a really_msleep() function for
> those who actually want millisecond resolution.  I'm not sure how to
> characterize what the cost could be, but it can only be buried by the
> fact that every call sleeps for some number of milliseconds.  On my
> system, the several hundred total msleep() calls can't cause any real
> overhead, and almost all happen at initialization time.

> I still think it would be useful for msleep() to do what it says it does
> and not vastly oversleep with small arguments.  A quick grep turns up
> 450 msleep(1) calls in the current mainline.  Andrew, if you agree, can
> you drop this into -mm?  If not, I guess I'll let it go.

> Current msleep() snoozes for at least two jiffies, causing msleep(1) to
> sleep for at least 20ms on HZ=100 systems.  Using hrtimers allows
> msleep() to sleep for something much closer to the requested time.

[snip]


Anything else I can try?

-- 
----------(  "There are times when I think Clint belongs in  )----------
----------( my RSS-hole" -- anonymous Debian Planet reader.  )----------
Simon ----(                                                  )---- Nomis
                             Htag.pl 0.0.22