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