Re: [PATCH/RFC] msleep() with hrtimers

[corbet@lwn.net (Jonathan Corbet)]

Roman Zippel <zippel@linux-m68k.org> wrote:

> > That's a possibility, I admit I haven't benchmarked it.  I will say that
> > I don't think it will be enough to matter - msleep() is not a hot-path
> > sort of function.  Once the system is up and running it almost never
> > gets called at all - at least, on my setup.
> 
> That's a bit my problem - we have to consider other setups as well.
> Is it worth converting all msleep users behind their back or should we 
> just a provide a separate function for those who care?

Any additional overhead is clearly small - enough not to disrupt a *high
resolution* timer, after all.  And msleep() is used mostly during
initialization time.  My box had a few hundred calls, almost all during
boot.  Any cost will be bounded by the fact that, well, it sleeps for
milliseconds on every call.

I could run a million-msleep profile if you really want, but I just
can't imagine it's going to have a measurable effect on any real-world
situation.  Is there a specific setup you're worried about?

I'm not *that* attached to this patch; if it causes heartburn we can
just forget it.  But I had thought it might be useful...

> Which driver is this? I'd like to look at this, in case there's some other 
> hidden problem. 

drivers/media/video/cafe_ccic.c, and cafe_smbus_write_data() in
particular.  The "hidden problem," though, is that the hardware has
periods where reading the status registers will send it off into its
room where it will hide under its bed and never come out.

I have an alternative which avoids the sleep at the cost of a small,
relatively harmless race; it may be my real solution in the end, we'll
see.

> > Except that then, with the current implementation, you're paying for the
> > higher HZ whenever the CPU is busy.  I bet that doesn't take long to
> > overwhelm any added overhead in the hrtimer msleep().
> 
> Actually if that's the case I'd consider this a bug, where is that extra 
> cost coming from?

My understanding is that the current dyntick code only turns off the
tick during idle periods; while things are running it's business as
usual.  Perhaps I misunderstood?

jon