diff mbox series

[RFC,v2,10/10] eventdev: remove single event enqueue and dequeue

Message ID 20241015182535.825098-11-mattias.ronnblom@ericsson.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers
Series eventdev: remove single-event enqueue and dequeue |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build fail github build: failed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing fail Testing issues
ci/iol-unit-arm64-testing warning Testing issues
ci/iol-compile-amd64-testing warning Testing issues
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Mattias Rönnblom Oct. 15, 2024, 6:25 p.m. UTC 
Remove the single event enqueue and dequeue, since they did not
provide any noticable performance benefits.

This is a change of the ABI, previously announced as a deprecation
notice. These functions were not directly called by the application,
so the API remains unaffected.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
---
 doc/guides/rel_notes/deprecation.rst |  6 +-----
 lib/eventdev/eventdev_pmd.h          |  4 ----
 lib/eventdev/eventdev_private.c      | 22 ----------------------
 lib/eventdev/rte_eventdev.h          | 21 ++++-----------------
 lib/eventdev/rte_eventdev_core.h     |  4 ----
 5 files changed, 5 insertions(+), 52 deletions(-)

Comments

Stephen Hemminger Oct. 15, 2024, 10 p.m. UTC | #1 
On Tue, 15 Oct 2024 20:25:35 +0200
Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:

> Remove the single event enqueue and dequeue, since they did not
> provide any noticable performance benefits.
> 
> This is a change of the ABI, previously announced as a deprecation
> notice. These functions were not directly called by the application,
> so the API remains unaffected.
> 
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>

Still have a build failure with one driver.


-------------------------------BEGIN LOGS----------------------------
####################################################################################
#### [Begin job log] "ubuntu-22.04-gcc-shared-aarch64" at step Build and test
####################################################################################
      |         ^~~~~~
../drivers/event/cnxk/cn9k_eventdev.c: In function ‘cn9k_sso_fp_fns_set’:
../drivers/event/cnxk/cn9k_eventdev.c:576:20: error: ‘struct rte_eventdev’ has no member named ‘enqueue’; did you mean ‘ca_enqueue’?
  576 |         event_dev->enqueue = cn9k_sso_hws_enq;
      |                    ^~~~~~~
      |                    ca_enqueue
../drivers/event/cnxk/cn9k_eventdev.c:576:30: error: ‘cn9k_sso_hws_enq’ undeclared (first use in this function); did you mean ‘cn9k_sso_hws_link’?
  576 |         event_dev->enqueue = cn9k_sso_hws_enq;
      |                              ^~~~~~~~~~~~~~~~
      |                              cn9k_sso_hws_link
../drivers/event/cnxk/cn9k_eventdev.c:584:28: error: ‘struct rte_eventdev’ has no member named ‘enqueue’; did you mean ‘ca_enqueue’?
  584 |                 event_dev->enqueue = cn9k_sso_hws_dual_enq;
      |                            ^~~~~~~
      |                            ca_enqueue
../drivers/event/cnxk/cn9k_eventdev.c:584:38: error: ‘cn9k_sso_hws_dual_enq’ undeclared (first use in this function); did you mean ‘cn9k_sso_hws_dual_ca_enq’?
  584 |                 event_dev->enqueue = cn9k_sso_hws_dual_enq;
      |                                      ^~~~~~~~~~~~~~~~~~~~~
      |                                      cn9k_sso_hws_dual_ca_enq
[2451/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at sta/event_cnxk_deq_cn9k_deq_64_79_seg_burst.c.o'.
[2452/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at sta/event_cnxk_deq_cn9k_deq_80_95_seg_burst.c.o'.
[2453/4290] Generating rte_crypto_octeontx.sym_chk with a meson_exe.py custom command.
[2454/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at sta/event_cnxk_deq_cn9k_deq_96_111_seg_burst.c.o'.
[2455/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at sta/event_cnxk_deq_cn9k_deq_112_127_seg_burst.c.o'.
ninja: build stopped: subcommand failed.
##[error]Process completed with exit code 1.
####################################################################################
#### [End job log] "ubuntu-22.04-gcc-shared-aarch64" at step Build and test
####################################################################################
--------------------------------END LOGS-----------------------------
Mattias Rönnblom Oct. 16, 2024, 4:36 a.m. UTC | #2 
On 2024-10-16 00:00, Stephen Hemminger wrote:
> On Tue, 15 Oct 2024 20:25:35 +0200
> Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:
> 
>> Remove the single event enqueue and dequeue, since they did not
>> provide any noticable performance benefits.
>>
>> This is a change of the ABI, previously announced as a deprecation
>> notice. These functions were not directly called by the application,
>> so the API remains unaffected.
>>
>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> 
> Still have a build failure with one driver.
> 

The wonders of #ifdef <arch>.

> 
> -------------------------------BEGIN LOGS----------------------------
> ####################################################################################
> #### [Begin job log] "ubuntu-22.04-gcc-shared-aarch64" at step Build and test
> ####################################################################################
>        |         ^~~~~~
> ../drivers/event/cnxk/cn9k_eventdev.c: In function ‘cn9k_sso_fp_fns_set’:
> ../drivers/event/cnxk/cn9k_eventdev.c:576:20: error: ‘struct rte_eventdev’ has no member named ‘enqueue’; did you mean ‘ca_enqueue’?
>    576 |         event_dev->enqueue = cn9k_sso_hws_enq;
>        |                    ^~~~~~~
>        |                    ca_enqueue
> ../drivers/event/cnxk/cn9k_eventdev.c:576:30: error: ‘cn9k_sso_hws_enq’ undeclared (first use in this function); did you mean ‘cn9k_sso_hws_link’?
>    576 |         event_dev->enqueue = cn9k_sso_hws_enq;
>        |                              ^~~~~~~~~~~~~~~~
>        |                              cn9k_sso_hws_link
> ../drivers/event/cnxk/cn9k_eventdev.c:584:28: error: ‘struct rte_eventdev’ has no member named ‘enqueue’; did you mean ‘ca_enqueue’?
>    584 |                 event_dev->enqueue = cn9k_sso_hws_dual_enq;
>        |                            ^~~~~~~
>        |                            ca_enqueue
> ../drivers/event/cnxk/cn9k_eventdev.c:584:38: error: ‘cn9k_sso_hws_dual_enq’ undeclared (first use in this function); did you mean ‘cn9k_sso_hws_dual_ca_enq’?
>    584 |                 event_dev->enqueue = cn9k_sso_hws_dual_enq;
>        |                                      ^~~~~~~~~~~~~~~~~~~~~
>        |                                      cn9k_sso_hws_dual_ca_enq
> [2451/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at sta/event_cnxk_deq_cn9k_deq_64_79_seg_burst.c.o'.
> [2452/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at sta/event_cnxk_deq_cn9k_deq_80_95_seg_burst.c.o'.
> [2453/4290] Generating rte_crypto_octeontx.sym_chk with a meson_exe.py custom command.
> [2454/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at sta/event_cnxk_deq_cn9k_deq_96_111_seg_burst.c.o'.
> [2455/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at sta/event_cnxk_deq_cn9k_deq_112_127_seg_burst.c.o'.
> ninja: build stopped: subcommand failed.
> ##[error]Process completed with exit code 1.
> ####################################################################################
> #### [End job log] "ubuntu-22.04-gcc-shared-aarch64" at step Build and test
> ####################################################################################
> --------------------------------END LOGS-----------------------------
Mattias Rönnblom Oct. 16, 2024, 6:20 a.m. UTC | #3 
On 2024-10-16 06:36, Mattias Rönnblom wrote:
> On 2024-10-16 00:00, Stephen Hemminger wrote:
>> On Tue, 15 Oct 2024 20:25:35 +0200
>> Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:
>>
>>> Remove the single event enqueue and dequeue, since they did not
>>> provide any noticable performance benefits.
>>>
>>> This is a change of the ABI, previously announced as a deprecation
>>> notice. These functions were not directly called by the application,
>>> so the API remains unaffected.
>>>
>>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>
>> Still have a build failure with one driver.
>>
> 
> The wonders of #ifdef <arch>.
> 

I had a closer look at this, and given the elaborate macro-based 
machinery and the ARM-only conditional compilation it's better if a 
driver maintainer does the required changes.

(It's probably better to start from scratch on the cnxk patch, rather 
than looking at anything I did.)

Ashwin Sekhar or Pavan Nikhilesh, is this something you can do?

>>
>> -------------------------------BEGIN LOGS----------------------------
>> ####################################################################################
>> #### [Begin job log] "ubuntu-22.04-gcc-shared-aarch64" at step Build 
>> and test
>> ####################################################################################
>>        |         ^~~~~~
>> ../drivers/event/cnxk/cn9k_eventdev.c: In function ‘cn9k_sso_fp_fns_set’:
>> ../drivers/event/cnxk/cn9k_eventdev.c:576:20: error: ‘struct 
>> rte_eventdev’ has no member named ‘enqueue’; did you mean ‘ca_enqueue’?
>>    576 |         event_dev->enqueue = cn9k_sso_hws_enq;
>>        |                    ^~~~~~~
>>        |                    ca_enqueue
>> ../drivers/event/cnxk/cn9k_eventdev.c:576:30: error: 
>> ‘cn9k_sso_hws_enq’ undeclared (first use in this function); did you 
>> mean ‘cn9k_sso_hws_link’?
>>    576 |         event_dev->enqueue = cn9k_sso_hws_enq;
>>        |                              ^~~~~~~~~~~~~~~~
>>        |                              cn9k_sso_hws_link
>> ../drivers/event/cnxk/cn9k_eventdev.c:584:28: error: ‘struct 
>> rte_eventdev’ has no member named ‘enqueue’; did you mean ‘ca_enqueue’?
>>    584 |                 event_dev->enqueue = cn9k_sso_hws_dual_enq;
>>        |                            ^~~~~~~
>>        |                            ca_enqueue
>> ../drivers/event/cnxk/cn9k_eventdev.c:584:38: error: 
>> ‘cn9k_sso_hws_dual_enq’ undeclared (first use in this function); did 
>> you mean ‘cn9k_sso_hws_dual_ca_enq’?
>>    584 |                 event_dev->enqueue = cn9k_sso_hws_dual_enq;
>>        |                                      ^~~~~~~~~~~~~~~~~~~~~
>>        |                                      cn9k_sso_hws_dual_ca_enq
>> [2451/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at 
>> sta/event_cnxk_deq_cn9k_deq_64_79_seg_burst.c.o'.
>> [2452/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at 
>> sta/event_cnxk_deq_cn9k_deq_80_95_seg_burst.c.o'.
>> [2453/4290] Generating rte_crypto_octeontx.sym_chk with a meson_exe.py 
>> custom command.
>> [2454/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at 
>> sta/event_cnxk_deq_cn9k_deq_96_111_seg_burst.c.o'.
>> [2455/4290] Compiling C object 'drivers/a715181@@tmp_rte_event_cnxk at 
>> sta/event_cnxk_deq_cn9k_deq_112_127_seg_burst.c.o'.
>> ninja: build stopped: subcommand failed.
>> ##[error]Process completed with exit code 1.
>> ####################################################################################
>> #### [End job log] "ubuntu-22.04-gcc-shared-aarch64" at step Build and 
>> test
>> ####################################################################################
>> --------------------------------END LOGS-----------------------------
>
Jerin Jacob Oct. 16, 2024, 2:14 p.m. UTC | #4 
On Wed, Oct 16, 2024 at 12:14 AM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> Remove the single event enqueue and dequeue, since they did not
> provide any noticable performance benefits.
>
> This is a change of the ABI, previously announced as a deprecation
> notice. These functions were not directly called by the application,
> so the API remains unaffected.
>
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> ---
>  doc/guides/rel_notes/deprecation.rst |  6 +-----
>  lib/eventdev/eventdev_pmd.h          |  4 ----
>  lib/eventdev/eventdev_private.c      | 22 ----------------------
>  lib/eventdev/rte_eventdev.h          | 21 ++++-----------------
>  lib/eventdev/rte_eventdev_core.h     |  4 ----

Update “ABI Changes” section in doc/guides/rel_notes/release_24_11.rst
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 7bc2310bc4..6a6fd54444 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -173,11 +173,7 @@  Deprecation Notices
 
 * eventdev: The single-event (non-burst) enqueue and dequeue operations,
   used by static inline burst enqueue and dequeue functions in ``rte_eventdev.h``,
-  will be removed in DPDK 23.11.
-  This simplification includes changing the layout and potentially also
-  the size of the public ``rte_event_fp_ops`` struct, breaking the ABI.
-  Since these functions are not called directly by the application,
-  the API remains unaffected.
+  are removed in DPDK 24.11.
 
 * pipeline: The pipeline library legacy API (functions rte_pipeline_*)
   will be deprecated and subsequently removed in DPDK 24.11 release.
diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
index af855e3467..36148f8d86 100644
--- a/lib/eventdev/eventdev_pmd.h
+++ b/lib/eventdev/eventdev_pmd.h
@@ -158,16 +158,12 @@  struct __rte_cache_aligned rte_eventdev {
 	uint8_t attached : 1;
 	/**< Flag indicating the device is attached */
 
-	event_enqueue_t enqueue;
-	/**< Pointer to PMD enqueue function. */
 	event_enqueue_burst_t enqueue_burst;
 	/**< Pointer to PMD enqueue burst function. */
 	event_enqueue_burst_t enqueue_new_burst;
 	/**< Pointer to PMD enqueue burst function(op new variant) */
 	event_enqueue_burst_t enqueue_forward_burst;
 	/**< Pointer to PMD enqueue burst function(op forward variant) */
-	event_dequeue_t dequeue;
-	/**< Pointer to PMD dequeue function. */
 	event_dequeue_burst_t dequeue_burst;
 	/**< Pointer to PMD dequeue burst function. */
 	event_maintain_t maintain;
diff --git a/lib/eventdev/eventdev_private.c b/lib/eventdev/eventdev_private.c
index b628f4a69e..6df129fc2d 100644
--- a/lib/eventdev/eventdev_private.c
+++ b/lib/eventdev/eventdev_private.c
@@ -5,15 +5,6 @@ 
 #include "eventdev_pmd.h"
 #include "rte_eventdev.h"
 
-static uint16_t
-dummy_event_enqueue(__rte_unused void *port,
-		    __rte_unused const struct rte_event *ev)
-{
-	RTE_EDEV_LOG_ERR(
-		"event enqueue requested for unconfigured event device");
-	return 0;
-}
-
 static uint16_t
 dummy_event_enqueue_burst(__rte_unused void *port,
 			  __rte_unused const struct rte_event ev[],
@@ -24,15 +15,6 @@  dummy_event_enqueue_burst(__rte_unused void *port,
 	return 0;
 }
 
-static uint16_t
-dummy_event_dequeue(__rte_unused void *port, __rte_unused struct rte_event *ev,
-		    __rte_unused uint64_t timeout_ticks)
-{
-	RTE_EDEV_LOG_ERR(
-		"event dequeue requested for unconfigured event device");
-	return 0;
-}
-
 static uint16_t
 dummy_event_dequeue_burst(__rte_unused void *port,
 			  __rte_unused struct rte_event ev[],
@@ -129,11 +111,9 @@  event_dev_fp_ops_reset(struct rte_event_fp_ops *fp_op)
 {
 	static void *dummy_data[RTE_MAX_QUEUES_PER_PORT];
 	static const struct rte_event_fp_ops dummy = {
-		.enqueue = dummy_event_enqueue,
 		.enqueue_burst = dummy_event_enqueue_burst,
 		.enqueue_new_burst = dummy_event_enqueue_burst,
 		.enqueue_forward_burst = dummy_event_enqueue_burst,
-		.dequeue = dummy_event_dequeue,
 		.dequeue_burst = dummy_event_dequeue_burst,
 		.maintain = dummy_event_maintain,
 		.txa_enqueue = dummy_event_tx_adapter_enqueue,
@@ -153,11 +133,9 @@  void
 event_dev_fp_ops_set(struct rte_event_fp_ops *fp_op,
 		     const struct rte_eventdev *dev)
 {
-	fp_op->enqueue = dev->enqueue;
 	fp_op->enqueue_burst = dev->enqueue_burst;
 	fp_op->enqueue_new_burst = dev->enqueue_new_burst;
 	fp_op->enqueue_forward_burst = dev->enqueue_forward_burst;
-	fp_op->dequeue = dev->dequeue;
 	fp_op->dequeue_burst = dev->dequeue_burst;
 	fp_op->maintain = dev->maintain;
 	fp_op->txa_enqueue = dev->txa_enqueue;
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index b5c3c16dd0..fabd1490db 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -2596,14 +2596,8 @@  __rte_event_enqueue_burst(uint8_t dev_id, uint8_t port_id,
 	}
 #endif
 	rte_eventdev_trace_enq_burst(dev_id, port_id, ev, nb_events, (void *)fn);
-	/*
-	 * Allow zero cost non burst mode routine invocation if application
-	 * requests nb_events as const one
-	 */
-	if (nb_events == 1)
-		return (fp_ops->enqueue)(port, ev);
-	else
-		return fn(port, ev, nb_events);
+
+	return fn(port, ev, nb_events);
 }
 
 /**
@@ -2852,15 +2846,8 @@  rte_event_dequeue_burst(uint8_t dev_id, uint8_t port_id, struct rte_event ev[],
 	}
 #endif
 	rte_eventdev_trace_deq_burst(dev_id, port_id, ev, nb_events);
-	/*
-	 * Allow zero cost non burst mode routine invocation if application
-	 * requests nb_events as const one
-	 */
-	if (nb_events == 1)
-		return (fp_ops->dequeue)(port, ev, timeout_ticks);
-	else
-		return (fp_ops->dequeue_burst)(port, ev, nb_events,
-					       timeout_ticks);
+
+	return (fp_ops->dequeue_burst)(port, ev, nb_events, timeout_ticks);
 }
 
 #define RTE_EVENT_DEV_MAINT_OP_FLUSH          (1 << 0)
diff --git a/lib/eventdev/rte_eventdev_core.h b/lib/eventdev/rte_eventdev_core.h
index 2706d5e6c8..78b06d1f2e 100644
--- a/lib/eventdev/rte_eventdev_core.h
+++ b/lib/eventdev/rte_eventdev_core.h
@@ -60,16 +60,12 @@  typedef void (*event_preschedule_t)(void *port,
 struct __rte_cache_aligned rte_event_fp_ops {
 	void **data;
 	/**< points to array of internal port data pointers */
-	event_enqueue_t enqueue;
-	/**< PMD enqueue function. */
 	event_enqueue_burst_t enqueue_burst;
 	/**< PMD enqueue burst function. */
 	event_enqueue_burst_t enqueue_new_burst;
 	/**< PMD enqueue burst new function. */
 	event_enqueue_burst_t enqueue_forward_burst;
 	/**< PMD enqueue burst fwd function. */
-	event_dequeue_t dequeue;
-	/**< PMD dequeue function. */
 	event_dequeue_burst_t dequeue_burst;
 	/**< PMD dequeue burst function. */
 	event_maintain_t maintain;