Yes, it's uncessary. Here is the answer :
http://linux.derkeiler.com/Mailing-Lists/Kernel/2004-10/2391.html
--
Regards,
Zhu Yanhai
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 - 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
>
--
Regards,
Zhu Yanhai
------------------------------------------------------------------------------
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
