Date: Fri, 29 Jun 2007 13:08:47 +0100 From: Doug Rabson <dfr@rabson.org> To: Hidetoshi Shimokawa <simokawa@FreeBSD.ORG> Cc: freebsd-firewire@FreeBSD.ORG Subject: Re: [CFT] MPSAFE firewire Message-ID: <EB9CAB3B-FD9E-4687-99E6-721023534696@rabson.org> In-Reply-To: <626eb4530706290430we45e352xf27592d7eac99cf8@mail.gmail.com> References: <626eb4530706060746u44226cfajcedb3e169996a51a@mail.gmail.com> <200706291024.56591.dfr@rabson.org> <626eb4530706290430we45e352xf27592d7eac99cf8@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <dfr@rabson.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?EB9CAB3B-FD9E-4687-99E6-721023534696>