Re: [ANNOUNCE] 3.8.4-rt1

["Luis Claudio R. Goncalves" <lclaudio@uudg.org>]

On Sat, Mar 23, 2013 at 07:38:27PM +0100, Sebastian Andrzej Siewior wrote:
| * Luis Claudio R. Goncalves | 2013-03-22 22:30:03 [-0300]:
| 
| What about this:
| 
| diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
| index de45b60..9b51712 100644
| --- a/drivers/gpu/drm/i915/i915_gem.c
| +++ b/drivers/gpu/drm/i915/i915_gem.c
| @@ -91,7 +91,6 @@ i915_gem_wait_for_error(struct drm_device *dev)
|  {
|  	struct drm_i915_private *dev_priv = dev->dev_private;
|  	struct completion *x = &dev_priv->error_completion;
| -	unsigned long flags;
|  	int ret;
|  
|  	if (!atomic_read(&dev_priv->mm.wedged))
| @@ -116,9 +115,7 @@ i915_gem_wait_for_error(struct drm_device *dev)
|  		 * end up waiting upon a subsequent completion event that
|  		 * will never happen.
|  		 */
| -		spin_lock_irqsave(&x->wait.lock, flags);
| -		x->done++;
| -		spin_unlock_irqrestore(&x->wait.lock, flags);
| +		complete(x);
|  	}
|  	return 0;
|  }
| @@ -946,12 +943,9 @@ i915_gem_check_wedge(struct drm_i915_private *dev_priv,
|  	if (atomic_read(&dev_priv->mm.wedged)) {
|  		struct completion *x = &dev_priv->error_completion;
|  		bool recovery_complete;
| -		unsigned long flags;
|  
|  		/* Give the error handler a chance to run. */
| -		spin_lock_irqsave(&x->wait.lock, flags);
| -		recovery_complete = x->done > 0;
| -		spin_unlock_irqrestore(&x->wait.lock, flags);
| +		recovery_complete = completion_done(x);
|  
|  		/* Non-interruptible callers can't handle -EAGAIN, hence return
|  		 * -EIO unconditionally for these. */

Good catch. The resulting code is cleaner. I took the simplest approach while
trying to fix the issue, but your solution is more elegant.

| @@ -4366,7 +4360,7 @@ static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct *task)
|  	if (!mutex_is_locked(mutex))
|  		return false;
|  
| -#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_MUTEXES)
| +#if (defined(CONFIG_SMP) || defined(CONFIG_DEBUG_MUTEXES)) && !defined(CONFIG_PREEMPT_RT_BASE)
|  	return mutex->owner == task;
|  #else
|  	/* Since UP may be pre-empted, we cannot assume that we own the lock */

I was trying to keep the behavior of CONFIG_DEBUG_MUTEXES, but we do have
the RT counterpart, CONFIG_DEBUG_RT_MUTEXES. So, yours is probably the
right one.

| I'm not very happy with the complete() here, I am not sure what kind of
| problem they try to solve here…

I was looking at the code and IMHO it seems to fit there perfectly.

Cheers,
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html