[v4,04/10] timer: remove deprecated code

Message ID 7eba69a7c008f5ab62fd3a207430ef6a182e1622.1571322634.git.anatoly.burakov@intel.com (mailing list archive)
State Superseded, archived
Headers
Series Implement the new ABI policy and add helper scripts |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Burakov, Anatoly Oct. 17, 2019, 2:31 p.m. UTC
  From: Marcin Baran <marcinx.baran@intel.com>

Remove code for old ABI versions ahead of ABI version bump.

Signed-off-by: Marcin Baran <marcinx.baran@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---

Notes:
    v2:
    - Moved this to before ABI version bump to avoid compile breakage

 lib/librte_timer/rte_timer.c | 90 ++----------------------------------
 lib/librte_timer/rte_timer.h | 15 ------
 2 files changed, 5 insertions(+), 100 deletions(-)
  

Comments

Carrillo, Erik G Oct. 17, 2019, 9:04 p.m. UTC | #1
> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Thursday, October 17, 2019 9:32 AM
> To: dev@dpdk.org
> Cc: Baran, MarcinX <marcinx.baran@intel.com>; Robert Sanford
> <rsanford@akamai.com>; Carrillo, Erik G <erik.g.carrillo@intel.com>;
> Mcnamara, John <john.mcnamara@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; thomas@monjalon.net;
> david.marchand@redhat.com
> Subject: [PATCH v4 04/10] timer: remove deprecated code
> 
> From: Marcin Baran <marcinx.baran@intel.com>
> 
> Remove code for old ABI versions ahead of ABI version bump.
> 
> Signed-off-by: Marcin Baran <marcinx.baran@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Looks good to me too:
Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
  
Kevin Traynor Oct. 21, 2019, 1:24 p.m. UTC | #2
On 17/10/2019 15:31, Anatoly Burakov wrote:
> From: Marcin Baran <marcinx.baran@intel.com>
> 
> Remove code for old ABI versions ahead of ABI version bump.
> 

I think there needs to be some doc updates for this.

Looking at http://doc.dpdk.org/guides/rel_notes/deprecation.html there
is nothing saying these functions are deprecated? (probably same issue
for other 'remove deprecated code' patches)

There should probably be an entry in the API/ABI changes section of the
release notes too.

