<snip>

> > > > > > +/**
> > > > > > + * RTE thread Quiescent State structure.
> > > > > > + */
> > > > > > +struct rte_rcu_qsbr {
> > > > > > +   uint64_t reg_thread_id[RTE_QSBR_BIT_MAP_ELEMS]
> > > > > __rte_cache_aligned;
> > > > > > +   /**< Registered reader thread IDs - reader threads reporting
> > > > > > +    * on this QS variable represented in a bit map.
> > > > > > +    */
> > > > > > +
> > > > > > +   uint64_t token __rte_cache_aligned;
> > > > > > +   /**< Counter to allow for multiple simultaneous QS queries
> > > > > > +*/
> > > > > > +
> > > > > > +   struct rte_rcu_qsbr_cnt w[RTE_RCU_MAX_THREADS]
> > > > > __rte_cache_aligned;
> > > > > > +   /**< QS counter for each reader thread, counts upto
> > > > > > +    * current value of token.
> > > > >
> > > > > As I understand you decided to stick with neutral thread_id and
> > > > > let user define what exactly thread_id is (lcore, syste, thread
> > > > > id, something
> > > else)?
> > > > Yes, that is correct. I will reply to the other thread to continue the
> discussion.
> > > >
> > > > > If so, can you probably get rid of RTE_RCU_MAX_THREADS limitation?
> > > > I am not seeing this as a limitation. The user can change this if
> > > > required. May
> > > be I should change it as follows:
> > > > #ifndef RTE_RCU_MAX_THREADS
> > > > #define RTE_RCU_MAX_THREADS 128
> > > > #endif
> > >
> > > Yep, that's better, though it would still require user to rebuild
> > > the code if he would like to increase total number of threads supported.
> > Agree
> >
> > > Though it seems relatively simply to extend current code to support
> > > dynamic max thread num here (2 variable arrays plus shift value plus
> mask).
> > Agree, supporting dynamic 'max thread num' is simple. But this means
> > memory needs to be allocated to the arrays. The API
> > 'rte_rcu_qsbr_init' has to take max thread num as the parameter. We also
> have to introduce another API to free this memory. This will become very
> similar to alloc/free APIs I had in the v1.
> > I hope I am following you well, please correct me if not.
> 
> I think we can still leave alloc/free tasks to the user.
> We probabply just need extra function rte_rcu_qsbr_size(uint32_
> max_threads) to help user calculate required size.
> rte_rcu_qsbr_init() might take as an additional parameter 'size' to make
> checks.
The size is returned by an API provided by the library. Why does it need to be 
validated again? If 'size' is required for rte_rcu_qsbr_init, it could 
calculate it again.

> Thought about something like that:
> 
> size_t sz = rte_rcu_qsbr_size(max_threads); struct rte_rcu_qsbr *qsbr =
> alloc_aligned(CACHE_LINE, sz); rte_rcu_qsbr_init(qsbr, max_threads, sz); ...
> 
Do you see any advantage for allowing the user to allocate the memory?
This approach requires the user to call 3 APIs (including memory allocation). 
These 3 can be abstracted in a rte_rcu_qsbr_alloc API, user has to call just 1 
API.

> Konstantin
> 
> >
> > >
> > > >
> > > > > I.E. struct rte_rcu_qsbr_cnt w[] and allow user at init time to
> > > > > define max number of threads allowed.
> > > > > Or something like:
> > > > > #define RTE_RCU_QSBR_DEF(name, max_thread) struct name { \
> > > > >   uint64_t reg_thread_id[ALIGN_CEIL(max_thread, 64)  >> 6]; \
> > > > >    ...
> > > > >    struct rte_rcu_qsbr_cnt w[max_thread]; \ }
> > > > I am trying to understand this. I am not following why 'name' is
> > > > required? Would the user call 'RTE_RCU_QSBR_DEF' in the
> > > > application
> > > header file?
> > >
> > > My thought here was to allow user to define his own structures,
> > > depending on the number of max threads he needs/wants:
> > > RTE_RCU_QSBR_DEF(rte_rcu_qsbr_128, 128);
> > > RTE_RCU_QSBR_DEF(rte_rcu_qsbr_64, 64); ...
> > Thank you for the clarification, I follow you now. However, it will
> > not solve the problem of dynamic max thread num. Changes to the max
> number of threads will require recompilation.
> >
> > > Konstantin

Reply via email to