#10177 NORM 1.0-sof: duplicate list_add() from libertas/cmd.c

[Zarro Boogs per Child]

#10177: duplicate list_add() from libertas/cmd.c
----------------------+-----------------------------------------------------
 Reporter:  pgf       |                 Owner:  dsaxena                           
     Type:  defect    |                Status:  new                               
 Priority:  normal    |             Milestone:  1.0-software-later                
Component:  kernel    |               Version:  Development source as of this date
 Keywords:  libertas  |           Next_action:  diagnose                          
 Verified:  0         |   Deployment_affected:                                    
Blockedby:            |              Blocking:                                    
----------------------+-----------------------------------------------------
 {{{
 the scenario is this:  when lbs_thread() times out a command
 ("Excessive timeouts submitting command 0x%04x\n"), it calls
 lbs_complete_command().  lbs_complete_command(), in turn, wakes
 up cmdwait_q, and then, depending on the value of cmd->callback,
 scrubs the command structure and requeues it on the free queue (by
 calling __lbs_cleanup_and_insert_cmd():


     void lbs_complete_command(struct lbs_private *priv, struct
 cmd_ctrl_node *cmd, int result)
     {
             if (cmd == priv->cur_cmd)
                     priv->cur_cmd_retcode = result;

             cmd->result = result;
             cmd->cmdwaitqwoken = 1;
             wake_up_interruptible(&cmd->cmdwait_q);

             if (!cmd->callback || cmd->callback == lbs_cmd_async_callback)
                     __lbs_cleanup_and_insert_cmd(priv, cmd);
             priv->cur_cmd = NULL;
     }


 however, it's possible that the process that's being woken up
 will run first.  in my case, that process is waiting in
 __lbs_cmd().  if __lbs_cmd() runs immediately after its wakeup,
 it will unconditionally call __lbs_cleanup_and_insert_cmd() to
 scrub and free the command buffer.  the scrubbing includes
 nulling the cmd->callback element.  when control eventually comes
 back to lbs_complete_command(), "callback" is null, and the
 command gets scrubbed and freed again.  this corrupts the cmdfreeq.

 i've attached a debug trace that shows this happening:
 duplicate_list_add.log (search for "Excessive timeouts".)

 as i said above, i'm not sure how this should be fixed.  it seems
 like the call to __lbs_cleanup_and_insert_cmd() should be made
 in just one place, but i don't know the driver well enough to
 say which place that should be.

 this may explain other list_add corruption issues we've had
 (like #9962), but that's not known yet.

 p.s. as an aside, note that __lbs_cleanup_and_insert_cmd() is called
 without taking the spinlock on driver_lock.  actually, i think
 lbs_thread() should be taking the lock before calling
 lbs_complete_command().  this is a bug, but it's not today's bug.
 }}}

-- 
Ticket URL: <http://dev.laptop.org/ticket/10177>
One Laptop Per Child <http://laptop.org/>
OLPC bug tracking system