no1wudi commented on a change in pull request #5498: URL: https://github.com/apache/incubator-nuttx/pull/5498#discussion_r807984699
########## File path: sched/task/task_tls_alloc.c ########## @@ -0,0 +1,113 @@ +/**************************************************************************** + * sched/task/task_tls_alloc.c + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <sys/types.h> +#include <unistd.h> +#include <sched.h> +#include <errno.h> + +#include <nuttx/tls.h> +#include <nuttx/semaphore.h> + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +#if CONFIG_TLS_TASK_NELEM > 0 + +static int g_tlsset = 0; +static sem_t g_tlssem = SEM_INITIALIZER(1); +static tls_dtor_t g_tlsdtor[CONFIG_TLS_TASK_NELEM]; + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +int task_tls_alloc(tls_dtor_t dtor) +{ + int ret; + int candidate; + + ret = nxsem_wait(&g_tlssem); + + if (ret < 0) + { + return ret; + } + + ret = -EAGAIN; + + for (candidate = 0; candidate < CONFIG_TLS_TASK_NELEM; candidate++) + { + int mask = 1 << candidate; + if ((g_tlsset & mask) == 0) + { + g_tlsset |= mask; + g_tlsdtor[candidate] = dtor; + ret = candidate; + break; + } + } + + nxsem_post(&g_tlssem); + return ret; +} + +/**************************************************************************** + * Name: task_tls_destruct + * + * Description: + * Destruct all TLS data element associated with allocated key + * + * Input Parameters: + * None + * + * Returned Value: + * A set of allocated TLS index + * + ****************************************************************************/ + +void task_tls_destruct(void) Review comment: These APIs is very similar to pthread_key_create series APIs, the only different is it works at task level, then: > 1. Is it expected that task_tls_destruct() will call TLS destructors for other tasks? Yes, if any tls key allocated, we must call task_tls_destruct for all tasks since scheduler don't know wether was the task specific context used. > If yes, then should we have a prerequisite that dtor() should check for NULL inside that call? Yes, we can check the data element as pthread does. > 3. Is it expected that after task_tls_destruct() is called the while all CONFIG_TLS_TASK_NELEM are allocated the new task_tls_alloc() will always fail because g_tlsset never resets bits? Yes in current implementation, maybe we can add a reference counter for each bit to recycle it ? Or maybe it's not a problem since in current workload, only few application or midlleware like libuv need it, and in such a complex system, the counter is hard to get chance to return zero. > 4. What it task_tls_destruct() is called in the middle of task_tls_alloc(), for example after g_tlsdtor[candidate] = dtor; and before ret = candidate;? Will the dtor be ready to accept NULL (that actually comes back to 2) It's safe in this situation since task_tls_destruct and task_tls_alloc is independent in different task, if task A use relative API (task_tls_alloc), it's dtor is used for all tasks but it's data element is only used in task A. > 5. Is it expected to have 3 * (N - 1) * sizeof(uintptr_t) (where N is the number of tasks in the system) of waste memory Yes, so these APIs disabled by default. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
