"Michael S. Tsirkin" <m...@redhat.com> writes:
> 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.
I don't think a pointer indirection can be called "bloat[ing] datapath
in an uncontrolled way".
We shouldn't focus on minor things like this, but rather on things like
notification mitigation and lock contention which we know has a direct
impact on performance.
Regards,
Anthony Liguori
>
>> >
>> > --
>> > MST
