Re: [PATCH] Reinit the RX DMA buffer to avoid skb conflict， Handle all RX packets in the DMA ring buffer

[YanBo]

On Fri, May 24, 2013 at 3:24 PM, Kalle Valo <kvalo at qca.qualcomm.com> wrote:
> YanBo <dreamfly281 at gmail.com> writes:
>
>> This patch is used to reallocation the rx skb buffer instead of clone
>> it, cause if just clone the skb, the hardware may refill the skb
>> buffer when the upper layer still ir. and this skb buffer is still in
>> DMA mapping status, it should not be access by the CPU before unmap
>> it.
>>
>> Another function of this patch is it will check every possible filled
>> RX descriptor in the ring buffer and handle them at once, normally the
>> hardware may fill more than one RX descriptors but just trigger one
>> interrupt to reduce CPU interrupt load.
>>
>> (This patch is just for pre review cause it is just generated after
>> rebased on the newest wcn36xx repo. I have tested it in the previous
>> wcn36xx repo.  but still not verify it after rebase due to lack of
>> time, welcome to get you feedback for this)
>
> The patch was severily whitspace damaged so it was pretty difficult to
> review. I recommend using git send-email to submit patches, it usually
> gets everything right.
>

Yes, seem the gmail wrapper some sentence automatic, any method to avoid this?
I will send the official  patch via the github pull request.


> I need to review this again, but I found one issue now:
>
>> +static int wcn36xx_dxe_fill_skb(struct wcn36xx_dxe_ctl *cur_dxe_ctl)
>> +{
>> + struct wcn36xx_dxe_desc *cur_dxe_desc = cur_dxe_ctl->desc;
>> + struct sk_buff *skb;
>> +
>> + skb = alloc_skb(WCN36XX_PKT_SIZE, GFP_ATOMIC);
>> +        if (skb == NULL) {
>> + wcn36xx_error("DMA buffer allocation failed");
>> + return -1;
>
> This needs to be -ENOMEM or similar. Also there's a general rule in
> Linux that memory allocation failures don't need any error messages, the
> memory subsystem handles that.
>
> --

Agree, will fix it and resubmit.

BR /Yanbo