[PATCH v2] virtio_scsi: Reject commands when virtqueue is broken

[Eric Farman]

In the case of a graceful set of detaches, where the virtio-scsi-ccw
disk is removed from the guest prior to the controller, the guest
behaves quite normally.  Specifically, the detach gets us into
sd_sync_cache to issue a Synchronize Cache(10) command, which
immediately fails (and is retried a couple of times) because the
device has been removed.  Later, the removal of the controller
sees two CRWs presented, but there's no further indication of the
removal from the guest viewpoint.

 [   17.217458] sd 0:0:0:0: [sda] Synchronizing SCSI cache
 [   17.219257] sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result: 
hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK
 [   21.449400] crw_info : CRW reports slct=0, oflw=0, chn=1, rsc=3, anc=0, 
erc=4, rsid=2
 [   21.449406] crw_info : CRW reports slct=0, oflw=0, chn=0, rsc=3, anc=0, 
erc=4, rsid=0

However, on s390, the SCSI disks can be removed "by surprise" when
an entire controller (host) is removed and all associated disks
are removed via the loop in scsi_forget_host.  The same call to
sd_sync_cache is made, but because the controller has already
been removed, the Synchronize Cache(10) command is neither issued
(and then failed) nor rejected.

That the I/O isn't returned means the guest cannot have other devices
added nor removed, and other tasks (such as shutdown or reboot) issued
by the guest will not complete either.  The virtio ring has already
been marked as broken (via virtio_break_device in virtio_ccw_remove),
but we still attempt to queue the command only to have it remain there.
The calling sequence provides a bit of distinction for us:

  virtscsi_queuecommand()
   -> virtscsi_kick_cmd()
-> virtscsi_add_cmd()
 -> virtqueue_add_sgs()
  -> virtqueue_add()
 if success
   return 0
 elseif vq->broken or vring_mapping_error()
   return -EIO
 else
   return -ENOSPC

A return of ENOSPC is generally a temporary condition, so returning
"host busy" from virtscsi_queuecommand makes sense here, to have it
redriven in a moment or two.  But the EIO return code is more of a
permanent error and so it would be wise to return the I/O itself and
allow the calling thread to finish gracefully.  The result is these
four kernel messages in the guest (the fourth one does not occur
prior to this patch):

 [   22.921562] crw_info : CRW reports slct=0, oflw=0, chn=1, rsc=3, anc=0, 
erc=4, rsid=2
 [   22.921580] crw_info : CRW reports slct=0, oflw=0, chn=0, rsc=3, anc=0, 
erc=4, rsid=0
 [   22.921978] sd 0:0:0:0: [sda] Synchronizing SCSI cache
 [   22.921993] sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result: 
hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK

I opted to fill in the same response data that is returned from the
more graceful device detach, where the disk device is removed prior
to the controller device.

Signed-off-by: Eric Farman 
---
 drivers/scsi/virtio_scsi.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index ec91bd0..c680d76 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -534,7 +534,9 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi,
 {
struct Scsi_Host *shost = virtio_scsi_host(vscsi->vdev);
struct virtio_scsi_cmd *cmd = scsi_cmd_priv(sc);
+   unsigned long flags;
int req_size;
+   int ret;
 
BUG_ON(scsi_sg_count(sc) > shost->sg_tablesize);
 
@@ -562,8 +564,15 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi,
req_size = sizeof(cmd->req.cmd);
}
 
-   if (virtscsi_kick_cmd(req_vq, cmd, req_size, sizeof(cmd->resp.cmd)) != 
0)
+   ret = virtscsi_kick_cmd(req_vq, cmd, req_size, sizeof(cmd->resp.cmd));
+   if (ret == -EIO) {
+   cmd->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
+   spin_lock_irqsave(_vq->vq_lock, flags);
+   virtscsi_complete_cmd(vscsi, cmd);
+   spin_unlock_irqrestore(_vq->vq_lock, flags);
+   } else if (ret != 0) {
return SCSI_MLQUEUE_HOST_BUSY;
+   }
return 0;
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] virtio_scsi: Reject commands when virtqueue is broken

[Eric Farman]

While doing some disruptive testing with QEMU/KVM, I have encountered some
guest problems during hot unplug of virtio-scsi devices depending on the
order of operations in which they are performed.  The following notes
describe my setup (s390x), and how I'm able to reproduce the error and
test the attached fix.

In both the "working" and "failing" case, the detaches appear to work
just fine.  Any sign of problems only begin to appear later based on
other actions I may perform, such as powering off the guest system.

Host:
 # lsscsi -g | grep sg6
 [6:0:6:1074151456]diskIBM  2107900  .217  /dev/sdg   /dev/sg6 

QEMU:
 - Include the following parameters
-device virtio-scsi-ccw,id=scsi0
-drive file=/dev/sg6,if=none,id=drive0,format=raw
-device 
scsi-generic,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive0,id=hostdev6
 - QMP commands (working)
- device_del hostdev6
- device_del scsi0
 - QMP commands (failing)
- device_del scsi0

Libvirt:
 - Note: A preventative fix went into Libvirt 2.5.0
   (libvirt commit 655429a0d4a5 ("qemu: Prevent detaching SCSI controller used 
by hostdev"))
 - Include the following XML
# cat scsicontroller.xml 

# cat scsihostdev.xml 

  


  

 - virsh commands (working)
- virsh detach-device guest scsihostdev.xml
- virsh detach-device guest scsicontroller.xml
 - virsh commands (failing)
- virsh detach-device guest scsicontroller.xml

v1->v2:
 - Hold vq_lock across virtscsi_complete_cmd call (Fam Zheng)

Eric Farman (1):
  virtio_scsi: Reject commands when virtqueue is broken

 drivers/scsi/virtio_scsi.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iscsi_trx going into D state

[Robert LeBlanc]

Laurance,

I'm really starting to think that the stars aligned with the phase of
the moon or something when I reproduced this in my lab before because
I've been unable to reproduce it on Infiniband the last two days. The
problem with this issue is that it is so hard to trigger, but causes a
lot of problems when it does happen. I really hate wasting people's
time when I can't reproduce it myself reliably. Please don't waste too
much time if you can't get it reproduced on Infiniband, I'll have to
wait until someone with the ConnectX-4-LX cards can replicate it.

Hmmm you do have ConnectX-4 cards which may have the same bug it
Ethernet mode. I don't see the RoCE bug on my ConnectX-3 cards, but
your ConnectX-4 cards may work. Try putting the cards into Ethernet
mode, set the speed and advertised speed to something lower than the
max speed and verify that the link speed is that (ethtool). On the
ConnectX-4-LX cards, I just had to set both interfaces down and then
back up at the same time, on the ConnectX-3 I had to pull the cable
(shutting down the client might have worked). Then set up target and
client with iSER, format and run the test and it should trigger
automatically.

Looking at release notes on the ConnectX-4-LX cards, the latest
firmware may fix the bug that so easily exposes the problem with that
card. My cards are SuperMicro branded cards and don't have the new
firmware available yet.

Good luck.

Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1


On Fri, Jan 13, 2017 at 8:10 AM, Laurence Oberman  wrote:
>
>
> - Original Message -
>> From: "Robert LeBlanc" 
>> To: "Laurence Oberman" 
>> Cc: "Doug Ledford" , "Nicholas A. Bellinger" 
>> , "Zhu Lingshan"
>> , "linux-rdma" , 
>> linux-scsi@vger.kernel.org, "Sagi Grimberg"
>> , "Christoph Hellwig" 
>> Sent: Thursday, January 12, 2017 4:26:05 PM
>> Subject: Re: iscsi_trx going into D state
>>
>> Sorry sent prematurely...
>>
>> On Thu, Jan 12, 2017 at 2:22 PM, Robert LeBlanc  wrote:
>> > I'm having trouble replicating the D state issue on Infiniband (I was
>> > able to trigger it reliably a couple weeks back, I don't know if OFED
>> > to verify the same results happen there as well.
>>
>> I'm having trouble replicating the D state issue on Infiniband (I was
>> able to trigger it reliably a couple weeks back, I don't know if OFED
>> being installed is altering things but it only installed for 3.10. The
>> ConnectX-4-LX exposes the issue easily if you have those cards.) to
>> verify the same results happen there as well.
>>
>> 
>> Robert LeBlanc
>> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> I am only back in the office next Wednesday.
> I have this all setup using ConnectX-4 with IB/ISER but have no way of 
> remotely creating the disconnect as I currently have it back-to-back.
> Have run multiple tests with IB and ISER hard resting the client to break the 
> IB connection but have not been able to reproduce as yet.
> So it will have to wait until I can pull cables next week as that seemed to 
> be the way you have been reproducing this.
>
> This is in a code area I also don't have a lot of knowledge of the flow but 
> have started trying to understand it better.
>
> Thanks
> Laurence
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: fix data transfer size caculation for special payload requests

[Jens Axboe]

On Fri, Jan 13 2017, Christoph Hellwig wrote:
> Hi all,
> 
> the first two fixes fix a regression in 4.10-rc1 where the data transfer
> size for discard commands wasn't set properly, leading to hangs with
> Hyper-V VMs.  The use the new data transfer size helper added in patch 1
> more widely.

Added for this series, thanks.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/11] qla2xxx: Add Dual mode support in the driver

[Madhani, Himanshu]

On 1/11/17, 11:51 AM, "Bart Van Assche"  wrote:

>On 12/23/2016 08:23 PM, Himanshu Madhani wrote:
>> -#define QLA_TGT_MODE_ENABLED() (ql2x_ini_mode != QLA2XXX_INI_MODE_ENABLED)
>> +#define QLA_TGT_MODE_ENABLED() \
>> +((ql2x_ini_mode != QLA2XXX_INI_MODE_ENABLED) || \
>> + (ql2x_ini_mode == QLA2XXX_INI_MODE_DUAL))
>
>Hello Himanshu and Quinn,
>
>Is this change needed? It looks redundant to me. For all possible
>ql2x_ini_mode values both the old and the new expression will yield the
>same value.

Agree. This is redundant. I’ll remove it.

>
>Anyway, dual mode is a welcome improvement!
>
>Bart.

Re: [PATCH 4/4] sd: remove __data_len hack for WRITE SAME

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] nvme: use blk_rq_payload_bytes

[Sagi Grimberg]

This looks good,

Reviewed-by: Sagi Grimberg 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] scsi: use blk_rq_payload_bytes

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] block: add blk_rq_payload_bytes

[Sagi Grimberg]

Add a helper to calculate the actual data transfer size for special
payload requests.

Signed-off-by: Christoph Hellwig 
---
 include/linux/blkdev.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ff3d774..1ca8e8f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1000,6 +1000,19 @@ static inline unsigned int blk_rq_cur_sectors(const 
struct request *rq)
return blk_rq_cur_bytes(rq) >> 9;
 }

