[Dri-devel] Re: Recent broken locking in DRM code..

[Michel <mi...@da...>]

On Mon, 2002-12-02 at 03:00, Linus Torvalds wrote:
> On 1 Dec 2002, Michel D=E4nzer wrote:
>=20
> > Does that also apply to kmalloc()/kfree(), or are they safe?
>=20
> kfree() (or free_pages() or others of that type) is always safe. =20
> kmalloc(x, GFP_ATOMIC) works, but has problems (ie being over-eager about=
=20
> using it can easily result in a system that just dies from being out of=20
> memory and not being able to do the required memory balancing and=20
> swap-out to replenish it).

Luckily, we only need kfree() in the interrupt here, kmalloc() is done
in the ioctl. Thanks to you and Ben for explaining this.


> The attached patch looked mostly ok, except this part:
>=20
> 	-       sema_init( &dev->vbl_sem, 0 );
> 	+       spin_lock_irqsave( &dev->vbl_lock, flags );
> 	..
>=20
> should almost certainly have been a
>=20
> 		spin_lock_init(&dev->vbl_lock);
>=20
> instead of locking and unlocking an uninitialized variable (from what I=20
> could tell from just reading the diff).

Fixed, thanks. I'll commit the attached updated patch soon unless
someone sees another mistake. It works for me. Now I hope someone will
fill in the missing pieces and/or do the cleanup necessary for BSD.


> Btw, from a testing standpoint it's then also worthwhile compiling with=20
> CONFIG_DEBUG_SPINLOCK, which will verify that spinlocks are initialized=20
> etc.

I hope to get around to that some time.


--=20
Earthling Michel D=E4nzer (MrCooper)/ Debian GNU/Linux (powerpc) developer
XFree86 and DRI project member   /  CS student, Free Software enthusiast