Hi,

I found two problems in latest DRM while working on the mach64. I'm
attaching a patch for the two.

1: drm_lock.c: drm_unlock uses a new tasklet_lock. That leads to hangs
on my system (for some reason the driver can't find an IRQ for my PCI
card). Fortunately the NMI watchdog detects the lockup and kills the
Xserver, so I can recover the system without rebooting.

I can see that tasklet_lock is only initialized in drm_irq_install and
only if the DRIVER_IRQ_VBL is enabled. Thus you can't unconditionally
use that spinlock in drm_unlock. The patch adds a condition
dev->irq_enabled, but I'm not sure if this is the right condition. I
believe dev->irq_enabled can be set even without DRIVER_IRQ_VBL being
enabled. Michel, what's would be the right way to handle this?

2: drm_drawable.c: make idx and id signed integers in drm_rmdraw. There
are some comparisons such as id < 0 and idx >= 0 in there that don't
make sense for unsigned integers. And I actually got kernel oopses when
running glxinfo or glxgears in a naked Xserver when it destroyed the
last drawable. It crapped out here:

        ...
        /* Can we shrink the arrays? */
        if (idx == bitfield_length - 1) {
                while (idx >= 0 && !bitfield[idx])
                        --idx;
                ...

idx will always be >= 0 if it's unsigned. --idx will make the value wrap
around and the next access to bitfield[idx] oopses.

Regards,
  Felix

-- 
| Felix Kühling <[EMAIL PROTECTED]>                     http://fxk.de.vu |
| PGP Fingerprint: 6A3C 9566 5B30 DDED 73C3  B152 151C 5CC1 D888 E595 |
diff --git a/linux-core/drm_lock.c b/linux-core/drm_lock.c
index fa677ba..b25bf08 100644
--- a/linux-core/drm_lock.c
+++ b/linux-core/drm_lock.c
@@ -163,15 +163,17 @@ int drm_unlock(struct inode *inode, stru
 		return -EINVAL;
 	}
 
-	spin_lock_irqsave(&dev->tasklet_lock, irqflags);
+	if (dev->irq_enabled) {
+		spin_lock_irqsave(&dev->tasklet_lock, irqflags);
 
-	if (dev->locked_tasklet_func) {
-		dev->locked_tasklet_func(dev);
+		if (dev->locked_tasklet_func) {
+			dev->locked_tasklet_func(dev);
 
-		dev->locked_tasklet_func = NULL;
-	}
+			dev->locked_tasklet_func = NULL;
+		}
 
-	spin_unlock_irqrestore(&dev->tasklet_lock, irqflags);
+		spin_unlock_irqrestore(&dev->tasklet_lock, irqflags);
+	}
 
 	atomic_inc(&dev->counts[_DRM_STAT_UNLOCKS]);
 
diff --git a/shared-core/drm_drawable.c b/shared-core/drm_drawable.c
index 5e2fc86..d203b24 100644
--- a/shared-core/drm_drawable.c
+++ b/shared-core/drm_drawable.c
@@ -132,7 +132,8 @@ int drm_rmdraw(DRM_IOCTL_ARGS)
 {
 	DRM_DEVICE;
 	drm_draw_t draw;
-	unsigned int id, idx, shift;
+	int id, idx;
+	unsigned int shift;
 	unsigned int irqflags;
 	u32 *bitfield = dev->drw_bitfield;
 	unsigned int bitfield_length = dev->drw_bitfield_length;
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to