[i2c] [patch 2.6.24-rc1-git] i2c-algo-bit, don't yield()

[David Brownell]

On Tuesday 06 November 2007, Jean Delvare wrote:
> On Sun, 4 Nov 2007 12:38:35 -0700, David Brownell wrote:
> > Don't use yield().  It can have nasty scheduling properties; and in any
> > case this code could be interrupted or otherwise preempted at any time.
> 
> Can you please be more specific? I can't see any problem here. We're
> between two I2C transactions, how could being rescheduled be an issue?

Use (== abuse) of yield() is one of Andrew Mortons pet bugaboos.

I forget the details -- I think they related to trashing state
that kept the scheduler well-behaved -- but the short version of
the story was that it shouldn't really be used.

Turn it around:  why use it at all?  Or why use yield() instead
of cond_resched()?


> > This address retry mechanism should be removed from the I2C stack at some
> > point.  Meanwhile, there's no point in having its primary implementation
> > misbehave.
> 
> Well, I seem to remember we agreed on that and nobody objected, so
> rather than trying to fix this code, what about sending a patch that
> rips the whole thing off?

NYET.  We also wanted to try a less bloodthirsty approach
than just ripping it out ... in case anyone relied on it.  :)

- Dave


> > Signed-off-by: David Brownell <dbrownell at users.sourceforge.net>
> > ---
> >  drivers/i2c/algos/i2c-algo-bit.c |    1 -
> >  1 files changed, 1 deletion(-)
> > 
> > --- a.orig/drivers/i2c/algos/i2c-algo-bit.c	2007-09-05 09:08:53.000000000 -0700
> > +++ a/drivers/i2c/algos/i2c-algo-bit.c	2007-09-05 12:01:35.000000000 -0700
> > @@ -322,7 +322,6 @@ static int try_address(struct i2c_adapte
> >  		bit_dbg(3, &i2c_adap->dev, "emitting stop condition\n");
> >  		i2c_stop(adap);
> >  		udelay(adap->udelay);
> > -		yield();
> >  		bit_dbg(3, &i2c_adap->dev, "emitting start condition\n");
> >  		i2c_start(adap);
> >  	}
> 
> Thanks,
> -- 
> Jean Delvare
>