netdev.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).