[CFT] MPSAFE firewire

[Doug Rabson]

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 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