[RFC,12/15] eventdev: move timer adapters memory to hugepage

Message ID 20210823194020.1229-12-pbhagavatula@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers
Series [RFC,01/15] eventdev: make driver interface as internal |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Pavan Nikhilesh Bhagavatula Aug. 23, 2021, 7:40 p.m. UTC
  From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Move memory used by timer adapters to hugepage.
Allocate memory on the first adapter create or lookup to address
both primary and secondary process usecases.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 lib/eventdev/rte_event_timer_adapter.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)
  

Comments

Carrillo, Erik G Aug. 24, 2021, 1:50 p.m. UTC | #1
Hi Pavan,

> -----Original Message-----
> From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> Sent: Monday, August 23, 2021 2:40 PM
> To: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
> Pavan Nikhilesh <pbhagavatula@marvell.com>
> Subject: [dpdk-dev] [RFC 12/15] eventdev: move timer adapters memory to
> hugepage
> 
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Move memory used by timer adapters to hugepage.
> Allocate memory on the first adapter create or lookup to address both
> primary and secondary process usecases.
> 

Is the motivation for this change performance or space improvement?  Can we add something to the commit message to say?

Thanks,
Erik

> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>  lib/eventdev/rte_event_timer_adapter.c | 24
> +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/eventdev/rte_event_timer_adapter.c
> b/lib/eventdev/rte_event_timer_adapter.c
> index ae55407042..c4dc7a5fd4 100644
> --- a/lib/eventdev/rte_event_timer_adapter.c
> +++ b/lib/eventdev/rte_event_timer_adapter.c
> @@ -33,7 +33,7 @@ RTE_LOG_REGISTER_SUFFIX(evtim_logtype,
> adapter.timer, NOTICE);
> RTE_LOG_REGISTER_SUFFIX(evtim_buffer_logtype, adapter.timer, NOTICE);
> RTE_LOG_REGISTER_SUFFIX(evtim_svc_logtype, adapter.timer.svc,
> NOTICE);
> 
> -static struct rte_event_timer_adapter
> adapters[RTE_EVENT_TIMER_ADAPTER_NUM_MAX];
> +static struct rte_event_timer_adapter *adapters;
> 
>  static const struct event_timer_adapter_ops swtim_ops;
> 
> @@ -138,6 +138,17 @@ rte_event_timer_adapter_create_ext(
>  	int n, ret;
>  	struct rte_eventdev *dev;
> 
> +	if (adapters == NULL) {
> +		adapters = rte_zmalloc("Eventdev",
> +				       sizeof(struct rte_event_timer_adapter) *
> +
> RTE_EVENT_TIMER_ADAPTER_NUM_MAX,
> +				       RTE_CACHE_LINE_SIZE);
> +		if (adapters == NULL) {
> +			rte_errno = ENOMEM;
> +			return NULL;
> +		}
> +	}
> +
>  	if (conf == NULL) {
>  		rte_errno = EINVAL;
>  		return NULL;
> @@ -312,6 +323,17 @@ rte_event_timer_adapter_lookup(uint16_t
> adapter_id)
>  	int ret;
>  	struct rte_eventdev *dev;
> 
> +	if (adapters == NULL) {
> +		adapters = rte_zmalloc("Eventdev",
> +				       sizeof(struct rte_event_timer_adapter) *
> +
> RTE_EVENT_TIMER_ADAPTER_NUM_MAX,
> +				       RTE_CACHE_LINE_SIZE);
> +		if (adapters == NULL) {
> +			rte_errno = ENOMEM;
> +			return NULL;
> +		}
> +	}
> +
>  	if (adapters[adapter_id].allocated)
>  		return &adapters[adapter_id]; /* Adapter is already loaded
> */
> 
> --
> 2.17.1
  
Pavan Nikhilesh Bhagavatula Sept. 1, 2021, 6:30 a.m. UTC | #2
>Hi Pavan,
>
>> -----Original Message-----
>> From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
>> Sent: Monday, August 23, 2021 2:40 PM
>> To: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>
>> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>;
>dev@dpdk.org;
>> Pavan Nikhilesh <pbhagavatula@marvell.com>
>> Subject: [dpdk-dev] [RFC 12/15] eventdev: move timer adapters
>memory to
>> hugepage
>>
>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>
>> Move memory used by timer adapters to hugepage.
>> Allocate memory on the first adapter create or lookup to address both
>> primary and secondary process usecases.
>>
>
>Is the motivation for this change performance or space improvement?
>Can we add something to the commit message to say?

This was supposed to be a perf improvement change, I will be dropping this 
for event device as it causes an additional load for getting the base address.

For timer adapter I think we can make this change as we return the pointer
to the adapter directly, so no additional lookup cost.

I will update the commit message in the next version.

>
>Thanks,
>Erik

Thanks,
Pavan.

>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> ---
>>  lib/eventdev/rte_event_timer_adapter.c | 24
>> +++++++++++++++++++++++-
>>  1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/eventdev/rte_event_timer_adapter.c
>> b/lib/eventdev/rte_event_timer_adapter.c
>> index ae55407042..c4dc7a5fd4 100644
>> --- a/lib/eventdev/rte_event_timer_adapter.c
>> +++ b/lib/eventdev/rte_event_timer_adapter.c
>> @@ -33,7 +33,7 @@ RTE_LOG_REGISTER_SUFFIX(evtim_logtype,
>> adapter.timer, NOTICE);
>> RTE_LOG_REGISTER_SUFFIX(evtim_buffer_logtype, adapter.timer,
>NOTICE);
>> RTE_LOG_REGISTER_SUFFIX(evtim_svc_logtype, adapter.timer.svc,
>> NOTICE);
>>
>> -static struct rte_event_timer_adapter
>> adapters[RTE_EVENT_TIMER_ADAPTER_NUM_MAX];
>> +static struct rte_event_timer_adapter *adapters;
>>
>>  static const struct event_timer_adapter_ops swtim_ops;
>>
>> @@ -138,6 +138,17 @@ rte_event_timer_adapter_create_ext(
>>  	int n, ret;
>>  	struct rte_eventdev *dev;
>>
>> +	if (adapters == NULL) {
>> +		adapters = rte_zmalloc("Eventdev",
>> +				       sizeof(struct
>rte_event_timer_adapter) *
>> +
>> RTE_EVENT_TIMER_ADAPTER_NUM_MAX,
>> +				       RTE_CACHE_LINE_SIZE);
>> +		if (adapters == NULL) {
>> +			rte_errno = ENOMEM;
>> +			return NULL;
>> +		}
>> +	}
>> +
>>  	if (conf == NULL) {
>>  		rte_errno = EINVAL;
>>  		return NULL;
>> @@ -312,6 +323,17 @@ rte_event_timer_adapter_lookup(uint16_t
>> adapter_id)
>>  	int ret;
>>  	struct rte_eventdev *dev;
>>
>> +	if (adapters == NULL) {
>> +		adapters = rte_zmalloc("Eventdev",
>> +				       sizeof(struct
>rte_event_timer_adapter) *
>> +
>> RTE_EVENT_TIMER_ADAPTER_NUM_MAX,
>> +				       RTE_CACHE_LINE_SIZE);
>> +		if (adapters == NULL) {
>> +			rte_errno = ENOMEM;
>> +			return NULL;
>> +		}
>> +	}
>> +
>>  	if (adapters[adapter_id].allocated)
>>  		return &adapters[adapter_id]; /* Adapter is already
>loaded
>> */
>>
>> --
>> 2.17.1
  

Patch

diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index ae55407042..c4dc7a5fd4 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -33,7 +33,7 @@  RTE_LOG_REGISTER_SUFFIX(evtim_logtype, adapter.timer, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(evtim_buffer_logtype, adapter.timer, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(evtim_svc_logtype, adapter.timer.svc, NOTICE);
 
-static struct rte_event_timer_adapter adapters[RTE_EVENT_TIMER_ADAPTER_NUM_MAX];
+static struct rte_event_timer_adapter *adapters;
 
 static const struct event_timer_adapter_ops swtim_ops;
 
@@ -138,6 +138,17 @@  rte_event_timer_adapter_create_ext(
 	int n, ret;
 	struct rte_eventdev *dev;
 
+	if (adapters == NULL) {
+		adapters = rte_zmalloc("Eventdev",
+				       sizeof(struct rte_event_timer_adapter) *
+					       RTE_EVENT_TIMER_ADAPTER_NUM_MAX,
+				       RTE_CACHE_LINE_SIZE);
+		if (adapters == NULL) {
+			rte_errno = ENOMEM;
+			return NULL;
+		}
+	}
+
 	if (conf == NULL) {
 		rte_errno = EINVAL;
 		return NULL;
@@ -312,6 +323,17 @@  rte_event_timer_adapter_lookup(uint16_t adapter_id)
 	int ret;
 	struct rte_eventdev *dev;
 
+	if (adapters == NULL) {
+		adapters = rte_zmalloc("Eventdev",
+				       sizeof(struct rte_event_timer_adapter) *
+					       RTE_EVENT_TIMER_ADAPTER_NUM_MAX,
+				       RTE_CACHE_LINE_SIZE);
+		if (adapters == NULL) {
+			rte_errno = ENOMEM;
+			return NULL;
+		}
+	}
+
 	if (adapters[adapter_id].allocated)
 		return &adapters[adapter_id]; /* Adapter is already loaded */