From: Mike Snitzer <snitzer@redhat.com>
To: kbuild-all@lists.01.org
Subject: Re: drivers/md/dm-mpath.c:524 multipath_clone_and_map() error: double unlocked 'm->lock' (orig line 516)
Date: Sat, 08 Aug 2020 10:35:26 -0400 [thread overview]
Message-ID: <20200808143526.GA6950@redhat.com> (raw)
In-Reply-To: <202008082059.puvi6HNx%lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 5163 bytes --]
On Sat, Aug 08 2020 at 8:10am -0400,
kernel test robot <lkp@intel.com> wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 449dc8c97089a6e09fb2dac4d92b1b7ac0eb7c1e
> commit: 374117ad4736c5a4f8012cfe59fc07d9d58191d5 dm mpath: use double checked locking in fast path
> date: 4 weeks ago
> config: arm-randconfig-m031-20200808 (attached as .config)
> compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
All 3 recent reports about smatch showing "double unlocked 'm->lock'"
appear to be bogus. Think smatch is generating false positives (and
given "Old smatch warnings" it seems it has been doing so for some
time).
In addition, does lkp(a)intel.com no longer test linux-next? The dm-mpath
locking changes that were just merged into mainline have been in
linux-next since July 13. Why were these tests only done against
mainline?
Mike
> New smatch warnings:
> drivers/md/dm-mpath.c:524 multipath_clone_and_map() error: double unlocked 'm->lock' (orig line 516)
>
> Old smatch warnings:
> drivers/md/dm-mpath.c:446 choose_pgpath() error: double unlocked 'm->lock' (orig line 416)
> drivers/md/dm-mpath.c:457 choose_pgpath() error: double unlocked 'm->lock' (orig line 403)
> drivers/md/dm-mpath.c:525 multipath_clone_and_map() error: double unlocked 'm->lock' (orig line 516)
> drivers/md/dm-mpath.c:526 multipath_clone_and_map() error: double unlocked 'm->lock' (orig line 524)
> drivers/md/dm-mpath.c:626 __map_bio() error: double unlocked 'm->lock' (orig line 615)
> drivers/md/dm-mpath.c:627 __map_bio() error: double unlocked 'm->lock' (orig line 615)
> drivers/md/dm-mpath.c:628 __map_bio() error: double unlocked 'm->lock' (orig line 626)
> drivers/md/dm-mpath.c:629 __map_bio() error: double unlocked 'm->lock' (orig line 628)
> drivers/md/dm-mpath.c:1607 pg_init_done() error: double unlocked 'm->lock' (orig line 1560)
> drivers/md/dm-mpath.c:1707 multipath_end_io_bio() error: double unlocked 'm->lock' (orig line 1704)
> drivers/md/dm-mpath.c:1988 multipath_prepare_ioctl() error: double unlocked 'm->lock' (orig line 1984)
> drivers/md/dm-mpath.c:2012 multipath_prepare_ioctl() error: double unlocked 'm->lock' (orig line 2001)
>
> vim +524 drivers/md/dm-mpath.c
>
> 498
> 499 /*
> 500 * Map cloned requests (request-based multipath)
> 501 */
> 502 static int multipath_clone_and_map(struct dm_target *ti, struct request *rq,
> 503 union map_info *map_context,
> 504 struct request **__clone)
> 505 {
> 506 struct multipath *m = ti->private;
> 507 size_t nr_bytes = blk_rq_bytes(rq);
> 508 struct pgpath *pgpath;
> 509 struct block_device *bdev;
> 510 struct dm_mpath_io *mpio = get_mpio(map_context);
> 511 struct request_queue *q;
> 512 struct request *clone;
> 513
> 514 /* Do we need to select a new pgpath? */
> 515 pgpath = READ_ONCE(m->current_pgpath);
> > 516 if (!pgpath || !mpath_double_check_test_bit(MPATHF_QUEUE_IO, m))
> 517 pgpath = choose_pgpath(m, nr_bytes);
> 518
> 519 if (!pgpath) {
> 520 if (must_push_back_rq(m))
> 521 return DM_MAPIO_DELAY_REQUEUE;
> 522 dm_report_EIO(m); /* Failed */
> 523 return DM_MAPIO_KILL;
> > 524 } else if (mpath_double_check_test_bit(MPATHF_QUEUE_IO, m) ||
> 525 mpath_double_check_test_bit(MPATHF_PG_INIT_REQUIRED, m)) {
> 526 pg_init_all_paths(m);
> 527 return DM_MAPIO_DELAY_REQUEUE;
> 528 }
> 529
> 530 mpio->pgpath = pgpath;
> 531 mpio->nr_bytes = nr_bytes;
> 532
> 533 bdev = pgpath->path.dev->bdev;
> 534 q = bdev_get_queue(bdev);
> 535 clone = blk_get_request(q, rq->cmd_flags | REQ_NOMERGE,
> 536 BLK_MQ_REQ_NOWAIT);
> 537 if (IS_ERR(clone)) {
> 538 /* EBUSY, ENODEV or EWOULDBLOCK: requeue */
> 539 if (blk_queue_dying(q)) {
> 540 atomic_inc(&m->pg_init_in_progress);
> 541 activate_or_offline_path(pgpath);
> 542 return DM_MAPIO_DELAY_REQUEUE;
> 543 }
> 544
> 545 /*
> 546 * blk-mq's SCHED_RESTART can cover this requeue, so we
> 547 * needn't deal with it by DELAY_REQUEUE. More importantly,
> 548 * we have to return DM_MAPIO_REQUEUE so that blk-mq can
> 549 * get the queue busy feedback (via BLK_STS_RESOURCE),
> 550 * otherwise I/O merging can suffer.
> 551 */
> 552 return DM_MAPIO_REQUEUE;
> 553 }
> 554 clone->bio = clone->biotail = NULL;
> 555 clone->rq_disk = bdev->bd_disk;
> 556 clone->cmd_flags |= REQ_FAILFAST_TRANSPORT;
> 557 *__clone = clone;
> 558
> 559 if (pgpath->pg->ps.type->start_io)
> 560 pgpath->pg->ps.type->start_io(&pgpath->pg->ps,
> 561 &pgpath->path,
> 562 nr_bytes);
> 563 return DM_MAPIO_REMAPPED;
> 564 }
> 565
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: kernel test robot <lkp@intel.com>
Cc: kbuild-all@lists.01.org, linux-kernel@vger.kernel.org
Subject: Re: drivers/md/dm-mpath.c:524 multipath_clone_and_map() error: double unlocked 'm->lock' (orig line 516)
Date: Sat, 8 Aug 2020 10:35:26 -0400 [thread overview]
Message-ID: <20200808143526.GA6950@redhat.com> (raw)
In-Reply-To: <202008082059.puvi6HNx%lkp@intel.com>
On Sat, Aug 08 2020 at 8:10am -0400,
kernel test robot <lkp@intel.com> wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 449dc8c97089a6e09fb2dac4d92b1b7ac0eb7c1e
> commit: 374117ad4736c5a4f8012cfe59fc07d9d58191d5 dm mpath: use double checked locking in fast path
> date: 4 weeks ago
> config: arm-randconfig-m031-20200808 (attached as .config)
> compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
All 3 recent reports about smatch showing "double unlocked 'm->lock'"
appear to be bogus. Think smatch is generating false positives (and
given "Old smatch warnings" it seems it has been doing so for some
time).
In addition, does lkp@intel.com no longer test linux-next? The dm-mpath
locking changes that were just merged into mainline have been in
linux-next since July 13. Why were these tests only done against
mainline?
Mike
> New smatch warnings:
> drivers/md/dm-mpath.c:524 multipath_clone_and_map() error: double unlocked 'm->lock' (orig line 516)
>
> Old smatch warnings:
> drivers/md/dm-mpath.c:446 choose_pgpath() error: double unlocked 'm->lock' (orig line 416)
> drivers/md/dm-mpath.c:457 choose_pgpath() error: double unlocked 'm->lock' (orig line 403)
> drivers/md/dm-mpath.c:525 multipath_clone_and_map() error: double unlocked 'm->lock' (orig line 516)
> drivers/md/dm-mpath.c:526 multipath_clone_and_map() error: double unlocked 'm->lock' (orig line 524)
> drivers/md/dm-mpath.c:626 __map_bio() error: double unlocked 'm->lock' (orig line 615)
> drivers/md/dm-mpath.c:627 __map_bio() error: double unlocked 'm->lock' (orig line 615)
> drivers/md/dm-mpath.c:628 __map_bio() error: double unlocked 'm->lock' (orig line 626)
> drivers/md/dm-mpath.c:629 __map_bio() error: double unlocked 'm->lock' (orig line 628)
> drivers/md/dm-mpath.c:1607 pg_init_done() error: double unlocked 'm->lock' (orig line 1560)
> drivers/md/dm-mpath.c:1707 multipath_end_io_bio() error: double unlocked 'm->lock' (orig line 1704)
> drivers/md/dm-mpath.c:1988 multipath_prepare_ioctl() error: double unlocked 'm->lock' (orig line 1984)
> drivers/md/dm-mpath.c:2012 multipath_prepare_ioctl() error: double unlocked 'm->lock' (orig line 2001)
>
> vim +524 drivers/md/dm-mpath.c
>
> 498
> 499 /*
> 500 * Map cloned requests (request-based multipath)
> 501 */
> 502 static int multipath_clone_and_map(struct dm_target *ti, struct request *rq,
> 503 union map_info *map_context,
> 504 struct request **__clone)
> 505 {
> 506 struct multipath *m = ti->private;
> 507 size_t nr_bytes = blk_rq_bytes(rq);
> 508 struct pgpath *pgpath;
> 509 struct block_device *bdev;
> 510 struct dm_mpath_io *mpio = get_mpio(map_context);
> 511 struct request_queue *q;
> 512 struct request *clone;
> 513
> 514 /* Do we need to select a new pgpath? */
> 515 pgpath = READ_ONCE(m->current_pgpath);
> > 516 if (!pgpath || !mpath_double_check_test_bit(MPATHF_QUEUE_IO, m))
> 517 pgpath = choose_pgpath(m, nr_bytes);
> 518
> 519 if (!pgpath) {
> 520 if (must_push_back_rq(m))
> 521 return DM_MAPIO_DELAY_REQUEUE;
> 522 dm_report_EIO(m); /* Failed */
> 523 return DM_MAPIO_KILL;
> > 524 } else if (mpath_double_check_test_bit(MPATHF_QUEUE_IO, m) ||
> 525 mpath_double_check_test_bit(MPATHF_PG_INIT_REQUIRED, m)) {
> 526 pg_init_all_paths(m);
> 527 return DM_MAPIO_DELAY_REQUEUE;
> 528 }
> 529
> 530 mpio->pgpath = pgpath;
> 531 mpio->nr_bytes = nr_bytes;
> 532
> 533 bdev = pgpath->path.dev->bdev;
> 534 q = bdev_get_queue(bdev);
> 535 clone = blk_get_request(q, rq->cmd_flags | REQ_NOMERGE,
> 536 BLK_MQ_REQ_NOWAIT);
> 537 if (IS_ERR(clone)) {
> 538 /* EBUSY, ENODEV or EWOULDBLOCK: requeue */
> 539 if (blk_queue_dying(q)) {
> 540 atomic_inc(&m->pg_init_in_progress);
> 541 activate_or_offline_path(pgpath);
> 542 return DM_MAPIO_DELAY_REQUEUE;
> 543 }
> 544
> 545 /*
> 546 * blk-mq's SCHED_RESTART can cover this requeue, so we
> 547 * needn't deal with it by DELAY_REQUEUE. More importantly,
> 548 * we have to return DM_MAPIO_REQUEUE so that blk-mq can
> 549 * get the queue busy feedback (via BLK_STS_RESOURCE),
> 550 * otherwise I/O merging can suffer.
> 551 */
> 552 return DM_MAPIO_REQUEUE;
> 553 }
> 554 clone->bio = clone->biotail = NULL;
> 555 clone->rq_disk = bdev->bd_disk;
> 556 clone->cmd_flags |= REQ_FAILFAST_TRANSPORT;
> 557 *__clone = clone;
> 558
> 559 if (pgpath->pg->ps.type->start_io)
> 560 pgpath->pg->ps.type->start_io(&pgpath->pg->ps,
> 561 &pgpath->path,
> 562 nr_bytes);
> 563 return DM_MAPIO_REMAPPED;
> 564 }
> 565
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
next prev parent reply other threads:[~2020-08-08 14:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-08 12:10 drivers/md/dm-mpath.c:524 multipath_clone_and_map() error: double unlocked 'm->lock' (orig line 516) kernel test robot
2020-08-08 12:10 ` kernel test robot
2020-08-08 14:35 ` Mike Snitzer [this message]
2020-08-08 14:35 ` Mike Snitzer
2020-08-10 5:13 ` Rong Chen
2020-08-10 5:13 ` [kbuild-all] " Rong Chen
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=20200808143526.GA6950@redhat.com \
--to=snitzer@redhat.com \
--cc=kbuild-all@lists.01.org \
/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.