Re: [dpdk-dev] [PATCH 4/4] vhost: define callfd and kickfd as int type

["Ouyang, Changchun" <changchun.ouyang@intel.com>]

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, September 9, 2015 9:55 AM
> To: Ouyang, Changchun
> Cc: dev@dpdk.org; Xie, Huawei
> Subject: Re: [PATCH 4/4] vhost: define callfd and kickfd as int type
> 
> On Wed, Sep 09, 2015 at 01:43:06AM +0000, Ouyang, Changchun wrote:
> >
> >
> > > -----Original Message-----
> > > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > > Sent: Monday, August 24, 2015 11:55 AM
> > > To: dev@dpdk.org
> > > Cc: Xie, Huawei; Ouyang, Changchun; Yuanhan Liu
> > > Subject: [PATCH 4/4] vhost: define callfd and kickfd as int type
> > >
> > > So that we can remove the redundant (int) cast.
> > >
> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > > ---
> >
> > > diff --git a/lib/librte_vhost/rte_virtio_net.h
> > > b/lib/librte_vhost/rte_virtio_net.h
> > > index b9bf320..a037c15 100644
> > > --- a/lib/librte_vhost/rte_virtio_net.h
> > > +++ b/lib/librte_vhost/rte_virtio_net.h
> > > @@ -87,8 +87,8 @@ struct vhost_virtqueue {
> > >  	uint16_t		vhost_hlen;		/**< Vhost header
> > > length (varies depending on RX merge buffers. */
> > >  	volatile uint16_t	last_used_idx;		/**< Last index used
> > > on the available ring */
> > >  	volatile uint16_t	last_used_idx_res;	/**< Used for
> > > multiple devices reserving buffers. */
> > > -	eventfd_t		callfd;			/**< Used to notify
> > > the guest (trigger interrupt). */
> > > -	eventfd_t		kickfd;			/**< Currently
> > > unused as polling mode is enabled. */
> > > +	int			callfd;			/**< Used to notify
> > > the guest (trigger interrupt). */
> > > +	int			kickfd;			/**< Currently
> > > unused as polling mode is enabled. */
> >
> > I don't think we have to change it from 8B(eventfd_t is defined as
> > uint64_t) to 4B, Any benefit for this change?
> 
> As I stated in the commit log, to remove the redundant (int) cast. Casts like
> following are a bit ugly:
> 
>         if ((int)dev->virtqueue[VIRTIO_RXQ]->callfd >= 0)
>                 close((int)dev->virtqueue[VIRTIO_RXQ]->callfd);
> 
> On the other hand, why it has to be uint64_t? The caller side sends the
> message(be more precisely, qemu) actually uses int type.
> 

Agree, qemu use 32 bit for the callfd and kickfd.
It could use int.
Well, there is another comment in other place in this patch, I will send out soon.


> 	--yliu