On 04/04 23:02:15, Maxim Uvarov wrote:
> On 04/04/17 22:48, Brian Brooks wrote:
> > On 04/04 21:57:29, Maxim Uvarov wrote:
> >> On 04/04/17 21:47, Brian Brooks wrote:
> >>> Signed-off-by: Ola Liljedahl <[email protected]>
> >>> Reviewed-by: Honnappa Nagarahalli <[email protected]>
> >>> Reviewed-by: Brian Brooks <[email protected]>
> >>> ---
> >>>  platform/linux-generic/include/odp_config_internal.h | 7 +++++++
> >>>  1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/platform/linux-generic/include/odp_config_internal.h 
> >>> b/platform/linux-generic/include/odp_config_internal.h
> >>> index dadd59e7..4822e69d 100644
> >>> --- a/platform/linux-generic/include/odp_config_internal.h
> >>> +++ b/platform/linux-generic/include/odp_config_internal.h
> >>> @@ -22,6 +22,13 @@ extern "C" {
> >>>  #define ODP_CONFIG_QUEUES 1024
> >>>  
> >>>  /*
> >>> + * Maximum queue depth. Maximum number of elements that can be stored in 
> >>> a
> >>> + * queue. This value is used only when the size is not explicitly 
> >>> provided
> >>> + * during queue creation.
> >>> + */
> >>> +#define ODP_CONFIG_QUEUE_SIZE 4096
> >>> +
> >>
> >> internal things should not start this ODP_ . Just follow syntax in that
> >> file.
> > 
> > Please take another look at the file, e.g. ODP_CONFIG_QUEUES, 
> > ODP_CONFIG_POOLS..
> > 
> 
> yes, some names has ODP_. That is bad and confusing. But we did it at
> the beginning and did not clean up later. But for new code it's better
> to not overlap with api prefixes.

OK. We can address it in a follow-up patch, otherwise it will still
remain 'bad and confusing' if we only address it in this patch.

> >> Patch has to be merge to patch where it is used.
> > 
> > Can you please explain?
> > 
> 
> This patch adds new define. But it's not used in code of this patch.
> That is very confusing for reading.

This is also another pre-existing condition.

> So this patch should be combined
> with patch which uses this define. So people can see why you moved this
> to config and how do you use it.
> 
> Maxim.
> 
> 
> 
> 
> >> Maxim.
> >>
> >>> +/*
> >>>   * Maximum number of ordered locks per queue
> >>>   */
> >>>  #define CONFIG_QUEUE_MAX_ORD_LOCKS 4
> >>>
> >>
> 

Reply via email to