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

David Woodhouse
2009-09-21T06:14:16Z

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