+/*
+ * Some commands like WRITE SAME have a payload or data transfer size which


Don't you mean here DISCARD?


+ * is different from the size of the request.  Any driver that supports such
+ * commands using the RQF_SPECIAL_PAYLOAD flag needs to use this helper to
+ * calculate the data transfer size.
+ */
+static inline unsigned int blk_rq_payload_bytes(struct request *rq)
+{
+   if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
+   return rq->special_vec.bv_len;
+   return blk_rq_bytes(rq);
+}
+
 static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
 int op)
 {


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ibmvscsis: fix sleeping in interrupt context

[Bart Van Assche]

On Wed, 2017-01-11 at 13:16 -0600, Bryant G. Ly wrote:
> Currently, dma_alloc_coherent is being called with a GFP_KERNEL
> flag which allows it to sleep in an interrupt context, need to
> change to GFP_ATOMIC.

This patch has been queued for kernel v4.10. Thanks for the patch.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ibmvscsis: Fix max transfer length

[Bart Van Assche]

On Wed, 2017-01-11 at 13:52 -0600, Bryant G. Ly wrote:
> Current code incorrectly calculates the max transfer length, since
> it is assuming a 4k page table, but ppc64 all run on 64k page tables.

This patch has been queued for kernel v4.10. Thanks for the patch.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[LSF/MM ATTEND] storage topics plus container filesystems and IMA

[James Bottomley]

I'd like to attend LSF/MM mostly to participate in the storage topics
discussion, but also because containers is starting to need some
features from the VFS.  Hopefully the container uid shifting is mostly
sorted out by the superblock user namespace, and I should have patches
to demonstrate this in shiftfs this month.  However, trusted containers
are also becoming an issue and, since containers are file based, not
image based, we really need to sort out the IMA interfaces to be
correctly integrated into the VFS, so I'd like to be present in those
discussions as well.

James

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [bug report] scsi: megaraid_sas: Add the Support for SAS3.5 Generic Megaraid Controllers Capabilities

[Sasikumar PC]

Hi Dan,

I will fix the static checker warning

Thanks
sasi

-Original Message-
From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
Sent: Friday, January 13, 2017 7:51 AM
To: sasikumar...@broadcom.com
Cc: megaraidlinux@broadcom.com; linux-scsi@vger.kernel.org
Subject: [bug report] scsi: megaraid_sas: Add the Support for SAS3.5
Generic Megaraid Controllers Capabilities

Hello Sasikumar Chandrasekaran,

This is a semi-automatic email about new static checker warnings.

The patch 9581ebebbe35: "scsi: megaraid_sas: Add the Support for
SAS3.5 Generic Megaraid Controllers Capabilities" from Jan 10, 2017, leads
to the following Smatch complaint:

drivers/scsi/megaraid/megaraid_sas_base.c:5245 megasas_init_fw()
 error: we previously assumed 'fusion' could be null (see line
5069)

drivers/scsi/megaraid/megaraid_sas_base.c
  5068  
  5069  if (fusion)
^^
Patch introduces a new NULL check.

  5070  instance->instancet =
_instance_template_fusion;
  5071  else {
  5072  switch (instance->pdev->device) {
  5073  case PCI_DEVICE_ID_LSI_SAS1078R:
  5074  case PCI_DEVICE_ID_LSI_SAS1078DE:
  5075  instance->instancet =
_instance_template_ppc;
  5076  break;
  5077  case PCI_DEVICE_ID_LSI_SAS1078GEN2:
  5078  case PCI_DEVICE_ID_LSI_SAS0079GEN2:
  5079  instance->instancet =
_instance_template_gen2;
  5080  break;
  5081  case PCI_DEVICE_ID_LSI_SAS0073SKINNY:
  5082  case PCI_DEVICE_ID_LSI_SAS0071SKINNY:
  5083  instance->instancet =
_instance_template_skinny;
  5084  break;
  5085  case PCI_DEVICE_ID_LSI_SAS1064R:
  5086  case PCI_DEVICE_ID_DELL_PERC5:
  5087  default:
  5088  instance->instancet =
_instance_template_xscale;
  5089  instance->pd_list_not_supported = 1;
  5090  break;
  5091  }
  5092  }
  5093  
  5094  if (megasas_transition_to_ready(instance, 0)) {
  5095  atomic_set(>fw_reset_no_pci_access, 1);
  5096  instance->instancet->adp_reset
  5097  (instance, instance->reg_set);
  5098  atomic_set(>fw_reset_no_pci_access, 0);
  5099  dev_info(>pdev->dev,
  5100  "FW restarted successfully from %s!\n",
  5101  __func__);
  5102  
  5103  /*waitting for about 30 second before retry*/
  5104  ssleep(30);
  5105  
  5106  if (megasas_transition_to_ready(instance, 0))
  5107  goto fail_ready_state;
  5108  }
  5109  
  5110  if (instance->is_ventura) {
  5111  scratch_pad_3 =
  5112
readl(>reg_set->outbound_scratch_pad_3);
  5113  #if VD_EXT_DEBUG
  5114  dev_info(>pdev->dev, "scratch_pad3
0x%x\n",
  5115  scratch_pad_3);
  5116  #endif
  5117  instance->max_raid_mapsize = ((scratch_pad_3 >>
  5118  MR_MAX_RAID_MAP_SIZE_OFFSET_SHIFT) &
  5119  MR_MAX_RAID_MAP_SIZE_MASK);
  5120  }
  5121  
  5122  /* Check if MSI-X is supported while in ready state */
  5123  msix_enable =
(instance->instancet->read_fw_status_reg(reg_set) &
  5124 0x400) >> 0x1a;
  5125  if (msix_enable && !msix_disable) {
  5126  int irq_flags = PCI_IRQ_MSIX;
  5127  
  5128  scratch_pad_2 = readl
  5129
(>reg_set->outbound_scratch_pad_2);
  5130  /* Check max MSI-X vectors */
  5131  if (fusion) {
  5132  if (fusion->adapter_type ==
THUNDERBOLT_SERIES) { /* Thunderbolt Series*/
  5133  instance->msix_vectors =
(scratch_pad_2
  5134  &
MR_MAX_REPLY_QUEUES_OFFSET) + 1;
  5135  fw_msix_count =
instance->msix_vectors;
  5136  } else { /* Invader series supports more
than 8 MSI-x vectors*/
  5137  instance->msix_vectors =
((scratch_pad_2
  5138  &
MR_MAX_REPLY_QUEUES_EXT_OFFSET)
  5139  >>
MR_MAX_REPLY_QUEUES_EXT_OFFSET_SHIFT) + 1;
  5140  if (instance->msix_vectors > 16)
  5141  instance->msix_combined =
true;
  5142  
  5143  if (rdpq_enable)
  5144  instance->is_rdpq =
(scratch_pad_2 & MR_RDPQ_MODE_OFFSET) ?
  

RE: [bug report] scsi: megaraid_sas: Dynamic Raid Map Changes for SAS3.5 Generic Megaraid Controllers

[Sasikumar PC]

Hi Dan,

I will fix the static checker warning

Thanks
sasi

-Original Message-
From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
Sent: Thursday, January 12, 2017 2:09 PM
To: sasikumar...@broadcom.com; Tomas Henzl
Cc: megaraidlinux@broadcom.com; linux-scsi@vger.kernel.org
Subject: [bug report] scsi: megaraid_sas: Dynamic Raid Map Changes for
SAS3.5 Generic Megaraid Controllers

Hello Sasikumar Chandrasekaran,

The patch d889344e4e59: "scsi: megaraid_sas: Dynamic Raid Map Changes for
SAS3.5 Generic Megaraid Controllers" from Jan 10, 2017, leads to the
following static checker warning:

drivers/scsi/megaraid/megaraid_sas_fusion.c:2043
megasas_build_ldio_fusion()
warn: curly braces intended?

drivers/scsi/megaraid/megaraid_sas_fusion.c
  2020  if (instance->is_ventura) {
  2021  if (io_info.isRead) {
  2022  if
((raid->cpuAffinity.pdRead.cpu0) &&
  2023
(raid->cpuAffinity.pdRead.cpu1))
  2024
praid_context->raid_context_g35.routing_flags.bits.cpu_sel
  2025  = MR_RAID_CTX_CPUSEL_FCFS;
  2026  else if
(raid->cpuAffinity.pdRead.cpu1)
  2027
praid_context->raid_context_g35.routing_flags.bits.cpu_sel
  2028  = MR_RAID_CTX_CPUSEL_1;
  2029  else
  2030
praid_context->raid_context_g35.routing_flags.bits.cpu_sel
  2031  = MR_RAID_CTX_CPUSEL_0;
  2032  } else {
  2033  if ((raid->cpuAffinity.pdWrite.cpu0)
  2034  && (raid->cpuAffinity.pdWrite.cpu1))
  2035
praid_context->raid_context_g35.routing_flags.bits.cpu_sel
  2036  = MR_RAID_CTX_CPUSEL_FCFS;
  2037  else if
(raid->cpuAffinity.pdWrite.cpu1)
  2038
praid_context->raid_context_g35.routing_flags.bits.cpu_sel
  2039  = MR_RAID_CTX_CPUSEL_1;
  2040  else
  2041
praid_context->raid_context_g35.routing_flags.bits.cpu_sel
  2042  = MR_RAID_CTX_CPUSEL_0;
  2043  if
(praid_context->raid_context_g35.routing_flags.bits.sld) {
  2044
praid_context->raid_context_g35.raid_flags
  2045  =
(MR_RAID_FLAGS_IO_SUB_TYPE_CACHE_BYPASS
  2046  <<
MR_RAID_CTX_RAID_FLAGS_IO_SUB_TYPE_SHIFT);
  2047  }
  2048  }
  2049  }
  2050  } else {

Wow...  You guys are probably already discussed this code, but I'm not on
the linux-scsi list.  Do we have a process issue where we are merging code
that we shouldn't be?  What's going on here?

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [bug report] scsi: megaraid_sas: SAS3.5 Generic Megaraid Controllers Stream Detection and IO Coalescing

[Sasikumar PC]

Hi Dan,

I will fix the static checker warning

Thanks
sasi

-Original Message-
From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
Sent: Thursday, January 12, 2017 1:50 PM
To: sasikumar...@broadcom.com
Cc: megaraidlinux@broadcom.com; linux-scsi@vger.kernel.org
Subject: [bug report] scsi: megaraid_sas: SAS3.5 Generic Megaraid
Controllers Stream Detection and IO Coalescing

Hello Sasikumar Chandrasekaran,

The patch fdd84e2514b0: "scsi: megaraid_sas: SAS3.5 Generic Megaraid
Controllers Stream Detection and IO Coalescing" from Jan 10, 2017, leads
to the following static checker warning:

drivers/scsi/megaraid/megaraid_sas_fusion.c:1771
megasas_stream_detect()
warn: inconsistent indenting

drivers/scsi/megaraid/megaraid_sas_fusion.c
  1747  static void megasas_stream_detect(struct megasas_instance
*instance,
  1748  struct megasas_cmd_fusion *cmd,
  1749  struct IO_REQUEST_INFO *io_info)
  1750  {
  1751  struct fusion_context *fusion = instance->ctrl_context;
  1752  u32 device_id = io_info->ldTgtId;
  1753  struct LD_STREAM_DETECT *current_ld_sd
  1754  = fusion->stream_detect_by_ld[device_id];
  1755  u32 *track_stream = _ld_sd->mru_bit_map,
stream_num;
  1756  u32 shifted_values, unshifted_values;
  1757  u32 index_value_mask, shifted_values_mask;
  1758  int i;
  1759  bool is_read_ahead = false;
  1760  struct STREAM_DETECT *current_sd;
  1761  /* find possible stream */
  1762  for (i = 0; i < MAX_STREAMS_TRACKED; ++i) {
  1763  stream_num =
  1764  (*track_stream >> (i * BITS_PER_INDEX_STREAM)) &
  1765  STREAM_MASK;
  1766  current_sd =
_ld_sd->stream_track[stream_num];
  1767  /* if we found a stream, update the raid
  1768   *  context and also update the mruBitMap
  1769   */
  1770  /*  boundary condition */
  1771  if ((current_sd->next_seq_lba) &&

We're still inside the for loop.  This isn't indented far enough.

  1772  (io_info->ldStartBlock >=
current_sd->next_seq_lba) &&
  1773  (io_info->ldStartBlock <=
(current_sd->next_seq_lba+32)) &&
  1774  (current_sd->is_read == io_info->isRead)) {
  1775
  1776  if ((io_info->ldStartBlock !=
current_sd->next_seq_lba)
  1777  && ((!io_info->isRead) ||
(!is_read_ahead)))
  1778  /*
  1779   * Once the API availible we need to
change this.
  1780   * At this point we are not allowing any
gap
  1781   */
  1782  continue;
  1783
  1784
cmd->io_request->RaidContext.raid_context_g35.stream_detected = true;
  1785  current_sd->next_seq_lba =
  1786  io_info->ldStartBlock + io_info->numBlocks;
  1787  /*
  1788   *  update the mruBitMap LRU
  1789   */


See also:

drivers/scsi/megaraid/megaraid_sas_base.c:5396 megasas_init_fw() warn:
inconsistent indenting
drivers/scsi/megaraid/megaraid_sas_fusion.c:4060 megasas_reset_fusion()
warn: inconsistent indenting

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/11] qla2xxx: Add framework for Async fabric discovery.

[Madhani, Himanshu]

On 1/11/17, 12:08 PM, "Bart Van Assche"  wrote:

>On 12/23/2016 08:23 PM, Himanshu Madhani wrote:
>> +sess->logout_completed = 0;
>> +be_sid[0] = sess->d_id.b.domain;
>> +be_sid[1] = sess->d_id.b.area;
>> +be_sid[2] = sess->d_id.b.al_pa;
>
>Hello Himanshu and Quinn,
>
>When building the qlt_create_sess() code with W=1 the compiler warns
>that values are assigned to the elements of the be_sid[] array but that
>these values are not used. Please review the code.

Thanks for review. will review code and update patch.
 
>
>Thanks,
>
>Bart.
N�r��yb�X��ǧv�^�)޺{.n�+{���"�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [LSF/MM TOPIC][LSF/MM ATTEND] multipath redesign

[Hannes Reinecke]

On 01/13/2017 05:07 PM, Mike Snitzer wrote:
> On Fri, Jan 13 2017 at 10:56am -0500,
> Hannes Reinecke  wrote:
> 
>> On 01/12/2017 06:29 PM, Benjamin Marzinski wrote:
[ .. ]
>>> While I've daydreamed of rewriting the multipath tools multiple times,
>>> and having nothing aginst you doing it in concept, I would be happier
>>> knowing that it won't simply mean that there are two sets of tools, that
>>> both need to be supported to deal with all customer configurations.
>>>
>> Sure. I feel the pain of supporting multipath-tools all too strongly.
>> Having two tools for the same thing is always a pain, and I would like
>> to avoid this if at all possible.
> 
> I welcome your work.  Should help us focus on what fat needs to be
> trimmed from both multipath-tools and kernel.
> 
> Might be a good time to branch multipath-tools and get very aggressive
> with trimming stuff that is outdated.
> 
> Things like the event stuff, using select interface, that Andy Grover is
> working on (and Mikulas is taking a stab at finishing/optimizing) is
> something that might help... but your approach described in this thread
> may prove better.
> 
> Point is, everything should be on the table for revitalizing multipath
> userspace (and kernel) to meet new requirements (e.g. NVMEoF, etc).
> 
> And yes, I'd prefer to ultimately see these advances land in terms of DM
> multipath advances but we'll take it as it comes.

I'm fully on board with that.
And it would be good if Ben Marzinski would be present, too;
he might have some insights which both of us might lack
(like the ominous dm-event interface into multipathd where we both
struggle to figure out what it's for ...)

Looking forward to that discussion.
And promising to have some results by then.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] csiostor: switch to pci_alloc_irq_vectors

[Christoph Hellwig]

And get automatic MSI-X affinity for free.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/csiostor/csio_hw.h   |   4 +-
 drivers/scsi/csiostor/csio_init.c |   9 +--
 drivers/scsi/csiostor/csio_isr.c  | 127 +-
 3 files changed, 51 insertions(+), 89 deletions(-)

diff --git a/drivers/scsi/csiostor/csio_hw.h b/drivers/scsi/csiostor/csio_hw.h
index 029bef8..a084d83 100644
--- a/drivers/scsi/csiostor/csio_hw.h
+++ b/drivers/scsi/csiostor/csio_hw.h
@@ -95,7 +95,6 @@ enum {
 };
 
 struct csio_msix_entries {
-   unsigned short  vector; /* Assigned MSI-X vector */
void*dev_id;/* Priv object associated w/ this msix*/
chardesc[24];   /* Description of this vector */
 };
@@ -591,8 +590,9 @@ int csio_enqueue_evt(struct csio_hw *, enum csio_evt, void 
*, uint16_t);
 void csio_evtq_flush(struct csio_hw *hw);
 
 int csio_request_irqs(struct csio_hw *);
+void csio_free_irqs(struct csio_hw *);
 void csio_intr_enable(struct csio_hw *);
-void csio_intr_disable(struct csio_hw *, bool);
+void csio_intr_disable(struct csio_hw *);
 void csio_hw_fatal_err(struct csio_hw *);
 
 struct csio_lnode *csio_lnode_alloc(struct csio_hw *);
diff --git a/drivers/scsi/csiostor/csio_init.c 
b/drivers/scsi/csiostor/csio_init.c
index dbe416f..7e60699 100644
--- a/drivers/scsi/csiostor/csio_init.c
+++ b/drivers/scsi/csiostor/csio_init.c
@@ -461,8 +461,7 @@ csio_config_queues(struct csio_hw *hw)
return 0;
 
 intr_disable:
-   csio_intr_disable(hw, false);
-
+   csio_intr_disable(hw);
return -EINVAL;
 }
 
@@ -575,7 +574,8 @@ static struct csio_hw *csio_hw_alloc(struct pci_dev *pdev)
 static void
 csio_hw_free(struct csio_hw *hw)
 {
-   csio_intr_disable(hw, true);
+   csio_free_irqs(hw);
+   csio_intr_disable(hw);
csio_hw_exit_workers(hw);
csio_hw_exit(hw);
iounmap(hw->regstart);
@@ -1068,7 +1068,8 @@ csio_pci_error_detected(struct pci_dev *pdev, 
pci_channel_state_t state)
spin_unlock_irq(>lock);
csio_lnodes_unblock_request(hw);
csio_lnodes_exit(hw, 0);
-   csio_intr_disable(hw, true);
+   csio_free_irqs(hw);
+   csio_intr_disable(hw);
pci_disable_device(pdev);
return state == pci_channel_io_perm_failure ?
PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET;
diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
index 2fb71c6..a5bf51b7 100644
--- a/drivers/scsi/csiostor/csio_isr.c
+++ b/drivers/scsi/csiostor/csio_isr.c
@@ -383,17 +383,15 @@ csio_request_irqs(struct csio_hw *hw)
int rv, i, j, k = 0;
struct csio_msix_entries *entryp = >msix_entries[0];
struct csio_scsi_cpu_info *info;
+   struct pci_dev *pdev = hw->pdev;
 
if (hw->intr_mode != CSIO_IM_MSIX) {
-   rv = request_irq(hw->pdev->irq, csio_fcoe_isr,
-   (hw->intr_mode == CSIO_IM_MSI) ?
-   0 : IRQF_SHARED,
-   KBUILD_MODNAME, hw);
+   rv = request_irq(pci_irq_vector(pdev, 0), csio_fcoe_isr,
+   hw->intr_mode == CSIO_IM_MSI ? 0 : IRQF_SHARED,
+   KBUILD_MODNAME, hw);
if (rv) {
-   if (hw->intr_mode == CSIO_IM_MSI)
-   pci_disable_msi(hw->pdev);
csio_err(hw, "Failed to allocate interrupt line.\n");
-   return -EINVAL;
+   goto out_free_irqs;
}
 
goto out;
@@ -402,22 +400,22 @@ csio_request_irqs(struct csio_hw *hw)
/* Add the MSIX vector descriptions */
csio_add_msix_desc(hw);
 
-   rv = request_irq(entryp[k].vector, csio_nondata_isr, 0,
+   rv = request_irq(pci_irq_vector(pdev, k), csio_nondata_isr, 0,
 entryp[k].desc, hw);
if (rv) {
csio_err(hw, "IRQ request failed for vec %d err:%d\n",
-entryp[k].vector, rv);
-   goto err;
+pci_irq_vector(pdev, k), rv);
+   goto out_free_irqs;
}
 
-   entryp[k++].dev_id = (void *)hw;
+   entryp[k++].dev_id = hw;
 
-   rv = request_irq(entryp[k].vector, csio_fwevt_isr, 0,
+   rv = request_irq(pci_irq_vector(pdev, k), csio_fwevt_isr, 0,
 entryp[k].desc, hw);
if (rv) {
csio_err(hw, "IRQ request failed for vec %d err:%d\n",
-entryp[k].vector, rv);
-   goto err;
+pci_irq_vector(pdev, k), rv);
+   goto out_free_irqs;
}
 
entryp[k++].dev_id = (void *)hw;
@@ -429,51 +427,31 @@ csio_request_irqs(struct csio_hw *hw)
struct csio_scsi_qset *sqset = >sqset[i][j];
  

[PATCH] be2iscsi: switch to pci_alloc_irq_vectors

[Christoph Hellwig]

And get automatic MSI-X affinity for free.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/be2iscsi/be_main.c | 127 +---
 drivers/scsi/be2iscsi/be_main.h |   2 -
 2 files changed, 42 insertions(+), 87 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 6372613..03faca8 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -801,12 +801,12 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
struct pci_dev *pcidev = phba->pcidev;
struct hwi_controller *phwi_ctrlr;
struct hwi_context_memory *phwi_context;
-   int ret, msix_vec, i, j;
+   int ret, i, j;
 
phwi_ctrlr = phba->phwi_ctrlr;
phwi_context = phwi_ctrlr->phwi_ctxt;
 
-   if (phba->msix_enabled) {
+   if (pcidev->msix_enabled) {
for (i = 0; i < phba->num_cpus; i++) {
phba->msi_name[i] = kzalloc(BEISCSI_MSI_NAME,
GFP_KERNEL);
@@ -817,9 +817,8 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
 
sprintf(phba->msi_name[i], "beiscsi_%02x_%02x",
phba->shost->host_no, i);
-   msix_vec = phba->msix_entries[i].vector;
-   ret = request_irq(msix_vec, be_isr_msix, 0,
- phba->msi_name[i],
+   ret = request_irq(pci_irq_vector(pcidev, i),
+ be_isr_msix, 0, phba->msi_name[i],
  _context->be_eq[i]);
if (ret) {
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
@@ -837,9 +836,8 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
}
sprintf(phba->msi_name[i], "beiscsi_mcc_%02x",
phba->shost->host_no);
-   msix_vec = phba->msix_entries[i].vector;
-   ret = request_irq(msix_vec, be_isr_mcc, 0, phba->msi_name[i],
- _context->be_eq[i]);
+   ret = request_irq(pci_irq_vector(pcidev, i), be_isr_mcc, 0,
+ phba->msi_name[i], _context->be_eq[i]);
if (ret) {
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT ,
"BM_%d : beiscsi_init_irqs-"
@@ -861,9 +859,8 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
return 0;
 free_msix_irqs:
for (j = i - 1; j >= 0; j--) {
+   free_irq(pci_irq_vector(pcidev, i), _context->be_eq[j]);
kfree(phba->msi_name[j]);
-   msix_vec = phba->msix_entries[j].vector;
-   free_irq(msix_vec, _context->be_eq[j]);
}
return ret;
 }
@@ -3039,7 +3036,7 @@ static int beiscsi_create_eqs(struct beiscsi_hba *phba,
num_eq_pages = PAGES_REQUIRED(phba->params.num_eq_entries * \
  sizeof(struct be_eq_entry));
 
-   if (phba->msix_enabled)
+   if (phba->pcidev->msix_enabled)
eq_for_mcc = 1;
else
eq_for_mcc = 0;
@@ -3549,7 +3546,7 @@ static int be_mcc_queues_create(struct beiscsi_hba *phba,
sizeof(struct be_mcc_compl)))
goto err;
/* Ask BE to create MCC compl queue; */
-   if (phba->msix_enabled) {
+   if (phba->pcidev->msix_enabled) {
if (beiscsi_cmd_cq_create(ctrl, cq, _context->be_eq
 [phba->num_cpus].q, false, true, 0))
goto mcc_cq_free;
@@ -3580,42 +3577,35 @@ static int be_mcc_queues_create(struct beiscsi_hba 
*phba,
return -ENOMEM;
 }
 
-/**
- * find_num_cpus()- Get the CPU online count
- * @phba: ptr to priv structure
- *
- * CPU count is used for creating EQ.
- **/
-static void find_num_cpus(struct beiscsi_hba *phba)
+static void be2iscsi_enable_msix(struct beiscsi_hba *phba)
 {
-   int  num_cpus = 0;
-
-   num_cpus = num_online_cpus();
+   int nvec = 1;
 
switch (phba->generation) {
case BE_GEN2:
case BE_GEN3:
-   phba->num_cpus = (num_cpus > BEISCSI_MAX_NUM_CPUS) ?
- BEISCSI_MAX_NUM_CPUS : num_cpus;
+   nvec = BEISCSI_MAX_NUM_CPUS + 1;
break;
case BE_GEN4:
-   /*
-* If eqid_count == 1 fall back to
-* INTX mechanism
-**/
-   if (phba->fw_config.eqid_count == 1) {
-   enable_msix = 0;
-   phba->num_cpus = 1;
-   return;
-   }
-
-   phba->num_cpus =
-   (num_cpus > (phba->fw_config.eqid_count - 1)) ?
-   (phba->fw_config.eqid_count - 1) : num_cpus;
+ 

Re: [LSF/MM TOPIC][LSF/MM ATTEND] multipath redesign

[Mike Snitzer]

On Fri, Jan 13 2017 at 10:56am -0500,
Hannes Reinecke  wrote:

> On 01/12/2017 06:29 PM, Benjamin Marzinski wrote:
> > On Thu, Jan 12, 2017 at 09:27:40AM +0100, Hannes Reinecke wrote:
> >> On 01/11/2017 11:23 PM, Mike Snitzer wrote:
> >>> On Wed, Jan 11 2017 at  4:44am -0500,
> >>> Hannes Reinecke  wrote:
> >>>
>  Hi all,
> 
>  I'd like to attend LSF/MM this year, and would like to discuss a
>  redesign of the multipath handling.
> 
>  With recent kernels we've got quite some functionality required for
>  multipathing already implemented, making some design decisions of the
>  original multipath-tools implementation quite pointless.
> 
>  I'm working on a proof-of-concept implementation which just uses a
>  simple configfs interface and doesn't require a daemon altogether.
> 
>  At LSF/MM I'd like to discuss how to move forward here, and whether we'd
>  like to stay with the current device-mapper integration or move away
> >>> >from that towards a stand-alone implementation.
> >>>
> >>> I'd really like open exchange of the problems you're having with the
> >>> current multipath-tools and DM multipath _before LSF_.  Last LSF only
> >>> scratched the surface on people having disdain for the complexity that is
> >>> the multipath-tools userspace.  But considering how much of the
> >>> multipath-tools you've written I find it fairly comical that you're the
> >>> person advocating switching away from it.
> >>>
> >> Yeah, I know.
> >>
> >> But I've stared long and hard at the code, and found some issues really 
> >> hard
> >> to overcome. Even more so as most things it does are really pointless.
> >>
> >> multipathd _insists_ on redoing the _entire_ device layout for basically 
> >> any
> >> operation (except for path checking).
> >> As the data structures allow only for a single setup it uses a lock per
> >> multipath device to protect against concurrent changes.
> >> When lots of uevents are to be processed this lock is heavily contended,
> >> leading to a slow-down of uevent processing.
> >> (cf the patchseries from Tang Junhui and my earlier pathset for
> >> lock pushdown)
> >>
> >> I've tried to move that lock down even further with distinct locks for
> >> device paths and multipath devices, but ultimately failed as it would 
> >> amount
> >> to essentially a rewrite of the core engine.
> > 
> > The multipath user-space tools locking IS horrible and touches
> > everything.  I could never see a way around it that didn't involve
> > a ground-up redesign.
> >  
> :-)
> 
> >>> But if less userspace involvement is needed then fix userspace.  Fail to
> >>> see how configfs is any different than the established DM ioctl interface.
> >>>
> >>> As I just said in another email DM multipath could benefit from
> >>> factoring out the SCSI-specific bits so that they are nicely optimized
> >>> away if using new transports (e.g. NVMEoF).
> >>>
> >>> Could be lessons can be learned from your approach but I'd prefer we
> >>> provably exhaust the utility of the current DM multipath kernel
> >>> implementation.  DM multipath is one of the most actively maintained and
> >>> updated DM targets (aside from thinp and cache).  As you know DM
> >>> multipath has grown blk-mq support which yielded serious performance
> >>> improvement.  You also noted (in an earlier email) that I reintroduced
> >>> bio-based DM multipath.  On a data path level we have all possible block
> >>> core interfaces plumbed.  And yes, they all involve cloning due to the
> >>> underlying Device Mapper core.  Open to any ideas on optimization.  If
> >>> DM is imposing some inherent performance limitation then please report
> >>> it accordingly.
> >>>
> >> Ah. And I thought you disliked request-based multipathing ...
> >>
> >> It's not _actually_ the DM interface which I'm objecting to, it's more the
> >> user-space implementation.
> >> The daemon is build around some design decisions which are simply not
> >> applicable anymore:
> >> - we now _do_ have reliable device identifications, so the the 'path_id'
> >> functionality is pointless.
> > 
> > This could be largely fixed in the existing code. The route that the
> > latest patch from Tang Junhui are going still grabs the wwid if we got
> > it from the uevent, but it isn't necesary, as long was we're careful.
> > Currently rbd devices don't get their wwid from the uevent but all other
> > devices do. It would probably be possible to write an rbd device udev
> > rule to set a variable so that they can work through udev environment
> > variables too.
> > 
> But this is still only working around the problem.
> We only should need to touch the device-mapper tables when setting up
> devices or during reconfiguration.
> 
> >> - The 'alua' device handler also provides you with reliable priority
> >> information, so it should be possible to do away with the 'prio' setting,
> >> too.
> > 
> > But this isn't true for all devices. 

Re: [dm-devel] [LSF/MM TOPIC][LSF/MM ATTEND] multipath redesign

[Hannes Reinecke]

On 01/12/2017 06:29 PM, Benjamin Marzinski wrote:
> On Thu, Jan 12, 2017 at 09:27:40AM +0100, Hannes Reinecke wrote:
>> On 01/11/2017 11:23 PM, Mike Snitzer wrote:
>>> On Wed, Jan 11 2017 at  4:44am -0500,
>>> Hannes Reinecke  wrote:
>>>
 Hi all,

 I'd like to attend LSF/MM this year, and would like to discuss a
 redesign of the multipath handling.

 With recent kernels we've got quite some functionality required for
 multipathing already implemented, making some design decisions of the
 original multipath-tools implementation quite pointless.

 I'm working on a proof-of-concept implementation which just uses a
 simple configfs interface and doesn't require a daemon altogether.

 At LSF/MM I'd like to discuss how to move forward here, and whether we'd
 like to stay with the current device-mapper integration or move away
>>> >from that towards a stand-alone implementation.
>>>
>>> I'd really like open exchange of the problems you're having with the
>>> current multipath-tools and DM multipath _before LSF_.  Last LSF only
>>> scratched the surface on people having disdain for the complexity that is
>>> the multipath-tools userspace.  But considering how much of the
>>> multipath-tools you've written I find it fairly comical that you're the
>>> person advocating switching away from it.
>>>
>> Yeah, I know.
>>
>> But I've stared long and hard at the code, and found some issues really hard
>> to overcome. Even more so as most things it does are really pointless.
>>
>> multipathd _insists_ on redoing the _entire_ device layout for basically any
>> operation (except for path checking).
>> As the data structures allow only for a single setup it uses a lock per
>> multipath device to protect against concurrent changes.
>> When lots of uevents are to be processed this lock is heavily contended,
>> leading to a slow-down of uevent processing.
>> (cf the patchseries from Tang Junhui and my earlier pathset for
>> lock pushdown)
>>
>> I've tried to move that lock down even further with distinct locks for
>> device paths and multipath devices, but ultimately failed as it would amount
>> to essentially a rewrite of the core engine.
> 
> The multipath user-space tools locking IS horrible and touches
> everything.  I could never see a way around it that didn't involve
> a ground-up redesign.
>  
:-)

>>> But if less userspace involvement is needed then fix userspace.  Fail to
>>> see how configfs is any different than the established DM ioctl interface.
>>>
>>> As I just said in another email DM multipath could benefit from
>>> factoring out the SCSI-specific bits so that they are nicely optimized
>>> away if using new transports (e.g. NVMEoF).
>>>
>>> Could be lessons can be learned from your approach but I'd prefer we
>>> provably exhaust the utility of the current DM multipath kernel
>>> implementation.  DM multipath is one of the most actively maintained and
>>> updated DM targets (aside from thinp and cache).  As you know DM
>>> multipath has grown blk-mq support which yielded serious performance
>>> improvement.  You also noted (in an earlier email) that I reintroduced
>>> bio-based DM multipath.  On a data path level we have all possible block
>>> core interfaces plumbed.  And yes, they all involve cloning due to the
>>> underlying Device Mapper core.  Open to any ideas on optimization.  If
>>> DM is imposing some inherent performance limitation then please report
>>> it accordingly.
>>>
>> Ah. And I thought you disliked request-based multipathing ...
>>
>> It's not _actually_ the DM interface which I'm objecting to, it's more the
>> user-space implementation.
>> The daemon is build around some design decisions which are simply not
>> applicable anymore:
>> - we now _do_ have reliable device identifications, so the the 'path_id'
>> functionality is pointless.
> 
> This could be largely fixed in the existing code. The route that the
> latest patch from Tang Junhui are going still grabs the wwid if we got
> it from the uevent, but it isn't necesary, as long was we're careful.
> Currently rbd devices don't get their wwid from the uevent but all other
> devices do. It would probably be possible to write an rbd device udev
> rule to set a variable so that they can work through udev environment
> variables too.
> 
But this is still only working around the problem.
We only should need to touch the device-mapper tables when setting up
devices or during reconfiguration.

>> - The 'alua' device handler also provides you with reliable priority
>> information, so it should be possible to do away with the 'prio' setting,
>> too.
> 
> But this isn't true for all devices. Also, Like I mentioned last year
> when this got brought up, no matter how we group the paths, there end up
> being users that have good reasons why they want them grouped
> differently in their case.  The path priority/grouping seems like one
> place where evidence has shown that we should 

Re: [LSF/MM TOPIC][LSF/MM ATTEND] NAPI polling for block drivers

[Johannes Thumshirn]

On Wed, Jan 11, 2017 at 08:13:02AM -0700, Jens Axboe wrote:
> On 01/11/2017 08:07 AM, Jens Axboe wrote:
> > On 01/11/2017 06:43 AM, Johannes Thumshirn wrote:
> >> Hi all,
> >>
> >> I'd like to attend LSF/MM and would like to discuss polling for block 
> >> drivers.
> >>
> >> Currently there is blk-iopoll but it is neither as widely used as NAPI in 
> >> the
> >> networking field and accoring to Sagi's findings in [1] performance with
> >> polling is not on par with IRQ usage.
> >>
> >> On LSF/MM I'd like to whether it is desirable to have NAPI like polling in
> >> more block drivers and how to overcome the currently seen performance 
> >> issues.
> > 
> > It would be an interesting topic to discuss, as it is a shame that 
> > blk-iopoll
> > isn't used more widely.
> 
> Forgot to mention - it should only be a topic, if experimentation has
> been done and results gathered to pin point what the issues are, so we
> have something concrete to discus. I'm not at all interested in a hand
> wavy discussion on the topic.

So here are my 1st real numbers on this topic w/ some spinning rust:

All is done with 4.10-rc3 and we at least have no performance degradation when
a poll budget of 128 or 256 (oddly the max that irq_poll currently does you
allow to have). Clearly it looks like the disk is the limiting factor here and
we already saturated it. I'll do AHCI SSD tests on Monday. Hannes did some 
tests 
with mptXsas and a SSD maybe he can share his findings as well.

scsi-sq:

baseline:
  read : io=66776KB, bw=1105.5KB/s, iops=276, runt= 60406msec
  write: io=65812KB, bw=1089.6KB/s, iops=272, runt= 60406msec

AHCI irq_poll budget 31:
  read : io=53372KB, bw=904685B/s, iops=220, runt= 60411msec
  write: io=52596KB, bw=891531B/s, iops=217, runt= 60411msec

AHCI irq_poll budget 128:
  read : io=4KB, bw=1106.4KB/s, iops=276, runt= 60257msec
  write: io=65608KB, bw=1088.9KB/s, iops=272, runt= 60257msec

AHCI irq_poll budget 256:
  read : io=67048KB, bw=.2KB/s, iops=277, runt= 60296msec
  write: io=65916KB, bw=1093.3KB/s, iops=273, runt= 60296msec

scsi-mq:

baseline:
  read : io=78220KB, bw=1300.7KB/s, iops=325, runt= 60140msec
  write: io=77104KB, bw=1282.8KB/s, iops=320, runt= 60140msec

AHCI irq_poll budget 256:
  read : io=78316KB, bw=1301.7KB/s, iops=325, runt= 60167msec
  write: io=77172KB, bw=1282.7KB/s, iops=320, runt= 60167msec


-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Set elsiocb contexts to NULL after freeing it

[Johannes Thumshirn]

[+Cc James S. I'm sorry]

On Tue, Jan 10, 2017 at 12:05:54PM +0100, Johannes Thumshirn wrote:
> Set the elsiocb contexts to NULL after freeing as others depend on it.
> 
> Signed-off-by: Johannes Thumshirn 
> ---
>  drivers/scsi/lpfc/lpfc_els.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index 236e4e5..7b6bd8e 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
> @@ -3590,12 +3590,14 @@ lpfc_els_free_iocb(struct lpfc_hba *phba, struct 
> lpfc_iocbq *elsiocb)
>   } else {
>   buf_ptr1 = (struct lpfc_dmabuf *) elsiocb->context2;
>   lpfc_els_free_data(phba, buf_ptr1);
> + elsiocb->context2 = NULL;
>   }
>   }
>  
>   if (elsiocb->context3) {
>   buf_ptr = (struct lpfc_dmabuf *) elsiocb->context3;
>   lpfc_els_free_bpl(phba, buf_ptr);
> + elsiocb->context3 = NULL;
>   }
>   lpfc_sli_release_iocbq(phba, elsiocb);
>   return 0;

Dick, James, any comments? I'd really like to get this in soon as it solves
customer issues.

Byte,
Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iscsi_trx going into D state

[Laurence Oberman]

- Original Message -
> From: "Robert LeBlanc" 
> To: "Laurence Oberman" 
> Cc: "Doug Ledford" , "Nicholas A. Bellinger" 
> , "Zhu Lingshan"
> , "linux-rdma" , 
> linux-scsi@vger.kernel.org, "Sagi Grimberg"
> , "Christoph Hellwig" 
> Sent: Thursday, January 12, 2017 4:26:05 PM
> Subject: Re: iscsi_trx going into D state
> 
> Sorry sent prematurely...
> 
> On Thu, Jan 12, 2017 at 2:22 PM, Robert LeBlanc  wrote:
> > I'm having trouble replicating the D state issue on Infiniband (I was
> > able to trigger it reliably a couple weeks back, I don't know if OFED
> > to verify the same results happen there as well.
> 
> I'm having trouble replicating the D state issue on Infiniband (I was
> able to trigger it reliably a couple weeks back, I don't know if OFED
> being installed is altering things but it only installed for 3.10. The
> ConnectX-4-LX exposes the issue easily if you have those cards.) to
> verify the same results happen there as well.
> 
> 
> Robert LeBlanc
> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

I am only back in the office next Wednesday.
I have this all setup using ConnectX-4 with IB/ISER but have no way of remotely 
creating the disconnect as I currently have it back-to-back.
Have run multiple tests with IB and ISER hard resting the client to break the 
IB connection but have not been able to reproduce as yet.
So it will have to wait until I can pull cables next week as that seemed to be 
the way you have been reproducing this.

This is in a code area I also don't have a lot of knowledge of the flow but 
have started trying to understand it better.

Thanks
Laurence
 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] SCSI fixes for 4.10-rc3

[James Bottomley]

The major fix is the bfa firmware, since the latest 10Gb cards fail
probing with the current firmware.  The rest is a set of minor fixes:
one missed Kconfig dependency causing randconfig failures, a missed
error return on an error leg, a change for how multiqueue waits on a
blocked device and a don't reset while in reset fix.

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes

The short changelog is:

Bart Van Assche (1):
  scsi: scsi-mq: Wait for .queue_rq() if necessary

Benjamin Poirier (1):
  scsi: bfa: Increase requested firmware version to 3.2.5.1

Burak Ok (1):
  scsi: snic: Return error code on memory allocation failure

Randy Dunlap (1):
  scsi: qedi: fix build, depends on UIO

Satish Kharat (1):
  scsi: fnic: Avoid sending reset to firmware when another reset is in 
progress

And the diffstat:

 drivers/scsi/bfa/bfad.c   |  6 +++---
 drivers/scsi/bfa/bfad_drv.h   |  2 +-
 drivers/scsi/fnic/fnic.h  |  1 +
 drivers/scsi/fnic/fnic_scsi.c | 16 
 drivers/scsi/qedi/Kconfig |  2 +-
 drivers/scsi/scsi_lib.c   |  2 +-
 drivers/scsi/snic/snic_main.c |  3 +++
 7 files changed, 26 insertions(+), 6 deletions(-)

With full diff below.

James

---

diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index d9e1521..5caf5f3 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -64,9 +64,9 @@ int   max_rport_logins = BFA_FCS_MAX_RPORT_LOGINS;
 u32bfi_image_cb_size, bfi_image_ct_size, bfi_image_ct2_size;
 u32*bfi_image_cb, *bfi_image_ct, *bfi_image_ct2;
 
-#define BFAD_FW_FILE_CB"cbfw-3.2.3.0.bin"
-#define BFAD_FW_FILE_CT"ctfw-3.2.3.0.bin"
-#define BFAD_FW_FILE_CT2   "ct2fw-3.2.3.0.bin"
+#define BFAD_FW_FILE_CB"cbfw-3.2.5.1.bin"
+#define BFAD_FW_FILE_CT"ctfw-3.2.5.1.bin"
+#define BFAD_FW_FILE_CT2   "ct2fw-3.2.5.1.bin"
 
 static u32 *bfad_load_fwimg(struct pci_dev *pdev);
 static void bfad_free_fwimg(void);
diff --git a/drivers/scsi/bfa/bfad_drv.h b/drivers/scsi/bfa/bfad_drv.h
index f9e8620..cfcfff4 100644
--- a/drivers/scsi/bfa/bfad_drv.h
+++ b/drivers/scsi/bfa/bfad_drv.h
@@ -58,7 +58,7 @@
 #ifdef BFA_DRIVER_VERSION
 #define BFAD_DRIVER_VERSIONBFA_DRIVER_VERSION
 #else
-#define BFAD_DRIVER_VERSION"3.2.25.0"
+#define BFAD_DRIVER_VERSION"3.2.25.1"
 #endif
 
 #define BFAD_PROTO_NAME FCPI_NAME
diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index 9ddc920..9e4b770 100644
--- a/drivers/scsi/fnic/fnic.h
+++ b/drivers/scsi/fnic/fnic.h
@@ -248,6 +248,7 @@ struct fnic {
struct completion *remove_wait; /* device remove thread blocks */
 
atomic_t in_flight; /* io counter */
+   bool internal_reset_inprogress;
u32 _reserved;  /* fill hole */
unsigned long state_flags;  /* protected by host lock */
enum fnic_state state;
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 2544a37..adb3d58 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -2581,6 +2581,19 @@ int fnic_host_reset(struct scsi_cmnd *sc)
unsigned long wait_host_tmo;
struct Scsi_Host *shost = sc->device->host;
struct fc_lport *lp = shost_priv(shost);
+   struct fnic *fnic = lport_priv(lp);
+   unsigned long flags;
+
+   spin_lock_irqsave(>fnic_lock, flags);
+   if (fnic->internal_reset_inprogress == 0) {
+   fnic->internal_reset_inprogress = 1;
+   } else {
+   spin_unlock_irqrestore(>fnic_lock, flags);
+   FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+   "host reset in progress skipping another host reset\n");
+   return SUCCESS;
+   }
+   spin_unlock_irqrestore(>fnic_lock, flags);
 
/*
 * If fnic_reset is successful, wait for fabric login to complete
@@ -2601,6 +2614,9 @@ int fnic_host_reset(struct scsi_cmnd *sc)
}
}
 
+   spin_lock_irqsave(>fnic_lock, flags);
+   fnic->internal_reset_inprogress = 0;
+   spin_unlock_irqrestore(>fnic_lock, flags);
return ret;
 }
 
diff --git a/drivers/scsi/qedi/Kconfig b/drivers/scsi/qedi/Kconfig
index 23ca8a2..2133145 100644
--- a/drivers/scsi/qedi/Kconfig
+++ b/drivers/scsi/qedi/Kconfig
@@ -1,6 +1,6 @@
 config QEDI
tristate "QLogic QEDI 25/40/100Gb iSCSI Initiator Driver Support"
-   depends on PCI && SCSI
+   depends on PCI && SCSI && UIO
depends on QED
select SCSI_ISCSI_ATTRS
select QED_LL2
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c35b6de..9fd9a97 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2893,7 +2893,7 @@ scsi_internal_device_block(struct scsi_device *sdev)
 * request queue. 
 */
if (q->mq_ops) {
-   blk_mq_stop_hw_queues(q);
+   

RE: [PATCH 3/4] hpsa: remove coalescing settings for ioaccel2

[Don Brace]

> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, October 27, 2016 4:04 AM
> To: Don Brace ; j...@linux.vnet.ibm.com; John
> Hall ; Kevin Barnett
> ; Mahesh Rajashekhara
> ; h...@infradead.org; Scott Teel
> ; Viswas G ; Justin
> Lindley ; Scott Benesh
> ; elli...@hpe.com; posw...@suse.com
> Cc: linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 3/4] hpsa: remove coalescing settings for ioaccel2
> 
> EXTERNAL EMAIL
> 
> 
> On 10/27/2016 12:21 AM, Don Brace wrote:
> > - Setting coalescing has a significant negative
> >   impact on low queue-depth performance.
> > - Does not help high queue-depth performance.
> >
> > Reviewed-by: Scott Benesh 
> > Reviewed-by: Scott Teel 
> > Reviewed-by: Kevin Barnett 
> > Signed-off-by: Don Brace 
> > ---
> >  drivers/scsi/hpsa.c |8 ++--
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> > index 9fb739c..810c300 100644
> > --- a/drivers/scsi/hpsa.c
> > +++ b/drivers/scsi/hpsa.c
> > @@ -9277,13 +9277,9 @@ static int hpsa_enter_performant_mode(struct
> ctlr_info *h, u32 trans_support)
> >   access = SA5_ioaccel_mode1_access;
> >   writel(10, >cfgtable->HostWrite.CoalIntDelay);
> >   writel(4, >cfgtable->HostWrite.CoalIntCount);
> > - } else {
> > - if (trans_support & CFGTBL_Trans_io_accel2) {
> > + } else
> > + if (trans_support & CFGTBL_Trans_io_accel2)
> >   access = SA5_ioaccel_mode2_access;
> > - writel(10, >cfgtable->HostWrite.CoalIntDelay);
> > - writel(4, >cfgtable->HostWrite.CoalIntCount);
> > - }
> > - }
> >   writel(CFGTBL_ChangeReq, h->vaddr + SA5_DOORBELL);
> >   if (hpsa_wait_for_mode_change_ack(h)) {
> >   dev_err(>pdev->dev,
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> Reviewed-by: Hannes Reinecke 
> 
> Cheers,
> 
> Hannes
> --
> Dr. Hannes ReineckeTeamlead Storage & Networking
> h...@suse.de   +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)

Wondering what has happened to this patch?

Thanks,
Don Brace

ESC - Smart Storage
Microsemi Corporation

[bug report] scsi: megaraid_sas: Add the Support for SAS3.5 Generic Megaraid Controllers Capabilities

[Dan Carpenter]

Hello Sasikumar Chandrasekaran,

This is a semi-automatic email about new static checker warnings.

The patch 9581ebebbe35: "scsi: megaraid_sas: Add the Support for 
SAS3.5 Generic Megaraid Controllers Capabilities" from Jan 10, 2017, 
leads to the following Smatch complaint:

drivers/scsi/megaraid/megaraid_sas_base.c:5245 megasas_init_fw()
 error: we previously assumed 'fusion' could be null (see line 5069)

drivers/scsi/megaraid/megaraid_sas_base.c
  5068  
  5069  if (fusion)
^^
Patch introduces a new NULL check.

  5070  instance->instancet = _instance_template_fusion;
  5071  else {
  5072  switch (instance->pdev->device) {
  5073  case PCI_DEVICE_ID_LSI_SAS1078R:
  5074  case PCI_DEVICE_ID_LSI_SAS1078DE:
  5075  instance->instancet = 
_instance_template_ppc;
  5076  break;
  5077  case PCI_DEVICE_ID_LSI_SAS1078GEN2:
  5078  case PCI_DEVICE_ID_LSI_SAS0079GEN2:
  5079  instance->instancet = 
_instance_template_gen2;
  5080  break;
  5081  case PCI_DEVICE_ID_LSI_SAS0073SKINNY:
  5082  case PCI_DEVICE_ID_LSI_SAS0071SKINNY:
  5083  instance->instancet = 
_instance_template_skinny;
  5084  break;
  5085  case PCI_DEVICE_ID_LSI_SAS1064R:
  5086  case PCI_DEVICE_ID_DELL_PERC5:
  5087  default:
  5088  instance->instancet = 
_instance_template_xscale;
  5089  instance->pd_list_not_supported = 1;
  5090  break;
  5091  }
  5092  }
  5093  
  5094  if (megasas_transition_to_ready(instance, 0)) {
  5095  atomic_set(>fw_reset_no_pci_access, 1);
  5096  instance->instancet->adp_reset
  5097  (instance, instance->reg_set);
  5098  atomic_set(>fw_reset_no_pci_access, 0);
  5099  dev_info(>pdev->dev,
  5100  "FW restarted successfully from %s!\n",
  5101  __func__);
  5102  
  5103  /*waitting for about 30 second before retry*/
  5104  ssleep(30);
  5105  
  5106  if (megasas_transition_to_ready(instance, 0))
  5107  goto fail_ready_state;
  5108  }
  5109  
  5110  if (instance->is_ventura) {
  5111  scratch_pad_3 =
  5112  
readl(>reg_set->outbound_scratch_pad_3);
  5113  #if VD_EXT_DEBUG
  5114  dev_info(>pdev->dev, "scratch_pad3 0x%x\n",
  5115  scratch_pad_3);
  5116  #endif
  5117  instance->max_raid_mapsize = ((scratch_pad_3 >>
  5118  MR_MAX_RAID_MAP_SIZE_OFFSET_SHIFT) &
  5119  MR_MAX_RAID_MAP_SIZE_MASK);
  5120  }
  5121  
  5122  /* Check if MSI-X is supported while in ready state */
  5123  msix_enable = (instance->instancet->read_fw_status_reg(reg_set) 
&
  5124 0x400) >> 0x1a;
  5125  if (msix_enable && !msix_disable) {
  5126  int irq_flags = PCI_IRQ_MSIX;
  5127  
  5128  scratch_pad_2 = readl
  5129  (>reg_set->outbound_scratch_pad_2);
  5130  /* Check max MSI-X vectors */
  5131  if (fusion) {
  5132  if (fusion->adapter_type == THUNDERBOLT_SERIES) 
{ /* Thunderbolt Series*/
  5133  instance->msix_vectors = (scratch_pad_2
  5134  & MR_MAX_REPLY_QUEUES_OFFSET) + 
1;
  5135  fw_msix_count = instance->msix_vectors;
  5136  } else { /* Invader series supports more than 8 
MSI-x vectors*/
  5137  instance->msix_vectors = ((scratch_pad_2
  5138  & 
MR_MAX_REPLY_QUEUES_EXT_OFFSET)
  5139  >> 
MR_MAX_REPLY_QUEUES_EXT_OFFSET_SHIFT) + 1;
  5140  if (instance->msix_vectors > 16)
  5141  instance->msix_combined = true;
  5142  
  5143  if (rdpq_enable)
  5144  instance->is_rdpq = 
(scratch_pad_2 & MR_RDPQ_MODE_OFFSET) ?
  5145  1 : 0;
  5146  fw_msix_count = instance->msix_vectors;
  5147  /* Save 1-15 reply post index address 
to local memory
  5148   * Index 0 is already saved from 

Re: [PATCH v2] scsi: qla4xxx: Use dma_pool_zalloc

[Javali, Nilesh]

On 02/01/17, 11:10 AM, "Souptick Joarder"  wrote:

>Hi Nilesh,
>
>On Thu, Dec 22, 2016 at 5:42 PM, Souptick Joarder 
>wrote:
>> We should use dma_pool_zalloc instead of dma_pool_alloc/memset
>>
>> Signed-off-by: Souptick joarder 
>> ---
>> v2 changes:
>>- Address comment from Nilesh to make same change in
>>  all applicable places inside qla4xxx source
>>
>>  drivers/scsi/qla4xxx/ql4_mbx.c | 6 ++
>>  drivers/scsi/qla4xxx/ql4_os.c  | 4 +---
>>  2 files changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c
>>b/drivers/scsi/qla4xxx/ql4_mbx.c
>> index c291fdf..8f97839 100644
>> --- a/drivers/scsi/qla4xxx/ql4_mbx.c
>> +++ b/drivers/scsi/qla4xxx/ql4_mbx.c
>> @@ -1587,12 +1587,11 @@ int qla4xxx_get_chap(struct scsi_qla_host *ha,
>>char *username, char *password,
>> struct ql4_chap_table *chap_table;
>> dma_addr_t chap_dma;
>>
>> -   chap_table = dma_pool_alloc(ha->chap_dma_pool, GFP_KERNEL,
>>_dma);
>> +   chap_table = dma_pool_zalloc(ha->chap_dma_pool, GFP_KERNEL,
>>_dma);
>> if (chap_table == NULL)
>> return -ENOMEM;
>>
>> chap_size = sizeof(struct ql4_chap_table);
>> -   memset(chap_table, 0, chap_size);
>>
>> if (is_qla40XX(ha))
>> offset = FLASH_CHAP_OFFSET | (idx * chap_size);
>> @@ -1651,13 +1650,12 @@ int qla4xxx_set_chap(struct scsi_qla_host *ha,
>>char *username, char *password,
>> uint32_t chap_size = 0;
>> dma_addr_t chap_dma;
>>
>> -   chap_table = dma_pool_alloc(ha->chap_dma_pool, GFP_KERNEL,
>>_dma);
>> +   chap_table = dma_pool_zalloc(ha->chap_dma_pool, GFP_KERNEL,
>>_dma);
>> if (chap_table == NULL) {
>> ret =  -ENOMEM;
>> goto exit_set_chap;
>> }
>>
>> -   memset(chap_table, 0, sizeof(struct ql4_chap_table));
>> if (bidi)
>> chap_table->flags |= BIT_6; /* peer */
>> else
>> diff --git a/drivers/scsi/qla4xxx/ql4_os.c
>>b/drivers/scsi/qla4xxx/ql4_os.c
>> index 01c3610..0c91c89 100644
>> --- a/drivers/scsi/qla4xxx/ql4_os.c
>> +++ b/drivers/scsi/qla4xxx/ql4_os.c
>> @@ -825,12 +825,10 @@ static int qla4xxx_delete_chap(struct Scsi_Host
>>*shost, uint16_t chap_tbl_idx)
>> uint32_t chap_size;
>> int ret = 0;
>>
>> -   chap_table = dma_pool_alloc(ha->chap_dma_pool, GFP_KERNEL,
>>_dma);
>> +   chap_table = dma_pool_zalloc(ha->chap_dma_pool, GFP_KERNEL,
>>_dma);
>> if (chap_table == NULL)
>> return -ENOMEM;
>>
>> -   memset(chap_table, 0, sizeof(struct ql4_chap_table));
>> -
>> if (is_qla80XX(ha))
>> max_chap_entries = (ha->hw.flt_chap_size / 2) /
>>sizeof(struct ql4_chap_table);
>> --
>> 1.9.1
>>
>
>Any further comment on this patch ?

Any reason for handling this only for CHAP related code and not take care
at all other places within ql4_os.c and ql4_init.c.
According to me the change should be consistent across all the affecting
files unless you think otherwise.

Thanks,
Nilesh

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] scsi: use blk_rq_payload_bytes

[Hannes Reinecke]

On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Without that we'll pass a wrong payload size in cmd->sdb, which
> can lead to hangs with drivers that need the total transfer size.
> 
> Signed-off-by: Christoph Hellwig 
> Reported-by: Chris Valean 
> Reported-by: Dexuan Cui 
> Fixes: f9d03f96 ("block: improve handling of the magic discard payload")
> ---
>  drivers/scsi/scsi_lib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index c35b6de..ad4ff8f 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1018,7 +1018,7 @@ static int scsi_init_sgtable(struct request *req, 
> struct scsi_data_buffer *sdb)
>   count = blk_rq_map_sg(req->q, req, sdb->table.sgl);
>   BUG_ON(count > sdb->table.nents);
>   sdb->table.nents = count;
> - sdb->length = blk_rq_bytes(req);
> + sdb->length = blk_rq_payload_bytes(req);
>   return BLKPREP_OK;
>  }
>  
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] sd: remove __data_len hack for WRITE SAME

[Hannes Reinecke]

On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Now that we have the blk_rq_payload_bytes helper available to determine
> the actual I/O size we don't need to mess around with __data_len for
> WRITE SAME.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/scsi/sd.c | 17 +
>  1 file changed, 1 insertion(+), 16 deletions(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b193304..1fbb1ec 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -836,7 +836,6 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
>   struct bio *bio = rq->bio;
>   sector_t sector = blk_rq_pos(rq);
>   unsigned int nr_sectors = blk_rq_sectors(rq);
> - unsigned int nr_bytes = blk_rq_bytes(rq);
>   int ret;
>  
>   if (sdkp->device->no_write_same)
> @@ -869,21 +868,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd 
> *cmd)
>  
>   cmd->transfersize = sdp->sector_size;
>   cmd->allowed = SD_MAX_RETRIES;
> -
> - /*
> -  * For WRITE_SAME the data transferred in the DATA IN buffer is
> -  * different from the amount of data actually written to the target.
> -  *
> -  * We set up __data_len to the amount of data transferred from the
> -  * DATA IN buffer so that blk_rq_map_sg set up the proper S/G list
> -  * to transfer a single sector of data first, but then reset it to
> -  * the amount of data to be written right after so that the I/O path
> -  * knows how much to actually write.
> -  */
> - rq->__data_len = sdp->sector_size;
> - ret = scsi_init_io(cmd);
> - rq->__data_len = nr_bytes;
> - return ret;
> + return scsi_init_io(cmd);
>  }
>  
>  static int sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] block: add blk_rq_payload_bytes

[Hannes Reinecke]

On 01/13/2017 12:29 PM, Christoph Hellwig wrote:
> Add a helper to calculate the actual data transfer size for special
> payload requests.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  include/linux/blkdev.h | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ff3d774..1ca8e8f 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1000,6 +1000,19 @@ static inline unsigned int blk_rq_cur_sectors(const 
> struct request *rq)
>   return blk_rq_cur_bytes(rq) >> 9;
>  }
>  
> +/*
> + * Some commands like WRITE SAME have a payload or data transfer size which
> + * is different from the size of the request.  Any driver that supports such
> + * commands using the RQF_SPECIAL_PAYLOAD flag needs to use this helper to
> + * calculate the data transfer size.
> + */
> +static inline unsigned int blk_rq_payload_bytes(struct request *rq)
> +{
> + if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
> + return rq->special_vec.bv_len;
> + return blk_rq_bytes(rq);
> +}
> +
>  static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
>int op)
>  {
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] sd: remove __data_len hack for WRITE SAME

[Christoph Hellwig]

Now that we have the blk_rq_payload_bytes helper available to determine
the actual I/O size we don't need to mess around with __data_len for
WRITE SAME.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/sd.c | 17 +
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b193304..1fbb1ec 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -836,7 +836,6 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
struct bio *bio = rq->bio;
sector_t sector = blk_rq_pos(rq);
unsigned int nr_sectors = blk_rq_sectors(rq);
-   unsigned int nr_bytes = blk_rq_bytes(rq);
int ret;
 
if (sdkp->device->no_write_same)
@@ -869,21 +868,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
 
cmd->transfersize = sdp->sector_size;
cmd->allowed = SD_MAX_RETRIES;
-
-   /*
-* For WRITE_SAME the data transferred in the DATA IN buffer is
-* different from the amount of data actually written to the target.
-*
-* We set up __data_len to the amount of data transferred from the
-* DATA IN buffer so that blk_rq_map_sg set up the proper S/G list
-* to transfer a single sector of data first, but then reset it to
-* the amount of data to be written right after so that the I/O path
-* knows how much to actually write.
-*/
-   rq->__data_len = sdp->sector_size;
-   ret = scsi_init_io(cmd);
-   rq->__data_len = nr_bytes;
-   return ret;
+   return scsi_init_io(cmd);
 }
 
 static int sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] scsi: use blk_rq_payload_bytes

[Christoph Hellwig]

Without that we'll pass a wrong payload size in cmd->sdb, which
can lead to hangs with drivers that need the total transfer size.

Signed-off-by: Christoph Hellwig 
Reported-by: Chris Valean 
Reported-by: Dexuan Cui 
Fixes: f9d03f96 ("block: improve handling of the magic discard payload")
---
 drivers/scsi/scsi_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c35b6de..ad4ff8f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1018,7 +1018,7 @@ static int scsi_init_sgtable(struct request *req, struct 
scsi_data_buffer *sdb)
count = blk_rq_map_sg(req->q, req, sdb->table.sgl);
BUG_ON(count > sdb->table.nents);
sdb->table.nents = count;
-   sdb->length = blk_rq_bytes(req);
+   sdb->length = blk_rq_payload_bytes(req);
return BLKPREP_OK;
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] nvme: use blk_rq_payload_bytes

[Christoph Hellwig]

The new blk_rq_payload_bytes generalizes the payload length hacks
that nvme_map_len did before.

Signed-off-by: Christoph Hellwig 
---
 drivers/nvme/host/fc.c   |  5 ++---
 drivers/nvme/host/nvme.h |  8 
 drivers/nvme/host/pci.c  | 19 ---
 drivers/nvme/host/rdma.c | 13 +
 4 files changed, 15 insertions(+), 30 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index aa0bc60..fcc9dcf 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1654,13 +1654,12 @@ nvme_fc_map_data(struct nvme_fc_ctrl *ctrl, struct 
request *rq,
struct nvme_fc_fcp_op *op)
 {
struct nvmefc_fcp_req *freq = >fcp_req;
-   u32 map_len = nvme_map_len(rq);
enum dma_data_direction dir;
int ret;
 
freq->sg_cnt = 0;
 
-   if (!map_len)
+   if (!blk_rq_payload_bytes(rq))
return 0;
 
freq->sg_table.sgl = freq->first_sgl;
@@ -1854,7 +1853,7 @@ nvme_fc_queue_rq(struct blk_mq_hw_ctx *hctx,
if (ret)
return ret;
 
-   data_len = nvme_map_len(rq);
+   data_len = blk_rq_payload_bytes(rq);
if (data_len)
io_dir = ((rq_data_dir(rq) == WRITE) ?
NVMEFC_FCP_WRITE : NVMEFC_FCP_READ);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 6377e14..aead6d0 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -225,14 +225,6 @@ static inline u64 nvme_block_nr(struct nvme_ns *ns, 
sector_t sector)
return (sector >> (ns->lba_shift - 9));
 }
 
-static inline unsigned nvme_map_len(struct request *rq)
-{
-   if (req_op(rq) == REQ_OP_DISCARD)
-   return sizeof(struct nvme_dsm_range);
-   else
-   return blk_rq_bytes(rq);
-}
-
 static inline void nvme_cleanup_cmd(struct request *req)
 {
if (req->rq_flags & RQF_SPECIAL_PAYLOAD) {
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 19beeb7..3faefab 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -306,11 +306,11 @@ static __le64 **iod_list(struct request *req)
return (__le64 **)(iod->sg + blk_rq_nr_phys_segments(req));
 }
 
-static int nvme_init_iod(struct request *rq, unsigned size,
-   struct nvme_dev *dev)
+static int nvme_init_iod(struct request *rq, struct nvme_dev *dev)
 {
struct nvme_iod *iod = blk_mq_rq_to_pdu(rq);
int nseg = blk_rq_nr_phys_segments(rq);
+   unsigned int size = blk_rq_payload_bytes(rq);
 
if (nseg > NVME_INT_PAGES || size > NVME_INT_BYTES(dev)) {
iod->sg = kmalloc(nvme_iod_alloc_size(dev, size, nseg), 
GFP_ATOMIC);
@@ -420,12 +420,11 @@ static void nvme_dif_complete(u32 p, u32 v, struct 
t10_pi_tuple *pi)
 }
 #endif
 
-static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req,
-   int total_len)
+static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req)
 {
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct dma_pool *pool;
-   int length = total_len;
+   int length = blk_rq_payload_bytes(req);
struct scatterlist *sg = iod->sg;
int dma_len = sg_dma_len(sg);
u64 dma_addr = sg_dma_address(sg);
@@ -501,7 +500,7 @@ static bool nvme_setup_prps(struct nvme_dev *dev, struct 
request *req,
 }
 
 static int nvme_map_data(struct nvme_dev *dev, struct request *req,
-   unsigned size, struct nvme_command *cmnd)
+   struct nvme_command *cmnd)
 {
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct request_queue *q = req->q;
@@ -519,7 +518,7 @@ static int nvme_map_data(struct nvme_dev *dev, struct 
request *req,
DMA_ATTR_NO_WARN))
goto out;
 
-   if (!nvme_setup_prps(dev, req, size))
+   if (!nvme_setup_prps(dev, req))
goto out_unmap;
 
ret = BLK_MQ_RQ_QUEUE_ERROR;
@@ -580,7 +579,6 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
struct nvme_dev *dev = nvmeq->dev;
struct request *req = bd->rq;
struct nvme_command cmnd;
-   unsigned map_len;
int ret = BLK_MQ_RQ_QUEUE_OK;
 
/*
@@ -600,13 +598,12 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
if (ret != BLK_MQ_RQ_QUEUE_OK)
return ret;
 
-   map_len = nvme_map_len(req);
-   ret = nvme_init_iod(req, map_len, dev);
+   ret = nvme_init_iod(req, dev);
if (ret != BLK_MQ_RQ_QUEUE_OK)
goto out_free_cmd;
 
if (blk_rq_nr_phys_segments(req))
-   ret = nvme_map_data(dev, req, map_len, );
+   ret = nvme_map_data(dev, req, );
 
if (ret != BLK_MQ_RQ_QUEUE_OK)
goto out_cleanup_iod;
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 34e5648..557f29b 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -981,8 +981,7 @@ 

[PATCH 1/4] block: add blk_rq_payload_bytes

[Christoph Hellwig]

Add a helper to calculate the actual data transfer size for special
payload requests.

Signed-off-by: Christoph Hellwig 
---
 include/linux/blkdev.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ff3d774..1ca8e8f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1000,6 +1000,19 @@ static inline unsigned int blk_rq_cur_sectors(const 
struct request *rq)
return blk_rq_cur_bytes(rq) >> 9;
 }
 
+/*
+ * Some commands like WRITE SAME have a payload or data transfer size which
+ * is different from the size of the request.  Any driver that supports such
+ * commands using the RQF_SPECIAL_PAYLOAD flag needs to use this helper to
+ * calculate the data transfer size.
+ */
+static inline unsigned int blk_rq_payload_bytes(struct request *rq)
+{
+   if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
+   return rq->special_vec.bv_len;
+   return blk_rq_bytes(rq);
+}
+
 static inline unsigned int blk_queue_get_max_sectors(struct request_queue *q,
 int op)
 {
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

fix data transfer size caculation for special payload requests

[Christoph Hellwig]

Hi all,

the first two fixes fix a regression in 4.10-rc1 where the data transfer
size for discard commands wasn't set properly, leading to hangs with
Hyper-V VMs.  The use the new data transfer size helper added in patch 1
more widely.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 191381] LIO ignores XCOPY source and destination descriptor IDs

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=191381

David Disseldorp  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |CODE_FIX

--- Comment #2 from David Disseldorp  ---
LIO XCOPY fixes merged to Linus' tree:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/?id=87156518da94a696f2b27ab8945d531af2f1d339=1

-- 
You are receiving this mail because:
You are the assignee for the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: bfa: Remove redundant memset before memcpy

[Shyam Saini]

The region set by the call to memset, immediately overwritten by the
subsequent call to memcpy and thus makes the  memset redundant

Signed-off-by: Shyam Saini 
---
 drivers/scsi/bfa/bfa_ioc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index a1ada4a..a3aa8d3 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -2763,7 +2763,6 @@ bfa_ioc_get_type(struct bfa_ioc_s *ioc)
 void
 bfa_ioc_get_adapter_serial_num(struct bfa_ioc_s *ioc, char *serial_num)
 {
-   memset((void *)serial_num, 0, BFA_ADAPTER_SERIAL_NUM_LEN);
memcpy((void *)serial_num,
(void *)ioc->attr->brcd_serialnum,
BFA_ADAPTER_SERIAL_NUM_LEN);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: megaraid: Remove redundant memset before memcpy

[Shyam Saini]

The region set by the call to memset, immediately overwritten by the
subsequent call to memcpy and thus makes the  memset redundant

Signed-off-by: Shyam Saini 
---
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c 
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 24778ba..32102f6 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -1406,8 +1406,6 @@ map_cmd_status(struct megasas_cmd_fusion *cmd, u8 status, 
u8 ext_status)
 
cmd->scmd->result = (DID_OK << 16) | ext_status;
if (ext_status == SAM_STAT_CHECK_CONDITION) {
-   memset(cmd->scmd->sense_buffer, 0,
-  SCSI_SENSE_BUFFERSIZE);
memcpy(cmd->scmd->sense_buffer, cmd->sense,
   SCSI_SENSE_BUFFERSIZE);
cmd->scmd->result |= DRIVER_SENSE << 24;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: qla4xxx: Remove redundant memset before memcpy

[Shyam Saini]

The region set by the call to memset, immediately overwritten by the
subsequent call to memcpy and thus makes the  memset redundant

Signed-off-by: Shyam Saini 
---
 drivers/scsi/qla4xxx/ql4_os.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 9fbb33f..21c1c78 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -619,7 +619,6 @@ static void qla4xxx_create_chap_list(struct scsi_qla_host 
*ha)
goto exit_chap_list;
}
 
-   memset(ha->chap_list, 0, chap_size);
memcpy(ha->chap_list, chap_flash_data, chap_size);
 
 exit_chap_list:
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: qla4xxx: Remove redundant memset before memcpy

[Shyam Saini]

The region set by the call to memset, immediately overwritten by the
subsequent call to memcpy and thus makes the  memset redundant

Signed-off-by: Shyam Saini 
---
 drivers/scsi/bfa/bfa_ioc.c  | 1 -
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 2 --
 drivers/scsi/qla4xxx/ql4_os.c   | 1 -
 3 files changed, 4 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index a1ada4a..a3aa8d3 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -2763,7 +2763,6 @@ bfa_ioc_get_type(struct bfa_ioc_s *ioc)
 void
 bfa_ioc_get_adapter_serial_num(struct bfa_ioc_s *ioc, char *serial_num)
 {
-   memset((void *)serial_num, 0, BFA_ADAPTER_SERIAL_NUM_LEN);
memcpy((void *)serial_num,
(void *)ioc->attr->brcd_serialnum,
BFA_ADAPTER_SERIAL_NUM_LEN);
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c 
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 24778ba..32102f6 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -1406,8 +1406,6 @@ map_cmd_status(struct megasas_cmd_fusion *cmd, u8 status, 
u8 ext_status)
 
cmd->scmd->result = (DID_OK << 16) | ext_status;
if (ext_status == SAM_STAT_CHECK_CONDITION) {
-   memset(cmd->scmd->sense_buffer, 0,
-  SCSI_SENSE_BUFFERSIZE);
memcpy(cmd->scmd->sense_buffer, cmd->sense,
   SCSI_SENSE_BUFFERSIZE);
cmd->scmd->result |= DRIVER_SENSE << 24;
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 9fbb33f..21c1c78 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -619,7 +619,6 @@ static void qla4xxx_create_chap_list(struct scsi_qla_host 
*ha)
goto exit_chap_list;
}
 
-   memset(ha->chap_list, 0, chap_size);
memcpy(ha->chap_list, chap_flash_data, chap_size);
 
 exit_chap_list:
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/15] dm: remove incomple BLOCK_PC support

[Christoph Hellwig]

On Thu, Jan 12, 2017 at 05:28:45PM -0500, Mike Snitzer wrote:
> What is "incomplete" about request-based DM's BLOCK_PC support?

BLOCK_PC requests are always aomething issued by the driver itself
(for a broad defintion of the driver, aka everything under the block
layer that works together is a driver for this purpose, e.g. all
of the SCSI subsystem).  If a driver doesn't issue BLOCK_PC
requests itself or through library functions only called from the
driver (e.g. scsi_cmd_ioctl) it is incomplete because it can't
be used.

> I'm also missing how you're saying the new blk-mq request-based DM will
> work with your new model.  I appreciate that we get the request from the
> underlying blk-mq request_queue and it'll be properly sized.  But
> wouldn't we need to pass data back up for these SCSI pass-through
> requests?  So wouldn't the top-level multipath request_queue need to
> setup cmd_size?

As said above - supporting BLOCK_PC for dm does not make sense, as
it's an internal passthrough mechanism for driver internal use.
It just happened we standardized it at the block layer because SCSI
commands are a standard supported by a few different drivers, e.g.
SCSI itself, the old ide code for ATAPI and CCISS and virtio_blk
which primarily are block drivers but allow some SCSI passthrough.

To be honest I'd love to just fold BLOCK_PC into the SCSI layer sooner
or later - the old IDE code and CCISS should die off sooner or later,
and virtio_blk scsi passthrough was a horrible idea to start with
(and I say that as the person who had the idea back then and implemented
it..)

> Sorry for the naive questions (that clearly speak to me not
> understanding how this aspect of the block and SCSI code work).. but I'd
> like to understand where DM will be lacking going forward.

At least in terms of BLOCK_PC nothing will change for dm, the code
was simply dead on arrival.  Maybe I should change the subject
to say that more clearly.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libfc: Fix variable name in fc_set_wwpn

[Johannes Thumshirn]

On Fri, Jan 13, 2017 at 11:40:01AM +0800, Fam Zheng wrote:
> The parameter name should be wwpn instead of wwnn.
> 
> Signed-off-by: Fam Zheng 
> ---

Yup, looks good
Acked-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html