diff mbox

[API-NEXT,PATCHv3,1/4] linux-gen: sched: solve ordered context inversion

Message ID 1484121033-15354-2-git-send-email-yi.he@linaro.org
State Accepted
Commit 34f63c0cb6421ce32fac1a409cbc75fd1cc8fdbc
Headers show

Commit Message

Yi He Jan. 11, 2017, 7:50 a.m. UTC
For ordered queue, a thread consumes events (dequeue) and
acquires its unique sequential context in two steps, non
atomic and preemptable.

This leads to potential ordered context inversion in case
the thread consumes prior events acquired subsequent context,
while the thread consumes subsequent events but acquired
prior context.

This patch insert the ordered context acquisition into
event dequeue operation to make these two steps atomic.

Signed-off-by: Yi He <yi.he@linaro.org>

---
 platform/linux-generic/include/odp_schedule_if.h | 3 +++
 platform/linux-generic/odp_queue.c               | 3 +++
 platform/linux-generic/odp_schedule.c            | 7 ++++++-
 platform/linux-generic/odp_schedule_sp.c         | 7 ++++++-
 4 files changed, 18 insertions(+), 2 deletions(-)

-- 
2.7.4

Comments

Bill Fischofer Jan. 11, 2017, 11:41 p.m. UTC | #1
For this series:

Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org>

On Wed, Jan 11, 2017 at 1:50 AM, Yi He <yi.he@linaro.org> wrote:
> For ordered queue, a thread consumes events (dequeue) and

> acquires its unique sequential context in two steps, non

> atomic and preemptable.

>

> This leads to potential ordered context inversion in case

> the thread consumes prior events acquired subsequent context,

> while the thread consumes subsequent events but acquired

> prior context.

>

> This patch insert the ordered context acquisition into

> event dequeue operation to make these two steps atomic.

>

> Signed-off-by: Yi He <yi.he@linaro.org>

> ---

>  platform/linux-generic/include/odp_schedule_if.h | 3 +++

>  platform/linux-generic/odp_queue.c               | 3 +++

>  platform/linux-generic/odp_schedule.c            | 7 ++++++-

>  platform/linux-generic/odp_schedule_sp.c         | 7 ++++++-

>  4 files changed, 18 insertions(+), 2 deletions(-)

>

> diff --git a/platform/linux-generic/include/odp_schedule_if.h b/platform/linux-generic/include/odp_schedule_if.h

> index 6c2b050..c0aee42 100644

> --- a/platform/linux-generic/include/odp_schedule_if.h

> +++ b/platform/linux-generic/include/odp_schedule_if.h

> @@ -12,6 +12,7 @@ extern "C" {

>  #endif

>

>  #include <odp/api/queue.h>

> +#include <odp_queue_internal.h>

>  #include <odp/api/schedule.h>

>

>  typedef void (*schedule_pktio_start_fn_t)(int pktio_index, int num_in_queue,

> @@ -33,6 +34,7 @@ typedef int (*schedule_term_local_fn_t)(void);

>  typedef void (*schedule_order_lock_fn_t)(void);

>  typedef void (*schedule_order_unlock_fn_t)(void);

>  typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);

> +typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);

>

>  typedef struct schedule_fn_t {

>         schedule_pktio_start_fn_t   pktio_start;

> @@ -50,6 +52,7 @@ typedef struct schedule_fn_t {

>         schedule_order_lock_fn_t    order_lock;

>         schedule_order_unlock_fn_t  order_unlock;

>         schedule_max_ordered_locks_fn_t max_ordered_locks;

> +       schedule_save_context_fn_t  save_context;

>  } schedule_fn_t;

>

>  /* Interface towards the scheduler */

> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c

> index d9cb9f3..3b6a68b 100644

> --- a/platform/linux-generic/odp_queue.c

> +++ b/platform/linux-generic/odp_queue.c

> @@ -568,6 +568,9 @@ static inline int deq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[],

>         if (hdr == NULL)

>                 queue->s.tail = NULL;

>

> +       if (queue->s.type == ODP_QUEUE_TYPE_SCHED)

> +               sched_fn->save_context(queue);

> +

>         UNLOCK(&queue->s.lock);

>

>         return i;

> diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c

> index 645630a..264c58f 100644

> --- a/platform/linux-generic/odp_schedule.c

> +++ b/platform/linux-generic/odp_schedule.c

> @@ -1205,6 +1205,10 @@ static int schedule_num_grps(void)

>         return NUM_SCHED_GRPS;

>  }

>

> +static void schedule_save_context(queue_entry_t *queue ODP_UNUSED)

> +{

> +}

> +

>  /* Fill in scheduler interface */

>  const schedule_fn_t schedule_default_fn = {

>         .pktio_start = schedule_pktio_start,

> @@ -1221,7 +1225,8 @@ const schedule_fn_t schedule_default_fn = {

>         .term_local  = schedule_term_local,

>         .order_lock = order_lock,

>         .order_unlock = order_unlock,

> -       .max_ordered_locks = schedule_max_ordered_locks

> +       .max_ordered_locks = schedule_max_ordered_locks,

> +       .save_context = schedule_save_context

>  };

>

>  /* Fill in scheduler API calls */

> diff --git a/platform/linux-generic/odp_schedule_sp.c b/platform/linux-generic/odp_schedule_sp.c

> index 5150d28..1406312 100644

> --- a/platform/linux-generic/odp_schedule_sp.c

> +++ b/platform/linux-generic/odp_schedule_sp.c

> @@ -803,6 +803,10 @@ static void order_unlock(void)

>  {

>  }

>

> +static void save_context(queue_entry_t *queue ODP_UNUSED)

> +{

> +}

> +

>  /* Fill in scheduler interface */

>  const schedule_fn_t schedule_sp_fn = {

>         .pktio_start   = pktio_start,

> @@ -819,7 +823,8 @@ const schedule_fn_t schedule_sp_fn = {

>         .term_local    = term_local,

>         .order_lock =    order_lock,

>         .order_unlock =  order_unlock,

> -       .max_ordered_locks = max_ordered_locks

> +       .max_ordered_locks = max_ordered_locks,

> +       .save_context  = save_context

>  };

>

>  /* Fill in scheduler API calls */

> --

> 2.7.4

>
Maxim Uvarov Jan. 12, 2017, 2:40 p.m. UTC | #2
Merged,
Maxim.

On 01/12/17 02:41, Bill Fischofer wrote:
> For this series:

> 

> Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org>

> 

> On Wed, Jan 11, 2017 at 1:50 AM, Yi He <yi.he@linaro.org> wrote:

>> For ordered queue, a thread consumes events (dequeue) and

>> acquires its unique sequential context in two steps, non

>> atomic and preemptable.

>>

>> This leads to potential ordered context inversion in case

>> the thread consumes prior events acquired subsequent context,

>> while the thread consumes subsequent events but acquired

>> prior context.

>>

>> This patch insert the ordered context acquisition into

>> event dequeue operation to make these two steps atomic.

>>

>> Signed-off-by: Yi He <yi.he@linaro.org>

>> ---

>>  platform/linux-generic/include/odp_schedule_if.h | 3 +++

>>  platform/linux-generic/odp_queue.c               | 3 +++

>>  platform/linux-generic/odp_schedule.c            | 7 ++++++-

>>  platform/linux-generic/odp_schedule_sp.c         | 7 ++++++-

>>  4 files changed, 18 insertions(+), 2 deletions(-)

>>

>> diff --git a/platform/linux-generic/include/odp_schedule_if.h b/platform/linux-generic/include/odp_schedule_if.h

>> index 6c2b050..c0aee42 100644

>> --- a/platform/linux-generic/include/odp_schedule_if.h

>> +++ b/platform/linux-generic/include/odp_schedule_if.h

>> @@ -12,6 +12,7 @@ extern "C" {

>>  #endif

>>

>>  #include <odp/api/queue.h>

>> +#include <odp_queue_internal.h>

>>  #include <odp/api/schedule.h>

>>

>>  typedef void (*schedule_pktio_start_fn_t)(int pktio_index, int num_in_queue,

>> @@ -33,6 +34,7 @@ typedef int (*schedule_term_local_fn_t)(void);

>>  typedef void (*schedule_order_lock_fn_t)(void);

>>  typedef void (*schedule_order_unlock_fn_t)(void);

>>  typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);

>> +typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);

>>

>>  typedef struct schedule_fn_t {

>>         schedule_pktio_start_fn_t   pktio_start;

>> @@ -50,6 +52,7 @@ typedef struct schedule_fn_t {

>>         schedule_order_lock_fn_t    order_lock;

>>         schedule_order_unlock_fn_t  order_unlock;

>>         schedule_max_ordered_locks_fn_t max_ordered_locks;

>> +       schedule_save_context_fn_t  save_context;

>>  } schedule_fn_t;

>>

>>  /* Interface towards the scheduler */

>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c

>> index d9cb9f3..3b6a68b 100644

>> --- a/platform/linux-generic/odp_queue.c

>> +++ b/platform/linux-generic/odp_queue.c

>> @@ -568,6 +568,9 @@ static inline int deq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[],

>>         if (hdr == NULL)

>>                 queue->s.tail = NULL;

>>

>> +       if (queue->s.type == ODP_QUEUE_TYPE_SCHED)

>> +               sched_fn->save_context(queue);

>> +

>>         UNLOCK(&queue->s.lock);

>>

>>         return i;

>> diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c

>> index 645630a..264c58f 100644

>> --- a/platform/linux-generic/odp_schedule.c

>> +++ b/platform/linux-generic/odp_schedule.c

>> @@ -1205,6 +1205,10 @@ static int schedule_num_grps(void)

>>         return NUM_SCHED_GRPS;

>>  }

>>

>> +static void schedule_save_context(queue_entry_t *queue ODP_UNUSED)

>> +{

>> +}

>> +

>>  /* Fill in scheduler interface */

>>  const schedule_fn_t schedule_default_fn = {

>>         .pktio_start = schedule_pktio_start,

>> @@ -1221,7 +1225,8 @@ const schedule_fn_t schedule_default_fn = {

>>         .term_local  = schedule_term_local,

>>         .order_lock = order_lock,

>>         .order_unlock = order_unlock,

>> -       .max_ordered_locks = schedule_max_ordered_locks

>> +       .max_ordered_locks = schedule_max_ordered_locks,

>> +       .save_context = schedule_save_context

>>  };

>>

>>  /* Fill in scheduler API calls */

>> diff --git a/platform/linux-generic/odp_schedule_sp.c b/platform/linux-generic/odp_schedule_sp.c

>> index 5150d28..1406312 100644

>> --- a/platform/linux-generic/odp_schedule_sp.c

>> +++ b/platform/linux-generic/odp_schedule_sp.c

>> @@ -803,6 +803,10 @@ static void order_unlock(void)

>>  {

>>  }

>>

>> +static void save_context(queue_entry_t *queue ODP_UNUSED)

>> +{

>> +}

>> +

>>  /* Fill in scheduler interface */

>>  const schedule_fn_t schedule_sp_fn = {

>>         .pktio_start   = pktio_start,

>> @@ -819,7 +823,8 @@ const schedule_fn_t schedule_sp_fn = {

>>         .term_local    = term_local,

>>         .order_lock =    order_lock,

>>         .order_unlock =  order_unlock,

>> -       .max_ordered_locks = max_ordered_locks

>> +       .max_ordered_locks = max_ordered_locks,

>> +       .save_context  = save_context

>>  };

>>

>>  /* Fill in scheduler API calls */

>> --

>> 2.7.4

>>
diff mbox

Patch

diff --git a/platform/linux-generic/include/odp_schedule_if.h b/platform/linux-generic/include/odp_schedule_if.h
index 6c2b050..c0aee42 100644
--- a/platform/linux-generic/include/odp_schedule_if.h
+++ b/platform/linux-generic/include/odp_schedule_if.h
@@ -12,6 +12,7 @@  extern "C" {
 #endif
 
 #include <odp/api/queue.h>
+#include <odp_queue_internal.h>
 #include <odp/api/schedule.h>
 
 typedef void (*schedule_pktio_start_fn_t)(int pktio_index, int num_in_queue,
@@ -33,6 +34,7 @@  typedef int (*schedule_term_local_fn_t)(void);
 typedef void (*schedule_order_lock_fn_t)(void);
 typedef void (*schedule_order_unlock_fn_t)(void);
 typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);
+typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);
 
 typedef struct schedule_fn_t {
 	schedule_pktio_start_fn_t   pktio_start;
@@ -50,6 +52,7 @@  typedef struct schedule_fn_t {
 	schedule_order_lock_fn_t    order_lock;
 	schedule_order_unlock_fn_t  order_unlock;
 	schedule_max_ordered_locks_fn_t max_ordered_locks;
+	schedule_save_context_fn_t  save_context;
 } schedule_fn_t;
 
 /* Interface towards the scheduler */
diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c
index d9cb9f3..3b6a68b 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -568,6 +568,9 @@  static inline int deq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[],
 	if (hdr == NULL)
 		queue->s.tail = NULL;
 
+	if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
+		sched_fn->save_context(queue);
+
 	UNLOCK(&queue->s.lock);
 
 	return i;
diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c
index 645630a..264c58f 100644
--- a/platform/linux-generic/odp_schedule.c
+++ b/platform/linux-generic/odp_schedule.c
@@ -1205,6 +1205,10 @@  static int schedule_num_grps(void)
 	return NUM_SCHED_GRPS;
 }
 
+static void schedule_save_context(queue_entry_t *queue ODP_UNUSED)
+{
+}
+
 /* Fill in scheduler interface */
 const schedule_fn_t schedule_default_fn = {
 	.pktio_start = schedule_pktio_start,
@@ -1221,7 +1225,8 @@  const schedule_fn_t schedule_default_fn = {
 	.term_local  = schedule_term_local,
 	.order_lock = order_lock,
 	.order_unlock = order_unlock,
-	.max_ordered_locks = schedule_max_ordered_locks
+	.max_ordered_locks = schedule_max_ordered_locks,
+	.save_context = schedule_save_context
 };
 
 /* Fill in scheduler API calls */
diff --git a/platform/linux-generic/odp_schedule_sp.c b/platform/linux-generic/odp_schedule_sp.c
index 5150d28..1406312 100644
--- a/platform/linux-generic/odp_schedule_sp.c
+++ b/platform/linux-generic/odp_schedule_sp.c
@@ -803,6 +803,10 @@  static void order_unlock(void)
 {
 }
 
+static void save_context(queue_entry_t *queue ODP_UNUSED)
+{
+}
+
 /* Fill in scheduler interface */
 const schedule_fn_t schedule_sp_fn = {
 	.pktio_start   = pktio_start,
@@ -819,7 +823,8 @@  const schedule_fn_t schedule_sp_fn = {
 	.term_local    = term_local,
 	.order_lock =    order_lock,
 	.order_unlock =  order_unlock,
-	.max_ordered_locks = max_ordered_locks
+	.max_ordered_locks = max_ordered_locks,
+	.save_context  = save_context
 };
 
 /* Fill in scheduler API calls */