Re: [Xen-devel] [bug report] xen/pvcalls: implement release command

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel