On 19 February 2014 15:18, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
> Michal Kazior <michal.kazior@xxxxxxxxx> writes:
>
>> On 19 February 2014 13:48, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
>>> Michal Kazior <michal.kazior@xxxxxxxxx> writes:
>>>
>>>> +struct ath10k_hif_sg_item {
>>>> + u16 transfer_id;
>>>> + void *transfer_context;
>>>> + void *vaddr; /* for debugging mostly */
>>>> + u32 paddr;
>>>> + u16 len;
>>>> +};
>>>
>>> This is the part I don't like. Instead of adding our own structs we
>>> instead should have everything in skb->cb and pass the skbs around. The
>>> sad part was that last fall I was working on cleaning up that but never
>>> found the time to finish it :(
>>
>> There's simply not enough room to keep it all in ath10k_skb_cb
>> directly.
>
> After my cleanups transfer_context was not used and when using sk_buffs
> properly vaddr and len would be useless. So we would have only transfer
> id and paddr left. And when I was working on this they did fit to cb.
You'd need `len` so you can use part of the original MSDU as a frame
prefetch for htt tx descriptor.
>> It doesn't really make any sense to keep it there anyway because
>> sg_item is used as means to pass a complex function argument to
>> sg_tx().
>
> If we used skbs we would just give a list/queue of them and no need to
> have any extra structs.
Managing skbs would incur extra overhead. Skbs are also less flexible
and probably unnecessary (at least at this point).
Think about the qos workaround removal - you have to submit htt tx
descriptor frame prefetch in 2 parts instead of 1 (to make it appear
to FW as there's no QoS field). With skbs you have to allocate another
skb, copy part of MSDU into it... aaand you've just traded a single
memmove() for dev_alloc_skb(), skb_put() and a memcpy().
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
