Re: [Qemu-devel] [PATCH 19/61] virtio-net : cleanup : use QOM cast.

[Michael S. Tsirkin]

On Mon, Jan 07, 2013 at 03:17:18PM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin" <address@hidden> writes:
> 
> > On Mon, Jan 07, 2013 at 07:40:32PM +0100, address@hidden 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