On Mon, 2002-12-02 at 03:00, Linus Torvalds wrote:
> On 1 Dec 2002, Michel Dänzer wrote:
> 
> > Does that also apply to kmalloc()/kfree(), or are they safe?
> 
> kfree() (or free_pages() or others of that type) is always safe.  
> kmalloc(x, GFP_ATOMIC) works, but has problems (ie being over-eager about 
> using it can easily result in a system that just dies from being out of 
> memory and not being able to do the required memory balancing and 
> swap-out to replenish it).

Luckily, we only need kfree() in the interrupt here, kmalloc() is done
in the ioctl. Thanks to you and Ben for explaining this.


> The attached patch looked mostly ok, except this part:
> 
>       -       sema_init( &dev->vbl_sem, 0 );
>       +       spin_lock_irqsave( &dev->vbl_lock, flags );
>       ..
> 
> should almost certainly have been a
> 
>               spin_lock_init(&dev->vbl_lock);
> 
> instead of locking and unlocking an uninitialized variable (from what I 
> could tell from just reading the diff).

Fixed, thanks. I'll commit the attached updated patch soon unless
someone sees another mistake. It works for me. Now I hope someone will
fill in the missing pieces and/or do the cleanup necessary for BSD.


> Btw, from a testing standpoint it's then also worthwhile compiling with 
> CONFIG_DEBUG_SPINLOCK, which will verify that spinlocks are initialized 
> etc.

I hope to get around to that some time.


-- 
Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer
XFree86 and DRI project member   /  CS student, Free Software enthusiast
Index: programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drmP.h
===================================================================
RCS file: /cvsroot/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drmP.h,v
retrieving revision 1.53
diff -p -u -r1.53 drmP.h
--- programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drmP.h	30 Nov 2002 14:24:06 -0000	1.53
+++ programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drmP.h	2 Dec 2002 11:43:05 -0000
@@ -591,8 +600,7 @@ typedef struct drm_device {
 #if __HAVE_VBL_IRQ
    	wait_queue_head_t vbl_queue;
    	atomic_t          vbl_received;
-	struct tq_struct  vbl_tq;
-	struct semaphore  vbl_sem;
+	spinlock_t        vbl_lock;
 	drm_vbl_sig_t     vbl_sigs;
 #endif
 	cycles_t	  ctx_start;
@@ -834,7 +845,7 @@ extern void          DRM(driver_irq_unin
 extern int           DRM(wait_vblank)(struct inode *inode, struct file *filp,
 				      unsigned int cmd, unsigned long arg);
 extern int           DRM(vblank_wait)(drm_device_t *dev, unsigned int *vbl_seq);
-extern void          DRM(vbl_immediate_bh)( void *arg );
+extern void          DRM(vbl_send_signals)( drm_device_t *dev );
 #endif
 #if __HAVE_DMA_IRQ_BH
 extern void          DRM(dma_immediate_bh)( void *dev );
Index: programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_dma.h
===================================================================
RCS file: /cvsroot/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_dma.h,v
retrieving revision 1.8
diff -p -u -r1.8 drm_dma.h
--- programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_dma.h	30 Nov 2002 14:24:07 -0000	1.8
+++ programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_dma.h	2 Dec 2002 11:43:11 -0000
@@ -509,6 +509,9 @@ int DRM(dma_get_buffers)(drm_device_t *d
 int DRM(irq_install)( drm_device_t *dev, int irq )
 {
 	int ret;
+#if __HAVE_VBL_IRQ
+	unsigned long flags;
+#endif
 
 	if ( !irq )
 		return -EINVAL;
@@ -541,16 +544,9 @@ int DRM(irq_install)( drm_device_t *dev,
 #if __HAVE_VBL_IRQ
 	init_waitqueue_head(&dev->vbl_queue);
 
-	sema_init( &dev->vbl_sem, 0 );
+	spin_lock_init( &dev->vbl_lock );
 
 	INIT_LIST_HEAD( &dev->vbl_sigs.head );
-
-	up( &dev->vbl_sem );
-
-	INIT_LIST_HEAD( &dev->vbl_tq.list );
-	dev->vbl_tq.sync = 0;
-	dev->vbl_tq.routine = DRM(vbl_immediate_bh);
-	dev->vbl_tq.data = dev;
 #endif
 
 				/* Before installing handler */
@@ -642,6 +638,7 @@ int DRM(wait_vblank)( DRM_IOCTL_ARGS )
 	flags = vblwait.request.type & _DRM_VBLANK_FLAGS_MASK;
 	
 	if ( flags & _DRM_VBLANK_SIGNAL ) {
+		unsigned long flags;
 		drm_vbl_sig_t *vbl_sig = DRM_MALLOC( sizeof( drm_vbl_sig_t ) );
 
 		if ( !vbl_sig )
@@ -656,11 +653,11 @@ int DRM(wait_vblank)( DRM_IOCTL_ARGS )
 		vblwait.reply.sequence = atomic_read( &dev->vbl_received );
 
 		/* Hook signal entry into list */
-		down( &dev->vbl_sem );
+		spin_lock_irqsave( &dev->vbl_lock, flags );
 
 		list_add_tail( (struct list_head *) vbl_sig, &dev->vbl_sigs.head );
 
-		up( &dev->vbl_sem );
+		spin_unlock_irqrestore( &dev->vbl_lock, flags );
 	} else {
 		ret = DRM(vblank_wait)( dev, &vblwait.request.sequence );
 
@@ -675,14 +672,14 @@ int DRM(wait_vblank)( DRM_IOCTL_ARGS )
 	return ret;
 }
 
-void DRM(vbl_immediate_bh)( void *arg )
+void DRM(vbl_send_signals)( drm_device_t *dev )
 {
-	drm_device_t *dev = (drm_device_t *) arg;
 	struct list_head *entry, *tmp;
 	drm_vbl_sig_t *vbl_sig;
 	unsigned int vbl_seq = atomic_read( &dev->vbl_received );
+	unsigned long flags;
 
-	down( &dev->vbl_sem );
+	spin_lock_irqsave( &dev->vbl_lock, flags );
 
 	list_for_each_safe( entry, tmp, &dev->vbl_sigs.head ) {
 
@@ -699,7 +696,7 @@ void DRM(vbl_immediate_bh)( void *arg )
 		}
 	}
 
-	up( &dev->vbl_sem );
+	spin_unlock_irqrestore( &dev->vbl_lock, flags );
 }
 
 #endif	/* __HAVE_VBL_IRQ */
Index: programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/mga_irq.c
===================================================================
RCS file: /cvsroot/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/mga_irq.c,v
retrieving revision 1.2
diff -p -u -r1.2 mga_irq.c
--- programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/mga_irq.c	30 Nov 2002 14:24:07 -0000	1.2
+++ programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/mga_irq.c	2 Dec 2002 11:43:11 -0000
@@ -50,10 +50,7 @@ void mga_dma_service( DRM_IRQ_ARGS )
 		MGA_WRITE( MGA_ICLEAR, MGA_VLINEICLR );
 		atomic_inc(&dev->vbl_received);
 		DRM_WAKEUP(&dev->vbl_queue);
-
-		/* kick off bottom half for signals */
-		queue_task(&dev->vbl_tq, &tq_immediate);
-		mark_bh(IMMEDIATE_BH);
+		DRM(vbl_send_signals)( dev );
 	}
 }
 
Index: programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/r128_irq.c
===================================================================
RCS file: /cvsroot/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/r128_irq.c,v
retrieving revision 1.2
diff -p -u -r1.2 r128_irq.c
--- programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/r128_irq.c	30 Nov 2002 14:24:07 -0000	1.2
+++ programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/r128_irq.c	2 Dec 2002 11:43:11 -0000
@@ -50,10 +50,7 @@ void r128_dma_service( DRM_IRQ_ARGS )
 		R128_WRITE( R128_GEN_INT_STATUS, R128_CRTC_VBLANK_INT_AK );
 		atomic_inc(&dev->vbl_received);
 		DRM_WAKEUP(&dev->vbl_queue);
-
-		/* kick off bottom half for signals */
-		queue_task(&dev->vbl_tq, &tq_immediate);
-		mark_bh(IMMEDIATE_BH);
+		DRM(vbl_send_signals)( dev );
 	}
 }
 
Index: programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/radeon_irq.c
===================================================================
RCS file: /cvsroot/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/radeon_irq.c,v
retrieving revision 1.9
diff -p -u -r1.9 radeon_irq.c
--- programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/radeon_irq.c	30 Nov 2002 14:24:07 -0000	1.9
+++ programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/radeon_irq.c	2 Dec 2002 11:43:11 -0000
@@ -74,10 +74,7 @@ void DRM(dma_service)( DRM_IRQ_ARGS )
 	if (stat & RADEON_CRTC_VBLANK_STAT) {
 		atomic_inc(&dev->vbl_received);
 		DRM_WAKEUP(&dev->vbl_queue);
-
-		/* kick off bottom half for signals */
-		queue_task(&dev->vbl_tq, &tq_immediate);
-		mark_bh(IMMEDIATE_BH);
+		DRM(vbl_send_signals)( dev );
 	}
 
 	/* Acknowledge all the bits in GEN_INT_STATUS -- seem to get

Reply via email to