From 48b46673186b2c1261182dbe52786d617be68e43 Mon Sep 17 00:00:00 2001 From: Jiuzhu Dong Date: Wed, 22 Jun 2022 21:37:14 +0800 Subject: [PATCH] driver/sensor: slove deadlock issue because ept/devlist using same lock CHAMPION-6418 rptun sensor: [ap] [ 5] [<0x2c352c68>] arm_switchcontext+0xc/0x10 [ap] [ 5] [<0x2c322eae>] sem_wait+0x5a/0xbc [ap] [ 5] [<0x2c3376cc>] sensor_rpmsg_find_dev+0x8/0x78 (wait g_lock) [ap] [ 5] [<0x2c338d96>] sensor_rpmsg_suback_handler+0xa/0x4c [ap] [ 5] [<0x2c7630f6>] rpmsg_virtio_rx_callback+0xba/0x1b4 [ap] [ 5] [<0x2c762788>] rproc_virtio_notified+0x44/0x5c [ap] [ 5] [<0x2c762154>] remoteproc_get_notification+0x1c/0x2c [ap] [ 5] [<0x2c335576>] rptun_thread+0x6e/0x164 [ap] [ 5] [<0x2c324aba>] nxtask_start+0x3a/0x60 apps:(note: ipc buffer is busy, it's not enough for dual core.) [ap] [26] [<0x2c323a5e>] nxsig_timedwait+0x8a/0x184 [ap] [26] [<0x2c324344>] nxsig_nanosleep+0x34/0xa4 [ap] [26] [<0x2c324446>] nxsig_usleep+0x26/0x38 [ap] [26] [<0x2c76338e>] rpmsg_virtio_get_tx_payload_buffer+0x5e/0x100 (wait ipc buffer) [ap] [26] [<0x2c33775a>] sensor_rpmsg_advsub_one+0x1e/0xdc (get g_lock) [ap] [26] [<0x2c338312>] sensor_rpmsg_open+0xee/0x178 [ap] [26] [<0x2c3365bc>] sensor_open+0x50/0x234 [ap] [26] [<0x2c749cb8>] open+0xc8/0x194 [ap] [26] [<0x2c375324>] orb_subscribe_multi+0x1c/0x94 Signed-off-by: Jiuzhu Dong --- drivers/sensors/sensor_rpmsg.c | 61 +++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/drivers/sensors/sensor_rpmsg.c b/drivers/sensors/sensor_rpmsg.c index 8338a592a9..074da1737f 100644 --- a/drivers/sensors/sensor_rpmsg.c +++ b/drivers/sensors/sensor_rpmsg.c @@ -279,7 +279,8 @@ static const rpmsg_ept_cb g_sensor_rpmsg_handler[] = static struct list_node g_devlist = LIST_INITIAL_VALUE(g_devlist); static struct list_node g_eptlist = LIST_INITIAL_VALUE(g_eptlist); -static mutex_t g_lock = MUTEX_INITIALIZER; +static mutex_t g_ept_lock = NXMUTEX_INITIALIZER; +static mutex_t g_dev_lock = NXMUTEX_INITIALIZER; /**************************************************************************** * Private Functions @@ -322,14 +323,14 @@ static void sensor_rpmsg_advsub(FAR struct sensor_rpmsg_dev_s *dev, /* Broadcast advertise/subscribe message to all ready ept */ - nxmutex_lock(&g_lock); + nxmutex_lock(&g_ept_lock); list_for_every_entry(&g_eptlist, sre, struct sensor_rpmsg_ept_s, node) { sensor_rpmsg_advsub_one(dev, &sre->ept, command); } - nxmutex_unlock(&g_lock); + nxmutex_unlock(&g_ept_lock); } static int sensor_rpmsg_ioctl(FAR struct sensor_rpmsg_dev_s *dev, @@ -843,17 +844,17 @@ sensor_rpmsg_find_dev(FAR const char *path) { FAR struct sensor_rpmsg_dev_s *dev; - nxmutex_lock(&g_lock); + nxmutex_lock(&g_dev_lock); list_for_every_entry(&g_devlist, dev, struct sensor_rpmsg_dev_s, node) { if (strcmp(dev->path, path) == 0) { - nxmutex_unlock(&g_lock); + nxmutex_unlock(&g_dev_lock); return dev; } } - nxmutex_unlock(&g_lock); + nxmutex_unlock(&g_dev_lock); return NULL; } @@ -884,7 +885,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_free_proxy(proxy); + nxrmutex_unlock(&dev->lock); snerr("ERROR: adv rpmsg send failed:%s, %d, %s\n", dev->path, ret, rpmsg_get_cpuname(ept->rdev)); } @@ -969,7 +972,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_free_stub(stub); + nxrmutex_unlock(&dev->lock); snerr("ERROR: sub rpmsg send failed:%s, %d, %s\n", dev->path, ret, rpmsg_get_cpuname(ept->rdev)); } @@ -1077,6 +1082,8 @@ static int sensor_rpmsg_ioctl_handler(FAR struct rpmsg_endpoint *ept, dev->path, ret, rpmsg_get_cpuname(ept->rdev)); } } + + break; } } @@ -1123,9 +1130,7 @@ static void sensor_rpmsg_ns_unbind_cb(FAR struct rpmsg_endpoint *ept) FAR struct sensor_rpmsg_ept_s *sre; FAR struct sensor_rpmsg_dev_s *dev; FAR struct sensor_rpmsg_stub_s *stub; - FAR struct sensor_rpmsg_stub_s *stmp; FAR struct sensor_rpmsg_proxy_s *proxy; - FAR struct sensor_rpmsg_proxy_s *ptmp; sre = container_of(ept, struct sensor_rpmsg_ept_s, ept); @@ -1133,37 +1138,43 @@ static void sensor_rpmsg_ns_unbind_cb(FAR struct rpmsg_endpoint *ept) * destoryed. */ - nxmutex_lock(&g_lock); + nxmutex_lock(&g_dev_lock); list_for_every_entry(&g_devlist, dev, struct sensor_rpmsg_dev_s, node) { nxrmutex_lock(&dev->lock); - list_for_every_entry_safe(&dev->proxylist, proxy, ptmp, - struct sensor_rpmsg_proxy_s, node) + list_for_every_entry(&dev->proxylist, proxy, + struct sensor_rpmsg_proxy_s, node) { if (proxy->ept == ept) { sensor_rpmsg_free_proxy(proxy); + break; } } - list_for_every_entry_safe(&dev->stublist, stub, stmp, - struct sensor_rpmsg_stub_s, node) + list_for_every_entry(&dev->stublist, stub, + struct sensor_rpmsg_stub_s, node) { if (stub->ept == ept) { sensor_rpmsg_free_stub(stub); + break; } } nxrmutex_unlock(&dev->lock); } + nxmutex_unlock(&g_dev_lock); + + nxmutex_lock(&g_ept_lock); list_delete(&sre->node); - nxmutex_unlock(&g_lock); - rpmsg_destroy_ept(ept); + nxmutex_unlock(&g_ept_lock); + nxmutex_destroy(&sre->lock); kmm_free(sre); + rpmsg_destroy_ept(ept); } static void sensor_rpmsg_device_ns_bound(FAR struct rpmsg_endpoint *ept) @@ -1173,11 +1184,13 @@ static void sensor_rpmsg_device_ns_bound(FAR struct rpmsg_endpoint *ept) sre = container_of(ept, struct sensor_rpmsg_ept_s, ept); - nxmutex_lock(&g_lock); + nxmutex_lock(&g_ept_lock); list_add_tail(&g_eptlist, &sre->node); + nxmutex_unlock(&g_ept_lock); /* Broadcast all device to ready ept */ + nxmutex_lock(&g_dev_lock); list_for_every_entry(&g_devlist, dev, struct sensor_rpmsg_dev_s, node) { @@ -1195,7 +1208,7 @@ static void sensor_rpmsg_device_ns_bound(FAR struct rpmsg_endpoint *ept) nxrmutex_unlock(&dev->lock); } - nxmutex_unlock(&g_lock); + nxmutex_unlock(&g_dev_lock); } static void sensor_rpmsg_device_created(FAR struct rpmsg_device *rdev, @@ -1267,6 +1280,7 @@ sensor_rpmsg_register(FAR struct sensor_lowerhalf_s *lower, nxrmutex_init(&dev->lock); strcpy(dev->path, path); + dev->nadvertisers = !!lower->ops->activate; dev->push_event = lower->push_event; dev->upper = lower->priv; lower->push_event = sensor_rpmsg_push_event; @@ -1277,19 +1291,20 @@ sensor_rpmsg_register(FAR struct sensor_lowerhalf_s *lower, /* If openamp is ready, send advertisement to remote proc */ - nxmutex_lock(&g_lock); + nxmutex_lock(&g_dev_lock); list_add_tail(&g_devlist, &dev->node); + nxmutex_unlock(&g_dev_lock); if (lower->ops->activate) { - dev->nadvertisers = 1; + nxmutex_lock(&g_ept_lock); list_for_every_entry(&g_eptlist, sre, struct sensor_rpmsg_ept_s, node) { sensor_rpmsg_advsub_one(dev, &sre->ept, SENSOR_RPMSG_ADVERTISE); } - } - nxmutex_unlock(&g_lock); + nxmutex_unlock(&g_ept_lock); + } return &dev->lower; } @@ -1314,9 +1329,9 @@ void sensor_rpmsg_unregister(FAR struct sensor_lowerhalf_s *lower) return; } - nxmutex_lock(&g_lock); + nxmutex_lock(&g_dev_lock); list_delete(&dev->node); - nxmutex_unlock(&g_lock); + nxmutex_unlock(&g_dev_lock); nxrmutex_destroy(&dev->lock); kmm_free(dev);