The DRI drawable spinlock

[Keith Whitwell]

Thomas Hellstrom wrote:
> Hi!
> 
> Does anybody have a clear understanding of the drawable spinlock?
> 
>  From my reading of code in the X server and dri_utilities.c it is ment 
> to be used to stop anyone but the context holding the lock to touch the 
> dri drawables in a way that would change their timestamp.
> 
> The X server has a very inefficient way of checking whether a client 
> died while holding the drawable spinlock. It waits for 10 seconds and 
> then grabs it by force.
> 
> Also the usage is dri_util.c is beyond my understanding. Basically, to 
> lock and validate drawable info, the following happens:
> 
> get_heavyweight_lock;
> 
> while drawable_stamps_mismatch {
>   release_heavyweight_lock;
>   get_drawable_spinlock;
>    //In dri_util.c
>   do_some_minor_processing_that_can_be_done_elsewhere;
>   release_drawable_spinlock;
>   call_X_to_update_drawable_info;
>   get_drawable_spinlock;
>   //In driver.
>   release_drawable_spinlock;
> }
> 
> Basically no driver seems to be using it for anything, except possibly 
> the gamma driver, which I figure is outdated anyway?
> 
> I have found some use for it in XvMC clients: To run the scaling engine 
> to render to a drawable without holding the heavyweight lock for 
> prolonged periods, but I strongly dislike the idea of freezing the X 
> server for 10 secs if the XvMC client accidently dies.
> 
> Proposed changes:
> 
> 1). Could we replace the locking value (which now seems to be 1) with 
> the context number | _DRM_LOCK_HELD. In this way the drm can detect when 
> the drawable lock is held by a killed client and release it.

This seems like a reasonable thing to do now.  Note that the X server 
could also be the one responsible for freeing the lock, which might be 
cleaner if the DRM is currenly unaware of this beast.

> 2). Could we replace the drawable_spinlock with a futex-like lock 
> similar to what's used in the via drm to reserve certain chip functions. 
> The purpose would be to sched_yield() if the lock is contended, with an 
> immediate wakeup when the lock is released. This would not be backwards 
> binary compatible with other drivers, but it seems no up-to-date drivers 
> is using this lock anyway.

Maybe this is something to put off to be part of Ian's forthcoming 
binary-compatibility breakages for X.org 6.7?

Keith