All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: rgoldwyn@suse.com
Cc: linux-raid@vger.kernel.org
Subject: [bug report] md-cluster: Fix adding of new disk with new reload code
Date: Wed, 26 Apr 2017 22:59:40 +0300	[thread overview]
Message-ID: <20170426195940.l64pymopsbv36o7e@mwanda> (raw)

Hello Goldwyn Rodrigues,

The patch dbb64f8635f5: "md-cluster: Fix adding of new disk with new
reload code" from Oct 1, 2015, leads to the following static checker
warning:

	drivers/md/md-cluster.c:1341 add_new_disk()
	warn: inconsistent returns 'cinfo->recv_mutex'.
	  Locked on  : 1315,1341
	  Unlocked on: 1341

drivers/md/md-cluster.c
  1300  static int add_new_disk(struct mddev *mddev, struct md_rdev *rdev)
  1301  {
  1302          struct md_cluster_info *cinfo = mddev->cluster_info;
  1303          struct cluster_msg cmsg;
  1304          int ret = 0;
  1305          struct mdp_superblock_1 *sb = page_address(rdev->sb_page);
  1306          char *uuid = sb->device_uuid;
  1307  
  1308          memset(&cmsg, 0, sizeof(cmsg));
  1309          cmsg.type = cpu_to_le32(NEWDISK);
  1310          memcpy(cmsg.uuid, uuid, 16);
  1311          cmsg.raid_slot = cpu_to_le32(rdev->desc_nr);
  1312          lock_comm(cinfo, 1);
                ^^^^^^^^^^^^^^^^^^^
We take the lock here.

  1313          ret = __sendmsg(cinfo, &cmsg);
  1314          if (ret)
  1315                  return ret;

Should we unlock on failure here?

  1316          cinfo->no_new_dev_lockres->flags |= DLM_LKF_NOQUEUE;
  1317          ret = dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_EX);
  1318          cinfo->no_new_dev_lockres->flags &= ~DLM_LKF_NOQUEUE;
  1319          /* Some node does not "see" the device */
  1320          if (ret == -EAGAIN)
  1321                  ret = -ENOENT;
  1322          if (ret)
  1323                  unlock_comm(cinfo);

Because we do here.  I think we're only supposed to hold the lock on
success but how this all works with cancel etc looked slightly
complicated so I decided to ask instead of sending a patch.

  1324          else {
  1325                  dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
  1326                  /* Since MD_CHANGE_DEVS will be set in add_bound_rdev which
  1327                   * will run soon after add_new_disk, the below path will be
  1328                   * invoked:
  1329                   *   md_wakeup_thread(mddev->thread)
  1330                   *      -> conf->thread (raid1d)
  1331                   *      -> md_check_recovery -> md_update_sb
  1332                   *      -> metadata_update_start/finish
  1333                   * MD_CLUSTER_SEND_LOCKED_ALREADY will be cleared eventually.
  1334                   *
  1335                   * For other failure cases, metadata_update_cancel and
  1336                   * add_new_disk_cancel also clear below bit as well.
  1337                   * */
  1338                  set_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
  1339                  wake_up(&cinfo->wait);
  1340          }
  1341          return ret;
  1342  }

regards,
dan carpenter

             reply	other threads:[~2017-04-26 19:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-26 19:59 Dan Carpenter [this message]
2017-04-27  2:52 ` [bug report] md-cluster: Fix adding of new disk with new reload code Guoqing Jiang
2017-05-05 22:30   ` Goldwyn Rodrigues

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170426195940.l64pymopsbv36o7e@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=linux-raid@vger.kernel.org \
    --cc=rgoldwyn@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.