lkml.org 
[lkml]   [2011]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 12/16] drbd: fix race when forcefully disconnecting
    Date
    From: Lars Ellenberg <lars.ellenberg@linbit.com>

    If a forced disconnect hits a restarting receiver right after it passed
    its final "if (C_DISCONNECTING)" test in drbdd_init(), but before it was
    actually restarted by drbd_thread_setup, we could be left with a
    connection stuck in C_DISCONNECTING, never reaching C_STANDALONE,
    which would be necessary to take it down or reconfigure it.

    Move the last cleanup into w_after_conn_state_ch(), and do an additional
    state change request in conn_try_disconnect(), just in case.

    Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
    Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
    ---
    drivers/block/drbd/drbd_nl.c | 85 ++++++++++++++++++++----------------
    drivers/block/drbd/drbd_receiver.c | 14 +------
    drivers/block/drbd/drbd_state.c | 13 ++++++
    3 files changed, 62 insertions(+), 50 deletions(-)

    diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
    index 0bf74b1..455b9a9 100644
    --- a/drivers/block/drbd/drbd_nl.c
    +++ b/drivers/block/drbd/drbd_nl.c
    @@ -2160,37 +2160,54 @@ out:
    static enum drbd_state_rv conn_try_disconnect(struct drbd_tconn *tconn, bool force)
    {
    enum drbd_state_rv rv;
    - if (force) {
    - spin_lock_irq(&tconn->req_lock);
    - rv = _conn_request_state(tconn, NS(conn, C_DISCONNECTING), CS_HARD);
    - spin_unlock_irq(&tconn->req_lock);
    - return rv;
    - }

    - rv = conn_request_state(tconn, NS(conn, C_DISCONNECTING), 0);
    + rv = conn_request_state(tconn, NS(conn, C_DISCONNECTING),
    + force ? CS_HARD : 0);

    switch (rv) {
    case SS_NOTHING_TO_DO:
    + break;
    case SS_ALREADY_STANDALONE:
    return SS_SUCCESS;
    case SS_PRIMARY_NOP:
    /* Our state checking code wants to see the peer outdated. */
    rv = conn_request_state(tconn, NS2(conn, C_DISCONNECTING,
    - pdsk, D_OUTDATED), CS_VERBOSE);
    + pdsk, D_OUTDATED), CS_VERBOSE);
    break;
    case SS_CW_FAILED_BY_PEER:
    /* The peer probably wants to see us outdated. */
    rv = conn_request_state(tconn, NS2(conn, C_DISCONNECTING,
    disk, D_OUTDATED), 0);
    if (rv == SS_IS_DISKLESS || rv == SS_LOWER_THAN_OUTDATED) {
    - conn_request_state(tconn, NS(conn, C_DISCONNECTING), CS_HARD);
    - rv = SS_SUCCESS;
    + rv = conn_request_state(tconn, NS(conn, C_DISCONNECTING),
    + CS_HARD);
    }
    break;
    default:;
    /* no special handling necessary */
    }

    + if (rv >= SS_SUCCESS) {
    + enum drbd_state_rv rv2;
    + /* No one else can reconfigure the network while I am here.
    + * The state handling only uses drbd_thread_stop_nowait(),
    + * we want to really wait here until the receiver is no more.
    + */
    + drbd_thread_stop(&adm_ctx.tconn->receiver);
    +
    + /* Race breaker. This additional state change request may be
    + * necessary, if this was a forced disconnect during a receiver
    + * restart. We may have "killed" the receiver thread just
    + * after drbdd_init() returned. Typically, we should be
    + * C_STANDALONE already, now, and this becomes a no-op.
    + */
    + rv2 = conn_request_state(tconn, NS(conn, C_STANDALONE),
    + CS_VERBOSE | CS_HARD);
    + if (rv2 < SS_SUCCESS)
    + conn_err(tconn,
    + "unexpected rv2=%d in conn_try_disconnect()\n",
    + rv2);
    + }
    return rv;
    }

    @@ -2221,19 +2238,9 @@ int drbd_adm_disconnect(struct sk_buff *skb, struct genl_info *info)

    rv = conn_try_disconnect(tconn, parms.force_disconnect);
    if (rv < SS_SUCCESS)
    - goto fail;
    -
    - /* No one else can reconfigure the network while I am here.
    - * The state handling only uses drbd_thread_stop_nowait(),
    - * we want to really wait here until the receiver is no more. */
    - drbd_thread_stop(&tconn->receiver);
    - if (wait_event_interruptible(tconn->ping_wait,
    - tconn->cstate == C_STANDALONE)) {
    - retcode = ERR_INTR;
    - goto fail;
    - }
    -
    - retcode = NO_ERROR;
    + retcode = rv; /* FIXME: Type mismatch. */
    + else
    + retcode = NO_ERROR;
    fail:
    drbd_adm_finish(info, retcode);
    return 0;
    @@ -3095,8 +3102,7 @@ out:

    int drbd_adm_down(struct sk_buff *skb, struct genl_info *info)
    {
    - enum drbd_ret_code retcode;
    - enum drbd_state_rv rv;
    + int retcode; /* enum drbd_ret_code rsp. enum drbd_state_rv */
    struct drbd_conf *mdev;
    unsigned i;

    @@ -3120,30 +3126,35 @@ int drbd_adm_down(struct sk_buff *skb, struct genl_info *info)
    goto out_unlock;
    }
    }
    + up_read(&drbd_cfg_rwsem);

    - /* disconnect */
    - rv = conn_try_disconnect(adm_ctx.tconn, 0);
    - if (rv < SS_SUCCESS) {
    - retcode = rv; /* enum type mismatch! */
    + /* disconnect; may stop the receiver;
    + * must not hold the drbd_cfg_rwsem */
    + retcode = conn_try_disconnect(adm_ctx.tconn, 0);
    + if (retcode < SS_SUCCESS) {
    drbd_msg_put_info("failed to disconnect");
    - goto out_unlock;
    + goto out;
    }

    - /* Make sure the network threads have actually stopped,
    - * state handling only does drbd_thread_stop_nowait(). */
    - drbd_thread_stop(&adm_ctx.tconn->receiver);
    -
    + down_read(&drbd_cfg_rwsem);
    /* detach */
    idr_for_each_entry(&adm_ctx.tconn->volumes, mdev, i) {
    - rv = adm_detach(mdev);
    - if (rv < SS_SUCCESS) {
    - retcode = rv; /* enum type mismatch! */
    + retcode = adm_detach(mdev);
    + if (retcode < SS_SUCCESS) {
    drbd_msg_put_info("failed to detach");
    goto out_unlock;
    }
    }
    up_read(&drbd_cfg_rwsem);

    + /* If we reach this, all volumes (of this tconn) are Secondary,
    + * Disconnected, Diskless, aka Unconfigured. Make sure all threads have
    + * actually stopped, state handling only does drbd_thread_stop_nowait().
    + * This needs to be done without holding drbd_cfg_rwsem. */
    + drbd_thread_stop(&adm_ctx.tconn->worker);
    +
    + /* Now, nothing can fail anymore */
    +
    /* delete volumes */
    down_write(&drbd_cfg_rwsem);
    idr_for_each_entry(&adm_ctx.tconn->volumes, mdev, i) {
    diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
    index 9c8bcce..956cdda 100644
    --- a/drivers/block/drbd/drbd_receiver.c
    +++ b/drivers/block/drbd/drbd_receiver.c
    @@ -4213,20 +4213,8 @@ static void drbd_disconnect(struct drbd_tconn *tconn)

    spin_unlock_irq(&tconn->req_lock);

    - if (oc == C_DISCONNECTING) {
    - struct net_conf *old_conf;
    -
    - mutex_lock(&tconn->net_conf_update);
    - old_conf = tconn->net_conf;
    - rcu_assign_pointer(tconn->net_conf, NULL);
    - conn_free_crypto(tconn);
    - mutex_unlock(&tconn->net_conf_update);
    -
    - synchronize_rcu();
    - kfree(old_conf);
    -
    + if (oc == C_DISCONNECTING)
    conn_request_state(tconn, NS(conn, C_STANDALONE), CS_VERBOSE | CS_HARD);
    - }
    }

    static int drbd_disconnected(int vnr, void *p, void *data)
    diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
    index 978dcef..f2e1535 100644
    --- a/drivers/block/drbd/drbd_state.c
    +++ b/drivers/block/drbd/drbd_state.c
    @@ -1398,6 +1398,19 @@ static int w_after_conn_state_ch(struct drbd_work *w, int unused)
    if (oc == C_STANDALONE && ns_max.conn == C_UNCONNECTED)
    drbd_thread_start(&tconn->receiver);

    + if (oc == C_DISCONNECTING && ns_max.conn == C_STANDALONE) {
    + struct net_conf *old_conf;
    +
    + mutex_lock(&tconn->net_conf_update);
    + old_conf = tconn->net_conf;
    + rcu_assign_pointer(tconn->net_conf, NULL);
    + conn_free_crypto(tconn);
    + mutex_unlock(&tconn->net_conf_update);
    +
    + synchronize_rcu();
    + kfree(old_conf);
    + }
    +
    if (ns_max.susp_fen) {
    /* case1: The outdate peer handler is successful: */
    if (ns_max.pdsk <= D_OUTDATED) {
    --
    1.7.4.1


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