Re: [PATCH v2 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.

[Rangankar, Manish]

Hi Hannes,

Please find the response below,

On 11/11/16 10:13 PM, "Hannes Reinecke"  wrote:

>On 11/08/2016 07:57 AM, Manish Rangankar wrote:
>> The QLogic FastLinQ Driver for iSCSI (qedi) is the iSCSI specific module
>> for 41000 Series Converged Network Adapters by QLogic.
>>
>> This patch consists of following changes:
>>   - MAINTAINERS Makefile and Kconfig changes for qedi,
>>   - PCI driver registration,
>>   - iSCSI host level initialization,
>>   - Debugfs and log level infrastructure.
>>
>> Signed-off-by: Nilesh Javali 
>> Signed-off-by: Adheer Chandravanshi 
>> Signed-off-by: Chad Dupuis 
>> Signed-off-by: Saurav Kashyap 
>> Signed-off-by: Arun Easi 
>> Signed-off-by: Manish Rangankar 
>> ---
>>  MAINTAINERS |6 +
>>  drivers/net/ethernet/qlogic/Kconfig |   12 -
>>  drivers/scsi/Kconfig|1 +
>>  drivers/scsi/Makefile   |1 +
>>  drivers/scsi/qedi/Kconfig   |   10 +
>>  drivers/scsi/qedi/Makefile  |5 +
>>  drivers/scsi/qedi/qedi.h|  291 +++
>>  drivers/scsi/qedi/qedi_dbg.c|  143 
>>  drivers/scsi/qedi/qedi_dbg.h|  144 
>>  drivers/scsi/qedi/qedi_debugfs.c|  244 ++
>>  drivers/scsi/qedi/qedi_hsi.h|   52 ++
>>  drivers/scsi/qedi/qedi_main.c   | 1616
>>+++
>>  drivers/scsi/qedi/qedi_sysfs.c  |   52 ++
>>  drivers/scsi/qedi/qedi_version.h|   14 +
>>  14 files changed, 2579 insertions(+), 12 deletions(-)
>>  create mode 100644 drivers/scsi/qedi/Kconfig
>>  create mode 100644 drivers/scsi/qedi/Makefile
>>  create mode 100644 drivers/scsi/qedi/qedi.h
>>  create mode 100644 drivers/scsi/qedi/qedi_dbg.c
>>  create mode 100644 drivers/scsi/qedi/qedi_dbg.h
>>  create mode 100644 drivers/scsi/qedi/qedi_debugfs.c
>>  create mode 100644 drivers/scsi/qedi/qedi_hsi.h
>>  create mode 100644 drivers/scsi/qedi/qedi_main.c
>>  create mode 100644 drivers/scsi/qedi/qedi_sysfs.c
>>  create mode 100644 drivers/scsi/qedi/qedi_version.h
>>
[...]
>>
>> +static enum qed_int_mode qedi_int_mode_to_enum(void)
>> +{
>> +switch (int_mode) {
>> +case 0: return QED_INT_MODE_MSIX;
>> +case 1: return QED_INT_MODE_INTA;
>> +case 2: return QED_INT_MODE_MSI;
>> +default:
>> +QEDI_ERR(NULL, "Unknown qede_int_mode=%08x; "
>> + "Defaulting to MSI-x\n", int_mode);
>> +return QED_INT_MODE_MSIX;
>> +}
>> +}
>Errm. A per-driver interrupt mode?
>How very curious.
>You surely want to make that per-HBA, right?

This was added for testing purpose, we will remove this code.
 

[...]
>> +static int qedi_request_msix_irq(struct qedi_ctx *qedi)
>> +{
>> +int i, rc, cpu;
>> +
>> +cpu = cpumask_first(cpu_online_mask);
>> +for (i = 0; i < MIN_NUM_CPUS_MSIX(qedi); i++) {
>> +rc = request_irq(qedi->int_info.msix[i].vector,
>> + qedi_msix_handler, 0, "qedi",
>> + >fp_array[i]);
>> +
>> +if (rc) {
>> +QEDI_WARN(>dbg_ctx, "request_irq failed.\n");
>> +qedi_sync_free_irqs(qedi);
>> +return rc;
>> +}
>> +qedi->int_info.used_cnt++;
>> +rc = irq_set_affinity_hint(qedi->int_info.msix[i].vector,
>> +   get_cpu_mask(cpu));
>> +cpu = cpumask_next(cpu, cpu_online_mask);
>> +}
>> +
>> +return 0;
>> +}
>Please use the irq-affinity rework from Christoph here; that'll save you
>the additional msix vectors allocation.

The existing qed* driver(s) and common module (qed) framework is built on
top of the older pci_enable_msix_*() API. The new framework requires
re-work on the existing qed common module API. That would need
co-ordination among other dependent drivers (e.g.: qede network driver,
which is already in the tree). We would prefer to add this as a follow on
(to the initial submission) effort, with additional testing done and
submission co-ordinated across protocol drivers.



Thanks,
Manish

--
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] blk-mq drivers: untangle 0 and BLK_MQ_RQ_QUEUE_OK

[Jens Axboe]

On 11/14/2016 04:49 PM, Omar Sandoval wrote:

From: Omar Sandoval 

Let's not depend on any of the BLK_MQ_RQ_QUEUE_* constants having
specific values. No functional change.

Signed-off-by: Omar Sandoval 
---
Hi, Jens,

Some more trivial cleanup, feel free to apply or not if it's too intrusive.


I think we should clean it up. But you should split this patch in two -
one for NVMe, and one for SCSI.

And it should be against for-4.10/block, the nvme parts don't apply
without fuzz.

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

[PATCH] blk-mq drivers: untangle 0 and BLK_MQ_RQ_QUEUE_OK

[Omar Sandoval]

From: Omar Sandoval 

Let's not depend on any of the BLK_MQ_RQ_QUEUE_* constants having
specific values. No functional change.

Signed-off-by: Omar Sandoval 
---
Hi, Jens,

