Brian Norris <[email protected]> writes: > Hi Ganapathi, > > On Thu, Sep 14, 2017 at 02:14:24PM +0000, Ganapathi Bhat wrote: >> > On Thu, 2017-08-31 at 01:21 +0530, Ganapathi Bhat wrote: >> > > @@ -300,9 +300,16 @@ static int mwifiex_usb_submit_rx_urb(struct >> > urb_context *ctx, int size) >> > > if (card->rx_cmd_ep != ctx->ep) { >> > > ctx->skb = dev_alloc_skb(size); >> > > if (!ctx->skb) { >> > > - mwifiex_dbg(adapter, ERROR, >> > > - "%s: dev_alloc_skb failed\n", >> > > __func__); >> > > + if (++card->rx_urb_failure_count > >> > > + MWIFIEX_RX_URB_FAILURE_THRESHOLD) { >> > > + mwifiex_dbg(adapter, ERROR, >> > > + "%s: dev_alloc_skb failed, >> > > failure >> > count = %u\n", >> > > + __func__, >> > > + card->rx_urb_failure_count); >> > > + } >> > > return -ENOMEM; >> > >> > Why not use a ratelimit? >> Since this is for receive, the packets are from AP side and we cannot >> lower the rate from AP. On some low performance systems this change >> will be helpful. > > I think Joe was referring to things like printk_ratelimited() or > dev_err_ratelimited(). Those automatically ratelimit prints for you, > using a static counter. You'd just need to make a small warpper for > mwifiex_dbg() using __ratelimit(). > > Those sort of rate limits are significantly different than yours though. > You were looking to avoid printing errors when there are only a few > failures in a row, whereas the existing rate-limiting infrastructure > looks to avoid printing errors if too many happen in a row. Those are > different goals.
Are you saying that this patch is good to take? Or should Ganapathi submit v2? -- Kalle Valo
