Re: iwlagn: order 2 page allocation failures

[Frans Pop]

Hi Reinette,

On Wednesday 09 September 2009, reinette chatre wrote:
> I agree that this patch may be the reason we are seeing this issue. We
> would like to keep using GFP_ATOMIC here, but it is not necessary for
> an allocation failure to be so noisy since the function doing the
> allocation (iwl_rx_allocate) is always followed by a call to
> iwl_rx_queue_restock which will schedule a refill if the buffers are
> running low. We can thus use ___GFP_NOWARN for the allocations in
> iwl_rx_allocate and leave it to the restocking to find the needed
> memory when it tried its allocations using GFP_KERNEL.
>
> I do think it is useful to let user know about these allocation
> failures, even if it does not result in packet loss. Wrapping it in
> net_ratelimit() will help though.
>
> How about the patch below?

A couple of comments/suggestions.

If the driver recovers cleanly from the error, shouldn't the priority of 
the message be lowered from CRIT to INFO. Or maybe even DEBUG; AFAIK most 
distros will have DEBUG_KERNEL enabled by default.
This means it will still show up in logs, but not the console.

Shouldn't the message include some indication that the driver will recover 
by itself and that no user action is needed? This will help avoid users 
(unnecessarily) reporting this as a bug when it does occur.
I expect that users would still report it if they get flooded with the 
message (or ratelimit warnings for it), which I think is what you'd want.

And maybe s/Can not/Cannot/ or s/Can not/Failed to/.

Cheers,
FJP
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/