completion issues in ks7010

Nicholas Mc Guire der.herr at hofr.at
Wed Jul 27 12:58:13 UTC 2016


Hi !

 staging ks7010 has a a few completion calls, some of which are using
 wait_for_completion_interruptible_timeout(). The key difference 
 betweeen wait_for_completion_interruptible_timeout() and
 wait_for_completion_timeout() is, if I got it right, only that
 the former will return on any pending fatal signals while the
 later will ither timeout (return 0) or complete (retrun >= 1)
 so as most of the timeouts are relatively long lived (500ms to
 5s) the interruptible variant seems resonable and the interupted
 case probably should be treated (atleast with a DPRINTK()) as
 failure case. The short lived case (20ms timeout) might be converted 
 to wait_for_completion_timeout().

 The case in ks7010_card_init() is not clear to me, the specific 
 sequence is:
    init_completion(&priv->confirm_wait);
    hostif_sme_enqueue(priv, SME_START);
       enqueue SME_START 
       call tasklet_schedule(&priv->sme_task);
          sme_task is hostif_sme_task()
              call hostif_sme_execute()
                   switch (event) {
                   case SME_START:
                                  if (priv->dev_state == DEVICE_STATE_BOOT) {
                                      hostif_mib_get_request(priv, DOT11_MAC_ADDRESS);
                                  }
                                  break;
    wait_for_completion_interruptible_timeout(...)

 but it seems that completion is never called in this path - so effectively 
 this is an interruptible wait only -> should the SME_START case be calling
 complete(&priv->confirm_wait); here ?


 The second general issue with completion is the use of complete()
 it is called with:

    /* wake_up_all(&priv->confirm_wait); */
    complete(&priv->confirm_wait);

 or

    /* wake_up_interruptible_all(&priv->confirm_wait); */
    complete(&priv->confirm_wait);

 the comment suggest that it would be expecting to wake all
 waiters but compete() is actually passing 1 in __wake_up_common 
 for nr_exclusive so only the first waiter would be woken 
 should this be a comlplete_all() - or the comment fixed ?

thx!
hofrat


More information about the devel mailing list