" If the lock is taken, then
nobody can take drm lock, so nobody will call drm_unlock() "
No, the lock owner will continue as usual, because __schedule() will
set it to TASK_RUNNING,
please take a look at __schedule() for more.
--
Regards,
Zhu Yanhai
2009/4/24 Shaohua Li <shaohua...@intel.com>:
> 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
>
------------------------------------------------------------------------------
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
