Re: [dpdk-dev] [PATCH v6 2/2] mbuf: implement generic format for sched field

[Olivier Matz <olivier.matz@6wind.com>]

Hi Cristian,

[...]

On Thu, Dec 20, 2018 at 11:28:01AM +0000, Dumitrescu, Cristian wrote:
> > > + * @param m
> > > + *   Mbuf to read
> > > + * @param queue_id
> > > + *  Returns the queue id
> > > + * @param traffic_class
> > > + *  Returns the traffic class id
> > > + * @param color
> > > + *  Returns the colour id
> > > + */
> > > +static inline void
> > > +rte_mbuf_sched_get(const struct rte_mbuf *m, uint32_t *queue_id,
> > > +			uint8_t *traffic_class,
> > > +			uint8_t *color)
> > > +{
> > > +	struct rte_mbuf_sched sched = m->hash.sched;
> > > +
> > > +	*queue_id = sched.queue_id;
> > > +	*traffic_class = sched.traffic_class;
> > > +	*color = sched.color;
> > 
> > I don't think there is a need to have an additional local copy.
> > 
> > *queue_id = m->hash.sched.queue_id;
> > *traffic_class = m->hash.sched.traffic_class;
> > *color = m->hash.sched.color;
> > 
> 
> With local copy, compiler typically generates a single 8-byte read instruction. Without the local copy, compiler typically generates 3x read instructions.
> 
> The set/get functions are used in some performance critical actions, so this is the reason to make sure we get them right.

Ok, that makes sense, thanks for clarification.


Regards,
Olivier