[bug report] scsi: scsi_debug: Add per_host_store option

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

 



Hello Douglas Gilbert,

The patch 87c715dcde63: "scsi: scsi_debug: Add per_host_store option"
from Apr 21, 2020, leads to the following static checker warning:

drivers/scsi/scsi_debug.c:3748 resp_write_same() warn: inconsistent returns '*macc_lckp'.
  Locked on  : 3728
  Unlocked on: 3708,3748
drivers/scsi/scsi_debug.c:3712 resp_write_same() error: we previously assumed 'sip' could be null (see line 3699)
drivers/scsi/scsi_debug.c:3902 resp_comp_write() error: we previously assumed 'sip' could be null (see line 3859)
drivers/scsi/scsi_debug.c:3965 resp_unmap() error: we previously assumed 'sip' could be null (see line 3926)
drivers/scsi/scsi_debug.c:4261 resp_verify() error: we previously assumed 'sip' could be null (see line 4208)


drivers/scsi/scsi_debug.c
  3688  static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
  3689                             u32 ei_lba, bool unmap, bool ndob)
  3690  {
  3691          struct scsi_device *sdp = scp->device;
  3692          struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
  3693          unsigned long long i;
  3694          u64 block, lbaa;
  3695          u32 lb_size = sdebug_sector_size;
  3696          int ret;
  3697          struct sdeb_store_info *sip = devip2sip((struct sdebug_dev_info *)
  3698                                                  scp->device->hostdata);
  3699          rwlock_t *macc_lckp = sip ? &sip->macc_lck : &sdeb_fake_rw_lck;
                          ^^^^^^^^^
If "sip" is non-NULL we use that lock.

  3700          u8 *fs1p;
  3701          u8 *fsp;
  3702  
  3703          write_lock(macc_lckp);
  3704  
  3705          ret = check_device_access_params(scp, lba, num, true);
  3706          if (ret) {
  3707                  write_unlock(macc_lckp);
  3708                  return ret;
  3709          }
  3710  
  3711          if (unmap && scsi_debug_lbp()) {
  3712                  unmap_region(sip, lba, num);

How do we know "sip" is non-NULL?

  3713                  goto out;
  3714          }
  3715          lbaa = lba;
  3716          block = do_div(lbaa, sdebug_store_sectors);
  3717          /* if ndob then zero 1 logical block, else fetch 1 logical block */
  3718          fsp = sip->storep;
                      ^^^^^^^^^^^
Same.

  3719          fs1p = fsp + (block * lb_size);
  3720          if (ndob) {
  3721                  memset(fs1p, 0, lb_size);
  3722                  ret = 0;
  3723          } else
  3724                  ret = fetch_to_dev_buffer(scp, fs1p, lb_size);
  3725  
  3726          if (-1 == ret) {
  3727                  write_unlock(&sip->macc_lck);

If we know that "sip" is non-NULL then this is fine, but it is probably
less confusing to use write_unlock(macc_lckp); consistently everywhere.

  3728                  return DID_ERROR << 16;
  3729          } else if (sdebug_verbose && !ndob && (ret < lb_size))
  3730                  sdev_printk(KERN_INFO, scp->device,
  3731                              "%s: %s: lb size=%u, IO sent=%d bytes\n",
  3732                              my_name, "write same", lb_size, ret);

regards,
dan carpenter



[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