| Message ID | 1417021388-15432-2-git-send-email-taras.kondratiuk@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 26, 2014 at 7:03 PM, Taras Kondratiuk <taras.kondratiuk@linaro.org> wrote: > Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> > --- > platform/linux-generic/odp_queue.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c > index 1318bcd..f44aba0 100644 > --- a/platform/linux-generic/odp_queue.c > +++ b/platform/linux-generic/odp_queue.c > @@ -192,6 +192,23 @@ odp_queue_t odp_queue_create(const char *name, odp_queue_type_t type, > return handle; > } > > +int odp_queue_destroy(odp_queue_t handle) > +{ > + queue_entry_t *queue; > + queue = queue_to_qentry(handle); > + > + if (queue->s.status == QUEUE_STATUS_FREE) > + return -1; /* Queue is alredy freed */ > + > + LOCK(&queue->s.lock); > + queue->s.status = QUEUE_STATUS_FREE; > + queue->s.head = NULL; > + queue->s.tail = NULL; > + queue->s.sched_buf = ODP_BUFFER_INVALID; What happens with sched_buf that was allocated with odp_schedule_buffer_alloc? It needs to be freed as well, not to mention that there should be a way to protect from scheduling a queue that was destroyed. With this implementation sched_buf is set to invalid, but if the queue was scheduled prior to destroying it then sched_buf would still exist in the priority queue (see https://git.linaro.org/lng/odp.git/blob/945e8be94ea2779f55cc2fe91d3098361589bcb0:/platform/linux-generic/odp_schedule.c#l200) > + UNLOCK(&queue->s.lock); > + > + return 0; > +} > > odp_buffer_t queue_sched_buf(odp_queue_t handle) > { > -- > 1.7.9.5 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On Thu, Nov 27, 2014 at 5:49 PM, Ciprian Barbu <ciprian.barbu@linaro.org> wrote: > On Wed, Nov 26, 2014 at 7:03 PM, Taras Kondratiuk > <taras.kondratiuk@linaro.org> wrote: >> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> >> --- >> platform/linux-generic/odp_queue.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c >> index 1318bcd..f44aba0 100644 >> --- a/platform/linux-generic/odp_queue.c >> +++ b/platform/linux-generic/odp_queue.c >> @@ -192,6 +192,23 @@ odp_queue_t odp_queue_create(const char *name, odp_queue_type_t type, >> return handle; >> } >> >> +int odp_queue_destroy(odp_queue_t handle) >> +{ >> + queue_entry_t *queue; >> + queue = queue_to_qentry(handle); >> + >> + if (queue->s.status == QUEUE_STATUS_FREE) >> + return -1; /* Queue is alredy freed */ >> + >> + LOCK(&queue->s.lock); >> + queue->s.status = QUEUE_STATUS_FREE; >> + queue->s.head = NULL; >> + queue->s.tail = NULL; >> + queue->s.sched_buf = ODP_BUFFER_INVALID; > > What happens with sched_buf that was allocated with > odp_schedule_buffer_alloc? It needs to be freed as well, not to > mention that there should be a way to protect from scheduling a queue > that was destroyed. With this implementation sched_buf is set to > invalid, but if the queue was scheduled prior to destroying it then > sched_buf would still exist in the priority queue (see > https://git.linaro.org/lng/odp.git/blob/945e8be94ea2779f55cc2fe91d3098361589bcb0:/platform/linux-generic/odp_schedule.c#l200) And here is where sched_buf is dequeued from the priority queue: https://git.linaro.org/lng/odp.git/blob/945e8be94ea2779f55cc2fe91d3098361589bcb0:/platform/linux-generic/odp_schedule.c#l286 Although the queue's sched_buf handle is set to ODP_BUFFER_INVALID the same is not true for the priority queue. > >> + UNLOCK(&queue->s.lock); >> + >> + return 0; >> +} >> >> odp_buffer_t queue_sched_buf(odp_queue_t handle) >> { >> -- >> 1.7.9.5 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp
On 11/27/2014 05:57 PM, Ciprian Barbu wrote: > On Thu, Nov 27, 2014 at 5:49 PM, Ciprian Barbu <ciprian.barbu@linaro.org> wrote: >> On Wed, Nov 26, 2014 at 7:03 PM, Taras Kondratiuk >> <taras.kondratiuk@linaro.org> wrote: >>> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> >>> --- >>> platform/linux-generic/odp_queue.c | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c >>> index 1318bcd..f44aba0 100644 >>> --- a/platform/linux-generic/odp_queue.c >>> +++ b/platform/linux-generic/odp_queue.c >>> @@ -192,6 +192,23 @@ odp_queue_t odp_queue_create(const char *name, odp_queue_type_t type, >>> return handle; >>> } >>> >>> +int odp_queue_destroy(odp_queue_t handle) >>> +{ >>> + queue_entry_t *queue; >>> + queue = queue_to_qentry(handle); >>> + >>> + if (queue->s.status == QUEUE_STATUS_FREE) >>> + return -1; /* Queue is alredy freed */ >>> + >>> + LOCK(&queue->s.lock); >>> + queue->s.status = QUEUE_STATUS_FREE; >>> + queue->s.head = NULL; >>> + queue->s.tail = NULL; >>> + queue->s.sched_buf = ODP_BUFFER_INVALID; >> >> What happens with sched_buf that was allocated with >> odp_schedule_buffer_alloc? It needs to be freed as well, not to >> mention that there should be a way to protect from scheduling a queue >> that was destroyed. With this implementation sched_buf is set to >> invalid, but if the queue was scheduled prior to destroying it then >> sched_buf would still exist in the priority queue (see >> https://git.linaro.org/lng/odp.git/blob/945e8be94ea2779f55cc2fe91d3098361589bcb0:/platform/linux-generic/odp_schedule.c#l200) > > And here is where sched_buf is dequeued from the priority queue: > https://git.linaro.org/lng/odp.git/blob/945e8be94ea2779f55cc2fe91d3098361589bcb0:/platform/linux-generic/odp_schedule.c#l286 > > Although the queue's sched_buf handle is set to ODP_BUFFER_INVALID the > same is not true for the priority queue. Thanks. I forgot about scheduler. Will think how to handle this.
diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c index 1318bcd..f44aba0 100644 --- a/platform/linux-generic/odp_queue.c +++ b/platform/linux-generic/odp_queue.c @@ -192,6 +192,23 @@ odp_queue_t odp_queue_create(const char *name, odp_queue_type_t type, return handle; } +int odp_queue_destroy(odp_queue_t handle) +{ + queue_entry_t *queue; + queue = queue_to_qentry(handle); + + if (queue->s.status == QUEUE_STATUS_FREE) + return -1; /* Queue is alredy freed */ + + LOCK(&queue->s.lock); + queue->s.status = QUEUE_STATUS_FREE; + queue->s.head = NULL; + queue->s.tail = NULL; + queue->s.sched_buf = ODP_BUFFER_INVALID; + UNLOCK(&queue->s.lock); + + return 0; +} odp_buffer_t queue_sched_buf(odp_queue_t handle) {
Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> --- platform/linux-generic/odp_queue.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)