From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57661) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZEwpW-0007CQ-NF for qemu-devel@nongnu.org; Tue, 14 Jul 2015 05:48:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZEwpQ-0006Op-4d for qemu-devel@nongnu.org; Tue, 14 Jul 2015 05:48:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60586) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZEwpP-0006Ol-UO for qemu-devel@nongnu.org; Tue, 14 Jul 2015 05:48:44 -0400 Date: Tue, 14 Jul 2015 17:48:40 +0800 From: Fam Zheng Message-ID: <20150714094840.GD27873@ad.nay.redhat.com> References: <1436860421-4604-1-git-send-email-famz@redhat.com> <1436860421-4604-7-git-send-email-famz@redhat.com> <55A4D775.40006@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55A4D775.40006@redhat.com> Subject: Re: [Qemu-devel] [PATCH for-2.4 06/12] etsec: Flush queue when rx buffer is consumed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang Cc: Peter Maydell , Peter Crosthwaite , Rob Herring , qemu-devel@nongnu.org, Michael Walle , Gerd Hoffmann , stefanha@redhat.com, "Edgar E. Iglesias" On Tue, 07/14 17:33, Jason Wang wrote: > > > On 07/14/2015 03:53 PM, Fam Zheng wrote: > > The BH will be scheduled when etsec->rx_buffer_len is becoming 0, which > > is the condition of queuing. > > > > Signed-off-by: Fam Zheng > > I suggest to squash this into patch 05/12 to unbreak bisection. 05/12 didn't change the logic, it just moved the semantics from returning 0 in .can_receive() to .receive(). So I think it's OK to split to make reviewing easier. > > And can we do this without a bh? Otherwise, we may need to stop and > restart the bh during vm stop and start? A bh doesn't hurt when vm stop and restart (we get superfluous flush), otherwise the call stack could go very deap with a long queue, something like: etsec_receive etsec_rx_ring_write etsec_walk_rx_ring etsec_flush_rx_queue qemu_flush_queued_packets ... etsec_receive etsec_rx_ring_write etsec_walk_rx_ring etsec_flush_rx_queue qemu_flush_queued_packets /* loop */ ... > > > --- > > hw/net/fsl_etsec/etsec.c | 9 +++++++++ > > hw/net/fsl_etsec/etsec.h | 2 ++ > > hw/net/fsl_etsec/rings.c | 1 + > > 3 files changed, 12 insertions(+) > > > > diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c > > index f5170ae..fcde9b4 100644 > > --- a/hw/net/fsl_etsec/etsec.c > > +++ b/hw/net/fsl_etsec/etsec.c > > @@ -366,6 +366,13 @@ static NetClientInfo net_etsec_info = { > > .link_status_changed = etsec_set_link_status, > > }; > > > > +static void etsec_flush_rx_queue(void *opaque) > > +{ > > + eTSEC *etsec = opaque; > > + > > + qemu_flush_queued_packets(qemu_get_queue(etsec->nic)); > > +} > > + > > static void etsec_realize(DeviceState *dev, Error **errp) > > { > > eTSEC *etsec = ETSEC_COMMON(dev); > > @@ -378,6 +385,8 @@ static void etsec_realize(DeviceState *dev, Error **errp) > > etsec->bh = qemu_bh_new(etsec_timer_hit, etsec); > > etsec->ptimer = ptimer_init(etsec->bh); > > ptimer_set_freq(etsec->ptimer, 100); > > + > > + etsec->flush_bh = qemu_bh_new(etsec_flush_rx_queue, etsec); > > } > > > > static void etsec_instance_init(Object *obj) > > diff --git a/hw/net/fsl_etsec/etsec.h b/hw/net/fsl_etsec/etsec.h > > index fc41773..05bb7f8 100644 > > --- a/hw/net/fsl_etsec/etsec.h > > +++ b/hw/net/fsl_etsec/etsec.h > > @@ -144,6 +144,8 @@ typedef struct eTSEC { > > QEMUBH *bh; > > struct ptimer_state *ptimer; > > > > + QEMUBH *flush_bh; > > + > > } eTSEC; > > > > #define TYPE_ETSEC_COMMON "eTSEC" > > diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c > > index a11280b..7aae93e 100644 > > --- a/hw/net/fsl_etsec/rings.c > > +++ b/hw/net/fsl_etsec/rings.c > > @@ -646,6 +646,7 @@ void etsec_walk_rx_ring(eTSEC *etsec, int ring_nbr) > > } else { > > etsec->rx_buffer_len = 0; > > etsec->rx_buffer = NULL; > > + qemu_bh_schedule(etsec->flush_bh); > > } > > > > RING_DEBUG("eTSEC End of ring_write: remaining_data:%zu\n", remaining_data); >