Re: [PATCH 2/2] mwifiex: print URB submit failure error after threshold attemtps

[Brian Norris <briannorris@chromium.org>]

Hi Kalle,

On Wed, Sep 20, 2017 at 07:30:29AM +0300, Kalle Valo wrote:
> Brian Norris <briannorris@chromium.org> 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:

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

If you're asking me...

All I was saying was that I don't think Joe's suggestion will help
Ganapathi. I'd expect Ganapathi could confirm/deny that part. (Or Joe
could correct me if my interpretation is wrong.)

I'm also not familiar with how we expect dev_alloc_skb() failures to be
handled. If that's a common expected failure mode in low-memory
situations (seems reasonable?) and the driver handles these failure just
fine (completely unreviewed by me, so far; I suspect it doesn't do this
completely correctly), then sure, being less noisy about it as done in
this patch should be fine.

IOW, I don't have concrete feedback for Ganapathi to address, but I'm
not exactly "ack"ing it myself.

Brian