CC: [email protected] TO: "Hannes, Reinecke," <[email protected]>
tree: https://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel.git xarray.v4 head: 4196b2277d94baee7703987a4a6b9c30214e0da7 commit: 4196b2277d94baee7703987a4a6b9c30214e0da7 [6/6] scsi: avoid pointless memory allocation in scsi_alloc_target() :::::: branch date: 2 days ago :::::: commit date: 2 days ago config: i386-randconfig-m021-20200603 (attached as .config) compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <[email protected]> Reported-by: Dan Carpenter <[email protected]> New smatch warnings: drivers/scsi/scsi_scan.c:445 scsi_alloc_target() error: double unlocked '*shost->host_lock' (orig line 416) Old smatch warnings: drivers/scsi/scsi_scan.c:481 scsi_alloc_target() error: double unlocked '*shost->host_lock' (orig line 416) drivers/scsi/scsi_scan.c:1893 scsi_forget_host() error: double unlocked '*shost->host_lock' (orig line 1885) # https://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel.git/commit/?id=4196b2277d94baee7703987a4a6b9c30214e0da7 git remote add hare-scsi-devel https://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel.git git remote update hare-scsi-devel git checkout 4196b2277d94baee7703987a4a6b9c30214e0da7 vim +445 drivers/scsi/scsi_scan.c e63ed0d7a98014 James Bottomley 2014-01-21 376 884d25cc4fda20 James Bottomley 2006-09-05 377 /** 884d25cc4fda20 James Bottomley 2006-09-05 378 * scsi_alloc_target - allocate a new or find an existing target 884d25cc4fda20 James Bottomley 2006-09-05 379 * @parent: parent of the target (need not be a scsi host) 884d25cc4fda20 James Bottomley 2006-09-05 380 * @channel: target channel number (zero if no channels) 884d25cc4fda20 James Bottomley 2006-09-05 381 * @id: target id number 884d25cc4fda20 James Bottomley 2006-09-05 382 * 884d25cc4fda20 James Bottomley 2006-09-05 383 * Return an existing target if one exists, provided it hasn't already 884d25cc4fda20 James Bottomley 2006-09-05 384 * gone into STARGET_DEL state, otherwise allocate a new target. 884d25cc4fda20 James Bottomley 2006-09-05 385 * 884d25cc4fda20 James Bottomley 2006-09-05 386 * The target is returned with an incremented reference, so the caller 884d25cc4fda20 James Bottomley 2006-09-05 387 * is responsible for both reaping and doing a last put 884d25cc4fda20 James Bottomley 2006-09-05 388 */ ^1da177e4c3f41 Linus Torvalds 2005-04-16 389 static struct scsi_target *scsi_alloc_target(struct device *parent, 4196b2277d94ba Hannes Reinecke 2020-06-02 390 u16 channel, u16 id) ^1da177e4c3f41 Linus Torvalds 2005-04-16 391 { ^1da177e4c3f41 Linus Torvalds 2005-04-16 392 struct Scsi_Host *shost = dev_to_shost(parent); ^1da177e4c3f41 Linus Torvalds 2005-04-16 393 struct device *dev = NULL; ^1da177e4c3f41 Linus Torvalds 2005-04-16 394 unsigned long flags; ^1da177e4c3f41 Linus Torvalds 2005-04-16 395 const int size = sizeof(struct scsi_target) ^1da177e4c3f41 Linus Torvalds 2005-04-16 396 + shost->transportt->target_size; 5c44cd2afad3f7 [email protected] 2005-06-10 397 struct scsi_target *starget; ^1da177e4c3f41 Linus Torvalds 2005-04-16 398 struct scsi_target *found_target; e63ed0d7a98014 James Bottomley 2014-01-21 399 int error, ref_got; 4196b2277d94ba Hannes Reinecke 2020-06-02 400 unsigned long tid = (channel << 16) | id; 4196b2277d94ba Hannes Reinecke 2020-06-02 401 4196b2277d94ba Hannes Reinecke 2020-06-02 402 /* 4196b2277d94ba Hannes Reinecke 2020-06-02 403 * Try if the target is already present, and save us 4196b2277d94ba Hannes Reinecke 2020-06-02 404 * a pointless memory allocation if so. 4196b2277d94ba Hannes Reinecke 2020-06-02 405 */ 4196b2277d94ba Hannes Reinecke 2020-06-02 406 spin_lock_irqsave(shost->host_lock, flags); 4196b2277d94ba Hannes Reinecke 2020-06-02 407 found_target = xa_load(&shost->__targets, tid); 4196b2277d94ba Hannes Reinecke 2020-06-02 408 if (found_target) { 4196b2277d94ba Hannes Reinecke 2020-06-02 409 get_device(&found_target->dev); 4196b2277d94ba Hannes Reinecke 2020-06-02 410 if (kref_get_unless_zero(&found_target->reap_ref)) { 4196b2277d94ba Hannes Reinecke 2020-06-02 411 spin_unlock_irqrestore(shost->host_lock, flags); 4196b2277d94ba Hannes Reinecke 2020-06-02 412 return found_target; 4196b2277d94ba Hannes Reinecke 2020-06-02 413 } 4196b2277d94ba Hannes Reinecke 2020-06-02 414 put_device(&found_target->dev); 4196b2277d94ba Hannes Reinecke 2020-06-02 415 } 4196b2277d94ba Hannes Reinecke 2020-06-02 @416 spin_unlock_irqrestore(shost->host_lock, flags); ^1da177e4c3f41 Linus Torvalds 2005-04-16 417 24669f75a3231f Jes Sorensen 2006-01-16 418 starget = kzalloc(size, GFP_KERNEL); ^1da177e4c3f41 Linus Torvalds 2005-04-16 419 if (!starget) { cadbd4a5e36dde Harvey Harrison 2008-07-03 420 printk(KERN_ERR "%s: allocation failure\n", __func__); ^1da177e4c3f41 Linus Torvalds 2005-04-16 421 return NULL; ^1da177e4c3f41 Linus Torvalds 2005-04-16 422 } ^1da177e4c3f41 Linus Torvalds 2005-04-16 423 dev = &starget->dev; ^1da177e4c3f41 Linus Torvalds 2005-04-16 424 device_initialize(dev); e63ed0d7a98014 James Bottomley 2014-01-21 425 kref_init(&starget->reap_ref); ^1da177e4c3f41 Linus Torvalds 2005-04-16 426 dev->parent = get_device(parent); 71610f55fa4db6 Kay Sievers 2008-12-03 427 dev_set_name(dev, "target%d:%d:%d", shost->host_no, channel, id); b0ed43360fdca2 Hannes Reinecke 2008-03-18 428 dev->bus = &scsi_bus_type; b0ed43360fdca2 Hannes Reinecke 2008-03-18 429 dev->type = &scsi_target_type; ^1da177e4c3f41 Linus Torvalds 2005-04-16 430 starget->id = id; ^1da177e4c3f41 Linus Torvalds 2005-04-16 431 starget->channel = channel; f0c0a376d0fcd4 Mike Christie 2008-08-17 432 starget->can_queue = 0; a3747dc5606bc6 Hannes Reinecke 2020-05-27 433 xa_init_flags(&starget->__devices, XA_FLAGS_ALLOC); 643eb2d932c97a James Bottomley 2008-03-22 434 starget->state = STARGET_CREATED; 7c9d6f16f50d3a Alan Stern 2007-01-08 435 starget->scsi_level = SCSI_2; c53a284f8be237 Edward Goggin 2009-04-09 436 starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED; ffedb4522571ac James Bottomley 2006-02-23 437 retry: ^1da177e4c3f41 Linus Torvalds 2005-04-16 438 spin_lock_irqsave(shost->host_lock, flags); 89cdad03f4ab20 Hannes Reinecke 2020-05-27 439 found_target = xa_load(&shost->__targets, tid); 89cdad03f4ab20 Hannes Reinecke 2020-05-27 440 if (found_target) { 89cdad03f4ab20 Hannes Reinecke 2020-05-27 441 get_device(&found_target->dev); ^1da177e4c3f41 Linus Torvalds 2005-04-16 442 goto found; 89cdad03f4ab20 Hannes Reinecke 2020-05-27 443 } 89cdad03f4ab20 Hannes Reinecke 2020-05-27 444 error = xa_insert(&shost->__targets, tid, starget, GFP_ATOMIC); ^1da177e4c3f41 Linus Torvalds 2005-04-16 @445 spin_unlock_irqrestore(shost->host_lock, flags); 89cdad03f4ab20 Hannes Reinecke 2020-05-27 446 if (error) { 89cdad03f4ab20 Hannes Reinecke 2020-05-27 447 dev_printk(KERN_ERR, dev, 89cdad03f4ab20 Hannes Reinecke 2020-05-27 448 "target %u:%u index allocation failed, error %d\n", 89cdad03f4ab20 Hannes Reinecke 2020-05-27 449 channel, id, error); 89cdad03f4ab20 Hannes Reinecke 2020-05-27 450 put_device(dev); 89cdad03f4ab20 Hannes Reinecke 2020-05-27 451 kfree(starget); 89cdad03f4ab20 Hannes Reinecke 2020-05-27 452 return NULL; 89cdad03f4ab20 Hannes Reinecke 2020-05-27 453 } ^1da177e4c3f41 Linus Torvalds 2005-04-16 454 /* allocate and add */ a283bd37d00e92 James Bottomley 2005-05-24 455 transport_setup_device(dev); a283bd37d00e92 James Bottomley 2005-05-24 456 if (shost->hostt->target_alloc) { 32f95792500794 Brian King 2006-02-22 457 error = shost->hostt->target_alloc(starget); a283bd37d00e92 James Bottomley 2005-05-24 458 a283bd37d00e92 James Bottomley 2005-05-24 459 if(error) { 89cdad03f4ab20 Hannes Reinecke 2020-05-27 460 dev_printk(KERN_ERR, dev, 89cdad03f4ab20 Hannes Reinecke 2020-05-27 461 "target %u:%u allocation failed, error %d\n", 89cdad03f4ab20 Hannes Reinecke 2020-05-27 462 channel, id, error); a283bd37d00e92 James Bottomley 2005-05-24 463 /* don't want scsi_target_reap to do the final a283bd37d00e92 James Bottomley 2005-05-24 464 * put because it will be under the host lock */ 643eb2d932c97a James Bottomley 2008-03-22 465 scsi_target_destroy(starget); a283bd37d00e92 James Bottomley 2005-05-24 466 return NULL; a283bd37d00e92 James Bottomley 2005-05-24 467 } a283bd37d00e92 James Bottomley 2005-05-24 468 } 884d25cc4fda20 James Bottomley 2006-09-05 469 get_device(dev); a283bd37d00e92 James Bottomley 2005-05-24 470 ^1da177e4c3f41 Linus Torvalds 2005-04-16 471 return starget; ^1da177e4c3f41 Linus Torvalds 2005-04-16 472 ^1da177e4c3f41 Linus Torvalds 2005-04-16 473 found: e63ed0d7a98014 James Bottomley 2014-01-21 474 /* e63ed0d7a98014 James Bottomley 2014-01-21 475 * release routine already fired if kref is zero, so if we can still e63ed0d7a98014 James Bottomley 2014-01-21 476 * take the reference, the target must be alive. If we can't, it must e63ed0d7a98014 James Bottomley 2014-01-21 477 * be dying and we need to wait for a new target e63ed0d7a98014 James Bottomley 2014-01-21 478 */ e63ed0d7a98014 James Bottomley 2014-01-21 479 ref_got = kref_get_unless_zero(&found_target->reap_ref); e63ed0d7a98014 James Bottomley 2014-01-21 480 ^1da177e4c3f41 Linus Torvalds 2005-04-16 481 spin_unlock_irqrestore(shost->host_lock, flags); e63ed0d7a98014 James Bottomley 2014-01-21 482 if (ref_got) { 12fb8c1574d7d0 Alan Stern 2010-03-18 483 put_device(dev); ^1da177e4c3f41 Linus Torvalds 2005-04-16 484 return found_target; ^1da177e4c3f41 Linus Torvalds 2005-04-16 485 } e63ed0d7a98014 James Bottomley 2014-01-21 486 /* e63ed0d7a98014 James Bottomley 2014-01-21 487 * Unfortunately, we found a dying target; need to wait until it's e63ed0d7a98014 James Bottomley 2014-01-21 488 * dead before we can get a new one. There is an anomaly here. We e63ed0d7a98014 James Bottomley 2014-01-21 489 * *should* call scsi_target_reap() to balance the kref_get() of the e63ed0d7a98014 James Bottomley 2014-01-21 490 * reap_ref above. However, since the target being released, it's e63ed0d7a98014 James Bottomley 2014-01-21 491 * already invisible and the reap_ref is irrelevant. If we call e63ed0d7a98014 James Bottomley 2014-01-21 492 * scsi_target_reap() we might spuriously do another device_del() on e63ed0d7a98014 James Bottomley 2014-01-21 493 * an already invisible target. e63ed0d7a98014 James Bottomley 2014-01-21 494 */ ffedb4522571ac James Bottomley 2006-02-23 495 put_device(&found_target->dev); e63ed0d7a98014 James Bottomley 2014-01-21 496 /* e63ed0d7a98014 James Bottomley 2014-01-21 497 * length of time is irrelevant here, we just want to yield the CPU e63ed0d7a98014 James Bottomley 2014-01-21 498 * for a tick to avoid busy waiting for the target to die. e63ed0d7a98014 James Bottomley 2014-01-21 499 */ e63ed0d7a98014 James Bottomley 2014-01-21 500 msleep(1); ffedb4522571ac James Bottomley 2006-02-23 501 goto retry; ffedb4522571ac James Bottomley 2006-02-23 502 } ^1da177e4c3f41 Linus Torvalds 2005-04-16 503 :::::: The code at line 445 was first introduced by commit :::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2 :::::: TO: Linus Torvalds <[email protected]> :::::: CC: Linus Torvalds <[email protected]> --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/[email protected]
.config.gz
Description: application/gzip
_______________________________________________ kbuild mailing list -- [email protected] To unsubscribe send an email to [email protected]
