Re: [CFT] MPSAFE firewire

Doug Rabson <[email protected]>
Newsgroups gmane.os.freebsd.devel.firewire
Message-ID <[email protected]>
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 <[email protected]> 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
> \/  [email protected]

_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-firewire
To unsubscribe, send any mail to "[email protected]"
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.