On 28 February 2014 10:06, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
> Michal Kazior <michal.kazior@xxxxxxxxx> writes:
>
>> Going through full htc tx path for htt tx is a
>> waste of resources. By skipping it it's possible
>> to easily submit scatter-gather to the pci hif for
>> reduced host cpu load and improved performance.
>>
>> The new approach uses dma pool to store the
>> following metadata for each tx request:
>> * msdu fragment list
>> * htc header
>> * htt tx command
>>
>> The htt tx command contains a msdu prefetch.
>> Instead of copying it original mapped msdu address
>> is used to submit a second scatter-gather item to
>> hif to make a complete htt tx command.
>>
>> The htt tx command itself hands over dma mapped
>> pointers to msdus and completion of the command
>> itself doesn't mean the frame has been sent and
>> can be unmapped/freed. This is why htc tx
>> completion is skipped for htt tx as all tx related
>> resources are freed upon htt tx completion
>> indication event (which also implicitly means htt
>> tx command itself was completed).
>>
>> Since now each htt tx request effectively consists
>> of 2 copy engine items CE_HTT_H2T_MSG_SRC_NENTRIES
>> is updated to allow maximum of
>> TARGET_10X_NUM_MSDU_DESC msdus being queued. This
>> keeps the tx path resource management simple.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@xxxxxxxxx>
>> ---
>> v2:
>> * improve commit log
>> * improve comment in code
>> * fix sparse/checkpatch/buildbot warnings
>
> [...]
>
>> --- a/drivers/net/wireless/ath/ath10k/htc.c
>> +++ b/drivers/net/wireless/ath/ath10k/htc.c
>> @@ -202,10 +202,8 @@ static int ath10k_htc_tx_completion_handler(struct ath10k *ar,
>> struct ath10k_htc *htc = &ar->htc;
>> struct ath10k_htc_ep *ep = &htc->endpoint[eid];
>>
>> - if (!skb) {
>> - ath10k_warn("invalid sk_buff completion - NULL pointer. firmware crashed?\n");
>> + if (WARN_ON(!skb))
>> return 0;
>> - }
>
> WARN_ON() is a bit dangerous here as it might cause excessive spamming.
> Why did you want to change this? I think either ath10k_warn() or
> WARN_ON_ONCE() would be safer, but not sure which one to use.
After the scatter-gather patch no NULL skb should be ever passed to tx
completion handler as those are ignored by hif/pci. Perhaps the hunk
should be moved from this patch to the scatter-gather one.
Perhaps WARN_ON() is a bit over the top here, but since this is now
more of a logic issue rather than runtime issue I decided to change it
from ath10k_warn to WARN_ON(). It's probably still a good idea to make
it _ONCE generally, although if you actually get skbuff it's already
too late or it should be screaming loudly because someone must've
changed the code in an incorrect/incomplete way.
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
