[v2,4/4] lib: reduce global variable usage

  Some global variables can be eliminated, since they are not part of
public interface, it is free to remove them.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 lib/librte_compressdev/rte_compressdev.c     | 32 ++++++++----------
 lib/librte_compressdev/rte_compressdev_pmd.h |  5 ---
 lib/librte_cryptodev/rte_cryptodev.c         | 34 +++++++++-----------
 lib/librte_cryptodev/rte_cryptodev_pmd.h     |  3 --
 lib/librte_eventdev/rte_eventdev.c           |  8 ++---
 lib/librte_eventdev/rte_eventdev_pmd.h       |  2 --
 lib/librte_rawdev/rte_rawdev.c               | 10 +++---
 lib/librte_rawdev/rte_rawdev_pmd.h           |  2 --
 8 files changed, 37 insertions(+), 59 deletions(-)
  

-----Original Message-----
> Date: Fri, 5 Oct 2018 17:26:11 +0100
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> To: Reshma Pattan <reshma.pattan@intel.com>, Hemant Agrawal
>  <hemant.agrawal@nxp.com>, Shreyansh Jain <shreyansh.jain@nxp.com>, Ashish
>  Gupta <ashish.gupta@cavium.com>, Fiona Trahe <fiona.trahe@intel.com>,
>  Pablo de Lara <pablo.de.lara.guarch@intel.com>, Akhil Goyal
>  <akhil.goyal@nxp.com>, Declan Doherty <declan.doherty@intel.com>, Fan
>  Zhang <roy.fan.zhang@intel.com>, Rahul Lakkireddy
>  <rahul.lakkireddy@chelsio.com>, Gaetan Rivet <gaetan.rivet@6wind.com>, Qi
>  Zhang <qi.z.zhang@intel.com>, Xiao Wang <xiao.w.wang@intel.com>, Beilei
>  Xing <beilei.xing@intel.com>, Wenzhuo Lu <wenzhuo.lu@intel.com>,
>  Konstantin Ananyev <konstantin.ananyev@intel.com>, Rasesh Mody
>  <rasesh.mody@cavium.com>, Harish Patil <harish.patil@cavium.com>, Shahed
>  Shaikh <shahed.shaikh@cavium.com>, Andrew Rybchenko
>  <arybchenko@solarflare.com>, Jasvinder Singh <jasvinder.singh@intel.com>,
>  Cristian Dumitrescu <cristian.dumitrescu@intel.com>, Keith Wiles
>  <keith.wiles@intel.com>, Nipun Gupta <nipun.gupta@nxp.com>, Anatoly
>  Burakov <anatoly.burakov@intel.com>, Thomas Monjalon
>  <thomas@monjalon.net>, Jerin Jacob <jerin.jacob@caviumnetworks.com>,
>  Olivier Matz <olivier.matz@6wind.com>, Ashish Gupta
>  <ashish.gupta@caviumnetworks.com>
> CC: dev@dpdk.org, Ferruh Yigit <ferruh.yigit@intel.com>
> Subject: [PATCH v2 4/4] lib: reduce global variable usage
> X-Mailer: git-send-email 2.17.1
> 
> 
> Some global variables can be eliminated, since they are not part of
> public interface, it is free to remove them.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
  

On Friday 05 October 2018 09:56 PM, Ferruh Yigit wrote:
> Some global variables can be eliminated, since they are not part of
> public interface, it is free to remove them.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>   lib/librte_compressdev/rte_compressdev.c     | 32 ++++++++----------
>   lib/librte_compressdev/rte_compressdev_pmd.h |  5 ---
>   lib/librte_cryptodev/rte_cryptodev.c         | 34 +++++++++-----------
>   lib/librte_cryptodev/rte_cryptodev_pmd.h     |  3 --
>   lib/librte_eventdev/rte_eventdev.c           |  8 ++---
>   lib/librte_eventdev/rte_eventdev_pmd.h       |  2 --
>   lib/librte_rawdev/rte_rawdev.c               | 10 +++---
>   lib/librte_rawdev/rte_rawdev_pmd.h           |  2 --
>   8 files changed, 37 insertions(+), 59 deletions(-)

[...]

> diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c
> index 62b6b97ef..9f1e3592d 100644
> --- a/lib/librte_rawdev/rte_rawdev.c
> +++ b/lib/librte_rawdev/rte_rawdev.c
> @@ -35,21 +35,19 @@
>   /* dynamic log identifier */
>   int librawdev_logtype;
>   
> -struct rte_rawdev rte_rawdevices[RTE_RAWDEV_MAX_DEVS];
> +static struct rte_rawdev rte_rawdevices[RTE_RAWDEV_MAX_DEVS];
>   
> -struct rte_rawdev *rte_rawdevs = &rte_rawdevices[0];
> +struct rte_rawdev *rte_rawdevs = rte_rawdevices;
>   
>   static struct rte_rawdev_global rawdev_globals = {
>   	.nb_devs		= 0
>   };
>   
> -struct rte_rawdev_global *rte_rawdev_globals = &rawdev_globals;
> -
>   /* Raw device, northbound API implementation */
>   uint8_t
>   rte_rawdev_count(void)
>   {
> -	return rte_rawdev_globals->nb_devs;
> +	return rawdev_globals.nb_devs;
>   }
>   
>   uint16_t
> @@ -60,7 +58,7 @@ rte_rawdev_get_dev_id(const char *name)
>   	if (!name)
>   		return -EINVAL;
>   
> -	for (i = 0; i < rte_rawdev_globals->nb_devs; i++)
> +	for (i = 0; i < rawdev_globals.nb_devs; i++)
>   		if ((strcmp(rte_rawdevices[i].name, name)
>   				== 0) &&
>   				(rte_rawdevices[i].attached ==
> diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h
> index bb9bbc350..811e51d07 100644
> --- a/lib/librte_rawdev/rte_rawdev_pmd.h
> +++ b/lib/librte_rawdev/rte_rawdev_pmd.h
> @@ -73,8 +73,6 @@ struct rte_rawdev_global {
>   	uint16_t nb_devs;
>   };
>   
> -extern struct rte_rawdev_global *rte_rawdev_globals;
> -/** Pointer to global raw devices data structure. */
>   extern struct rte_rawdev *rte_rawdevs;
>   /** The pool of rte_rawdev structures. */

I think `rte_rawdevs` can also be removed from an exposed symbol. In all 
cases (at least for now), this is being accessed using an API (and no 
macros). There should be no need for people to read this directly if 
sufficient get_by_* APIs are available.

Wouldn't same be valid for rte_compressdevs as well?

Besides the above:

Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
  

On 10/8/2018 9:45 AM, Shreyansh Jain wrote:
> On Friday 05 October 2018 09:56 PM, Ferruh Yigit wrote:
>> Some global variables can be eliminated, since they are not part of
>> public interface, it is free to remove them.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>>   lib/librte_compressdev/rte_compressdev.c     | 32 ++++++++----------
>>   lib/librte_compressdev/rte_compressdev_pmd.h |  5 ---
>>   lib/librte_cryptodev/rte_cryptodev.c         | 34 +++++++++-----------
>>   lib/librte_cryptodev/rte_cryptodev_pmd.h     |  3 --
>>   lib/librte_eventdev/rte_eventdev.c           |  8 ++---
>>   lib/librte_eventdev/rte_eventdev_pmd.h       |  2 --
>>   lib/librte_rawdev/rte_rawdev.c               | 10 +++---
>>   lib/librte_rawdev/rte_rawdev_pmd.h           |  2 --
>>   8 files changed, 37 insertions(+), 59 deletions(-)
> 
> [...]
> 
>> diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c
>> index 62b6b97ef..9f1e3592d 100644
>> --- a/lib/librte_rawdev/rte_rawdev.c
>> +++ b/lib/librte_rawdev/rte_rawdev.c
>> @@ -35,21 +35,19 @@
>>   /* dynamic log identifier */
>>   int librawdev_logtype;
>>   
>> -struct rte_rawdev rte_rawdevices[RTE_RAWDEV_MAX_DEVS];
>> +static struct rte_rawdev rte_rawdevices[RTE_RAWDEV_MAX_DEVS];
>>   
>> -struct rte_rawdev *rte_rawdevs = &rte_rawdevices[0];
>> +struct rte_rawdev *rte_rawdevs = rte_rawdevices;
>>   
>>   static struct rte_rawdev_global rawdev_globals = {
>>   	.nb_devs		= 0
>>   };
>>   
>> -struct rte_rawdev_global *rte_rawdev_globals = &rawdev_globals;
>> -
>>   /* Raw device, northbound API implementation */
>>   uint8_t
>>   rte_rawdev_count(void)
>>   {
>> -	return rte_rawdev_globals->nb_devs;
>> +	return rawdev_globals.nb_devs;
>>   }
>>   
>>   uint16_t
>> @@ -60,7 +58,7 @@ rte_rawdev_get_dev_id(const char *name)
>>   	if (!name)
>>   		return -EINVAL;
>>   
>> -	for (i = 0; i < rte_rawdev_globals->nb_devs; i++)
>> +	for (i = 0; i < rawdev_globals.nb_devs; i++)
>>   		if ((strcmp(rte_rawdevices[i].name, name)
>>   				== 0) &&
>>   				(rte_rawdevices[i].attached ==
>> diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h
>> index bb9bbc350..811e51d07 100644
>> --- a/lib/librte_rawdev/rte_rawdev_pmd.h
>> +++ b/lib/librte_rawdev/rte_rawdev_pmd.h
>> @@ -73,8 +73,6 @@ struct rte_rawdev_global {
>>   	uint16_t nb_devs;
>>   };
>>   
>> -extern struct rte_rawdev_global *rte_rawdev_globals;
>> -/** Pointer to global raw devices data structure. */
>>   extern struct rte_rawdev *rte_rawdevs;
>>   /** The pool of rte_rawdev structures. */
> 
> I think `rte_rawdevs` can also be removed from an exposed symbol. In all 
> cases (at least for now), this is being accessed using an API (and no 
> macros). There should be no need for people to read this directly if 
> sufficient get_by_* APIs are available.

+1 to make it library internal.

But `rte_rawdevs` is both in public header and .map file, so it is part of
public API, we can't remove it without deprecation process.
And I want to leave the decision of removing an public symbol to library
maintainers.

> 
> Wouldn't same be valid for rte_compressdevs as well?

`rte_compressdevs` is already removed, but if you mean `rte_cryptodevs` it has
same problem with `rte_rawdevs`, it is part of API.

> Besides the above:
> 
> Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>
  

On Monday 08 October 2018 02:41 PM, Ferruh Yigit wrote:
> On 10/8/2018 9:45 AM, Shreyansh Jain wrote:
>> On Friday 05 October 2018 09:56 PM, Ferruh Yigit wrote:
>>> Some global variables can be eliminated, since they are not part of
>>> public interface, it is free to remove them.
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>> ---
>>>    lib/librte_compressdev/rte_compressdev.c     | 32 ++++++++----------
>>>    lib/librte_compressdev/rte_compressdev_pmd.h |  5 ---
>>>    lib/librte_cryptodev/rte_cryptodev.c         | 34 +++++++++-----------
>>>    lib/librte_cryptodev/rte_cryptodev_pmd.h     |  3 --
>>>    lib/librte_eventdev/rte_eventdev.c           |  8 ++---
>>>    lib/librte_eventdev/rte_eventdev_pmd.h       |  2 --
>>>    lib/librte_rawdev/rte_rawdev.c               | 10 +++---
>>>    lib/librte_rawdev/rte_rawdev_pmd.h           |  2 --
>>>    8 files changed, 37 insertions(+), 59 deletions(-)
>>
>> [...]
>>
>>> diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c
>>> index 62b6b97ef..9f1e3592d 100644
>>> --- a/lib/librte_rawdev/rte_rawdev.c
>>> +++ b/lib/librte_rawdev/rte_rawdev.c
>>> @@ -35,21 +35,19 @@
>>>    /* dynamic log identifier */
>>>    int librawdev_logtype;
>>>    
>>> -struct rte_rawdev rte_rawdevices[RTE_RAWDEV_MAX_DEVS];
>>> +static struct rte_rawdev rte_rawdevices[RTE_RAWDEV_MAX_DEVS];
>>>    
>>> -struct rte_rawdev *rte_rawdevs = &rte_rawdevices[0];
>>> +struct rte_rawdev *rte_rawdevs = rte_rawdevices;
>>>    
>>>    static struct rte_rawdev_global rawdev_globals = {
>>>    	.nb_devs		= 0
>>>    };
>>>    
>>> -struct rte_rawdev_global *rte_rawdev_globals = &rawdev_globals;
>>> -
>>>    /* Raw device, northbound API implementation */
>>>    uint8_t
>>>    rte_rawdev_count(void)
>>>    {
>>> -	return rte_rawdev_globals->nb_devs;
>>> +	return rawdev_globals.nb_devs;
>>>    }
>>>    
>>>    uint16_t
>>> @@ -60,7 +58,7 @@ rte_rawdev_get_dev_id(const char *name)
>>>    	if (!name)
>>>    		return -EINVAL;
>>>    
>>> -	for (i = 0; i < rte_rawdev_globals->nb_devs; i++)
>>> +	for (i = 0; i < rawdev_globals.nb_devs; i++)
>>>    		if ((strcmp(rte_rawdevices[i].name, name)
>>>    				== 0) &&
>>>    				(rte_rawdevices[i].attached ==
>>> diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h
>>> index bb9bbc350..811e51d07 100644
>>> --- a/lib/librte_rawdev/rte_rawdev_pmd.h
>>> +++ b/lib/librte_rawdev/rte_rawdev_pmd.h
>>> @@ -73,8 +73,6 @@ struct rte_rawdev_global {
>>>    	uint16_t nb_devs;
>>>    };
>>>    
>>> -extern struct rte_rawdev_global *rte_rawdev_globals;
>>> -/** Pointer to global raw devices data structure. */
>>>    extern struct rte_rawdev *rte_rawdevs;
>>>    /** The pool of rte_rawdev structures. */
>>
>> I think `rte_rawdevs` can also be removed from an exposed symbol. In all
>> cases (at least for now), this is being accessed using an API (and no
>> macros). There should be no need for people to read this directly if
>> sufficient get_by_* APIs are available.
> 
> +1 to make it library internal.
> 
> But `rte_rawdevs` is both in public header and .map file, so it is part of
> public API, we can't remove it without deprecation process.
> And I want to leave the decision of removing an public symbol to library
> maintainers.

Yes, I missed that part :)
I will fix it in next release by sending deprecation in this.

> 
>>
>> Wouldn't same be valid for rte_compressdevs as well?
> 
> `rte_compressdevs` is already removed, but if you mean `rte_cryptodevs` it has
> same problem with `rte_rawdevs`, it is part of API.

Ah yes, my mistake.
I was thinking about rte_eventdevs and rte_cryptodevs (but, this symbol 
is being used by a crypto/scheduler)

> 
>> Besides the above:
>>
>> Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>
>