qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 2/2] virtio-scsi: Optimize virtio_scsi_init_r


From: Fam Zheng
Subject: Re: [Qemu-devel] [PATCH v2 2/2] virtio-scsi: Optimize virtio_scsi_init_req
Date: Tue, 16 Sep 2014 15:16:11 +0800
User-agent: Mutt/1.5.23 (2014-03-12)

On Mon, 09/15 12:17, Paolo Bonzini wrote:
> Il 15/09/2014 07:23, Fam Zheng ha scritto:
> >      SCSIRequest *sreq;
> >      size_t resp_size;
> >      enum SCSIXferMode mode;
> > -    QEMUIOVector resp_iov;
> >      union {
> >          VirtIOSCSICmdResp     cmd;
> >          VirtIOSCSICtrlTMFResp tmf;
> > @@ -68,23 +75,27 @@ static inline SCSIDevice 
> > *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
> >  static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq)
> >  {
> >      VirtIOSCSIReq *req;
> > -    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
> > -
> > -    req = g_malloc0(sizeof(*req) + vs->cdb_size);
> > +    VirtIOSCSICommon *vs = (VirtIOSCSICommon *)s;
> > +    const size_t zero_skip = offsetof(VirtIOSCSIReq, elem)
> > +                             + sizeof(VirtQueueElement);
> >  
> > +    req = g_slice_alloc(sizeof(*req) + vs->cdb_size);
> 
> Looks good, but why do you need to zero the union?  You only need to
> zero sreq, resp_size and mode, don't you (and at this point, memset
> becomes superfluous)?
> 

The structures in unions are not zeroed by caller, also leaving them breaks
virtio-scsi in my test.

FWIW, I will remove the "req->sreq = NULL;" two lines below in v3. At this
point tuning these small fields are subtle optimization compared to the arrays,
I say let's just simply keep the memset so that adding more fields in the
future are also safe.

Fam



reply via email to

[Prev in Thread] Current Thread [Next in Thread]