locking in a device driver

Dinesh Nair dinesh at alphaque.com
Tue Nov 1 09:54:40 PST 2005

On 11/02/05 00:03 Scott Long said the following:
> I think this thread has gone too far into hyperbole and conjecture. What 
> is your code trying to do, and what problems are you seeing?

apologies, scott. i'm actually trying to get a driver written for freebsd 
5.x backported to 4.x. the driver works like a charm on 5.x.

under 4.x, i seem to be getting problems with synchronization/locking.

the driver set consists of two drivers, a pseudo driver which the userland 
reads/writes/ioctls to, and the actual device driver which reads/writes 
from the device thru DMA and places the data into a ring buffer.

this is the sequence of events for data from the device:

1. interrupt handler of device driver called
2. device driver reads data from DMA memory space
3. device driver writes to a shared buffer
4. device driver calls a function inside pseudo driver
5. pseudo driver copies data from shared buffer to another buffer
6. wakeup() is called
7. device driver waits for next interrupt

the interrupt handler uses splhigh()/splx() to mask out interrupts during 
the time it's executing. interrupts happen 1000 times a second consistently.

when a read on the pseudo device is called from a -lc_r threaded userland 
process, the following happens in pseudo device driver:

7. tsleep() is called, (with the same ident as the wakeup() in #6)
8. pseudo device reads from buffer in #5 and uses uiomove to return data to 
calling process

exactly the reverse happens for a write.

i believe that steps 3,5,8 need to be protected/synchronized with locks.

the code uses mtx_lock/mtx_unlock in the 5.x version, and uses simple_lock 
in the 4.x version. digging thru the include files, i've noticed that 
simple_lock is a NOOP in 4.x if you're on a single processor.

could i replace the mtx_lock/mtx_unlock with 
lockmgr(...,LK_EXCLUSIVE/LK_RELEASE) instead ? the earlier notion of using
splhigh()/splx() to protect the common areas didnt seem like the right way 
to do it since the pseudo device driver is not interrupt driven, but rather 
is driven by open/read/write/ioctl from the userland.

also, when the threaded userland reads, it's first put to a tsleep with 
PZERO|PCATCH and this tsleep is surrounded by a mtx_lock(&Giant) and 
mtx_unlock(&Giant). what should this be replaced with in 4.x ?

when the real device driver has data from the device, it uses wakeup() to 
wake the tsleep'ed bits in the pseudo device driver. is this the correct 
way to do this ?

also, is there any equivalent for PROC_LOCK()/PROC_UNLOCK() in 4.x or is it 
unnecessary ?

Regards,                           /\_/\   "All dogs go to heaven."
dinesh at alphaque.com                (0 0)    http://www.alphaque.com/
| for a in past present future; do                                        |
|   for b in clients employers associates relatives neighbours pets; do   |
|   echo "The opinions here in no way reflect the opinions of my $a $b."  |
| done; done                                                              |

More information about the freebsd-hackers mailing list