[dpdk-dev] [PATCH 1/5] vhost: refactor rte_vhost_dequeue_burst

Yuanhan Liu yuanhan.liu at linux.intel.com
Thu Dec 3 08:25:01 CET 2015


On Wed, Dec 02, 2015 at 11:02:44PM -0800, Stephen Hemminger wrote:
> On Thu,  3 Dec 2015 14:06:09 +0800
> Yuanhan Liu <yuanhan.liu at linux.intel.com> wrote:
> 
> > +#define COPY(dst, src) do {						\
> > +	cpy_len = RTE_MIN(desc_avail, mbuf_avail);			\
> > +	rte_memcpy((void *)(uintptr_t)(dst),				\
> > +		   (const void *)(uintptr_t)(src), cpy_len);		\
> > +									\
> > +	mbuf_avail  -= cpy_len;						\
> > +	mbuf_offset += cpy_len;						\
> > +	desc_avail  -= cpy_len;						\
> > +	desc_offset += cpy_len;						\
> > +} while(0)
> > +
> 
> I see lots of issues here.
> 
> All those void * casts are unnecessary, C casts arguements already.

Hi Stephen,

Without the cast, the compile will not work, as dst is actully with
uint64_t type.

> rte_memcpy is slower for constant size values than memcpy()

Sorry, what does it have something to do with this patch?

> This macro violates the rule that ther should be no hidden variables
> in a macro. I.e you are assuming cpy_len, desc_avail, and mbuf_avail
> are defined in all code using the macro.

Yes, I'm aware of that. And I agree that it's not a good usage
_in general_.

But I'm thinking that it's okay to use it here, for the three
major functions has quite similar logic.  And if my memory
servers me right, there are also quite many codes like that
in Linux kernel.

> Why use an un-typed macro when an inline function would be just
> as fast and give type safety?

It references too many variables, upto 6. And there are 4 vars
needed to be updated. Therefore, making it to be a function
would make the caller long and ugly. That's why I was thinking
to use a macro to remove few lines of repeat code.

So, if hidden var macro is forbidden here, I guess I would go
with just unfolding those lines of code, but not introducing
a helper function.

	--yliu


More information about the dev mailing list