> Signed-off-by: Marcin Baran <marcinx.baran@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> 
> Notes:
>     v2:
>     - Moved this to before ABI version bump to avoid compile breakage
> 
>  lib/librte_timer/rte_timer.c | 90 ++----------------------------------
>  lib/librte_timer/rte_timer.h | 15 ------
>  2 files changed, 5 insertions(+), 100 deletions(-)
> 
> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
> index bdcf05d06b..de6959b809 100644
> --- a/lib/librte_timer/rte_timer.c
> +++ b/lib/librte_timer/rte_timer.c
> @@ -68,9 +68,6 @@ static struct rte_timer_data *rte_timer_data_arr;
>  static const uint32_t default_data_id;
>  static uint32_t rte_timer_subsystem_initialized;
>  
> -/* For maintaining older interfaces for a period */
> -static struct rte_timer_data default_timer_data;
> -
>  /* when debug is enabled, store some statistics */
>  #ifdef RTE_LIBRTE_TIMER_DEBUG
>  #define __TIMER_STAT_ADD(priv_timer, name, n) do {			\
> @@ -131,22 +128,6 @@ rte_timer_data_dealloc(uint32_t id)
>  	return 0;
>  }
>  
> -void
> -rte_timer_subsystem_init_v20(void)
> -{
> -	unsigned lcore_id;
> -	struct priv_timer *priv_timer = default_timer_data.priv_timer;
> -
> -	/* since priv_timer is static, it's zeroed by default, so only init some
> -	 * fields.
> -	 */
> -	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id ++) {
> -		rte_spinlock_init(&priv_timer[lcore_id].list_lock);
> -		priv_timer[lcore_id].prev_lcore = lcore_id;
> -	}
> -}
> -VERSION_SYMBOL(rte_timer_subsystem_init, _v20, 2.0);
> -
>  /* Init the timer library. Allocate an array of timer data structs in shared
>   * memory, and allocate the zeroth entry for use with original timer
>   * APIs. Since the intersection of the sets of lcore ids in primary and
> @@ -154,7 +135,7 @@ VERSION_SYMBOL(rte_timer_subsystem_init, _v20, 2.0);
>   * multiple processes.
>   */
>  int
> -rte_timer_subsystem_init_v1905(void)
> +rte_timer_subsystem_init(void)
>  {
>  	const struct rte_memzone *mz;
>  	struct rte_timer_data *data;
> @@ -209,9 +190,6 @@ rte_timer_subsystem_init_v1905(void)
>  
>  	return 0;
>  }
> -MAP_STATIC_SYMBOL(int rte_timer_subsystem_init(void),
> -		  rte_timer_subsystem_init_v1905);
> -BIND_DEFAULT_SYMBOL(rte_timer_subsystem_init, _v1905, 19.05);
>  
>  void
>  rte_timer_subsystem_finalize(void)
> @@ -552,42 +530,13 @@ __rte_timer_reset(struct rte_timer *tim, uint64_t expire,
>  
>  /* Reset and start the timer associated with the timer handle tim */
>  int
> -rte_timer_reset_v20(struct rte_timer *tim, uint64_t ticks,
> -		    enum rte_timer_type type, unsigned int tim_lcore,
> -		    rte_timer_cb_t fct, void *arg)
> -{
> -	uint64_t cur_time = rte_get_timer_cycles();
> -	uint64_t period;
> -
> -	if (unlikely((tim_lcore != (unsigned)LCORE_ID_ANY) &&
> -			!(rte_lcore_is_enabled(tim_lcore) ||
> -			  rte_lcore_has_role(tim_lcore, ROLE_SERVICE))))
> -		return -1;
> -
> -	if (type == PERIODICAL)
> -		period = ticks;
> -	else
> -		period = 0;
> -
> -	return __rte_timer_reset(tim,  cur_time + ticks, period, tim_lcore,
> -			  fct, arg, 0, &default_timer_data);
> -}
> -VERSION_SYMBOL(rte_timer_reset, _v20, 2.0);
> -
> -int
> -rte_timer_reset_v1905(struct rte_timer *tim, uint64_t ticks,
> +rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
>  		      enum rte_timer_type type, unsigned int tim_lcore,
>  		      rte_timer_cb_t fct, void *arg)
>  {
>  	return rte_timer_alt_reset(default_data_id, tim, ticks, type,
>  				   tim_lcore, fct, arg);
>  }
> -MAP_STATIC_SYMBOL(int rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
> -				      enum rte_timer_type type,
> -				      unsigned int tim_lcore,
> -				      rte_timer_cb_t fct, void *arg),
> -		  rte_timer_reset_v1905);
> -BIND_DEFAULT_SYMBOL(rte_timer_reset, _v1905, 19.05);
>  
>  int
>  rte_timer_alt_reset(uint32_t timer_data_id, struct rte_timer *tim,
> @@ -658,20 +607,10 @@ __rte_timer_stop(struct rte_timer *tim, int local_is_locked,
>  
>  /* Stop the timer associated with the timer handle tim */
>  int
> -rte_timer_stop_v20(struct rte_timer *tim)
> -{
> -	return __rte_timer_stop(tim, 0, &default_timer_data);
> -}
> -VERSION_SYMBOL(rte_timer_stop, _v20, 2.0);
> -
> -int
> -rte_timer_stop_v1905(struct rte_timer *tim)
> +rte_timer_stop(struct rte_timer *tim)
>  {
>  	return rte_timer_alt_stop(default_data_id, tim);
>  }
> -MAP_STATIC_SYMBOL(int rte_timer_stop(struct rte_timer *tim),
> -		  rte_timer_stop_v1905);
> -BIND_DEFAULT_SYMBOL(rte_timer_stop, _v1905, 19.05);
>  
>  int
>  rte_timer_alt_stop(uint32_t timer_data_id, struct rte_timer *tim)
> @@ -817,15 +756,8 @@ __rte_timer_manage(struct rte_timer_data *timer_data)
>  	priv_timer[lcore_id].running_tim = NULL;
>  }
>  
> -void
> -rte_timer_manage_v20(void)
> -{
> -	__rte_timer_manage(&default_timer_data);
> -}
> -VERSION_SYMBOL(rte_timer_manage, _v20, 2.0);
> -
>  int
> -rte_timer_manage_v1905(void)
> +rte_timer_manage(void)
>  {
>  	struct rte_timer_data *timer_data;
>  
> @@ -835,8 +767,6 @@ rte_timer_manage_v1905(void)
>  
>  	return 0;
>  }
> -MAP_STATIC_SYMBOL(int rte_timer_manage(void), rte_timer_manage_v1905);
> -BIND_DEFAULT_SYMBOL(rte_timer_manage, _v1905, 19.05);
>  
>  int
>  rte_timer_alt_manage(uint32_t timer_data_id,
> @@ -1074,21 +1004,11 @@ __rte_timer_dump_stats(struct rte_timer_data *timer_data __rte_unused, FILE *f)
>  #endif
>  }
>  
> -void
> -rte_timer_dump_stats_v20(FILE *f)
> -{
> -	__rte_timer_dump_stats(&default_timer_data, f);
> -}
> -VERSION_SYMBOL(rte_timer_dump_stats, _v20, 2.0);
> -
>  int
> -rte_timer_dump_stats_v1905(FILE *f)
> +rte_timer_dump_stats(FILE *f)
>  {
>  	return rte_timer_alt_dump_stats(default_data_id, f);
>  }
> -MAP_STATIC_SYMBOL(int rte_timer_dump_stats(FILE *f),
> -		  rte_timer_dump_stats_v1905);
> -BIND_DEFAULT_SYMBOL(rte_timer_dump_stats, _v1905, 19.05);
>  
>  int
>  rte_timer_alt_dump_stats(uint32_t timer_data_id __rte_unused, FILE *f)
> diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
> index 05d287d8f2..9dc5fc3092 100644
> --- a/lib/librte_timer/rte_timer.h
> +++ b/lib/librte_timer/rte_timer.h
> @@ -181,8 +181,6 @@ int rte_timer_data_dealloc(uint32_t id);
>   *      subsystem
>   */
>  int rte_timer_subsystem_init(void);
> -int rte_timer_subsystem_init_v1905(void);
> -void rte_timer_subsystem_init_v20(void);
>  
>  /**
>   * @warning
> @@ -250,13 +248,6 @@ void rte_timer_init(struct rte_timer *tim);
>  int rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
>  		    enum rte_timer_type type, unsigned tim_lcore,
>  		    rte_timer_cb_t fct, void *arg);
> -int rte_timer_reset_v1905(struct rte_timer *tim, uint64_t ticks,
> -			  enum rte_timer_type type, unsigned int tim_lcore,
> -			  rte_timer_cb_t fct, void *arg);
> -int rte_timer_reset_v20(struct rte_timer *tim, uint64_t ticks,
> -			enum rte_timer_type type, unsigned int tim_lcore,
> -			rte_timer_cb_t fct, void *arg);
> -
>  
>  /**
>   * Loop until rte_timer_reset() succeeds.
> @@ -313,8 +304,6 @@ rte_timer_reset_sync(struct rte_timer *tim, uint64_t ticks,
>   *   - (-1): The timer is in the RUNNING or CONFIG state.
>   */
>  int rte_timer_stop(struct rte_timer *tim);
> -int rte_timer_stop_v1905(struct rte_timer *tim);
> -int rte_timer_stop_v20(struct rte_timer *tim);
>  
>  /**
>   * Loop until rte_timer_stop() succeeds.
> @@ -358,8 +347,6 @@ int rte_timer_pending(struct rte_timer *tim);
>   *   - -EINVAL: timer subsystem not yet initialized
>   */
>  int rte_timer_manage(void);
> -int rte_timer_manage_v1905(void);
> -void rte_timer_manage_v20(void);
>  
>  /**
>   * Dump statistics about timers.
> @@ -371,8 +358,6 @@ void rte_timer_manage_v20(void);
>   *   - -EINVAL: timer subsystem not yet initialized
>   */
>  int rte_timer_dump_stats(FILE *f);
> -int rte_timer_dump_stats_v1905(FILE *f);
> -void rte_timer_dump_stats_v20(FILE *f);
>  
>  /**
>   * @warning
>
  
Burakov, Anatoly Oct. 24, 2019, 9:07 a.m. UTC | #3
On 21-Oct-19 2:24 PM, Kevin Traynor wrote:
> On 17/10/2019 15:31, Anatoly Burakov wrote:
>> From: Marcin Baran <marcinx.baran@intel.com>
>>
>> Remove code for old ABI versions ahead of ABI version bump.
>>
> 
> I think there needs to be some doc updates for this.
> 
> Looking at http://doc.dpdk.org/guides/rel_notes/deprecation.html there
> is nothing saying these functions are deprecated? (probably same issue
> for other 'remove deprecated code' patches)
> 
> There should probably be an entry in the API/ABI changes section of the
> release notes too.
> 

The new ABI policy implies such deprecation, because everything now 
becomes one ABI version across the board. I'm not changing the API - 
just removing old ABI.

Regarding doc patches, i had a chat with John and he agreed that doc 
patches for this can come later.
  

Patch

diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index bdcf05d06b..de6959b809 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -68,9 +68,6 @@  static struct rte_timer_data *rte_timer_data_arr;
 static const uint32_t default_data_id;
 static uint32_t rte_timer_subsystem_initialized;
 
-/* For maintaining older interfaces for a period */
-static struct rte_timer_data default_timer_data;
-
 /* when debug is enabled, store some statistics */
 #ifdef RTE_LIBRTE_TIMER_DEBUG
 #define __TIMER_STAT_ADD(priv_timer, name, n) do {			\
@@ -131,22 +128,6 @@  rte_timer_data_dealloc(uint32_t id)
 	return 0;
 }
 
