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

[Kevin Wolf <kwolf@redhat.com>]

Am 31.10.2023 um 14:48 hat Richard W.M. Jones geschrieben:
> 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

Oh, this one (kvm-build-on-device) doesn't even show the object cast.
I was looking at kvm-build, it seems, where both the class and the
object cast are visible:

http://oirase.annexia.org/tmp/2023-kvm-build.svg?s=scsi_req_new&x=455.4&y=533

Kevin