On Mon, Jan 07, 2013 at 03:17:18PM -0600, Anthony Liguori wrote: > "Michael S. Tsirkin" <m...@redhat.com> writes: > > > On Mon, Jan 07, 2013 at 07:40:32PM +0100, fred.kon...@greensocs.com wrote: > >> @@ -130,7 +124,9 @@ static void virtio_net_set_status(struct VirtIODevice > >> *vdev, uint8_t status) > >> > >> static void virtio_net_set_link_status(NetClientState *nc) > >> { > >> - VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque; > >> + void *opaque = DO_UPCAST(NICState, nc, nc)->opaque; > >> + VirtIONet *n = VIRTIO_NET(opaque); > >> + VirtIODevice *vdev = VIRTIO_DEVICE(n); > >> uint16_t old_status = n->status; > >> > >> if (nc->link_down) > > > > I note this adds more pointer chasing due to runtime casts on data path > > operations. Can well be trivial but this really needs to be verified > > with a performance test. Was this done? Same comment applies to block. > > An alternative is to add _fast casts without runtime checks. > > I'm pretty sure other things like the one big global mutex are more > important than the number of pointer dereferences...
True. But we have plans to fix that, right? > Do you have any evidence to suggest that this would be a problem in practice? > > Regards, > > Anthony Liguori No. It's not this change specifically, I'm generally concerned if we let ourselves bloat datapath in an uncontrolled way, the waste will add up over time. > > > > -- > > MST