-void
-rte_timer_subsystem_init_v20(void)
-{
-	unsigned lcore_id;
-	struct priv_timer *priv_timer = default_timer_data.priv_timer;
-
-	/* since priv_timer is static, it's zeroed by default, so only init some
-	 * fields.
-	 */
-	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id ++) {
-		rte_spinlock_init(&priv_timer[lcore_id].list_lock);
-		priv_timer[lcore_id].prev_lcore = lcore_id;
-	}
-}
-VERSION_SYMBOL(rte_timer_subsystem_init, _v20, 2.0);
-
 /* Init the timer library. Allocate an array of timer data structs in shared
  * memory, and allocate the zeroth entry for use with original timer
  * APIs. Since the intersection of the sets of lcore ids in primary and
@@ -154,7 +135,7 @@  VERSION_SYMBOL(rte_timer_subsystem_init, _v20, 2.0);
  * multiple processes.
  */
 int
-rte_timer_subsystem_init_v1905(void)
+rte_timer_subsystem_init(void)
 {
 	const struct rte_memzone *mz;
 	struct rte_timer_data *data;
@@ -209,9 +190,6 @@  rte_timer_subsystem_init_v1905(void)
 
 	return 0;
 }
-MAP_STATIC_SYMBOL(int rte_timer_subsystem_init(void),
-		  rte_timer_subsystem_init_v1905);
-BIND_DEFAULT_SYMBOL(rte_timer_subsystem_init, _v1905, 19.05);
 
 void
 rte_timer_subsystem_finalize(void)
