[CFT] MPSAFE firewire
Hidetoshi Shimokawa
simokawa at FreeBSD.ORG
Fri Jun 29 11:36:37 UTC 2007
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 at 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 at FreeBSD.ORG
More information about the freebsd-firewire
mailing list