aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJosep Puigdemont <josep.puigdemont@linaro.org>2018-04-23 13:48:22 +0200
committerMaxim Uvarov <maxim.uvarov@linaro.org>2018-04-24 19:27:02 +0300
commit6947621e1b3a3c73c3ee351325f505e5e191474f (patch)
tree9bd538185d8010dd3a1a12028d6f42889d3fa44e
parent29cb860583cb906bc16eddf9a4c98d6bb37333e5 (diff)
downloadodp-master.tar.gz
linux-gen: fdserver: remove unnecessary lockingHEADmaster
The locks in fdserver's operations don't serve any purpose, there is no need to acquire a lock when registering a file descriptor to the server, specially because the only place where the registering function is used is already protected by another lock, and the same goes for deregistering. Also, the fdserver handles requests sequentially. On the other hand, removing the lock from the lookup function may return a fd that is being deregistered, but this is not protecting us from misusing it because the operation to be unregistered may be queued right after the lookup request, at which point the fd is already invalid anyway. Signed-off-by: Josep Puigdemont <josep.puigdemont@linaro.org> Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
-rw-r--r--platform/linux-generic/odp_fdserver.c45
1 files changed, 4 insertions, 41 deletions
diff --git a/platform/linux-generic/odp_fdserver.c b/platform/linux-generic/odp_fdserver.c
index 0e9fb0e..774fe36 100644
--- a/platform/linux-generic/odp_fdserver.c
+++ b/platform/linux-generic/odp_fdserver.c
@@ -39,7 +39,6 @@
*/
#include <odp_posix_extensions.h>
-#include <odp/api/spinlock.h>
#include <odp_internal.h>
#include <odp_debug_internal.h>
#include <odp_fdserver_internal.h>
@@ -78,9 +77,6 @@
ODP_DBG(fmt, ##__VA_ARGS__);\
} while (0)
-/* when accessing the client functions, clients should be mutexed: */
-static odp_spinlock_t *client_lock;
-
/* define the tables of file descriptors handled by this server: */
#define FDSERVER_MAX_ENTRIES 256
typedef struct fdentry_s {
@@ -288,23 +284,18 @@ int _odp_fdserver_register_fd(fd_server_context_e context, uint64_t key,
int command;
int fd;
- odp_spinlock_lock(client_lock);
-
FD_ODP_DBG("FD client register: pid=%d key=%" PRIu64 ", fd=%d\n",
getpid(), key, fd_to_send);
s_sock = get_socket();
- if (s_sock < 0) {
- odp_spinlock_unlock(client_lock);
+ if (s_sock < 0)
return -1;
- }
res = send_fdserver_msg(s_sock, FD_REGISTER_REQ, context, key,
fd_to_send);
if (res < 0) {
ODP_ERR("fd registration failure\n");
close(s_sock);
- odp_spinlock_unlock(client_lock);
return -1;
}
@@ -313,13 +304,11 @@ int _odp_fdserver_register_fd(fd_server_context_e context, uint64_t key,
if ((res < 0) || (command != FD_REGISTER_ACK)) {
ODP_ERR("fd registration failure\n");
close(s_sock);
- odp_spinlock_unlock(client_lock);
return -1;
}
close(s_sock);
- odp_spinlock_unlock(client_lock);
return 0;
}
@@ -334,22 +323,17 @@ int _odp_fdserver_deregister_fd(fd_server_context_e context, uint64_t key)
int command;
int fd;
- odp_spinlock_lock(client_lock);
-
FD_ODP_DBG("FD client deregister: pid=%d key=%" PRIu64 "\n",
getpid(), key);
s_sock = get_socket();
- if (s_sock < 0) {
- odp_spinlock_unlock(client_lock);
+ if (s_sock < 0)
return -1;
- }
res = send_fdserver_msg(s_sock, FD_DEREGISTER_REQ, context, key, -1);
if (res < 0) {
ODP_ERR("fd de-registration failure\n");
close(s_sock);
- odp_spinlock_unlock(client_lock);
return -1;
}
@@ -358,13 +342,11 @@ int _odp_fdserver_deregister_fd(fd_server_context_e context, uint64_t key)
if ((res < 0) || (command != FD_DEREGISTER_ACK)) {
ODP_ERR("fd de-registration failure\n");
close(s_sock);
- odp_spinlock_unlock(client_lock);
return -1;
}
close(s_sock);
- odp_spinlock_unlock(client_lock);
return 0;
}
@@ -380,19 +362,14 @@ int _odp_fdserver_lookup_fd(fd_server_context_e context, uint64_t key)
int command;
int fd;
- odp_spinlock_lock(client_lock);
-
s_sock = get_socket();
- if (s_sock < 0) {
- odp_spinlock_unlock(client_lock);
+ if (s_sock < 0)
return -1;
- }
res = send_fdserver_msg(s_sock, FD_LOOKUP_REQ, context, key, -1);
if (res < 0) {
ODP_ERR("fd lookup failure\n");
close(s_sock);
- odp_spinlock_unlock(client_lock);
return -1;
}
@@ -401,7 +378,6 @@ int _odp_fdserver_lookup_fd(fd_server_context_e context, uint64_t key)
if ((res < 0) || (command != FD_LOOKUP_ACK)) {
ODP_ERR("fd lookup failure\n");
close(s_sock);
- odp_spinlock_unlock(client_lock);
return -1;
}
@@ -409,7 +385,6 @@ int _odp_fdserver_lookup_fd(fd_server_context_e context, uint64_t key)
ODP_DBG("FD client lookup: pid=%d, key=%" PRIu64 ", fd=%d\n",
getpid(), key, fd);
- odp_spinlock_unlock(client_lock);
return fd;
}
@@ -421,27 +396,21 @@ static int stop_server(void)
int s_sock; /* server socket */
int res;
- odp_spinlock_lock(client_lock);
-
FD_ODP_DBG("FD sending server stop request\n");
s_sock = get_socket();
- if (s_sock < 0) {
- odp_spinlock_unlock(client_lock);
+ if (s_sock < 0)
return -1;
- }
res = send_fdserver_msg(s_sock, FD_SERVERSTOP_REQ, 0, 0, -1);
if (res < 0) {
ODP_ERR("fd stop request failure\n");
close(s_sock);
- odp_spinlock_unlock(client_lock);
return -1;
}
close(s_sock);
- odp_spinlock_unlock(client_lock);
return 0;
}
@@ -595,12 +564,6 @@ int _odp_fdserver_init_global(void)
pid_t server_pid;
int res;
- /* create the client spinlock that any client can see: */
- client_lock = mmap(NULL, sizeof(odp_spinlock_t), PROT_READ | PROT_WRITE,
- MAP_SHARED | MAP_ANONYMOUS, -1, 0);
-
- odp_spinlock_init(client_lock);
-
snprintf(sockpath, FDSERVER_SOCKPATH_MAXLEN, FDSERVER_SOCKDIR_FORMAT,
odp_global_data.shm_dir,
odp_global_data.uid);