tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Strange crash of DIAGNOSTIC kernel on cv_destroy(9)



> Date: Mon, 24 Jul 2023 12:05:34 +0900
> From: PHO <pho%cielonegro.org@localhost>
> 
> I realized the cause of this:
> [...]
> There were cases where the function was destroying a condvar that it 
> didn't initialize! Ugh, this is the very reason why I dislike C...

Oops...  Sorry, was blowing through the vmwgfx code a little too
hastily there.  Here's a patch to simplify it and make it easier to
read; might work better.
diff --git a/sys/external/bsd/drm2/dist/drm/vmwgfx/vmwgfx_fence.c b/sys/external/bsd/drm2/dist/drm/vmwgfx/vmwgfx_fence.c
index 970c6ee38235..68a654ddb29a 100644
--- a/sys/external/bsd/drm2/dist/drm/vmwgfx/vmwgfx_fence.c
+++ b/sys/external/bsd/drm2/dist/drm/vmwgfx/vmwgfx_fence.c
@@ -195,87 +195,37 @@ static long vmw_fence_wait(struct dma_fence *f, bool intr, signed long timeout)
 	if (likely(vmw_fence_obj_signaled(fence)))
 		return timeout;
 
+	DRM_INIT_WAITQUEUE(&cb.wq, "vmwgfxwf");
+
 	vmw_fifo_ping_host(dev_priv, SVGA_SYNC_GENERIC);
 	vmw_seqno_waiter_add(dev_priv);
 
-	spin_lock(f->lock);
-
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &f->flags))
-		goto out;
-
-	if (intr && signal_pending(current)) {
-		ret = -ERESTARTSYS;
+	if (likely(vmw_fence_obj_signaled(fence))) {
+		ret = timeout;
 		goto out;
 	}
 
-#ifdef __NetBSD__
-	DRM_INIT_WAITQUEUE(&cb.wq, "vmwgfxwf");
-#else
-	cb.task = current;
-#endif
-	spin_unlock(f->lock);
 	ret = dma_fence_add_callback(f, &cb.base, vmwgfx_wait_cb);
-	spin_lock(f->lock);
-	if (ret)
+	if (ret) {
+		ret = timeout;
 		goto out;
+	}
 
-#ifdef __NetBSD__
 #define	C	(__vmw_fences_update(fman), dma_fence_is_signaled_locked(f))
+	spin_lock(f->lock);
 	if (intr) {
 		DRM_SPIN_TIMED_WAIT_UNTIL(ret, &cb.wq, f->lock, timeout, C);
 	} else {
 		DRM_SPIN_TIMED_WAIT_NOINTR_UNTIL(ret, &cb.wq, f->lock, timeout,
 		    C);
 	}
-#else
-	for (;;) {
-		__vmw_fences_update(fman);
-
-		/*
-		 * We can use the barrier free __set_current_state() since
-		 * DMA_FENCE_FLAG_SIGNALED_BIT + wakeup is protected by the
-		 * fence spinlock.
-		 */
-		if (intr)
-			__set_current_state(TASK_INTERRUPTIBLE);
-		else
-			__set_current_state(TASK_UNINTERRUPTIBLE);
-
-		if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &f->flags)) {
-			if (ret == 0 && timeout > 0)
-				ret = 1;
-			break;
-		}
-
-		if (intr && signal_pending(current)) {
-			ret = -ERESTARTSYS;
-			break;
-		}
-
-		if (ret == 0)
-			break;
-
-		spin_unlock(f->lock);
-
-		ret = schedule_timeout(ret);
-
-		spin_lock(f->lock);
-	}
-	__set_current_state(TASK_RUNNING);
-	if (!list_empty(&cb.base.node))
-		list_del(&cb.base.node);
-#endif
 	spin_unlock(f->lock);
+
 	dma_fence_remove_callback(f, &cb.base);
-	spin_lock(f->lock);
 
-out:
-	spin_unlock(f->lock);
-#ifdef __NetBSD__
-	DRM_DESTROY_WAITQUEUE(&cb.wq);
-#endif
+out:	vmw_seqno_waiter_remove(dev_priv);
 
-	vmw_seqno_waiter_remove(dev_priv);
+	DRM_DESTROY_WAITQUEUE(&cb.wq);
 
 	return ret;
 }


Home | Main Index | Thread Index | Old Index