From owner-freebsd-firewire@FreeBSD.ORG Fri Jun 29 11:36:37 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 59E8C16A54F for ; Fri, 29 Jun 2007 11:36:37 +0000 (UTC) (envelope-from freebsd@gm.nunu.org) Received: from py-out-1112.google.com (py-out-1112.google.com [64.233.166.176]) by mx1.freebsd.org (Postfix) with ESMTP id 1D9F113C44B for ; Fri, 29 Jun 2007 11:36:36 +0000 (UTC) (envelope-from freebsd@gm.nunu.org) Received: by py-out-1112.google.com with SMTP id u77so1257074pyb for ; Fri, 29 Jun 2007 04:36:27 -0700 (PDT) Received: by 10.35.90.1 with SMTP id s1mr464352pyl.1183116605807; Fri, 29 Jun 2007 04:30:05 -0700 (PDT) Received: by 10.35.71.5 with HTTP; Fri, 29 Jun 2007 04:30:05 -0700 (PDT) Message-ID: <626eb4530706290430we45e352xf27592d7eac99cf8@mail.gmail.com> Date: Fri, 29 Jun 2007 20:30:05 +0900 From: "Hidetoshi Shimokawa" Sender: freebsd@gm.nunu.org To: "Doug Rabson" In-Reply-To: <200706291024.56591.dfr@rabson.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <626eb4530706060746u44226cfajcedb3e169996a51a@mail.gmail.com> <200706291024.56591.dfr@rabson.org> X-Google-Sender-Auth: 338ecdfdb5c1999a 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 11:36:37 -0000 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