Re: [Qemu-devel] interrupt mitigation for e1000

[Avi Kivity <avi@redhat.com>]

On 07/24/2012 07:58 PM, Luigi Rizzo wrote:
> I noticed that the various NIC modules in qemu/kvm do not implement
> interrupt mitigation, which is very beneficial as it dramatically
> reduces exits from the hypervisor.
> 
> As a proof of concept i tried to implement it for the e1000 driver
> (patch below), and it brings tx performance from 9 to 56Kpps on
> qemu-softmmu, and from ~20 to 140Kpps on qemu-kvm.
> 
> I am going to measure the rx interrupt mitigation in the next couple
> of days.
> 
> Is there any interest in having this code in ?

Indeed.  But please drop the #ifdef MITIGATIONs.

> +
> +#ifdef MITIGATION
> +    QEMUBH *int_bh;	// interrupt mitigation handler
> +    int tx_ics_count;	// pending tx int requests
> +    int rx_ics_count;	// pending rx int requests
> +    int int_cause;	// int cause

Use uint32_t for int_cause, also a correctly sized type for the packet
counts.

>  
> +#ifdef MITIGATION
> +    /* we transmit the first few packets, or we do if we are
> +     * approaching a full ring. in the latter case, also
> +     * send an ics.
> +     * 
> +     */
> +{
> +    int len, pending;
> +    len = s->mac_reg[TDLEN] / sizeof(desc) ;
> +    pending = s->mac_reg[TDT] - s->mac_reg[TDH];
> +    if (pending < 0)
> +	pending += len;
> +    /* ignore requests after the first few ones, as long as
> +     * we are not approaching a full ring.
> +     * Otherwise, deliver packets to the backend.
> +     */
> +    if (s->tx_ics_count > 4 && s->tx_ics_count + pending < len - 5)
> +	return;

Where do the 4 and 5 come from?  Shouldn't they be controlled by the
guest using a device register?

>      }
> +#ifdef MITIGATION
> +    s->int_cause |= cause; // remember the interrupt cause.
> +    s->tx_ics_count += pending;
> +    if (s->tx_ics_count >= len - 5) {
> +        // if the ring is about to become full, generate an interrupt

Another magic number.

> +	set_ics(s, 0, s->int_cause);
> +	s->tx_ics_count = 0;
> +	s->int_cause = 0;
> +    } else {	// otherwise just schedule it for later.
> +        qemu_bh_schedule_idle(s->int_bh);
> +    }
> +}
> +#else /* !MITIGATION */
>      set_ics(s, 0, cause);
> +#endif
>  }
>  

-- 
error compiling committee.c: too many arguments to function