On Fri, 2009-04-24 at 16:10 +0800, Thomas Hellstrom wrote:
> Dave Airlie wrote:
> > On Wed, Apr 22, 2009 at 11:48 AM, Shaohua Li <shaohua...@intel.com> wrote:
> >
> >> drm_lock() does:
> >> for (;;) {
> >> __set_current_state(TASK_INTERRUPTIBLE);
> >> ...
> >> if (drm_lock_take(&master->lock, lock->context)) {
> >> <==== schedule() here
> >> master->lock.file_priv = file_priv;
> >> master->lock.lock_time = jiffies;
> >> atomic_inc(&dev->counts[_DRM_STAT_LOCKS]);
> >> break; /* Got lock */
> >> }
> >> ...
> >> }
> >> If a preempt occurs in marked line, the task already holds the lock but
> >> set to interruptible, then nobody can wakeup the task (except signal) and
> >> other tasks can't get the lock again. Am I missing anything?
> >>
> >
> > Thomas you seem to have the best understanding of this code, can you
> > take a look and ack this?
> >
> > Dave.
> >
> At a first glance this looks like a sane patch.
> In essence what's said is that a TASK_INTERRUPTIBLE task must never be
> preempted, because it might be that nobody's there to wake it up.
> But we would've most likely hit the consequences by now, wouldn't we?
>
> Also looking at similar code (for example __wait_event_interruptible) in
> <linux/wait.h> there's no
> preempt_disable.
>
> Before we adopt this patch we'd need to understand why that is. Could it
> be that the scheduler is
> smart enough never to put (!TASK_RUNNING) processes to sleep when
> they're preempted?
no. If yes, then the schedule() just below the code will report error.
I guess this is case by case. In this case, if no lock is taken, the
preempt() in interruptible is ok, because other thread will eventually
wake up the thread with a drm_unlock(). If the lock is taken, then
nobody can take drm lock, so nobody will call drm_unlock()
Thanks,
Shaohua
------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensign option that enables unlimited
royalty-free distribution of the report engine for externally facing
server and web deployment.
http://p.sf.net/sfu/businessobjects
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
