Re: [patch 02/13] mtd: SST25L (non JEDEC) SPI Flash driver

[David Woodhouse <dwmw2@infradead.org>]

On Mon, 2009-09-21 at 16:47 +1200, Ryan Mallon wrote:
> >  while (busy) {
> >     if (timed_out) return -ETIMEDOUT;
> >     cond_resched();
> >  }
> > 
> 
> Just thinking about this a bit more. We don't want to call cond_resched
> if the device is ready immediately, and we want to do the check _after_
> cond_resched each time through the loop.

Right, absolutely.

And that's exactly what the loop above was doing, isn't it? If you
unroll the loop, it goes...

 if (busy) {
  if (timed_out) return -ETIMEDOUT;
  cond_resched();
  if (busy) {
    if (timed_out) return -ETIMEDOUT;
    cond_resched();
    if (busy) {
      if (timed_out) return -ETIMEDOUT;
      cond_resched();
      if (busy) {
        ...

It _isn't_ calling cond_resched() if the device is ready immediately,
and it _is_ doing the check _after_ cond_resched() each time. Yes?
  
> There are probably enough places that this sort of thing gets used
> that it may be worth having a generic function like this (untested):
> 
> int cond_resched_wait_timeout(unsigned long timeout_jiffies,
>                               int (*cond_check)(void *cookie),
>                               void *data)

Yes, it's probably a good idea. Maybe better off as a macro, rather than
having a function call to evaluate the condition. Much like wait_event?

-- 
dwmw2