From fe1d83c41b4ff752ad17c596998cbaa522ba0c96 Mon Sep 17 00:00:00 2001 From: Jiuzhu Dong Date: Thu, 23 Jun 2022 12:15:12 +0800 Subject: [PATCH] driver/sensor: solve the deadlock problem caused by sensor_close note: replace dev lock by upper driver lock rptun: sem_wait sched/semaphore/sem_wait.c:273 nxmutex_lock /home/neo/projects/monkey/nuttx/include/nuttx/mutex.h:161 sensor_close (wait upper driver lock) drivers/sensors/sensor.c:509 sensor_rpmsg_free_stub (get dev lock) drivers/sensors/sensor_rpmsg.c:546 sensor_rpmsg_unsub_handler drivers/sensors/sensor_rpmsg.c:1025 rptun_thread apps: sem_wait sched/semaphore/sem_wait.c:273 sensor_rpmsg_close (wait dev lock) drivers/sensors/sensor_rpmsg.c:613 sensor_close (get upper driver lock) drivers/sensors/sensor.c:512 Signed-off-by: Jiuzhu Dong --- drivers/sensors/sensor_rpmsg.c | 77 ++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/drivers/sensors/sensor_rpmsg.c b/drivers/sensors/sensor_rpmsg.c index 074da1737f..8aa9f263d2 100644 --- a/drivers/sensors/sensor_rpmsg.c +++ b/drivers/sensors/sensor_rpmsg.c @@ -82,7 +82,6 @@ struct sensor_rpmsg_dev_s { struct sensor_lowerhalf_s lower; FAR struct sensor_lowerhalf_s *drv; - rmutex_t lock; struct list_node node; struct list_node stublist; struct list_node proxylist; @@ -286,6 +285,16 @@ static mutex_t g_dev_lock = NXMUTEX_INITIALIZER; * Private Functions ****************************************************************************/ +static void sensor_rpmsg_lock(FAR struct sensor_rpmsg_dev_s *dev) +{ + dev->lower.sensor_lock(dev->upper); +} + +static void sensor_rpmsg_unlock(FAR struct sensor_rpmsg_dev_s *dev) +{ + dev->lower.sensor_unlock(dev->upper); +} + static void sensor_rpmsg_advsub_one(FAR struct sensor_rpmsg_dev_s *dev, FAR struct rpmsg_endpoint *ept, int command) @@ -355,7 +364,7 @@ static int sensor_rpmsg_ioctl(FAR struct sensor_rpmsg_dev_s *dev, * if device doesn't have proxy, it must return -ENOTTY. */ - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); list_for_every_entry(&dev->proxylist, proxy, struct sensor_rpmsg_proxy_s, node) { @@ -395,9 +404,9 @@ static int sensor_rpmsg_ioctl(FAR struct sensor_rpmsg_dev_s *dev, continue; } - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); ret = rpmsg_wait(proxy->ept, &cookie.sem); - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); if (ret < 0) { snerr("ERROR: ioctl rpmsg wait failed:%s, %d, %s\n", @@ -412,7 +421,7 @@ static int sensor_rpmsg_ioctl(FAR struct sensor_rpmsg_dev_s *dev, } } - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); if (wait) { nxsem_destroy(&cookie.sem); @@ -431,15 +440,19 @@ sensor_rpmsg_alloc_proxy(FAR struct sensor_rpmsg_dev_s *dev, struct file file; int ret; + sensor_rpmsg_lock(dev); list_for_every_entry(&dev->proxylist, proxy, struct sensor_rpmsg_proxy_s, node) { if (proxy->ept == ept && proxy->cookie == msg->cookie) { + sensor_rpmsg_unlock(dev); return proxy; } } + sensor_rpmsg_unlock(dev); + /* Create new proxy to represent a remote advertiser */ proxy = kmm_malloc(sizeof(*proxy)); @@ -461,7 +474,7 @@ sensor_rpmsg_alloc_proxy(FAR struct sensor_rpmsg_dev_s *dev, file_ioctl(&file, SNIOC_GET_STATE, &state); file_close(&file); - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); if (msg->persist) { dev->drv->persist = true; @@ -469,7 +482,7 @@ sensor_rpmsg_alloc_proxy(FAR struct sensor_rpmsg_dev_s *dev, } list_add_tail(&dev->proxylist, &proxy->node); - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); /* sync interval and latency */ @@ -495,15 +508,19 @@ sensor_rpmsg_alloc_stub(FAR struct sensor_rpmsg_dev_s *dev, FAR struct sensor_rpmsg_stub_s *stub; int ret; + sensor_rpmsg_lock(dev); list_for_every_entry(&dev->stublist, stub, struct sensor_rpmsg_stub_s, node) { if (stub->ept == ept && stub->cookie == cookie) { + sensor_rpmsg_unlock(dev); return stub; } } + sensor_rpmsg_unlock(dev); + /* Create new stub to represent a remote subscribers */ stub = kmm_malloc(sizeof(*stub)); @@ -523,9 +540,9 @@ sensor_rpmsg_alloc_stub(FAR struct sensor_rpmsg_dev_s *dev, } file_ioctl(&stub->file, SNIOC_READLAST, false); - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); list_add_tail(&dev->stublist, &stub->node); - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); if (dev->lower.persist) { @@ -569,7 +586,7 @@ static int sensor_rpmsg_open(FAR struct file *filep, return 0; } - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); if (filep->f_oflags & O_WROK) { if (dev->nadvertisers++ == 0) @@ -586,7 +603,7 @@ static int sensor_rpmsg_open(FAR struct file *filep, } } - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); return 0; } @@ -611,7 +628,7 @@ static int sensor_rpmsg_close(FAR struct file *filep, return ret; } - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); if (filep->f_oflags & O_WROK) { if (dev->nadvertisers == 1) @@ -642,7 +659,7 @@ static int sensor_rpmsg_close(FAR struct file *filep, dev->nsubscribers--; } - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); return ret; } @@ -828,14 +845,14 @@ static ssize_t sensor_rpmsg_push_event(FAR void *priv, FAR const void *data, * is successful, and must return length of written. */ - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); list_for_every_entry(&dev->stublist, stub, struct sensor_rpmsg_stub_s, node) { sensor_rpmsg_push_event_one(dev, stub); } - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); return ret; } @@ -885,9 +902,9 @@ static int sensor_rpmsg_adv_handler(FAR struct rpmsg_endpoint *ept, ret = rpmsg_send(ept, msg, len); if (ret < 0) { - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); sensor_rpmsg_free_proxy(proxy); - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); snerr("ERROR: adv rpmsg send failed:%s, %d, %s\n", dev->path, ret, rpmsg_get_cpuname(ept->rdev)); } @@ -928,7 +945,7 @@ static int sensor_rpmsg_unadv_handler(FAR struct rpmsg_endpoint *ept, return 0; } - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); list_for_every_entry(&dev->proxylist, proxy, struct sensor_rpmsg_proxy_s, node) { @@ -939,7 +956,7 @@ static int sensor_rpmsg_unadv_handler(FAR struct rpmsg_endpoint *ept, } } - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); return 0; } @@ -972,9 +989,9 @@ static int sensor_rpmsg_sub_handler(FAR struct rpmsg_endpoint *ept, ret = rpmsg_send(ept, msg, len); if (ret < 0) { - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); sensor_rpmsg_free_stub(stub); - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); snerr("ERROR: sub rpmsg send failed:%s, %d, %s\n", dev->path, ret, rpmsg_get_cpuname(ept->rdev)); } @@ -1015,7 +1032,7 @@ static int sensor_rpmsg_unsub_handler(FAR struct rpmsg_endpoint *ept, return 0; } - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); list_for_every_entry(&dev->stublist, stub, struct sensor_rpmsg_stub_s, node) { @@ -1026,7 +1043,7 @@ static int sensor_rpmsg_unsub_handler(FAR struct rpmsg_endpoint *ept, } } - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); return 0; } @@ -1065,7 +1082,7 @@ static int sensor_rpmsg_ioctl_handler(FAR struct rpmsg_endpoint *ept, arg = msg->arglen > 0 ? (unsigned long)(uintptr_t)msg->argbuf : msg->arg; dev = (FAR struct sensor_rpmsg_dev_s *)(uintptr_t)msg->proxy; - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); list_for_every_entry(&dev->stublist, stub, struct sensor_rpmsg_stub_s, node) { @@ -1087,7 +1104,7 @@ static int sensor_rpmsg_ioctl_handler(FAR struct rpmsg_endpoint *ept, } } - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); return 0; } @@ -1142,7 +1159,7 @@ static void sensor_rpmsg_ns_unbind_cb(FAR struct rpmsg_endpoint *ept) list_for_every_entry(&g_devlist, dev, struct sensor_rpmsg_dev_s, node) { - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); list_for_every_entry(&dev->proxylist, proxy, struct sensor_rpmsg_proxy_s, node) { @@ -1163,7 +1180,7 @@ static void sensor_rpmsg_ns_unbind_cb(FAR struct rpmsg_endpoint *ept) } } - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); } nxmutex_unlock(&g_dev_lock); @@ -1194,7 +1211,7 @@ static void sensor_rpmsg_device_ns_bound(FAR struct rpmsg_endpoint *ept) list_for_every_entry(&g_devlist, dev, struct sensor_rpmsg_dev_s, node) { - nxrmutex_lock(&dev->lock); + sensor_rpmsg_lock(dev); if (dev->nadvertisers > 0) { sensor_rpmsg_advsub_one(dev, ept, SENSOR_RPMSG_ADVERTISE); @@ -1205,7 +1222,7 @@ static void sensor_rpmsg_device_ns_bound(FAR struct rpmsg_endpoint *ept) sensor_rpmsg_advsub_one(dev, ept, SENSOR_RPMSG_SUBSCRIBE); } - nxrmutex_unlock(&dev->lock); + sensor_rpmsg_unlock(dev); } nxmutex_unlock(&g_dev_lock); @@ -1277,7 +1294,6 @@ sensor_rpmsg_register(FAR struct sensor_lowerhalf_s *lower, list_initialize(&dev->stublist); list_initialize(&dev->proxylist); - nxrmutex_init(&dev->lock); strcpy(dev->path, path); dev->nadvertisers = !!lower->ops->activate; @@ -1333,7 +1349,6 @@ void sensor_rpmsg_unregister(FAR struct sensor_lowerhalf_s *lower) list_delete(&dev->node); nxmutex_unlock(&g_dev_lock); - nxrmutex_destroy(&dev->lock); kmm_free(dev); }