On Thu, 2012-06-21 at 15:19 -0400, Neil Horman wrote:
> On Thu, Jun 21, 2012 at 10:13:17AM -0700, Vasu Dev wrote:
> > On Wed, 2012-06-20 at 08:48 -0400, Neil Horman wrote:
> > > Noticed that we can shuffle the code around in fcoe_percpu_receive_thread
> > > a bit
> > > and avoid taking the fcoe_rx_list lock twice per iteration. This should
> > > improve
> > > throughput somewhat. With this change we take the lock, and check for new
> > > frames in a single critical section. Only if the list is empty do we
> > > drop the
> > > lock and re-acquire it after being signaled to wake up.
> > >
> > > Signed-off-by: Neil Horman <nhor...@tuxdriver.com>
> > > CC: Robert Love <robert.w.l...@intel.com>
> > > ---
> > > drivers/scsi/fcoe/fcoe.c | 18 ++++++++++--------
> > > 1 files changed, 10 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> > > index 0b48c7d..082951f 100644
> > > --- a/drivers/scsi/fcoe/fcoe.c
> > > +++ b/drivers/scsi/fcoe/fcoe.c
> > > @@ -1820,19 +1820,21 @@ static int fcoe_percpu_receive_thread(void *arg)
> > >
> > > spin_lock_bh(&p->fcoe_rx_list.lock);
> > > skb_queue_splice_init(&p->fcoe_rx_list, &tmp);
> > > - spin_unlock_bh(&p->fcoe_rx_list.lock);
> > > -
> > > - while ((skb = __skb_dequeue(&tmp)) != NULL)
> > > - fcoe_recv_frame(skb);
> > >
> > > - spin_lock_bh(&p->fcoe_rx_list.lock);
> > > - if (!skb_queue_len(&p->fcoe_rx_list)) {
> > > + while (!skb_queue_len(&tmp) && !kthread_should_stop()) {
> >
> > If skb_queue_len check is done on locked fcoe_rx_list then
> > skb_queue_splice_init() can be moved outside loop below with its only
> > one instance calling instead having twice. Just very minor pick along
> > this cleanup and anyway it should get called once on each thread wakeup
> > with you patch.
> >
> Good call, you're right. Although it gets a little tricky because you need to
> hold the list lock to prevent a race with the softirq waking you up right
> before
> you go to sleep.
Yes, check and thread state change is needed under the lock for that
reason.
> I can replace the additional lock/splice_init with a goto
> however, just to make it a bit mroe consice. That also lets me eliminate the
> second kthread_should_stop check.
Even more better with one less check, thanks Neil.
Vasu
_______________________________________________
devel mailing list
devel@open-fcoe.org
https://lists.open-fcoe.org/mailman/listinfo/devel
