On Wed, Feb 06, 2013 at 03:23:41PM +0100, Luigi Rizzo wrote: > The following patch implements interrupt moderation registers > for the e1000 adapter. These feature is normally used by OS > drivers, and their implementation improves performance significantly, > especially on the transmit path. > The feature can be disabled through a command line option. > We have seen only benefits in throughput, while latency slightly > increases (that is an inherent feature of interrupt moderation) > because very short delays cannot be emulated precisely. > > For those curious on performance, there is a set of measurements > (of this and other changes that we will post separately) at > > http://info.iet.unipi.it/~luigi/research.html#qemu
http://info.iet.unipi.it/~luigi/papers/20130206-qemu.pdf is 404. > +static void > +mit_set_ics(E1000State *s, uint32_t cause) > +{ > + if (s->mit_on == 0) { > + set_ics(s, 0, cause); > + return; > + } > + s->mit_cause |= cause; > + if (!s->mit_timer_on) > + mit_rearm_and_int(s); Please run scripts/checkpatch.pl. QEMU coding style always uses braces, even for an if statement with a single line body. > @@ -1302,6 +1377,10 @@ static int pci_e1000_init(PCIDevice *pci_dev) > > d->autoneg_timer = qemu_new_timer_ms(vm_clock, e1000_autoneg_timer, d); > > + d->mit_cause = 0; > + d->mit_timer_on = 0; > + d->mit_timer = qemu_new_timer_ns(vm_clock, mit_rearm_and_int, d); Missing qemu_del_timer(d->mit_timer) and qemu_free_timer(d->mit_timer) in pci_e1000_uninit(). > + > return 0; > } > > @@ -1313,6 +1392,7 @@ static void qdev_e1000_reset(DeviceState *dev) > > static Property e1000_properties[] = { > DEFINE_NIC_PROPERTIES(E1000State, conf), > + DEFINE_PROP_UINT32("mit_on", E1000State, mit_on, 1), /* default on */ Can access the performance results you linked to. Therefore it's hard to know whether default on makes sense. We must avoid performance regressions. Stefan
