RE: [PATCH v2 02/23] bfa: Do not call pci_enable_msix() after it failed once
Patch look good.
Acked-by: Anil Gurumurthy anil.gurumur...@qlogic.com
-Original Message-
From: linux-scsi-ow...@vger.kernel.org
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Alexander Gordeev
Sent: 24 February 2014 13:32
To: linux-kernel
Cc: Alexander Gordeev; Anil Gurumurthy; Vijaya Mohan Guvva; linux-scsi;
linux-pci
Subject: [PATCH v2 02/23] bfa: Do not call pci_enable_msix() after it failed
once
Function pci_enable_msix() should not be called in case it threw a negative
errno from a previous call.
Signed-off-by: Alexander Gordeev agord...@redhat.com
Cc: Anil Gurumurthy aguru...@brocade.com
Cc: Vijaya Mohan Guvva vmo...@brocade.com
Cc: linux-scsi@vger.kernel.org
Cc: linux-...@vger.kernel.org
---
drivers/scsi/bfa/bfad.c | 48 ++
1 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c index
cc0fbcd..972ff8d 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -1235,33 +1235,31 @@ bfad_setup_intr(struct bfad_s *bfad)
(bfa_asic_id_cb(pdev-device) !msix_disable_cb)) {
error = pci_enable_msix(bfad-pcidev, msix_entries, bfad-nvec);
- if (error) {
- /* In CT1 CT2, try to allocate just one vector */
- if (bfa_asic_id_ctc(pdev-device)) {
- printk(KERN_WARNING bfa %s: trying one msix
- vector failed to allocate %d[%d]\n,
- bfad-pci_name, bfad-nvec, error);
- bfad-nvec = 1;
- error = pci_enable_msix(bfad-pcidev,
+ /* In CT1 CT2, try to allocate just one vector */
+ if (error 0 bfa_asic_id_ctc(pdev-device)) {
+ printk(KERN_WARNING bfa %s: trying one msix
+ vector failed to allocate %d[%d]\n,
+ bfad-pci_name, bfad-nvec, error);
+ bfad-nvec = 1;
+ error = pci_enable_msix(bfad-pcidev,
msix_entries, bfad-nvec);
- }
+ }
- /*
-* Only error number of vector is available.
-* We don't have a mechanism to map multiple
-* interrupts into one vector, so even if we
-* can try to request less vectors, we don't
-* know how to associate interrupt events to
-* vectors. Linux doesn't duplicate vectors
-* in the MSIX table for this case.
-*/
- if (error) {
- printk(KERN_WARNING bfad%d:
- pci_enable_msix failed (%d),
- use line based.\n,
- bfad-inst_no, error);
- goto line_based;
- }
+ /*
+* Only error number of vector is available.
+* We don't have a mechanism to map multiple
+* interrupts into one vector, so even if we
+* can try to request less vectors, we don't
+* know how to associate interrupt events to
+* vectors. Linux doesn't duplicate vectors
+* in the MSIX table for this case.
+*/
+ if (error) {
+ printk(KERN_WARNING bfad%d:
+ pci_enable_msix failed (%d),
+ use line based.\n,
+ bfad-inst_no, error);
+ goto line_based;
}
/* Disable INTX in MSI-X mode */
--
1.7.7.6
--
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
This message and any attached documents contain information from QLogic
Corporation or its wholly-owned subsidiaries that may be confidential. If you
are not the intended recipient, you may not read, copy, distribute, or use this
information. If you have received this transmission in error, please notify the
sender immediately by reply e-mail and then delete this message.
attachment: winmail.dat
[PATCH/RESEND 4/4] megaraid_sas : Performance boost fixes
Please ignore last sent patch on performance boost with subject line - [PATCH
4/4] megaraid_sas : Performance boost fixes
Resending the performance boost patch with one change being reverted back-
Remove host lock for queuecommand for asynchronous IO submission
, Host lock is added back around queuecommand. Host lock removal can create
race conditon between ISR path(when RAID map update interrupt
is raised) and IO build path of driver, since IO build path is making use of
RAID map, and in case of RAID map update interrupt, old RAID
map copy is memset to zero, which some IOs may be referencing in build IO path.
Changes done for performance boost-
1) Added code to set SMP IRQ affinity per CPU.
2) Pass MSI-x index, while issuing sysPD IO.
Signed-off-by: Kashyap Desai kashyap.de...@lsi.com
Signed-off-by: Sumit Saxena sumit.sax...@lsi.com
---
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index 74cd884..d2faa61 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -4246,7 +4246,7 @@ fail_set_dma_mask:
static int megasas_probe_one(struct pci_dev *pdev,
const struct pci_device_id *id)
{
- int rval, pos, i, j;
+ int rval, pos, i, j, cpu;
struct Scsi_Host *host;
struct megasas_instance *instance;
u16 control = 0;
@@ -4419,7 +4419,8 @@ retry_irq_register:
* Register IRQ
*/
if (instance-msix_vectors) {
- for (i = 0 ; i instance-msix_vectors; i++) {
+ cpu = cpumask_first(cpu_online_mask);
+ for (i = 0; i instance-msix_vectors; i++) {
instance-irq_context[i].instance = instance;
instance-irq_context[i].MSIxIndex = i;
if (request_irq(instance-msixentry[i].vector,
@@ -4428,14 +4429,22 @@ retry_irq_register:
instance-irq_context[i])) {
printk(KERN_DEBUG megasas: Failed to
register IRQ for vector %d.\n, i);
- for (j = 0 ; j i ; j++)
+ for (j = 0; j i; j++) {
+ irq_set_affinity_hint(
+ instance-msixentry[j].vector,
NULL);
free_irq(
instance-msixentry[j].vector,
instance-irq_context[j]);
+ }
/* Retry irq register for IO_APIC */
instance-msix_vectors = 0;
goto retry_irq_register;
}
+ if (irq_set_affinity_hint(instance-msixentry[i].vector,
+ get_cpu_mask(cpu)))
+ dev_err(instance-pdev-dev, Error setting
+ affinity hint for cpu %d\n, cpu);
+ cpu = cpumask_next(cpu, cpu_online_mask);
}
} else {
instance-irq_context[0].instance = instance;
@@ -4489,9 +4498,12 @@ retry_irq_register:
instance-instancet-disable_intr(instance);
if (instance-msix_vectors)
- for (i = 0 ; i instance-msix_vectors; i++)
+ for (i = 0; i instance-msix_vectors; i++) {
+ irq_set_affinity_hint(
+ instance-msixentry[i].vector, NULL);
free_irq(instance-msixentry[i].vector,
instance-irq_context[i]);
+ }
else
free_irq(instance-pdev-irq, instance-irq_context[0]);
fail_irq:
@@ -4645,9 +4657,12 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
instance-instancet-disable_intr(instance);
if (instance-msix_vectors)
- for (i = 0 ; i instance-msix_vectors; i++)
+ for (i = 0; i instance-msix_vectors; i++) {
+ irq_set_affinity_hint(
+ instance-msixentry[i].vector, NULL);
free_irq(instance-msixentry[i].vector,
instance-irq_context[i]);
+ }
else
free_irq(instance-pdev-irq, instance-irq_context[0]);
if (instance-msix_vectors)
@@ -4668,7 +4683,7 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
static int
megasas_resume(struct pci_dev *pdev)
{
- int rval, i, j;
+ int rval, i, j, cpu;
struct Scsi_Host *host;
struct megasas_instance *instance;
@@ -4740,6 +4755,7 @@ megasas_resume(struct pci_dev *pdev)
* Register IRQ
*/
if (instance-msix_vectors) {
+ cpu =
RE: [PATCH v2 03/23] bfa: Cleanup bfad_setup_intr() function
Patch look good.
Acked-by: Anil Gurumurthy anil.gurumur...@qlogic.com
-Original Message-
From: linux-scsi-ow...@vger.kernel.org
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Alexander Gordeev
Sent: 24 February 2014 13:32
To: linux-kernel
Cc: Alexander Gordeev; Anil Gurumurthy; Vijaya Mohan Guvva; linux-scsi;
linux-pci
Subject: [PATCH v2 03/23] bfa: Cleanup bfad_setup_intr() function
Signed-off-by: Alexander Gordeev agord...@redhat.com
Cc: Anil Gurumurthy aguru...@brocade.com
Cc: Vijaya Mohan Guvva vmo...@brocade.com
Cc: linux-scsi@vger.kernel.org
Cc: linux-...@vger.kernel.org
---
drivers/scsi/bfa/bfad.c | 18 --
1 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c index
972ff8d..e7e4774 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -1219,7 +1219,7 @@ bfad_install_msix_handler(struct bfad_s *bfad) int
bfad_setup_intr(struct bfad_s *bfad) {
- int error = 0;
+ int error;
u32 mask = 0, i, num_bit = 0, max_bit = 0;
struct msix_entry msix_entries[MAX_MSIX_ENTRY];
struct pci_dev *pdev = bfad-pcidev;
@@ -1279,20 +1279,18 @@ bfad_setup_intr(struct bfad_s *bfad)
bfad-bfad_flags |= BFAD_MSIX_ON;
- return error;
+ return 0;
}
line_based:
- error = 0;
- if (request_irq
- (bfad-pcidev-irq, (irq_handler_t) bfad_intx, BFAD_IRQ_FLAGS,
-BFAD_DRIVER_NAME, bfad) != 0) {
- /* Enable interrupt handler failed */
- return 1;
- }
+ error = request_irq(bfad-pcidev-irq, (irq_handler_t)bfad_intx,
+ BFAD_IRQ_FLAGS, BFAD_DRIVER_NAME, bfad);
+ if (error)
+ return error;
+
bfad-bfad_flags |= BFAD_INTX_ON;
- return error;
+ return 0;
}
void
--
1.7.7.6
--
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
This message and any attached documents contain information from QLogic
Corporation or its wholly-owned subsidiaries that may be confidential. If you
are not the intended recipient, you may not read, copy, distribute, or use this
information. If you have received this transmission in error, please notify the
sender immediately by reply e-mail and then delete this message.
attachment: winmail.dat
Re: [PATCH 2/3] Add EVPD page 0x83 to sysfs
On 02/13/14 11:28, Hannes Reinecke wrote:
static ssize_t
-show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
char *buf)
+show_vpd_pg(const unsigned char *pg_buf, int pg_len, char *buf)
+{
+ int len = 0, i;
+
+ if (!pg_buf)
+ return -EINVAL;
+
+ len = 0;
+ for (i = 0; i pg_len; i += 16) {
+ hex_dump_to_buffer(pg_buf + i, pg_len, 16, 1,
+buf + len, PAGE_SIZE, false);
+ strcat(buf + len, \n);
+ len += strlen(buf + len);
+ }
+ return len;
+}
It might be a good idea to add the output buffer length as an argument
in show_vpd_pg() and to check explicitly whether or not there is
sufficient space left in the output buffer.
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 2/3] Add EVPD page 0x83 to sysfs
On 02/13/14 11:28, Hannes Reinecke wrote: - return 0; + return (buffer[2] 8) + buffer[3] + 4; Has it been considered to use get_unaligned_be16() instead of open coding this function ? 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: [PATCHv7 0/3][Resend] Display EVPD pages in sysfs
On 02/13/14 11:27, Hannes Reinecke wrote: After discussion with jejb I've dropped the EVPD parsing. So with this version we're just displaying the EVPD page 0x80 and 0x83 as hexdumps; no parsing is attempted. This drastically simplifies the patch, and we don't have to worry about any parsing errors in kernel space. Of course we'll need a parser in userspace, but that doesn't need to do any I/O. So it's still a very nice gain. A general comment about this patch series: I think the cached copies of these pages should be refreshed at least after an INQUIRY DATA HAS CHANGED unit attention code has been received. Some SCSI target implementations allow to change this data after a LUN has been created. 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: [PATCHv2 00/16] scsi_dh_alua updates
On 01/31/14 10:29, Hannes Reinecke wrote: here's an update for the ALUA device handler I've been hoarding for quite some time. The major bit here is the asynchronous RTPG handling. With the original design we would treat every LUN independently, despite the fact that several LUNs might in fact belong to the same target port group. So any change on one LUN will affect the others, too. And we now can treat LUNs in 'transitioning' ALUA mode correctly, as now we'll be blocking any I/O in the prep_fn() until the controller is in a working state again. Also for this patch series, I think the cached INQUIRY data should be refreshed at least after an INQUIRY DATA HAS CHANGED unit attention code has been received. 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 v1 11/13] SCSI/libiscsi: Add check_protection callback for transports
On 02/27/2014 05:13 AM, Sagi Grimberg wrote:
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 4046241..a58a6bb 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -395,6 +395,10 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task
*task)
if (rc)
return rc;
}
+
+ if (scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
+ task-protected = true;
+
if (sc-sc_data_direction == DMA_TO_DEVICE) {
unsigned out_len = scsi_out(sc)-length;
struct iscsi_r2t_info *r2t = task-unsol_r2t;
@@ -823,6 +827,33 @@ static void iscsi_scsi_cmd_rsp(struct iscsi_conn *conn,
struct iscsi_hdr *hdr,
sc-result = (DID_OK 16) | rhdr-cmd_status;
+ if (task-protected) {
+ sector_t sector;
+ u8 ascq;
+
+ /**
+ * Transports that didn't implement check_protection
+ * callback but still published T10-PI support to scsi-mid
+ * deserve this BUG_ON.
+ **/
+ BUG_ON(!session-tt-check_protection);
Extra space before BUG_ON.
+
+ ascq = session-tt-check_protection(task, sector);
+ if (ascq) {
+ sc-result = DRIVER_SENSE 24 | DID_ABORT 16 |
+ SAM_STAT_CHECK_CONDITION;
I am not sure what the reason for the DID_ABORT is here. I do not think
we want that, because we just want scsi-ml to evaluate the sense error
part of the failure. It works ok today, but the DID_ABORT error can
possibly be evaluated before the sense so you might miss passing that
info to upper layers.
+ scsi_build_sense_buffer(1, sc-sense_buffer,
+ ILLEGAL_REQUEST, 0x10, ascq);
+ sc-sense_buffer[7] = 0xc; /* Additional sense length */
+ sc-sense_buffer[8] = 0; /* Information desc type */
+ sc-sense_buffer[9] = 0xa; /* Additional desc length */
+ sc-sense_buffer[10] = 0x80; /* Validity bit */
+
+ put_unaligned_be64(sector, sc-sense_buffer[12]);
+ goto out;
+ }
+ }
+
--
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 v1 10/13] IB/iser: Support T10-PI operations
On 02/27/2014 05:13 AM, Sagi Grimberg wrote:
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c
b/drivers/infiniband/ulp/iser/iser_initiator.c
index 58e14c7..7fd95fe 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -62,6 +62,17 @@ static int iser_prepare_read_cmd(struct iscsi_task *task,
if (err)
return err;
+ if (scsi_prot_sg_count(iser_task-sc)) {
+ struct iser_data_buf *pbuf_in = iser_task-prot[ISER_DIR_IN];
+
+ err = iser_dma_map_task_data(iser_task,
+ pbuf_in,
+ ISER_DIR_IN,
+ DMA_FROM_DEVICE);
+ if (err)
+ return err;
+ }
+
if (edtl iser_task-data[ISER_DIR_IN].data_len) {
iser_err(Total data length: %ld, less than EDTL:
%d, in READ cmd BHS itt: %d, conn: 0x%p\n,
@@ -113,6 +124,17 @@ iser_prepare_write_cmd(struct iscsi_task *task,
if (err)
return err;
+ if (scsi_prot_sg_count(iser_task-sc)) {
+ struct iser_data_buf *pbuf_out = iser_task-prot[ISER_DIR_OUT];
+
+ err = iser_dma_map_task_data(iser_task,
+ pbuf_out,
+ ISER_DIR_OUT,
+ DMA_TO_DEVICE);
+ if (err)
+ return err;
+ }
+
The xmit_task callout does handle failures like EINVAL. If the above map
calls fail then you would get infinite retries. You would currently want
to do the mapping in the init_task callout instead.
If it makes it easier on the driver implementation then it is ok to
modify the xmit_task callers so that they handle multiple error codes
for drivers like iser that have the xmit_task callout called from
iscsi_queuecommand.
--
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 v12 1/4] PHY: Add function set_speed to generic PHY framework
Hi Felipe and Kishon,
This patch adds function set_speed to the generic PHY framework
operation
structure. This function can be called to instruct the PHY underlying
layer
at specified lane to configure for specified speed in hertz.
why ? looks like clk_set_rate() is your friend here. Can you be more
descriptive of the use case ? When will this be used ?
The phy_set_speed is used to configure the operation speed of the PHY
at run-time. The clock interface in general is used to configure the
clock input to the IP. I don't believe they are the same thing. Maybe
it will be clear in my response to your second email
The problem with this is that you end up adding SATA-specific details to
something which is supposed to be generic.
Setting the operation speed is pretty generic from an interface point
of view. An generic multi-purpose PHY can support multiple speed. If
no it's not. Specially when you consider that your speed argument can
be just about anything and depending on the underlying bus, it *will* be
treated differently. Note that e.g. in OMAP devices the exact *same* PHY
IP is used for PCIe, SATA and USB... now, let's assume for the sake of
argument that we were to implement -set_speed() in that environment,
how do you plan to do that ? a 6MHz arguments isn't valid from USB stand
point and could mean different things in PCIe or SATA.
Correct me if I am wrong here. If the same PHY is used for PCIe, SATA,
and USB, would you not need to indicate what speed the PHY should be
operated at - unless the underlying IP magically negotiate and
configured automatically? If so, what about PHY isn't so intelligent?
How should you suggest that we handle these?
the upper layer wish to operate at an specified speed (say for testing
purpose and etc), it can be allowed.
anything for testing purposes, should be limited to test scenarios.
Testing purpose is only one use case. Another use case is to limit the
speed so that I can confirm the driver actually works with various
speed of the device and handle correctly.
After negoatiation, don't you
get any interrupt from your PHY indicating that link speed negotiation
is done ? Or is that IRQ only on AHCI IP ?
There is NO interrupt from the PHY. The IRQ is assoicated with the
AHCI IP. With SATA host flow, it starts off with an COMRESET to start
the link negotiation. At that point, it will poll for completion.
Any other concerns?
hey, calm down... just trying to prevent us from applying something
which isn't truly generic and I don't think -set_speed is generic
enough. The semantics of the speed argument won't be valid for all use
cases.
I can already see people using that to pass
USB_SPEED_{LOW,FULL,HIGH,SUPER}, instead of actual speed numbers. We wil
end up with a mess to handle from a PHY driver, specially in cases such
as OMAP where, as mentioned above, the same IP is used for SATA, PCIe
and USB3.
This PHY isn't as intelligent as other PHY's. What would you suggest
as I need an method to indicate to the underlying PHY that I want to
operate at an specified speed?
Sorry if I am pinging you guys too fast here. I am look from an
solution and open to any solution in which it is acceptable for your
original intent of the generic PHY framework. I understand that you
don't believe set_speed is generic enough and may not apply to omap.
Or if you prefer we can try changing the init function to take an
initial MAX speed?
-Loc
--
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