Some more trivial cleanup, feel free to apply or not if it's too intrusive.

 drivers/nvme/host/core.c   | 4 ++--
 drivers/nvme/host/pci.c| 8 
 drivers/nvme/host/rdma.c   | 2 +-
 drivers/nvme/target/loop.c | 6 +++---
 drivers/scsi/scsi_lib.c| 6 +++---
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 79e679d..e7f9ef5 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -276,7 +276,7 @@ static inline int nvme_setup_discard(struct nvme_ns *ns, 
struct request *req,
 */
req->__data_len = nr_bytes;
 
-   return 0;
+   return BLK_MQ_RQ_QUEUE_OK;
 }
 
 static inline void nvme_setup_rw(struct nvme_ns *ns, struct request *req,
@@ -324,7 +324,7 @@ static inline void nvme_setup_rw(struct nvme_ns *ns, struct 
request *req,
 int nvme_setup_cmd(struct nvme_ns *ns, struct request *req,
struct nvme_command *cmd)
 {
-   int ret = 0;
+   int ret = BLK_MQ_RQ_QUEUE_OK;
 
if (req->cmd_type == REQ_TYPE_DRV_PRIV)
memcpy(cmd, req->cmd, sizeof(*cmd));
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 0248d0e..4afa2f7 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -328,7 +328,7 @@ static int nvme_init_iod(struct request *rq, unsigned size,
rq->retries = 0;
rq->cmd_flags |= REQ_DONTPREP;
}
-   return 0;
+   return BLK_MQ_RQ_QUEUE_OK;
 }
 
 static void nvme_free_iod(struct nvme_dev *dev, struct request *req)
@@ -598,17 +598,17 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
 
map_len = nvme_map_len(req);
ret = nvme_init_iod(req, map_len, dev);
-   if (ret)
+   if (ret != BLK_MQ_RQ_QUEUE_OK)
return ret;
 
ret = nvme_setup_cmd(ns, req, );
-   if (ret)
+   if (ret != BLK_MQ_RQ_QUEUE_OK)
goto out;
 
if (req->nr_phys_segments)
ret = nvme_map_data(dev, req, map_len, );
 
-   if (ret)
+   if (ret != BLK_MQ_RQ_QUEUE_OK)
goto out;
 
cmnd.common.command_id = req->tag;
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 5a83881..125c43b 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1399,7 +1399,7 @@ static int nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
sizeof(struct nvme_command), DMA_TO_DEVICE);
 
ret = nvme_setup_cmd(ns, rq, c);
-   if (ret)
+   if (ret != BLK_MQ_RQ_QUEUE_OK)
return ret;
 
c->common.command_id = rq->tag;
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index d5df77d..01035fb 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -168,7 +168,7 @@ static int nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx,
int ret;
 
ret = nvme_setup_cmd(ns, req, >cmd);
-   if (ret)
+   if (ret != BLK_MQ_RQ_QUEUE_OK)
return ret;
 
iod->cmd.common.flags |= NVME_CMD_SGL_METABUF;
@@ -178,7 +178,7 @@ static int nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx,
nvme_cleanup_cmd(req);
blk_mq_start_request(req);
nvme_loop_queue_response(>req);
-   return 0;
+   return BLK_MQ_RQ_QUEUE_OK;
}
 
if (blk_rq_bytes(req)) {
@@ -197,7 +197,7 @@ static int nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx,
blk_mq_start_request(req);
 
schedule_work(>work);
-   return 0;
+   return BLK_MQ_RQ_QUEUE_OK;
 }
 
 static void nvme_loop_submit_async_event(struct nvme_ctrl *arg, int aer_idx)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 2cca9cf..13887c0 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1801,7 +1801,7 @@ static inline int prep_to_mq(int ret)
 {
switch (ret) {
case BLKPREP_OK:
-   return 0;
+   return BLK_MQ_RQ_QUEUE_OK;
case BLKPREP_DEFER:
return BLK_MQ_RQ_QUEUE_BUSY;
default:
@@ -1888,7 +1888,7 @@ static int scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
int reason;
 
ret = prep_to_mq(scsi_prep_state_check(sdev, req));
-   if (ret)
+   if (ret != BLK_MQ_RQ_QUEUE_OK)
goto out;
 
ret = BLK_MQ_RQ_QUEUE_BUSY;
@@ -1905,7 +1905,7 @@ static int scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
 
if (!(req->cmd_flags & REQ_DONTPREP)) {
ret = prep_to_mq(scsi_mq_prep_fn(req));
-   if (ret)
+   if (ret != BLK_MQ_RQ_QUEUE_OK)
goto out_dec_host_busy;
req->cmd_flags |= REQ_DONTPREP;
} else {
-- 
2.10.2

--
To unsubscribe 

Re: [PATCH] fnic: remove a pointless test

[Martin K. Petersen]

> "Tomas" == Tomas Henzl  writes:

Tomas> rport can't be null here, it would have failed already in
Tomas> fc_remote_port_chkready

Cisco folks: Please review!

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 00/11] hisi_sas: some fixes, improvements, and new features

[Martin K. Petersen]

> "John" == John Garry  writes:

John> This patchset introduces some misc bug fixes, improvements, and
John> new features to the HiSilicon SAS driver.

This series needs review.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] megaraid-sas: request irqs later

[Martin K. Petersen]

> "Tomas" == Tomas Henzl  writes:

Tomas> It is not good when an irq arrives before driver structures are
Tomas> allocated.

Sumit, Kashyap: Please review!

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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: qed, qedi patchset submission

[Martin K. Petersen]

> "Arun" == Arun Easi  writes:

Arun,

Arun> Do you have any preference or thoughts on how the "qed" patches be
Arun> approached? Just as a reference, our rdma driver "qedr" went
Arun> through something similar[1], and eventually "qed" patches were
Arun> taken by David in the net tree and "qedr", in the rdma tree
Arun> (obviously) by Doug L.

DaveM can queue the whole series or I can hold the SCSI pieces for
rc1. Either way works for me.

However, I would like to do a review of the driver first. I will try to
get it done this week.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] hpsa: correct logical resets

