From owner-freebsd-firewire@FreeBSD.ORG Fri Jun 29 12:48:54 2007 Return-Path: X-Original-To: freebsd-firewire@FreeBSD.ORG Delivered-To: freebsd-firewire@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 1750116A484; Fri, 29 Jun 2007 12:48:54 +0000 (UTC) (envelope-from dfr@rabson.org) Received: from mail.qubesoft.com (gate.qubesoft.com [217.169.36.34]) by mx1.freebsd.org (Postfix) with ESMTP id 9055013C480; Fri, 29 Jun 2007 12:48:53 +0000 (UTC) (envelope-from dfr@rabson.org) Received: from [10.201.19.245] (doug02.dyn.qubesoft.com [10.201.19.245]) by mail.qubesoft.com (8.13.3/8.13.3) with ESMTP id l5TC926s045698; Fri, 29 Jun 2007 13:09:11 +0100 (BST) (envelope-from dfr@rabson.org) In-Reply-To: <626eb4530706290430we45e352xf27592d7eac99cf8@mail.gmail.com> References: <626eb4530706060746u44226cfajcedb3e169996a51a@mail.gmail.com> <200706291024.56591.dfr@rabson.org> <626eb4530706290430we45e352xf27592d7eac99cf8@mail.gmail.com> Mime-Version: 1.0 (Apple Message framework v752.2) Content-Type: text/plain; charset=US-ASCII; delsp=yes; format=flowed Message-Id: Content-Transfer-Encoding: 7bit From: Doug Rabson Date: Fri, 29 Jun 2007 13:08:47 +0100 To: Hidetoshi Shimokawa X-Mailer: Apple Mail (2.752.2) X-Spam-Status: No, score=-2.8 required=5.0 tests=ALL_TRUSTED autolearn=failed version=3.0.4 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.qubesoft.com X-Virus-Scanned: ClamAV 0.86.2/3554/Fri Jun 29 11:58:29 2007 on mail.qubesoft.com X-Virus-Status: Clean Cc: freebsd-firewire@FreeBSD.ORG Subject: Re: [CFT] MPSAFE firewire X-BeenThere: freebsd-firewire@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Firewire support in FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jun 2007 12:48:54 -0000 Thanks for clarifying, it makes sense to me now. I guess this also covers the fragment reassembly code in if_fwsubr.c. Perhaps a comment should be added to make it clear that the caller of firewire_input is responsible for serialising access. On 29 Jun 2007, at 12:30, Hidetoshi Shimokawa wrote: > All the access to the RX DMA is dispatched from a taskqueue, > so that they are serialized and they don't need a lock. > As far as I understand, the code paths you concern are running > in a single thread(fw_taskq). > > Correct me, if I'm wrong. > > FWXFERQ_HANDLER is currently used only by fwe and fwip. > For DV streams, since the queue is access by userland threads, > we need a lock. > > On 6/29/07, Doug Rabson wrote: >> >> >> On Wednesday 06 June 2007, Hidetoshi Shimokawa wrote: >> >> > I have just committed MPSAFE(Giant free) firewire driver to - >> current. >> >> > If you have any problem, please let me know. >> >> >> >> I've been looking through the code and I have a few questions. >> >> >> >> In fwohci_rbuf_update(), you only call FW_GLOCK() if the >> FWXFERQ_HANDLER >> flag is zero. Doesn't this cause problems for the if_fwip driver >> (the only >> one to set this flag)? As far as I can see, if this flag is set, >> there is no >> mutex protection for any of the dma queues. Shouln't FW_GLOCK be used >> always? Also, additional protection is needed in fwip_stream_input >> where it >> manipulates the stvalid and stfree queues. >> >> >> >> I'm a bit confused about the async read path too. I'm looking at >> the code in >> fwohci_arcv() and I can't see any mutex protection in this >> function while it >> manipulates the buffers. Is this correct? I see some fossil use of >> splfw() >> here which is why I ask. Following the input path back to the fwip >> driver >> again, I can't see any mutex protection for the driver's unicast >> packet >> input queues. >> >> >> >> The last possible problem I noticed reading through the code is >> that there >> is no mutex protection of the fragmented packet reassembly queues in >> firewire_input_fragment. Perhaps the fw_com structure should have >> a mutex >> pointer in it which can be initialised to the if_fwip code's mutex >> and used >> in this case. >> >> > > > -- > /\ Hidetoshi Shimokawa > \/ simokawa@FreeBSD.ORG