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