[Martin K. Petersen]

> "Don" == Don Brace  writes:

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 0b6eb5a..a296537 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -300,6 +300,10 @@ static bool hpsa_cmd_dev_match(struct ctlr_info *h, struct 
CommandList *c,
   struct hpsa_scsi_dev_t *dev,
   unsigned char *scsi3addr);
 
+static int wait_for_device_to_become_ready(struct ctlr_info *h,
+  unsigned char lunaddr[],
+  int reply_queue);
+
 static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev)
 {
unsigned long *priv = shost_priv(sdev->host);

Wouldn't it be nicer to put this with the rest of the function
prototypes at the beginning of the file?

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] qla2xxx: do not abort all commands in the adapter during EEH recovery

[Martin K. Petersen]

> "Mauricio" == Mauricio Faria de Oliveira  
> writes:

Mauricio> The previous commit ("qla2xxx: fix invalid DMA access after
Mauricio> command aborts in PCI device remove") introduced a regression
Mauricio> during an EEH recovery, since the change to the
Mauricio> qla2x00_abort_all_cmds() function calls qla2xxx_eh_abort(),
Mauricio> which verifies the EEH recovery condition but handles it
Mauricio> heavy-handed. (commit a465537ad1a4 "qla2xxx: Disable the
Mauricio> adapter and skip error recovery in case of register
Mauricio> disconnect.")

Applied to 4.9/scsi-fixes.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] scsi: hpsa: free irq on q indexed by h->intr_mode and not i

[Martin K. Petersen]

> "Colin" == Colin King  writes:

Colin> From: Colin Ian King  Use correct index
Colin> on q, use h->intr_mode instead of i. Issue detected using static
Colin> analysis with cppcheck

Applied to 4.10/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] isci: fix typo in deg_dbg message

[Martin K. Petersen]

> "Colin" == Colin King  writes:

Colin> From: Colin Ian King  Trivial fix to
Colin> typo "repsonse" to "response" in dev_dbg message.

Applied to 4.10/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] iscsi: fix spelling mistakes in dev_warn messages

[Martin K. Petersen]

> "Colin" == Colin King  writes:

Colin> From: Colin Ian King  Trivial fix to
Colin> spelling mistake "suspeneded" to "suspended" in dev_warn messages

Applied to 4.10/scsi-queue with s/iscsi/isci/.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] [SCSI] mpt3sas: Fix secure erase premature termination (v4)

[Martin K. Petersen]

> "Andrey" == Andrey Grodzovsky  writes:

Andrey,

Andrey> Regarding older code where there is still a separate mpt2sas
Andrey> driver, should a separate patch to be done or this fix will be
Andrey> ported there ?

Feel free to submit a mpt2sas patch to the pre-4.4 stable trees.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] scsi: megaraid_sas: add in missing white spaces in error messages text

[Martin K. Petersen]

> "Colin" == Colin King  writes:

Colin> A couple of dev_printk messages spans two lines and the literal
Colin> string is missing a white space between words. Add the white
Colin> space.

Applied to 4.10/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] scsi_transport_fc: Hold queue lock while calling blk_run_queue_async()

[Martin K. Petersen]

> "Bart" == Bart Van Assche  writes:

Bart> It is required to hold the queue lock when calling
Bart> blk_run_queue_async() to avoid that a race between
Bart> blk_run_queue_async() and blk_cleanup_queue() is
Bart> triggered. Additionally, remove the get_device() and put_device()
Bart> calls from fc_bsg_goose_queue. It is namely the responsibility of
Bart> the caller of fc_bsg_goose_queue() to ensure that the bsg queue
Bart> does not disappear while fc_bsg_goose_queue() is in progress.

Applied to 4.10/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] isci: fix typo in deg_dbg message

[Bart Van Assche]

On 11/12/2016 10:30 AM, Colin King wrote:

Trivial fix to typo "repsonse" to "response" in dev_dbg message.


Reviewed-by: Bart Van Assche 
--
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] scsi: megaraid_sas: add in missing white spaces in error messages text

[Bart Van Assche]

On 11/12/2016 08:25 AM, Colin King wrote:

A couple of dev_printk messages spans two lines and the literal string
is missing a white space between words. Add the white space.


Reviewed-by: Bart Van Assche 
--
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] iscsi: fix spelling mistakes in dev_warn messages

[Bart Van Assche]

On 11/12/2016 08:49 AM, Colin King wrote:

Trivial fix to spelling mistake "suspeneded" to "suspended" in
dev_warn messages


Reviewed-by: Bart Van Assche 
--
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

qed, qedi patchset submission

[Arun Easi]

Hi Martin, David,

This is regarding the submission of the recent patch series we have posted
to linux-scsi and netdev:

[PATCH v2 0/6] Add QLogic FastLinQ iSCSI (qedi) driver.
[PATCH v2 1/6] qed: Add support for hardware offloaded iSCSI.
[PATCH v2 2/6] qed: Add iSCSI out of order packet handling.
[PATCH v2 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.
[PATCH v2 4/6] qedi: Add LL2 iSCSI interface for offload iSCSI.
[PATCH v2 5/6] qedi: Add support for iSCSI session management.
[PATCH v2 6/6] qedi: Add support for data path.

Patches 1 & 2 are "qed" module patches that goes under
drivers/net/ethernet/qlogic/qed/ and include/linux/qed/ directory.
- These are the iSCSI enablement changes to the common "qed"
  module. There is no dependency for these patches and so
  can go independently.

Patches 3, 4, 5 & 6 are the qedi patches that is aimed towards
drivers/scsi/qedi/ directory.
- These are the core qedi changes and is dependent on the
  qed changes (invokes qed_XXX functions).

As qed sits in the net tree, the patches are usually submitted via netdev.

Do you have any preference or thoughts on how the "qed" patches be 
approached? Just as a reference, our rdma driver "qedr" went through 
something similar[1], and eventually "qed" patches were taken by David 
in the net tree and "qedr", in the rdma tree (obviously) by Doug L.

Hi David,

For the "qed" enablement sent with the v2 series, we did not prefix the 
qed patches with "[PATCH net-next]" prefix, so netdev folks may have 
failed to notice/review that, sorry about that. We will send the next (v3) 
series with that corrected.

Right now, we are basing the "qed" patches on top of latest net + net-next 
tree. FYI, I tried a test merge of net-next/master + qed patches with 
"net/master" and I see no conflict in qed.

Regards,
-Arun

[1] http://marc.info/?l=linux-rdma=147509152719831=2
--
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] qla2xxx: do not abort all commands in the adapter during EEH recovery

[Mauricio Faria de Oliveira]

The previous commit ("qla2xxx: fix invalid DMA access after command
aborts in PCI device remove") introduced a regression during an EEH
recovery, since the change to the qla2x00_abort_all_cmds() function
calls qla2xxx_eh_abort(), which verifies the EEH recovery condition
but handles it heavy-handed. (commit a465537ad1a4 "qla2xxx: Disable
the adapter and skip error recovery in case of register disconnect.")

This problem warrants a more general/optimistic solution right into
qla2xxx_eh_abort()  (eg in case a real command abort arrives during
EEH recovery, or if it takes long enough to trigger command aborts);
but it's still worth to add a check to ensure the code added by the
previous commit is correct and contained within its owner function.

This commit just adds a 'if (!ha->flags.eeh_busy)' check around it.
(ahem; a trivial fix for this -rc series; sorry for this oversight.)

With it applied, both PCI device remove and EEH recovery works fine.

Fixes: 1535aa75a3d8 ("scsi: qla2xxx: fix invalid DMA access after
command aborts in PCI device remove")
Signed-off-by: Mauricio Faria de Oliveira 
---
 drivers/scsi/qla2xxx/qla_os.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 567fa080e261..56d6142852a5 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1456,15 +1456,20 @@ uint32_t qla2x00_isp_reg_stat(struct qla_hw_data *ha)
for (cnt = 1; cnt < req->num_outstanding_cmds; cnt++) {
sp = req->outstanding_cmds[cnt];
if (sp) {
-   /* Get a reference to the sp and drop the lock.
-* The reference ensures this sp->done() call
-* - and not the call in qla2xxx_eh_abort() -
-* ends the SCSI command (with result 'res').
+   /* Don't abort commands in adapter during EEH
+* recovery as it's not accessible/responding.
 */
-   sp_get(sp);
-   spin_unlock_irqrestore(>hardware_lock, 
flags);
-   qla2xxx_eh_abort(GET_CMD_SP(sp));
-   spin_lock_irqsave(>hardware_lock, flags);
+   if (!ha->flags.eeh_busy) {
+   /* Get a reference to the sp and drop 
the lock.
+* The reference ensures this 
sp->done() call
+* - and not the call in 
qla2xxx_eh_abort() -
+* ends the SCSI command (with result 
'res').
+*/
+   sp_get(sp);
+   
spin_unlock_irqrestore(>hardware_lock, flags);
+   qla2xxx_eh_abort(GET_CMD_SP(sp));
+   spin_lock_irqsave(>hardware_lock, 
flags);
+   }
req->outstanding_cmds[cnt] = NULL;
sp->done(vha, sp, res);
}
-- 
1.8.3.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: [PATCH] sd_zbc: Force use of READ16/WRITE16

[Jens Axboe]

On 11/10/2016 10:53 PM, Damien Le Moal wrote:

Normally, sd_read_capacity sets sdp->use_16_for_rw to 1 based on the
disk capacity so that READ16/WRITE16 are used for large drives.
However, for a zoned disk with RC_BASIS set to 0, the capacity reported
through READ_CAPACITY may be very small, leading to use_16_for_rw not being
set and READ10/WRITE10 commands being used, even after the actual zoned disk
capacity is corrected in sd_zbc_read_zones. This causes LBA offset overflow for
accesses beyond 2TB.

As the ZBC standard makes it mandatory for ZBC drives to support
the READ16/WRITE16 commands anyway, make sure that use_16_for_rw is set.


Added to the 4.10 branch, 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

[PATCH 1/2] blk-mq: Fix failed allocation path when mapping queues

[Gabriel Krisman Bertazi]

In blk_mq_map_swqueue, there is a memory optimization that frees the
tags of a queue that has gone unmapped.  Later, if that hctx is remapped
after another topology change, the tags need to be reallocated.

If this allocation fails, a simple WARN_ON triggers, but the block layer
ends up with an active hctx without any corresponding set of tags.
Then, any income IO to that hctx can trigger an Oops.

I can reproduce it consistently by running IO, flipping CPUs on and off
and eventually injecting a memory allocation failure in that path.

In the fix below, if the system experiences a failed allocation of any
hctx's tags, we remap all the ctxs of that queue to the hctx_0, which
should always keep it's tags.  There is a minor performance hit, since
our mapping just got worse after the error path, but this is
the simplest solution to handle this error path.  The performance hit
will disappear after another successful remap.

I considered dropping the memory optimization all together, but it
seemed a bad trade-off to handle this very specific error case.

My one concern about this patch is if remapping an arbitrary queue to
hctx_0 could result in outstanding requests getting submitted to the
wrong hctx.  I couldn't observe this happening during tests, but I'm not
entirely sure it'll never happen.  I believe the queue will be empty if
we are trying to allocate tags for it, unless it was using another alive
hctx queue and for some reason got reassigned to this new hctx.  is this
possible at all?

This should apply cleanly on top of Jen's for-next branch.

The Oops is the one below:

SP (3fff935ce4d0) is in userspace
1:mon> e
cpu 0x1: Vector: 300 (Data Access) at [c00fe99eb110]
pc: c05e868c: __sbitmap_queue_get+0x2c/0x180
lr: c0575328: __bt_get+0x48/0xd0
sp: c00fe99eb390
   msr: 90010280b033
   dar: 28
 dsisr: 4000
  current = 0xc00fe9966800
  paca= 0xc7e80300   softe: 0irq_happened: 0x01
pid   = 11035, comm = aio-stress
Linux version 4.8.0-rc6+ (root@bean) (gcc version 5.4.0 20160609
(Ubuntu/IBM 5.4.0-6ubuntu1~16.04.2) ) #3 SMP Mon Oct 10 20:16:53 CDT 2016
1:mon> s
[c00fe99eb3d0] c0575328 __bt_get+0x48/0xd0
[c00fe99eb400] c0575838 bt_get.isra.1+0x78/0x2d0
[c00fe99eb480] c0575cb4 blk_mq_get_tag+0x44/0x100
[c00fe99eb4b0] c056f6f4 __blk_mq_alloc_request+0x44/0x220
[c00fe99eb500] c0570050 blk_mq_map_request+0x100/0x1f0
[c00fe99eb580] c0574650 blk_mq_make_request+0xf0/0x540
[c00fe99eb640] c0561c44 generic_make_request+0x144/0x230
[c00fe99eb690] c0561e00 submit_bio+0xd0/0x200
[c00fe99eb740] c03ef740 ext4_io_submit+0x90/0xb0
[c00fe99eb770] c03e95d8 ext4_writepages+0x588/0xdd0
[c00fe99eb910] c025a9f0 do_writepages+0x60/0xc0
[c00fe99eb940] c0246c88 __filemap_fdatawrite_range+0xf8/0x180
[c00fe99eb9e0] c0246f90 filemap_write_and_wait_range+0x70/0xf0
[c00fe99eba20] c03dd844 ext4_sync_file+0x214/0x540
[c00fe99eba80] c0364718 vfs_fsync_range+0x78/0x130
[c00fe99ebad0] c03dd46c ext4_file_write_iter+0x35c/0x430
[c00fe99ebb90] c038c280 aio_run_iocb+0x3b0/0x450
[c00fe99ebce0] c038dc28 do_io_submit+0x368/0x730
[c00fe99ebe30] c0009404 system_call+0x38/0xec

Signed-off-by: Gabriel Krisman Bertazi 
Cc: Brian King 
Cc: Douglas Miller 
Cc: linux-bl...@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
 block/blk-mq.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 3c5aaead71bb..7f7c4ba91adf 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1862,7 +1862,7 @@ static void blk_mq_init_cpu_queues(struct request_queue 
*q,
 static void blk_mq_map_swqueue(struct request_queue *q,
   const struct cpumask *online_mask)
 {
-   unsigned int i;
+   unsigned int i, hctx_idx;
struct blk_mq_hw_ctx *hctx;
struct blk_mq_ctx *ctx;
struct blk_mq_tag_set *set = q->tag_set;
@@ -1885,6 +1885,15 @@ static void blk_mq_map_swqueue(struct request_queue *q,
if (!cpumask_test_cpu(i, online_mask))
continue;
 
+   hctx_idx = q->mq_map[i];
+   /* unmapped hw queue can be remapped after CPU topo changed */
+   if (!set->tags[hctx_idx]) {
+   set->tags[hctx_idx] = blk_mq_init_rq_map(set, hctx_idx);
+
+   if (!set->tags[hctx_idx])
+   q->mq_map[i] = 0;
+   }
+
ctx = per_cpu_ptr(q->queue_ctx, i);
hctx = blk_mq_map_queue(q, i);
 
@@ -1901,7 +1910,10 @@ static void blk_mq_map_swqueue(struct request_queue *q,
 * disable it and free the request entries.
  

[PATCH 2/2] blk-mq: Avoid memory reclaim when remapping queues

[Gabriel Krisman Bertazi]

While stressing memory and IO at the same time we changed SMT settings,
we were able to consistently trigger deadlocks in the mm system, which
froze the entire machine.

I think that under memory stress conditions, the large allocations
performed by blk_mq_init_rq_map may trigger a reclaim, which stalls
waiting on the block layer remmaping completion, thus deadlocking the
system.  The trace below was collected after the machine stalled,
waiting for the hotplug event completion.

The simplest fix for this is to make allocations in this path
non-reclaimable, with GFP_NOWAIT.  With this patch, We couldn't hit the
issue anymore.

This should apply on top of Jen's for-next branch cleanly.

 Call Trace:
[c00f0160aaf0] [c00f0160ab50] 0xc00f0160ab50 (unreliable)
[c00f0160acc0] [c0016624] __switch_to+0x2e4/0x430
[c00f0160ad20] [c0b1a880] __schedule+0x310/0x9b0
[c00f0160ae00] [c0b1af68] schedule+0x48/0xc0
[c00f0160ae30] [c0b1b4b0] schedule_preempt_disabled+0x20/0x30
[c00f0160ae50] [c0b1d4fc] __mutex_lock_slowpath+0xec/0x1f0
[c00f0160aed0] [c0b1d678] mutex_lock+0x78/0xa0
[c00f0160af00] [d00019413cac] xfs_reclaim_inodes_ag+0x33c/0x380 [xfs]
[c00f0160b0b0] [d00019415164] xfs_reclaim_inodes_nr+0x54/0x70 [xfs]
[c00f0160b0f0] [d000194297f8] xfs_fs_free_cached_objects+0x38/0x60 [xfs]
[c00f0160b120] [c03172c8] super_cache_scan+0x1f8/0x210
[c00f0160b190] [c026301c] shrink_slab.part.13+0x21c/0x4c0
[c00f0160b2d0] [c0268088] shrink_zone+0x2d8/0x3c0
[c00f0160b380] [c026834c] do_try_to_free_pages+0x1dc/0x520
[c00f0160b450] [c026876c] try_to_free_pages+0xdc/0x250
[c00f0160b4e0] [c0251978] __alloc_pages_nodemask+0x868/0x10d0
[c00f0160b6f0] [c0567030] blk_mq_init_rq_map+0x160/0x380
[c00f0160b7a0] [c056758c] blk_mq_map_swqueue+0x33c/0x360
[c00f0160b820] [c0567904] blk_mq_queue_reinit+0x64/0xb0
[c00f0160b850] [c056a16c] blk_mq_queue_reinit_notify+0x19c/0x250
[c00f0160b8a0] [c00f5d38] notifier_call_chain+0x98/0x100
[c00f0160b8f0] [c00c5fb0] __cpu_notify+0x70/0xe0
[c00f0160b930] [c00c63c4] notify_prepare+0x44/0xb0
[c00f0160b9b0] [c00c52f4] cpuhp_invoke_callback+0x84/0x250
[c00f0160ba10] [c00c570c] cpuhp_up_callbacks+0x5c/0x120
[c00f0160ba60] [c00c7cb8] _cpu_up+0xf8/0x1d0
[c00f0160bac0] [c00c7eb0] do_cpu_up+0x120/0x150
[c00f0160bb40] [c06fe024] cpu_subsys_online+0x64/0xe0
[c00f0160bb90] [c06f5124] device_online+0xb4/0x120
[c00f0160bbd0] [c06f5244] online_store+0xb4/0xc0
[c00f0160bc20] [c06f0a68] dev_attr_store+0x68/0xa0
[c00f0160bc60] [c03ccc30] sysfs_kf_write+0x80/0xb0
[c00f0160bca0] [c03cbabc] kernfs_fop_write+0x17c/0x250
[c00f0160bcf0] [c030fe6c] __vfs_write+0x6c/0x1e0
[c00f0160bd90] [c0311490] vfs_write+0xd0/0x270
[c00f0160bde0] [c03131fc] SyS_write+0x6c/0x110
[c00f0160be30] [c0009204] system_call+0x38/0xec

Signed-off-by: Gabriel Krisman Bertazi 
Cc: Brian King 
Cc: Douglas Miller 
Cc: linux-bl...@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
 block/blk-mq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 7f7c4ba91adf..3e44303646cb 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1597,7 +1597,7 @@ static struct blk_mq_tags *blk_mq_init_rq_map(struct 
blk_mq_tag_set *set,
INIT_LIST_HEAD(>page_list);
 
tags->rqs = kzalloc_node(set->queue_depth * sizeof(struct request *),
-GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY,
+GFP_NOWAIT | __GFP_NOWARN | __GFP_NORETRY,
 set->numa_node);
if (!tags->rqs) {
blk_mq_free_tags(tags);
@@ -1623,7 +1623,7 @@ static struct blk_mq_tags *blk_mq_init_rq_map(struct 
blk_mq_tag_set *set,
 
do {
page = alloc_pages_node(set->numa_node,
-   GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY | 
__GFP_ZERO,
+   GFP_NOWAIT | __GFP_NOWARN | __GFP_NORETRY | 
__GFP_ZERO,
this_order);
if (page)
break;
@@ -1644,7 +1644,7 @@ static struct blk_mq_tags *blk_mq_init_rq_map(struct 
blk_mq_tag_set *set,
 * Allow kmemleak to scan these pages as they contain pointers
 * to additional allocations like via ops->init_request().
 */
-   kmemleak_alloc(p, order_to_size(this_order), 1, GFP_KERNEL);
+   kmemleak_alloc(p, order_to_size(this_order), 1, GFP_NOWAIT);

Re: [PATCH 0/2] sd: Fix a deadlock between event checking and device removal

[Bart Van Assche]

On 11/12/2016 09:47 PM, James Bottomley wrote:

On Fri, 2016-11-11 at 16:38 -0800, Bart Van Assche wrote:

Hello James and Martin,

This short patch series fixes a deadlock that I ran into for the
first time several months ago but for which I had not yet had the
time to post a fix. As usual, feedback is appreciated.


First question would be why do we need to push highly dm specific
knowledge into block, like REQ_FAIL_IF_NO_PATH.  Can't we just use
REQ_FAILFAST_X for this?

Also, I don't quite understand how you've configured multipath
underneath SCSI.  I thought dm-mp always went on top?


Hello James,

dm-multipath was running on top of SCSI in my setup which means that at 
least the patch description is wrong. Since it was some time ago that I 
ran into this deadlock, I will check first whether I can still reproduce it.


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] sd_zbc: Force use of READ16/WRITE16

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig 
--
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] scsi_transport_fc: Hold queue lock while calling blk_run_queue_async()

[James Smart]

Reviewed-By: James Smart 

(Bart: fyi - note my change in email address)

-- james


On 11/11/2016 4:55 PM, Bart Van Assche wrote:

It is required to hold the queue lock when calling blk_run_queue_async()
to avoid that a race between blk_run_queue_async() and
blk_cleanup_queue() is triggered. Additionally, remove the get_device()
and put_device() calls from fc_bsg_goose_queue. It is namely the
responsibility of the caller of fc_bsg_goose_queue() to ensure that
the bsg queue does not disappear while fc_bsg_goose_queue() is in
progress.

Signed-off-by: Bart Van Assche 
Cc: Hannes Reinecke 
Cc: James Smart 
---
  drivers/scsi/scsi_transport_fc.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 0f3a386..a32ab8e 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -3855,15 +3855,15 @@ fc_bsg_host_dispatch(struct request_queue *q, struct 
Scsi_Host *shost,
  static void
  fc_bsg_goose_queue(struct fc_rport *rport)
  {
-   if (!rport->rqst_q)
+   struct request_queue *q = rport->rqst_q;
+   unsigned long flags;
+
+   if (!q)
return;
  
-	/*

-* This get/put dance makes no sense
-*/
-   get_device(>dev);
-   blk_run_queue_async(rport->rqst_q);
-   put_device(>dev);
+   spin_lock_irqsave(q->queue_lock, flags);
+   blk_run_queue_async(q);
+   spin_unlock_irqrestore(q->queue_lock, flags);
  }
  
  /**


--
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] scsi: hpsa: free irq on q indexed by h->intr_mode and not i

[Christoph Hellwig]

Looks fine, I was actually about to send the same fix..

Reviewed-by: Christoph Hellwig 
--
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: hpsa: free irq on q indexed by h->intr_mode and not i

[Colin King]

From: Colin Ian King 

Use correct index on q, use h->intr_mode instead of i. Issue
detected using static analysis with cppcheck

Fixes: bc2bb1543e62a5d0 ("scsi: hpsa: use pci_alloc_irq_vectors and automatic 
irq affinity")
Signed-off-by: Colin Ian King 
---
 drivers/scsi/hpsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 9459925..0d4f21c 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -8220,7 +8220,7 @@ static void hpsa_free_irqs(struct ctlr_info *h)
 
if (!h->msix_vectors || h->intr_mode != PERF_MODE_INT) {
/* Single reply queue, only one irq to free */
-   free_irq(pci_irq_vector(h->pdev, 0), >q[i]);
+   free_irq(pci_irq_vector(h->pdev, 0), >q[h->intr_mode]);
h->q[h->intr_mode] = 0;
return;
}
-- 
2.10.2

--
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/5] megaraid_sas: switch to pci_alloc_irq_vectors

[Christoph Hellwig]

>   if (instance->msix_vectors)
>   for (i = 0; i < instance->msix_vectors; i++) {
> + free_irq(pci_irq_vector(instance->pdev, i),
>>irq_context[i]);
>   }
>   else
> + free_irq(pci_irq_vector(instance->pdev, 0),
> +  >irq_context[0]);
>  }

Don't forget to replace the call to pci_disable_msix with one to 
pci_free_irq_vectors.

>  
>  /**
> @@ -5018,6 +5004,7 @@ static int megasas_init_fw(struct megasas_instance 
> *instance)
>   int i, loop, fw_msix_count = 0;
>   struct IOV_111 *iovPtr;
>   struct fusion_context *fusion;
> + int irq_flags = PCI_IRQ_MSIX;
>  
>   fusion = instance->ctrl_context;
>  
> @@ -5134,15 +5121,18 @@ static int megasas_init_fw(struct megasas_instance 
> *instance)
>   /* Don't bother allocating more MSI-X vectors than cpus */
>   instance->msix_vectors = min(instance->msix_vectors,
>(unsigned int)num_online_cpus());
> - for (i = 0; i < instance->msix_vectors; i++)
> - instance->msixentry[i].entry = i;
> - i = pci_enable_msix_range(instance->pdev, instance->msixentry,
> -   1, instance->msix_vectors);
> + if (smp_affinity_enable)
> + irq_flags |= PCI_IRQ_AFFINITY;
> + i = pci_alloc_irq_vectors(instance->pdev, 1,
> +   instance->msix_vectors, irq_flags);
>   if (i > 0)
>   instance->msix_vectors = i;
>   else
>   instance->msix_vectors = 0;
>   }
> + i = pci_alloc_irq_vectors(instance->pdev, 1, 1, PCI_IRQ_LEGACY);
> + if (i < 0)
> + goto fail_setup_irqs;

This looks wrong - you have to call to pci_alloc_irq_vectors right next
to each other here, so for the MSI-X case you'll still end up calling
the second one as well, which will then fail.  I think the better way
to structure the driver would be to do the following here.

 - Rename the msix_vectors field to irq_vectors, and make sure it's
   initialized to 1 for non-MSI-X capable adapters.
 - Have a single call to pci_alloc_irq_vectors here.
 - Have all the request_irq/free_irq code iterate over ->irq_vectors
   instead of duplicating it
 - use pdev->msix_enabled for the few cases that need to check for
   MSI-X mode specificly.

>   /* Now re-enable MSI-X */
> - if (instance->msix_vectors &&
> - pci_enable_msix_exact(instance->pdev, instance->msixentry,
> -   instance->msix_vectors))
> + if (instance->msix_vectors)
> + irq_flags = PCI_IRQ_MSIX;
> + if (smp_affinity_enable)
> + irq_flags |= PCI_IRQ_AFFINITY;
> + rval = pci_alloc_irq_vectors(instance->pdev, 1,
> +  instance->msix_vectors ?
> +  instance->msix_vectors : 1, irq_flags);
> + if (rval < 0)

The old code is doing a pci_enable_msix_exact so you also need to pass
the number of vectors as the first argument here.
--
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/5] megaraid_sas: switch to pci_alloc_irq_vectors

[Christoph Hellwig]

On Fri, Nov 11, 2016 at 03:59:05PM +0100, Hannes Reinecke wrote:
> You are right; irq affinity only makes sense for MSI-X.
> I'll be fixing it up.

It works for multi-MSI and MSI-X.  And even for single-MSI or
INTx it's harmless as it will be ignored if only a single
vector is present.
--
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/5] megaraid_sas: scsi-mq support

[Kashyap Desai]

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Hannes Reinecke
> Sent: Friday, November 11, 2016 3:15 PM
> To: Martin K. Petersen
> Cc: Christoph Hellwig; James Bottomley; Sumit Saxena; linux-
> s...@vger.kernel.org; Hannes Reinecke; Hannes Reinecke
> Subject: [PATCH 4/5] megaraid_sas: scsi-mq support
>
> This patch enables full scsi multiqueue support. But as I have been
seeing
> performance regressions I've also added a module parameter 'use_blk_mq',
> allowing multiqueue to be switched off if required.

Hannes - As discussed for hpsa driver related thread for similar support,
I don't think this code changes are helping MR as well.

>
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c   | 22 +
>  drivers/scsi/megaraid/megaraid_sas_fusion.c | 38
+
> 
>  2 files changed, 50 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index d580406..1af47e6 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -48,6 +48,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -104,6 +105,9 @@
>  module_param(scmd_timeout, int, S_IRUGO);
> MODULE_PARM_DESC(scmd_timeout, "scsi command timeout (10-90s), default
> 90s. See megasas_reset_timer.");
>
> +bool use_blk_mq = true;
> +module_param_named(use_blk_mq, use_blk_mq, bool, 0);
> +
>  MODULE_LICENSE("GPL");
>  MODULE_VERSION(MEGASAS_VERSION);
>  MODULE_AUTHOR("megaraidlinux@avagotech.com");
> @@ -1882,6 +1886,17 @@ static void megasas_slave_destroy(struct
> scsi_device *sdev)
>   sdev->hostdata = NULL;
>  }
>
> +static int megasas_map_queues(struct Scsi_Host *shost) {
> + struct megasas_instance *instance = (struct megasas_instance *)
> + shost->hostdata;
> +
> + if (!instance->ctrl_context)
> + return 0;
> +
> + return blk_mq_pci_map_queues(>tag_set, instance->pdev, 0);
}
> +
>  /*
>  * megasas_complete_outstanding_ioctls - Complete outstanding ioctls
after a
>  *   kill adapter
> @@ -2986,6 +3001,7 @@ struct device_attribute *megaraid_host_attrs[] = {
>   .slave_configure = megasas_slave_configure,
>   .slave_alloc = megasas_slave_alloc,
>   .slave_destroy = megasas_slave_destroy,
> + .map_queues = megasas_map_queues,
>   .queuecommand = megasas_queue_command,
>   .eh_target_reset_handler = megasas_reset_target,
>   .eh_abort_handler = megasas_task_abort, @@ -5610,6 +5626,12 @@
> static int megasas_io_attach(struct megasas_instance *instance)
>   host->max_id = MEGASAS_MAX_DEV_PER_CHANNEL;
>   host->max_lun = MEGASAS_MAX_LUN;
>   host->max_cmd_len = 16;
> + host->nr_hw_queues = instance->msix_vectors ?
> + instance->msix_vectors : 1;
> + if (use_blk_mq) {
> + host->can_queue = instance->max_scsi_cmds / host-
> >nr_hw_queues;
> + host->use_blk_mq = 1;
> + }
>
>   /*
>* Notify the mid-layer about the new controller diff --git
> a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> index eb3cb0f..aba53c0 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> @@ -1703,6 +1703,7 @@ static int megasas_create_sg_sense_fusion(struct
> megasas_instance *instance)
>   struct IO_REQUEST_INFO io_info;
>   struct fusion_context *fusion;
>   struct MR_DRV_RAID_MAP_ALL *local_map_ptr;
> + u16 msix_index;
>   u8 *raidLUN;
>
>   device_id = MEGASAS_DEV_INDEX(scp);
> @@ -1792,11 +1793,13 @@ static int megasas_create_sg_sense_fusion(struct
> megasas_instance *instance)
>   fp_possible = io_info.fpOkForIo;
>   }
>
> - /* Use raw_smp_processor_id() for now until cmd->request->cpu is
CPU
> -id by default, not CPU group id, otherwise all MSI-X queues
won't
> -be utilized */
> - cmd->request_desc->SCSIIO.MSIxIndex = instance->msix_vectors ?
> - raw_smp_processor_id() % instance->msix_vectors : 0;
> + if (shost_use_blk_mq(instance->host)) {
> + u32 blk_tag = blk_mq_unique_tag(scp->request);
> + msix_index = blk_mq_unique_tag_to_hwq(blk_tag);
> + } else
> + msix_index = instance->msix_vectors ?
> + raw_smp_processor_id() % instance->msix_vectors :
0;
> + cmd->request_desc->SCSIIO.MSIxIndex = msix_index;
>
>   if (fp_possible) {
>   megasas_set_pd_lba(io_request, scp->cmd_len, _info,
scp,
> @@ -1971,6 +1974,7 @@ static void megasas_build_ld_nonrw_fusion(struct
> megasas_instance *instance,
>   struct RAID_CONTEXT *pRAID_Context;
>   struct MR_PD_CFG_SEQ_NUM_SYNC *pd_sync;
>   struct 

RE: [PATCH] hpsa: scsi-mq support

[Kashyap Desai]

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Christoph Hellwig
> Sent: Sunday, November 13, 2016 5:29 PM
> To: Hannes Reinecke
> Cc: Christoph Hellwig; Hannes Reinecke; Christoph Hellwig; Martin K.
Petersen;
> James Bottomley; Don Brace; linux-scsi@vger.kernel.org; Jens Axboe
> Subject: Re: [PATCH] hpsa: scsi-mq support
>
> On Sun, Nov 13, 2016 at 10:44:47AM +0100, Hannes Reinecke wrote:
> > One day to mark with bright red in the calendar.
> >
> > Christoph Hellwig is telling me _NOT_ to use scsi-mq.
>
> That's not what I'm doing.
>
> > This patch was done so see what would needed to be done to convert a
> > legacy driver.
> > As I was under the impression that scsi-mq is the way forward, seeing
> > that it should be enabled per default.
> > But I must have been mistaken. Apparently.
>
> What I am doing is to tell you you should not expose multiple queues
unless the
> hardware actually has multiple submissions queues.  The blk-mq and
scsi-mq
> code works just fine with a single submission queue, and the hpsa driver
in
> particular works really well with scsi-mq and a single submission queue.
E.g. the
> RAID HBA on slide 19 of this presentations is an hpsa one:

I have similar results for MegaRaid where seen MR driver gives significant
improvement for Single Submission Queue and multiple Completion Queue.
Having said that scsi-mq is enabled but with single Queue is more than
enough to maximize improvement of SCSI-MQ.

Major advantage was seen while IO load is cross the boundary of Physical
CPU socket.

>From this discussion I understood that - Similar logical changes proposed
for megaraid_sas  and we are not really going to gain with fake multiple
submission exposed to SML.

Kashyap

>
> http://events.linuxfoundation.org/sites/events/files/slides/scsi.pdf
> --
> 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
--
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