From owner-freebsd-hackers@FreeBSD.ORG Tue Nov 1 19:07:18 2005 Return-Path: X-Original-To: freebsd-hackers@freebsd.org Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C1F4916A422 for ; Tue, 1 Nov 2005 19:07:18 +0000 (GMT) (envelope-from julian@elischer.org) Received: from a50.ironport.com (a50.ironport.com [63.251.108.112]) by mx1.FreeBSD.org (Postfix) with ESMTP id 4EDF143D46 for ; Tue, 1 Nov 2005 19:07:18 +0000 (GMT) (envelope-from julian@elischer.org) Received: from unknown (HELO [10.251.23.117]) ([10.251.23.117]) by a50.ironport.com with ESMTP; 01 Nov 2005 11:07:18 -0800 X-IronPort-Anti-Spam-Filtered: true Message-ID: <4367BCE5.2070900@elischer.org> Date: Tue, 01 Nov 2005 11:07:17 -0800 From: Julian Elischer User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.11) Gecko/20050727 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Julian Elischer References: <4360B8EE.4070605@alphaque.com> <4360DD7B.20900@samsco.org> <4361044B.50807@alphaque.com> <20051027.205250.55834228.imp@bsdimp.com> <4361E3E0.4090409@alphaque.com> <43676121.4030801@alphaque.com> <436791ED.8010808@samsco.org> <4367AA8D.3060506@alphaque.com> <4367BBDB.7020005@elischer.org> In-Reply-To: <4367BBDB.7020005@elischer.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-hackers@freebsd.org, Scott Long Subject: Re: locking in a device driver X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Nov 2005 19:07:18 -0000 Julian Elischer wrote: > Dinesh Nair wrote: > >> >> 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 > > > handler called at splxxx() level (see later) > >> 2. device driver reads data from DMA memory space > > > still at splxxx level > >> 3. device driver writes to a shared buffer > > > still at splxxx level > >> >> 4. device driver calls a function inside pseudo driver > > > still at splxxx level > >> 5. pseudo driver copies data from shared buffer to another buffer > > > still at splxx level > >> 6. wakeup() is called > > > still at splxxx level > >> 7. device driver waits for next interrupt > > > drops to splzero or similar,.. > woken process called, > starts manipulating "another buffer" > collides with next interrupt. > > what ever is woken up needs to call splxxx() to block the interupt > routine while > manipulating "another buffer" for as long as it takes to get the data out > or swap the b uffer pointers or whatever it does. > > >> >> the interrupt handler uses splhigh()/splx() to mask out interrupts >> during the time it's executing. interrupts happen 1000 times a second >> consistently. > the interrupt handler is ALREADY blocking it's own interupt. you don't need to do anything. You need to block interrupts for a momen tin #8, not in the interrupt handler. > > > When you register an interrupt handler, you specify which of several > groups you are a part of.. > Network, disk io, etc. > > once you specify you are part of a particular set, then you should only > block interrupts from that set.. > > e.g, a netowrk driver would use s = splimp(); ...; splx(s) > > Try not to use splhigh().. > it is ok for getting your driver going but may block too much. > >> >> 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 > > > > it needs to call splxxx() while it is doing it.. > I would suggest having two buffers and swapping them under splxxx() so > that > the one that the driver is accessing is not the one you are draining. > that way teh splxxx() levle needs to only be held for the small time > you are doing the swap. > >> >> exactly the reverse happens for a write. >> >> i believe that steps 3,5,8 need to be protected/synchronized with locks. > > > > not locks, but spl, > and only step 8 needs to be changed because all teh rest are already > done at high spl. > >> >> 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 ? >> > > use spl???()/splx uinstead. > your top end needs to raise to the same level as the interrupt you have > registered for as long as it is manupulating data that the bottom end > might manipulate, (e.g. buffer pointers or linked lists, etc.) > _______________________________________________ > freebsd-hackers@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-hackers > To unsubscribe, send any mail to > "freebsd-hackers-unsubscribe@freebsd.org"