Yanhai Zhu wrote:
> Yes, it's uncessary. Here is the answer :
>
> http://linux.derkeiler.com/Mailing-Lists/Kernel/2004-10/2391.html
>
>
> --
> Regards,
> Zhu Yanhai
>
>   
Indeed.
Given this (preempt never removes (!TASK_RUNNING) tasks from the run 
queue), it's concluded that the patch is not needed.


/Thomas

> 2009/4/24 Thomas Hellstrom <thellst...@vmware.com>:
>   
>> 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?
>>
>> That said, we should fix wait code like this to use prepare_to_wait /
>> finish_wait to mimic what's in linux/wait.h. In this case it should even
>> be possible to use wait_event_interruptible(). There seems to be some
>> hairy memory_barrier things that we currently don't get right on
>> processors that reorder write operations. See comments on
>> set_current_state in <linux/sched.h>
>>
>> So I won't ack this until we have an explanation why <linux/wait.h>
>> isn't doing the same thing.
>>
>> /Thomas
>>
>>
>> ------------------------------------------------------------------------------
>> Crystal Reports &#45; New Free Runtime and 30 Day Trial
>> Check out the new simplified licensign option that enables unlimited
>> royalty&#45;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 &#45; New Free Runtime and 30 Day Trial
Check out the new simplified licensign option that enables unlimited
royalty&#45;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

Reply via email to