From: kbuild test robot <lkp@intel.com>
To: Hannes Reinecke <hare@suse.de>,
"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: kbuild-all@lists.01.org, Christoph Hellwig <hch@lst.de>,
Doug Gilbert <dgilbert@interlog.com>,
Daniel Wagner <daniel.wagner@suse.com>,
James Bottomley <james.bottomley@hansenpartnership.com>,
linux-scsi@vger.kernel.org, Hannes Reinecke <hare@suse.de>
Subject: Re: [PATCH 1/4] scsi: convert target lookup to xarray
Date: Fri, 29 May 2020 13:01:21 +0800 [thread overview]
Message-ID: <20200529050121.GA9122@xsang-OptiPlex-9020> (raw)
In-Reply-To: <20200527141400.58087-2-hare@suse.de>
[-- Attachment #1: Type: text/plain, Size: 11065 bytes --]
Hi Hannes,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.7-rc7 next-20200526]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Hannes-Reinecke/scsi-use-xarray-for-devices-and-targets/20200527-231824
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
:::::: branch date: 12 hours ago
:::::: commit date: 12 hours ago
config: i386-randconfig-s001-20200528 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-240-gf0fe1cd9-dirty
# save the attached .config to linux build tree
make W=1 C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/scsi/scsi_scan.c:392:27: sparse: sparse: context imbalance in 'scsi_alloc_target' - different lock contexts for basic block
# https://github.com/0day-ci/linux/commit/45b149b239ea9a86968ddbd8ecda1e6c44937b68
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 45b149b239ea9a86968ddbd8ecda1e6c44937b68
vim +/scsi_alloc_target +392 drivers/scsi/scsi_scan.c
e63ed0d7a98014 James Bottomley 2014-01-21 379
884d25cc4fda20 James Bottomley 2006-09-05 380 /**
884d25cc4fda20 James Bottomley 2006-09-05 381 * scsi_alloc_target - allocate a new or find an existing target
884d25cc4fda20 James Bottomley 2006-09-05 382 * @parent: parent of the target (need not be a scsi host)
884d25cc4fda20 James Bottomley 2006-09-05 383 * @channel: target channel number (zero if no channels)
884d25cc4fda20 James Bottomley 2006-09-05 384 * @id: target id number
884d25cc4fda20 James Bottomley 2006-09-05 385 *
884d25cc4fda20 James Bottomley 2006-09-05 386 * Return an existing target if one exists, provided it hasn't already
884d25cc4fda20 James Bottomley 2006-09-05 387 * gone into STARGET_DEL state, otherwise allocate a new target.
884d25cc4fda20 James Bottomley 2006-09-05 388 *
884d25cc4fda20 James Bottomley 2006-09-05 389 * The target is returned with an incremented reference, so the caller
884d25cc4fda20 James Bottomley 2006-09-05 390 * is responsible for both reaping and doing a last put
884d25cc4fda20 James Bottomley 2006-09-05 391 */
^1da177e4c3f41 Linus Torvalds 2005-04-16 @392 static struct scsi_target *scsi_alloc_target(struct device *parent,
^1da177e4c3f41 Linus Torvalds 2005-04-16 393 int channel, uint id)
^1da177e4c3f41 Linus Torvalds 2005-04-16 394 {
^1da177e4c3f41 Linus Torvalds 2005-04-16 395 struct Scsi_Host *shost = dev_to_shost(parent);
^1da177e4c3f41 Linus Torvalds 2005-04-16 396 struct device *dev = NULL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 397 unsigned long flags;
^1da177e4c3f41 Linus Torvalds 2005-04-16 398 const int size = sizeof(struct scsi_target)
^1da177e4c3f41 Linus Torvalds 2005-04-16 399 + shost->transportt->target_size;
5c44cd2afad3f7 James.Smart@Emulex.Com 2005-06-10 400 struct scsi_target *starget;
^1da177e4c3f41 Linus Torvalds 2005-04-16 401 struct scsi_target *found_target;
e63ed0d7a98014 James Bottomley 2014-01-21 402 int error, ref_got;
45b149b239ea9a Hannes Reinecke 2020-05-27 403 unsigned long tid;
^1da177e4c3f41 Linus Torvalds 2005-04-16 404
24669f75a3231f Jes Sorensen 2006-01-16 405 starget = kzalloc(size, GFP_KERNEL);
^1da177e4c3f41 Linus Torvalds 2005-04-16 406 if (!starget) {
cadbd4a5e36dde Harvey Harrison 2008-07-03 407 printk(KERN_ERR "%s: allocation failure\n", __func__);
^1da177e4c3f41 Linus Torvalds 2005-04-16 408 return NULL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 409 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 410 dev = &starget->dev;
^1da177e4c3f41 Linus Torvalds 2005-04-16 411 device_initialize(dev);
e63ed0d7a98014 James Bottomley 2014-01-21 412 kref_init(&starget->reap_ref);
^1da177e4c3f41 Linus Torvalds 2005-04-16 413 dev->parent = get_device(parent);
71610f55fa4db6 Kay Sievers 2008-12-03 414 dev_set_name(dev, "target%d:%d:%d", shost->host_no, channel, id);
b0ed43360fdca2 Hannes Reinecke 2008-03-18 415 dev->bus = &scsi_bus_type;
b0ed43360fdca2 Hannes Reinecke 2008-03-18 416 dev->type = &scsi_target_type;
^1da177e4c3f41 Linus Torvalds 2005-04-16 417 starget->id = id;
^1da177e4c3f41 Linus Torvalds 2005-04-16 418 starget->channel = channel;
f0c0a376d0fcd4 Mike Christie 2008-08-17 419 starget->can_queue = 0;
^1da177e4c3f41 Linus Torvalds 2005-04-16 420 INIT_LIST_HEAD(&starget->devices);
643eb2d932c97a James Bottomley 2008-03-22 421 starget->state = STARGET_CREATED;
7c9d6f16f50d3a Alan Stern 2007-01-08 422 starget->scsi_level = SCSI_2;
c53a284f8be237 Edward Goggin 2009-04-09 423 starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED;
45b149b239ea9a Hannes Reinecke 2020-05-27 424 tid = scsi_target_index(starget);
ffedb4522571ac James Bottomley 2006-02-23 425 retry:
^1da177e4c3f41 Linus Torvalds 2005-04-16 426 spin_lock_irqsave(shost->host_lock, flags);
^1da177e4c3f41 Linus Torvalds 2005-04-16 427
45b149b239ea9a Hannes Reinecke 2020-05-27 428 found_target = xa_load(&shost->__targets, tid);
^1da177e4c3f41 Linus Torvalds 2005-04-16 429 if (found_target)
^1da177e4c3f41 Linus Torvalds 2005-04-16 430 goto found;
45b149b239ea9a Hannes Reinecke 2020-05-27 431 if (xa_insert(&shost->__targets, tid, starget, GFP_KERNEL)) {
45b149b239ea9a Hannes Reinecke 2020-05-27 432 dev_printk(KERN_ERR, dev, "target index busy\n");
45b149b239ea9a Hannes Reinecke 2020-05-27 433 kfree(starget);
45b149b239ea9a Hannes Reinecke 2020-05-27 434 return NULL;
45b149b239ea9a Hannes Reinecke 2020-05-27 435 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 436 spin_unlock_irqrestore(shost->host_lock, flags);
^1da177e4c3f41 Linus Torvalds 2005-04-16 437 /* allocate and add */
a283bd37d00e92 James Bottomley 2005-05-24 438 transport_setup_device(dev);
a283bd37d00e92 James Bottomley 2005-05-24 439 if (shost->hostt->target_alloc) {
32f95792500794 Brian King 2006-02-22 440 error = shost->hostt->target_alloc(starget);
a283bd37d00e92 James Bottomley 2005-05-24 441
a283bd37d00e92 James Bottomley 2005-05-24 442 if(error) {
a283bd37d00e92 James Bottomley 2005-05-24 443 dev_printk(KERN_ERR, dev, "target allocation failed, error %d\n", error);
a283bd37d00e92 James Bottomley 2005-05-24 444 /* don't want scsi_target_reap to do the final
a283bd37d00e92 James Bottomley 2005-05-24 445 * put because it will be under the host lock */
643eb2d932c97a James Bottomley 2008-03-22 446 scsi_target_destroy(starget);
a283bd37d00e92 James Bottomley 2005-05-24 447 return NULL;
a283bd37d00e92 James Bottomley 2005-05-24 448 }
a283bd37d00e92 James Bottomley 2005-05-24 449 }
884d25cc4fda20 James Bottomley 2006-09-05 450 get_device(dev);
a283bd37d00e92 James Bottomley 2005-05-24 451
^1da177e4c3f41 Linus Torvalds 2005-04-16 452 return starget;
^1da177e4c3f41 Linus Torvalds 2005-04-16 453
^1da177e4c3f41 Linus Torvalds 2005-04-16 454 found:
e63ed0d7a98014 James Bottomley 2014-01-21 455 /*
e63ed0d7a98014 James Bottomley 2014-01-21 456 * release routine already fired if kref is zero, so if we can still
e63ed0d7a98014 James Bottomley 2014-01-21 457 * take the reference, the target must be alive. If we can't, it must
e63ed0d7a98014 James Bottomley 2014-01-21 458 * be dying and we need to wait for a new target
e63ed0d7a98014 James Bottomley 2014-01-21 459 */
e63ed0d7a98014 James Bottomley 2014-01-21 460 ref_got = kref_get_unless_zero(&found_target->reap_ref);
e63ed0d7a98014 James Bottomley 2014-01-21 461
^1da177e4c3f41 Linus Torvalds 2005-04-16 462 spin_unlock_irqrestore(shost->host_lock, flags);
e63ed0d7a98014 James Bottomley 2014-01-21 463 if (ref_got) {
12fb8c1574d7d0 Alan Stern 2010-03-18 464 put_device(dev);
^1da177e4c3f41 Linus Torvalds 2005-04-16 465 return found_target;
^1da177e4c3f41 Linus Torvalds 2005-04-16 466 }
e63ed0d7a98014 James Bottomley 2014-01-21 467 /*
e63ed0d7a98014 James Bottomley 2014-01-21 468 * Unfortunately, we found a dying target; need to wait until it's
e63ed0d7a98014 James Bottomley 2014-01-21 469 * dead before we can get a new one. There is an anomaly here. We
e63ed0d7a98014 James Bottomley 2014-01-21 470 * *should* call scsi_target_reap() to balance the kref_get() of the
e63ed0d7a98014 James Bottomley 2014-01-21 471 * reap_ref above. However, since the target being released, it's
e63ed0d7a98014 James Bottomley 2014-01-21 472 * already invisible and the reap_ref is irrelevant. If we call
e63ed0d7a98014 James Bottomley 2014-01-21 473 * scsi_target_reap() we might spuriously do another device_del() on
e63ed0d7a98014 James Bottomley 2014-01-21 474 * an already invisible target.
e63ed0d7a98014 James Bottomley 2014-01-21 475 */
ffedb4522571ac James Bottomley 2006-02-23 476 put_device(&found_target->dev);
e63ed0d7a98014 James Bottomley 2014-01-21 477 /*
e63ed0d7a98014 James Bottomley 2014-01-21 478 * length of time is irrelevant here, we just want to yield the CPU
e63ed0d7a98014 James Bottomley 2014-01-21 479 * for a tick to avoid busy waiting for the target to die.
e63ed0d7a98014 James Bottomley 2014-01-21 480 */
e63ed0d7a98014 James Bottomley 2014-01-21 481 msleep(1);
ffedb4522571ac James Bottomley 2006-02-23 482 goto retry;
ffedb4522571ac James Bottomley 2006-02-23 483 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 484
:::::: The code at line 392 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35320 bytes --]
next prev parent reply other threads:[~2020-05-29 4:49 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-27 14:13 [RFC PATCH 0/4] scsi: use xarray for devices and targets Hannes Reinecke
2020-05-27 14:13 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
2020-05-27 14:57 ` Johannes Thumshirn
2020-05-27 15:06 ` Hannes Reinecke
2020-05-28 7:24 ` Daniel Wagner
2020-05-28 7:52 ` Hannes Reinecke
2020-05-28 16:28 ` Bart Van Assche
2020-05-29 5:01 ` kbuild test robot [this message]
2020-05-31 9:10 ` Dan Carpenter
2020-05-27 14:13 ` [PATCH 2/4] target_core_pscsi: use __scsi_device_lookup() Hannes Reinecke
2020-05-27 15:05 ` Johannes Thumshirn
2020-05-27 14:13 ` [PATCH 3/4] scsi: move target device list to xarray Hannes Reinecke
2020-05-27 15:34 ` Johannes Thumshirn
2020-05-27 20:13 ` kbuild test robot
2020-05-30 2:47 ` kbuild test robot
2020-05-27 14:14 ` [PATCH 4/4] scsi: remove direct device lookup per host Hannes Reinecke
2020-05-28 8:00 ` kbuild test robot
2020-05-27 16:36 ` [RFC PATCH 0/4] scsi: use xarray for devices and targets Bart Van Assche
2020-05-27 16:59 ` Hannes Reinecke
2020-05-28 3:59 ` Douglas Gilbert
-- strict thread matches above, loose matches on Subject: below --
2020-05-28 8:42 [PATCHv2 0/4] Hannes Reinecke
2020-05-28 8:42 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
2020-05-28 8:57 ` Johannes Thumshirn
2020-05-28 16:36 [PATCHv3 0/4] scsi: use xarray for devices and targets Hannes Reinecke
2020-05-28 16:36 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
2020-05-28 17:18 ` Douglas Gilbert
2020-05-28 19:08 ` Douglas Gilbert
2020-05-28 17:48 ` Bart Van Assche
2020-05-29 6:24 ` Hannes Reinecke
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=20200529050121.GA9122@xsang-OptiPlex-9020 \
--to=lkp@intel.com \
--cc=daniel.wagner@suse.com \
--cc=dgilbert@interlog.com \
--cc=hare@suse.de \
--cc=hch@lst.de \
--cc=james.bottomley@hansenpartnership.com \
--cc=kbuild-all@lists.01.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox