From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:41224)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <shmulik.ladkani@ravellosystems.com>)
	id 1axv5Y-0006Vx-58
	for qemu-devel@nongnu.org; Wed, 04 May 2016 07:35:38 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <shmulik.ladkani@ravellosystems.com>)
	id 1axv5M-0003js-EB
	for qemu-devel@nongnu.org; Wed, 04 May 2016 07:35:26 -0400
Received: from mail-wm0-x22b.google.com ([2a00:1450:400c:c09::22b]:38710)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <shmulik.ladkani@ravellosystems.com>)
	id 1axv5L-0003Zy-1Z
	for qemu-devel@nongnu.org; Wed, 04 May 2016 07:35:20 -0400
Received: by mail-wm0-x22b.google.com with SMTP id g17so88515799wme.1
	for <qemu-devel@nongnu.org>; Wed, 04 May 2016 04:35:04 -0700 (PDT)
Date: Wed, 4 May 2016 14:34:59 +0300
From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Message-ID: <20160504143459.69178f46@pixies>
In-Reply-To: <1458200278-11940-2-git-send-email-sameeh@daynix.com>
References: <1458200278-11940-1-git-send-email-sameeh@daynix.com>
	<1458200278-11940-2-git-send-email-sameeh@daynix.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/2] e1000: Fixing interrupts pace.
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Sameeh Jubran <sameeh@daynix.com>
Cc: qemu-devel@nongnu.org, Jason Wang <jasowang@redhat.com>, Dmitry Fleytman <dmitry@daynix.com>, Yan Vugenfirer <yan@daynix.com>

Hi Sameeh,

On Thu, 17 Mar 2016 09:37:57 +0200, sameeh@daynix.com wrote:
> @@ -357,6 +357,14 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val)
>              }
>              mit_update_delay(&mit_delay, s->mac_reg[ITR]);
>  
> +            /*
> +             * According to e1000 SPEC, the Ethernet controller guarantees
> +             * a maximum observable interrupt rate of 7813 interrupts/sec.
> +             * Thus if mit_delay < 500 then the delay should be set to the
> +             * minimum delay possible which is 500.
> +             */
> +            mit_delay = (mit_delay < 500) ? 500 : mit_delay;
> +
>              if (mit_delay) {
>                  s->mit_timer_on = 1;
>                  timer_mod(s->mit_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +

Sorry for late response.

Formerly, 'mit_delay' could possibly be 0 (as being not updated by
any of the mit_update_delay calls), thus 'mit_timer' wouldn't be
armed.

The new logic forces mit_delay to be set to 500, even if it was 0
("unset").

Which approach is correct:
- Either the 'if (mit_delay)' is now superflous,
- Or, do we need to keep the "unset" sematics (i.e. mit_delay==0 means
  don't use the timer)

Regards,
Shmulik