On Sat, 4 Nov 2017, Dan Carpenter wrote:
> Hello Stefano Stabellini,
>
> The patch 235a71c53903: "xen/pvcalls: implement release command" from
> Oct 30, 2017, leads to the following static checker warning:
>
> drivers/xen/pvcalls-front.c:1051 pvcalls_front_release()
> error: double lock 'mutex:&map->active.in_mutex'
>
> drivers/xen/pvcalls-front.c
> 1037 if (map->active_socket) {
> 1038 /*
> 1039 * Set in_error and wake up inflight_conn_req to force
> 1040 * recvmsg waiters to exit.
> 1041 */
> 1042 map->active.ring->in_error = -EBADF;
> 1043 wake_up_interruptible(&map->active.inflight_conn_req);
> 1044
> 1045 /*
> 1046 * Wait until there are no more waiters on the
> mutexes.
> 1047 * We know that no new waiters can be added because
> sk_send_head
> 1048 * is set to NULL -- we only need to wait for the
> existing
> 1049 * waiters to return.
> 1050 */
> 1051 while (!mutex_trylock(&map->active.in_mutex) ||
> 1052 !mutex_trylock(&map->active.out_mutex))
> 1053 cpu_relax();
>
> mutex_trylock() returns 1 if you take the lock and 0 if not. So I think
> the static checker is right that this code has an issue.
>
> Assume you take in_mutex on the first try, but you can't take out_mutex.
> That means you the next times you call mutex_trylock() in_mutex is going
> to fail. So it's an endless loop.
>
> But it could also be that I haven't slept well recently and my eyes are
> cross eyed.
Yes, you are right. Thanks for your help. I think the code should be:
while (!mutex_trylock(&map->active.in_mutex))
cpu_relax();
while (!mutex_trylock(&map->active.out_mutex))
cpu_relax();
I'll send a patch.
> 1054
> 1055 pvcalls_front_free_map(bedata, map);
> 1056 } else {
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
