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
next 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.