Mel and Frans,
Thank you very much for digging into this.
On Wed, 2009-09-09 at 09:55 -0700, Mel Gorman wrote:
> Conceivably a better candidate for this problem is commit
> 4752c93c30441f98f7ed723001b1a5e3e5619829 introduced in May 2009. If there
> are less than RX_QUEUE_SIZE/2 left, it starts replenishing buffers. Mohamed,
> is it absolutly necessary it use GFP_ATOMIC there? If an allocation fails,
> does it always mean frames are dropped or could it just replenish what it
> can and try again later printing a warning only if allocation failures are
> resulting in packet loss?
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?
diff --git a/drivers/net/wireless/iwlwifi/iwl-rx.c b/drivers/net/wireless/iwlwifi/iwl-rx.c
index b90adcb..f0ee72e 100644
--- a/drivers/net/wireless/iwlwifi/iwl-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-rx.c
@@ -252,10 +252,11 @@ void iwl_rx_allocate(struct iwl_priv *priv, gfp_t priority)
/* Alloc a new receive buffer */
skb = alloc_skb(priv->hw_params.rx_buf_size + 256,
- priority);
+ priority | __GFP_NOWARN);
if (!skb) {
- IWL_CRIT(priv, "Can not allocate SKB buffers\n");
+ if (net_ratelimit())
+ IWL_CRIT(priv, "Can not allocate SKB buffer.\n");
/* We don't reschedule replenish work here -- we will
* call the restock method and if it still needs
* more buffers it will schedule replenish */
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 0909668..5d9fb78 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -1147,10 +1147,10 @@ static void iwl3945_rx_allocate(struct iwl_priv *priv, gfp_t priority)
spin_unlock_irqrestore(&rxq->lock, flags);
/* Alloc a new receive buffer */
- skb = alloc_skb(priv->hw_params.rx_buf_size, priority);
+ skb = alloc_skb(priv->hw_params.rx_buf_size, priority | __GFP_NOWARN);
if (!skb) {
if (net_ratelimit())
- IWL_CRIT(priv, ": Can not allocate SKB buffers\n");
+ IWL_CRIT(priv, "Can not allocate SKB buffer.\n");
/* We don't reschedule replenish work here -- we will
* call the restock method and if it still needs
* more buffers it will schedule replenish */
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
