Re: [PATCH] bttv driver II

From: David Woodhouse (dwmw2@infradead.org)
Date: Mon May 22 2000 - 08:11:34 EST


alan@lxorguk.ukuu.org.uk said:
> > + spin_unlock_irqrestore(&btv->s_lock, flags);
> > interruptible_sleep_on(&btv->vbiq);
> > - sti();

> Nope. You added a race. The existing code is not right either. You
> need to change this to use add_wait_queue and remove the race.

> > + spin_unlock_irqrestore(&btv->s_lock, flags);
> > interruptible_sleep_on(&btv->vbiq);
> > - sti();

> Again you need to fix this with add_wait_queue

How many _valid_ (i.e. non-racing) uses of sleep_on() exist in the kernel at
the moment? I suspect it's a miniscule proportion of the occurrences.

I believe that the only circumstances in which it's truly safe are when
you're holding the big kernel lock, and when the waker will require that
lock too. In 2.3, that probably covers a fair amount of filesystem code, but
not a lot else.

That being the case, wouldn't it make sense just to remove sleep_on()
completely, and hence also the temptation for people to use it?

Yes, this breaks a lot of code at this late stage in the game, but most of
it is code which is probably subtly broken anyway, in which case breaking
its compilation is a GoodThing.

--
dwmw2

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue May 23 2000 - 21:00:21 EST