@@ -552,42 +530,13 @@  __rte_timer_reset(struct rte_timer *tim, uint64_t expire,
 
 /* Reset and start the timer associated with the timer handle tim */
 int
-rte_timer_reset_v20(struct rte_timer *tim, uint64_t ticks,
-		    enum rte_timer_type type, unsigned int tim_lcore,
-		    rte_timer_cb_t fct, void *arg)
-{
-	uint64_t cur_time = rte_get_timer_cycles();
-	uint64_t period;
-
-	if (unlikely((tim_lcore != (unsigned)LCORE_ID_ANY) &&
-			!(rte_lcore_is_enabled(tim_lcore) ||
-			  rte_lcore_has_role(tim_lcore, ROLE_SERVICE))))
-		return -1;
-
-	if (type == PERIODICAL)
-		period = ticks;
-	else
-		period = 0;
-
-	return __rte_timer_reset(tim,  cur_time + ticks, period, tim_lcore,
-			  fct, arg, 0, &default_timer_data);
-}
-VERSION_SYMBOL(rte_timer_reset, _v20, 2.0);
-
-int
-rte_timer_reset_v1905(struct rte_timer *tim, uint64_t ticks,
+rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
 		      enum rte_timer_type type, unsigned int tim_lcore,
 		      rte_timer_cb_t fct, void *arg)
 {
 	return rte_timer_alt_reset(default_data_id, tim, ticks, type,
 				   tim_lcore, fct, arg);
 }
-MAP_STATIC_SYMBOL(int rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
-				      enum rte_timer_type type,
-				      unsigned int tim_lcore,
-				      rte_timer_cb_t fct, void *arg),
-		  rte_timer_reset_v1905);
-BIND_DEFAULT_SYMBOL(rte_timer_reset, _v1905, 19.05);
 
 int
 rte_timer_alt_reset(uint32_t timer_data_id, struct rte_timer *tim,
@@ -658,20 +607,10 @@  __rte_timer_stop(struct rte_timer *tim, int local_is_locked,
 
 /* Stop the timer associated with the timer handle tim */
 int
-rte_timer_stop_v20(struct rte_timer *tim)
-{
-	return __rte_timer_stop(tim, 0, &default_timer_data);
-}
-VERSION_SYMBOL(rte_timer_stop, _v20, 2.0);
-
-int
-rte_timer_stop_v1905(struct rte_timer *tim)
+rte_timer_stop(struct rte_timer *tim)
 {
 	return rte_timer_alt_stop(default_data_id, tim);
 }
-MAP_STATIC_SYMBOL(int rte_timer_stop(struct rte_timer *tim),
-		  rte_timer_stop_v1905);
-BIND_DEFAULT_SYMBOL(rte_timer_stop, _v1905, 19.05);
 
 int
 rte_timer_alt_stop(uint32_t timer_data_id, struct rte_timer *tim)
@@ -817,15 +756,8 @@  __rte_timer_manage(struct rte_timer_data *timer_data)
 	priv_timer[lcore_id].running_tim = NULL;
 }
 
-void
-rte_timer_manage_v20(void)
-{
-	__rte_timer_manage(&default_timer_data);
-}
-VERSION_SYMBOL(rte_timer_manage, _v20, 2.0);
-
 int
-rte_timer_manage_v1905(void)
+rte_timer_manage(void)
 {
 	struct rte_timer_data *timer_data;
 
@@ -835,8 +767,6 @@  rte_timer_manage_v1905(void)
 
 	return 0;
 }
-MAP_STATIC_SYMBOL(int rte_timer_manage(void), rte_timer_manage_v1905);
-BIND_DEFAULT_SYMBOL(rte_timer_manage, _v1905, 19.05);
 
 int
 rte_timer_alt_manage(uint32_t timer_data_id,
@@ -1074,21 +1004,11 @@  __rte_timer_dump_stats(struct rte_timer_data *timer_data __rte_unused, FILE *f)
 #endif
 }
 
-void
-rte_timer_dump_stats_v20(FILE *f)
-{
-	__rte_timer_dump_stats(&default_timer_data, f);
-}
-VERSION_SYMBOL(rte_timer_dump_stats, _v20, 2.0);
-
 int
-rte_timer_dump_stats_v1905(FILE *f)
+rte_timer_dump_stats(FILE *f)
 {
 	return rte_timer_alt_dump_stats(default_data_id, f);
 }
-MAP_STATIC_SYMBOL(int rte_timer_dump_stats(FILE *f),
-		  rte_timer_dump_stats_v1905);
-BIND_DEFAULT_SYMBOL(rte_timer_dump_stats, _v1905, 19.05);
 
 int
 rte_timer_alt_dump_stats(uint32_t timer_data_id __rte_unused, FILE *f)
diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
index 05d287d8f2..9dc5fc3092 100644
--- a/lib/librte_timer/rte_timer.h
+++ b/lib/librte_timer/rte_timer.h
@@ -181,8 +181,6 @@  int rte_timer_data_dealloc(uint32_t id);
  *      subsystem
  */
 int rte_timer_subsystem_init(void);
-int rte_timer_subsystem_init_v1905(void);
-void rte_timer_subsystem_init_v20(void);
 
 /**
  * @warning
@@ -250,13 +248,6 @@  void rte_timer_init(struct rte_timer *tim);
 int rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
 		    enum rte_timer_type type, unsigned tim_lcore,
 		    rte_timer_cb_t fct, void *arg);
-int rte_timer_reset_v1905(struct rte_timer *tim, uint64_t ticks,
-			  enum rte_timer_type type, unsigned int tim_lcore,
-			  rte_timer_cb_t fct, void *arg);
-int rte_timer_reset_v20(struct rte_timer *tim, uint64_t ticks,
-			enum rte_timer_type type, unsigned int tim_lcore,
-			rte_timer_cb_t fct, void *arg);
-
 
 /**
  * Loop until rte_timer_reset() succeeds.
@@ -313,8 +304,6 @@  rte_timer_reset_sync(struct rte_timer *tim, uint64_t ticks,
  *   - (-1): The timer is in the RUNNING or CONFIG state.
  */
 int rte_timer_stop(struct rte_timer *tim);
-int rte_timer_stop_v1905(struct rte_timer *tim);
-int rte_timer_stop_v20(struct rte_timer *tim);
 
 /**
  * Loop until rte_timer_stop() succeeds.
@@ -358,8 +347,6 @@  int rte_timer_pending(struct rte_timer *tim);
  *   - -EINVAL: timer subsystem not yet initialized
  */
 int rte_timer_manage(void);
-int rte_timer_manage_v1905(void);
-void rte_timer_manage_v20(void);
 
 /**
  * Dump statistics about timers.
@@ -371,8 +358,6 @@  void rte_timer_manage_v20(void);
  *   - -EINVAL: timer subsystem not yet initialized
  */
 int rte_timer_dump_stats(FILE *f);
-int rte_timer_dump_stats_v1905(FILE *f);
-void rte_timer_dump_stats_v20(FILE *f);
 
 /**
  * @warning