Re: [PATCH 4/7] hw/scsi/virtio-scsi: Use VIRTIO_SCSI_COMMON() macro

["Richard W.M. Jones" <rjones@redhat.com>]

On Tue, Oct 31, 2023 at 02:17:56PM +0100, Kevin Wolf wrote:
> Am 17.10.2023 um 16:01 hat Philippe Mathieu-Daudé geschrieben:
> > Access QOM parent with the proper QOM VIRTIO_SCSI_COMMON() macro.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >  hw/scsi/virtio-scsi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> > index 45b95ea070..fa53f0902c 100644
> > --- a/hw/scsi/virtio-scsi.c
> > +++ b/hw/scsi/virtio-scsi.c
> > @@ -761,7 +761,7 @@ static void virtio_scsi_fail_cmd_req(VirtIOSCSIReq *req)
> >  
> >  static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
> >  {
> > -    VirtIOSCSICommon *vs = &s->parent_obj;
> > +    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
> >      SCSIDevice *d;
> >      int rc;
> 
> Why is a dynamic cast more "proper" than a static type-safe access, even
> more so in a hot I/O path?
> 
> Rich Jones posted a flamegraph the other day that surprised me because
> object_class_dynamic_class_assert() and object_dynamic_cast_assert()
> were shown to be a big part of scsi_req_new(). In the overall
> performance, it's probably dwarved by other issues, but unnecessary
> little things can add up, too.

I think Kevin is referring to one of these flamegraphs:

  http://oirase.annexia.org/tmp/2023-kvm-build-on-device.svg
  http://oirase.annexia.org/tmp/2023-kvm-build.svg

Here's a zoom showing scsi_req_new (hopefully this URL is stable ...):

  http://oirase.annexia.org/tmp/2023-kvm-build-on-device.svg?s=scsi_req_new&x=512.9&y=501

Note that qemu has been compiled with QOM cast debug.  This is the
default for Fedora (not RHEL) because we'd like to get early detection
of bugs from Fedora users.

There was another patch recently where a simple change saved about 5%
of total runtime in RISC-V TCG guests (admittedly a much more hot path
than this one).

  https://lists.nongnu.org/archive/html/qemu-devel/2023-10/msg02388.html

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW