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 - 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