All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: dgilbert@interlog.com
Cc: linux-scsi@vger.kernel.org
Subject: [bug report] scsi: scsi_debug: Add per_host_store option
Date: Tue, 12 May 2020 13:31:23 +0300	[thread overview]
Message-ID: <20200512103123.GA261906@mwanda> (raw)

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

             reply	other threads:[~2020-05-12 10:31 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-12 10:31 Dan Carpenter [this message]
2020-05-12 18:50 ` [bug report] scsi: scsi_debug: Add per_host_store option Douglas Gilbert

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=20200512103123.GA261906@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=dgilbert@interlog.com \
    --cc=linux-scsi@vger.kernel.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.