lkml.org 
[lkml]   [2011]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[038/264] target: Prevent cmd->se_queue_node double add
    3.1-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Roland Dreier <roland@purestorage.com>

    commit 79a7fef26431830e22e282053d050af790117db8 upstream.

    This patch addresses a bug with the lio-core-2.6.git conversion of
    transport_add_cmd_to_queue() to use a single embedded list_head, instead
    of individual struct se_queue_req allocations allowing a single se_cmd to
    be added to the queue mulitple times. This was changed in the following:

    commit 2a9e4d5ca5d99f4c600578d6285d45142e7e5208
    Author: Andy Grover <agrover@redhat.com>
    Date: Tue Apr 26 17:45:51 2011 -0700

    target: Embed qr in struct se_cmd

    The problem is that some target code still assumes performing multiple
    adds is allowed via transport_add_cmd_to_queue(), which ends up causing
    list corruption in qobj->qobj_list code. This patch addresses this
    by removing an existing struct se_cmd from the list before the add, and
    removes an unnecessary list walk in transport_remove_cmd_from_queue()

    It also changes cmd->t_transport_queue_active to use explict sets intead
    of increment/decrement to prevent confusion during exception path handling.

    Signed-off-by: Roland Dreier <roland@purestorage.com>
    Cc: Andy Grover <agrover@redhat.com>
    Signed-off-by: Nicholas Bellinger <nab@risingtidesystems.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

    ---
    drivers/target/target_core_tmr.c | 2 +-
    drivers/target/target_core_transport.c | 31 +++++++++++++++----------------
    2 files changed, 16 insertions(+), 17 deletions(-)

    --- a/drivers/target/target_core_tmr.c
    +++ b/drivers/target/target_core_tmr.c
    @@ -340,7 +340,7 @@ int core_tmr_lun_reset(

    atomic_dec(&cmd->t_transport_queue_active);
    atomic_dec(&qobj->queue_cnt);
    - list_del(&cmd->se_queue_node);
    + list_del_init(&cmd->se_queue_node);
    spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);

    pr_debug("LUN_RESET: %s from Device Queue: cmd: %p t_state:"
    --- a/drivers/target/target_core_transport.c
    +++ b/drivers/target/target_core_transport.c
    @@ -621,8 +621,6 @@ static void transport_add_cmd_to_queue(
    struct se_queue_obj *qobj = &dev->dev_queue_obj;
    unsigned long flags;

    - INIT_LIST_HEAD(&cmd->se_queue_node);
    -
    if (t_state) {
    spin_lock_irqsave(&cmd->t_state_lock, flags);
    cmd->t_state = t_state;
    @@ -631,15 +629,21 @@ static void transport_add_cmd_to_queue(
    }

    spin_lock_irqsave(&qobj->cmd_queue_lock, flags);
    +
    + /* If the cmd is already on the list, remove it before we add it */
    + if (!list_empty(&cmd->se_queue_node))
    + list_del(&cmd->se_queue_node);
    + else
    + atomic_inc(&qobj->queue_cnt);
    +
    if (cmd->se_cmd_flags & SCF_EMULATE_QUEUE_FULL) {
    cmd->se_cmd_flags &= ~SCF_EMULATE_QUEUE_FULL;
    list_add(&cmd->se_queue_node, &qobj->qobj_list);
    } else
    list_add_tail(&cmd->se_queue_node, &qobj->qobj_list);
    - atomic_inc(&cmd->t_transport_queue_active);
    + atomic_set(&cmd->t_transport_queue_active, 1);
    spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);

    - atomic_inc(&qobj->queue_cnt);
    wake_up_interruptible(&qobj->thread_wq);
    }

    @@ -656,9 +660,9 @@ transport_get_cmd_from_queue(struct se_q
    }
    cmd = list_first_entry(&qobj->qobj_list, struct se_cmd, se_queue_node);

    - atomic_dec(&cmd->t_transport_queue_active);
    + atomic_set(&cmd->t_transport_queue_active, 0);

    - list_del(&cmd->se_queue_node);
    + list_del_init(&cmd->se_queue_node);
    atomic_dec(&qobj->queue_cnt);
    spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);

    @@ -668,7 +672,6 @@ transport_get_cmd_from_queue(struct se_q
    static void transport_remove_cmd_from_queue(struct se_cmd *cmd,
    struct se_queue_obj *qobj)
    {
    - struct se_cmd *t;
    unsigned long flags;

    spin_lock_irqsave(&qobj->cmd_queue_lock, flags);
    @@ -676,14 +679,9 @@ static void transport_remove_cmd_from_qu
    spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);
    return;
    }
    -
    - list_for_each_entry(t, &qobj->qobj_list, se_queue_node)
    - if (t == cmd) {
    - atomic_dec(&cmd->t_transport_queue_active);
    - atomic_dec(&qobj->queue_cnt);
    - list_del(&cmd->se_queue_node);
    - break;
    - }
    + atomic_set(&cmd->t_transport_queue_active, 0);
    + atomic_dec(&qobj->queue_cnt);
    + list_del_init(&cmd->se_queue_node);
    spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);

    if (atomic_read(&cmd->t_transport_queue_active)) {
    @@ -1067,7 +1065,7 @@ static void transport_release_all_cmds(s
    list_for_each_entry_safe(cmd, tcmd, &dev->dev_queue_obj.qobj_list,
    se_queue_node) {
    t_state = cmd->t_state;
    - list_del(&cmd->se_queue_node);
    + list_del_init(&cmd->se_queue_node);
    spin_unlock_irqrestore(&dev->dev_queue_obj.cmd_queue_lock,
    flags);

    @@ -1598,6 +1596,7 @@ void transport_init_se_cmd(
    INIT_LIST_HEAD(&cmd->se_delayed_node);
    INIT_LIST_HEAD(&cmd->se_ordered_node);
    INIT_LIST_HEAD(&cmd->se_qf_node);
    + INIT_LIST_HEAD(&cmd->se_queue_node);

    INIT_LIST_HEAD(&cmd->t_task_list);
    init_completion(&cmd->transport_lun_fe_stop_comp);



    \
     
     \ /
      Last update: 2011-11-10 05:07    [W:0.030 / U:151.796 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site