Re: [PATCH 1/4] scsi: convert target lookup to xarray

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>


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@xxxxxxxxxx 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@xxxxxxxxxxxxxxx>
:::::: CC: Linus Torvalds <torvalds@xxxxxxxxxxxxxxx>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux