All of lore.kernel.org
 help / color / mirror / Atom feed
From: Santiago Leon <santil@us.ibm.com>
To: Andrew Morton <akpm@osdl.org>, netdev <netdev@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>
Cc: Santiago Leon <santil@us.ibm.com>, Jeff Garzik <jgarzik@pobox.com>
Subject: [PATCH 3/5] ibmveth fix buffer replenishing
Date: Wed, 26 Oct 2005 10:47:08 -0600	[thread overview]
Message-ID: <20051026164555.21820.85804.sendpatchset@localhost.localdomain> (raw)
In-Reply-To: <20051026164532.21820.72673.sendpatchset@localhost.localdomain>

This patch removes the allocation of RX skb's  buffers from a workqueue
to be called directly at RX processing time.  This change was suggested
by Dave Miller when the driver was starving the RX buffers and
deadlocking under heavy traffic:


> Allocating RX SKBs via tasklet is, IMHO, the worst way to
> do it.  It is no surprise that there are starvation cases.
> 
> If tasklets or work queues get delayed in any way, you lose,
> and it's very easy for a card to catch up with the driver RX'ing
> packets very fast, no matter how aggressive you make the
> replenishing.  By the time you detect that you need to be
> "more aggressive" it is already too late.
> The only pseudo-reliable way is to allocate at RX processing time.
> 

Signed-off-by: Santiago Leon <santil@us.ibm.com>
---

 drivers/net/ibmveth.c |   48 ++++++++----------------------------------------
 drivers/net/ibmveth.h |    4 ----
 2 files changed, 8 insertions(+), 44 deletions(-)

--- a/drivers/net/ibmveth.c	2005-10-17 12:04:58.000000000 -0500
+++ b/drivers/net/ibmveth.c	2005-10-17 12:23:22.000000000 -0500
@@ -96,7 +96,6 @@
 static void ibmveth_proc_register_adapter(struct ibmveth_adapter *adapter);
 static void ibmveth_proc_unregister_adapter(struct ibmveth_adapter *adapter);
 static irqreturn_t ibmveth_interrupt(int irq, void *dev_instance, struct pt_regs *regs);
-static inline void ibmveth_schedule_replenishing(struct ibmveth_adapter*);
 static inline void ibmveth_rxq_harvest_buffer(struct ibmveth_adapter *adapter);
 
 #ifdef CONFIG_PROC_FS
@@ -257,29 +256,7 @@
 	atomic_add(buffers_added, &(pool->available));
 }
 
-/* check if replenishing is needed.  */
-static inline int ibmveth_is_replenishing_needed(struct ibmveth_adapter *adapter)
-{
-	int i;
-
-	for(i = 0; i < IbmVethNumBufferPools; i++)
-		if(adapter->rx_buff_pool[i].active &&
-		  (atomic_read(&adapter->rx_buff_pool[i].available) <
-		   adapter->rx_buff_pool[i].threshold))
-			return 1;
-	return 0;
-}
-
-/* kick the replenish tasklet if we need replenishing and it isn't already running */
-static inline void ibmveth_schedule_replenishing(struct ibmveth_adapter *adapter)
-{
-	if(ibmveth_is_replenishing_needed(adapter) &&
-	   (atomic_dec_if_positive(&adapter->not_replenishing) == 0)) {
-		schedule_work(&adapter->replenish_task);
-	}
-}
-
-/* replenish tasklet routine */
+/* replenish routine */
 static void ibmveth_replenish_task(struct ibmveth_adapter *adapter) 
 {
 	int i;
@@ -292,10 +269,6 @@
 						     &adapter->rx_buff_pool[i]);
 
 	adapter->rx_no_buffer = *(u64*)(((char*)adapter->buffer_list_addr) + 4096 - 8);
-
-	atomic_inc(&adapter->not_replenishing);
-
-	ibmveth_schedule_replenishing(adapter);
 }
 
 /* empty and free ana buffer pool - also used to do cleanup in error paths */
@@ -563,10 +536,10 @@
 		return rc;
 	}
 
-	netif_start_queue(netdev);
+	ibmveth_debug_printk("initial replenish cycle\n");
+	ibmveth_replenish_task(adapter);
 
-	ibmveth_debug_printk("scheduling initial replenish cycle\n");
-	ibmveth_schedule_replenishing(adapter);
+	netif_start_queue(netdev);
 
 	ibmveth_debug_printk("open complete\n");
 
@@ -584,9 +557,6 @@
 
 	free_irq(netdev->irq, netdev);
 
-	cancel_delayed_work(&adapter->replenish_task);
-	flush_scheduled_work();
-
 	do {
 		lpar_rc = h_free_logical_lan(adapter->vdev->unit_address);
 	} while (H_isLongBusy(lpar_rc) || (lpar_rc == H_Busy));
@@ -795,7 +765,7 @@
 		}
 	} while(more_work && (frames_processed < max_frames_to_process));
 
-	ibmveth_schedule_replenishing(adapter);
+	ibmveth_replenish_task(adapter);
 
 	if(more_work) {
 		/* more work to do - return that we are not done yet */
@@ -931,8 +901,10 @@
 
 	}
 
+	/* kick the interrupt handler so that the new buffer pools get
+	   replenished or deallocated */
+	ibmveth_interrupt(dev->irq, dev, NULL);
 
-	ibmveth_schedule_replenishing(adapter);
 	dev->mtu = new_mtu;
 	return 0;	
 }
@@ -1017,14 +989,10 @@
 
 	ibmveth_debug_printk("adapter @ 0x%p\n", adapter);
 
-	INIT_WORK(&adapter->replenish_task, (void*)ibmveth_replenish_task, (void*)adapter);
-
 	adapter->buffer_list_dma = DMA_ERROR_CODE;
 	adapter->filter_list_dma = DMA_ERROR_CODE;
 	adapter->rx_queue.queue_dma = DMA_ERROR_CODE;
 
-	atomic_set(&adapter->not_replenishing, 1);
-
 	ibmveth_debug_printk("registering netdev...\n");
 
 	rc = register_netdev(netdev);
diff -urN a/drivers/net/ibmveth.h b/drivers/net/ibmveth.h
--- a/drivers/net/ibmveth.h	2005-10-17 12:04:58.000000000 -0500
+++ b/drivers/net/ibmveth.h	2005-10-17 12:23:22.000000000 -0500
@@ -118,10 +118,6 @@
     dma_addr_t filter_list_dma;
     struct ibmveth_buff_pool rx_buff_pool[IbmVethNumBufferPools];
     struct ibmveth_rx_q rx_queue;
-    atomic_t not_replenishing;
-
-    /* helper tasks */
-    struct work_struct replenish_task;
 
     /* adapter specific stats */
     u64 replenish_task_cycles;

  parent reply	other threads:[~2005-10-26 16:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-10-26 16:46 [PATCH 0/5] ibmveth resending various fixes Santiago Leon
2005-10-26 16:46 ` [PATCH 1/5] ibmveth fix bonding Santiago Leon
2005-10-28 20:07   ` Jeff Garzik
2005-10-26 16:47 ` [PATCH 2/5] ibmveth fix buffer pool management Santiago Leon
2005-10-26 16:47 ` Santiago Leon [this message]
2005-10-26 16:47 ` [PATCH 4/5] ibmveth lockless TX Santiago Leon
2005-10-26 16:47 ` [PATCH 5/5] ibmveth fix failed addbuf Santiago Leon
  -- strict thread matches above, loose matches on Subject: below --
2005-10-25 21:34 [PATCH] 3/5 ibmveth fix buffer replenishing Santiago Leon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20051026164555.21820.85804.sendpatchset@localhost.localdomain \
    --to=santil@us.ibm.com \
    --cc=akpm@osdl.org \
    --cc=jgarzik@pobox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.