* [PATCH V4 1/9] aacraid: SCSI blk tag support
2016-01-29 22:45 [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
@ 2016-01-29 22:45 ` Raghava Aditya Renukunta
2016-01-29 22:45 ` [PATCH V4 2/9] aacraid: Fix RRQ overload Raghava Aditya Renukunta
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-01-29 22:45 UTC (permalink / raw)
To: James.Bottomley, martin.petersen, linux-scsi
Cc: Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, thenzl, shane.seymour,
RaghavaAditya.Renukunta
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
The method to allocate and free FIB's in the present code utilizes
spinlocks.Multiple IO's have to wait on the spinlock to acquire or
free fibs creating a performance bottleneck.
An alternative solution would be to use block layer tags to keep track
of the fibs allocated and freed. To this end aac_fib_alloc_tag was
created to utilize the blk layer tags to plug into the Fib pool.These
functions are used exclusively in the IO path. 8 fibs are reserved for
the use of AIF management software and utilize the previous spinlock based
implementations.
Changes in V2:
Removed aac_fib_free_tag since it was a stub function
Moved population of fib fields that are constant to aac_fib_setup
Changes in V3:
None
Changes in V4:
None
Signed-off-by: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@pmcs.com>
Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
---
drivers/scsi/aacraid/aachba.c | 27 ++++++++++++---------------
drivers/scsi/aacraid/aacraid.h | 1 +
drivers/scsi/aacraid/commsup.c | 32 +++++++++++++++++++++++++++++---
drivers/scsi/aacraid/dpcsup.c | 2 --
drivers/scsi/aacraid/linit.c | 2 ++
5 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index e4c2437..7dfd0fa 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -323,7 +323,6 @@ static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
if (unlikely(!scsicmd || !scsicmd->scsi_done)) {
dprintk((KERN_WARNING "aac_valid_context: scsi command corrupt\n"));
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
return 0;
}
scsicmd->SCp.phase = AAC_OWNER_MIDLEVEL;
@@ -331,7 +330,6 @@ static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
if (unlikely(!device || !scsi_device_online(device))) {
dprintk((KERN_WARNING "aac_valid_context: scsi device corrupt\n"));
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
return 0;
}
return 1;
@@ -541,7 +539,6 @@ static void get_container_name_callback(void *context, struct fib * fibptr)
scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
scsicmd->scsi_done(scsicmd);
}
@@ -557,7 +554,8 @@ static int aac_get_container_name(struct scsi_cmnd * scsicmd)
dev = (struct aac_dev *)scsicmd->device->host->hostdata;
- if (!(cmd_fibcontext = aac_fib_alloc(dev)))
+ cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+ if (!cmd_fibcontext)
return -ENOMEM;
aac_fib_init(cmd_fibcontext);
@@ -586,7 +584,6 @@ static int aac_get_container_name(struct scsi_cmnd * scsicmd)
printk(KERN_WARNING "aac_get_container_name: aac_fib_send failed with status: %d.\n", status);
aac_fib_complete(cmd_fibcontext);
- aac_fib_free(cmd_fibcontext);
return -1;
}
@@ -1024,7 +1021,6 @@ static void get_container_serial_callback(void *context, struct fib * fibptr)
scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
scsicmd->scsi_done(scsicmd);
}
@@ -1040,7 +1036,8 @@ static int aac_get_container_serial(struct scsi_cmnd * scsicmd)
dev = (struct aac_dev *)scsicmd->device->host->hostdata;
- if (!(cmd_fibcontext = aac_fib_alloc(dev)))
+ cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+ if (!cmd_fibcontext)
return -ENOMEM;
aac_fib_init(cmd_fibcontext);
@@ -1068,7 +1065,6 @@ static int aac_get_container_serial(struct scsi_cmnd * scsicmd)
printk(KERN_WARNING "aac_get_container_serial: aac_fib_send failed with status: %d.\n", status);
aac_fib_complete(cmd_fibcontext);
- aac_fib_free(cmd_fibcontext);
return -1;
}
@@ -1869,7 +1865,6 @@ static void io_callback(void *context, struct fib * fibptr)
break;
}
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
scsicmd->scsi_done(scsicmd);
}
@@ -1954,7 +1949,8 @@ static int aac_read(struct scsi_cmnd * scsicmd)
/*
* Alocate and initialize a Fib
*/
- if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
+ cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+ if (!cmd_fibcontext) {
printk(KERN_WARNING "aac_read: fib allocation failed\n");
return -1;
}
@@ -2051,7 +2047,8 @@ static int aac_write(struct scsi_cmnd * scsicmd)
/*
* Allocate and initialize a Fib then setup a BlockWrite command
*/
- if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
+ cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+ if (!cmd_fibcontext) {
/* FIB temporarily unavailable,not catastrophic failure */
/* scsicmd->result = DID_ERROR << 16;
@@ -2285,7 +2282,7 @@ static int aac_start_stop(struct scsi_cmnd *scsicmd)
/*
* Allocate and initialize a Fib
*/
- cmd_fibcontext = aac_fib_alloc(aac);
+ cmd_fibcontext = aac_fib_alloc_tag(aac, scsicmd);
if (!cmd_fibcontext)
return SCSI_MLQUEUE_HOST_BUSY;
@@ -3157,7 +3154,6 @@ static void aac_srb_callback(void *context, struct fib * fibptr)
scsicmd->result |= le32_to_cpu(srbreply->scsi_status);
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
scsicmd->scsi_done(scsicmd);
}
@@ -3187,9 +3183,10 @@ static int aac_send_srb_fib(struct scsi_cmnd* scsicmd)
/*
* Allocate and initialize a Fib then setup a BlockWrite command
*/
- if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
+ cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+ if (!cmd_fibcontext)
return -1;
- }
+
status = aac_adapter_scsi(cmd_fibcontext, scsicmd);
/*
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 074878b..f51f0a0 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2114,6 +2114,7 @@ int aac_acquire_irq(struct aac_dev *dev);
void aac_free_irq(struct aac_dev *dev);
const char *aac_driverinfo(struct Scsi_Host *);
struct fib *aac_fib_alloc(struct aac_dev *dev);
+struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd);
int aac_fib_setup(struct aac_dev *dev);
void aac_fib_map_free(struct aac_dev *dev);
void aac_fib_free(struct fib * context);
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index a1f90fe..46a2a2f 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -137,6 +137,7 @@ int aac_fib_setup(struct aac_dev * dev)
i++, fibptr++)
{
fibptr->flags = 0;
+ fibptr->size = sizeof(struct fib);
fibptr->dev = dev;
fibptr->hw_fib_va = hw_fib;
fibptr->data = (void *) fibptr->hw_fib_va->data;
@@ -156,13 +157,38 @@ int aac_fib_setup(struct aac_dev * dev)
*/
dev->fibs[dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB - 1].next = NULL;
/*
- * Enable this to debug out of queue space
- */
- dev->free_fib = &dev->fibs[0];
+ * Set 8 fibs aside for management tools
+ */
+ dev->free_fib = &dev->fibs[dev->scsi_host_ptr->can_queue];
return 0;
}
/**
+ * aac_fib_alloc_tag-allocate a fib using tags
+ * @dev: Adapter to allocate the fib for
+ *
+ * Allocate a fib from the adapter fib pool using tags
+ * from the blk layer.
+ */
+
+struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd)
+{
+ struct fib *fibptr;
+
+ fibptr = &dev->fibs[scmd->request->tag];
+ /*
+ * Null out fields that depend on being zero at the start of
+ * each I/O
+ */
+ fibptr->hw_fib_va->header.XferState = 0;
+ fibptr->type = FSAFS_NTC_FIB_CONTEXT;
+ fibptr->callback_data = NULL;
+ fibptr->callback = NULL;
+
+ return fibptr;
+}
+
+/**
* aac_fib_alloc - allocate a fib
* @dev: Adapter to allocate the fib for
*
diff --git a/drivers/scsi/aacraid/dpcsup.c b/drivers/scsi/aacraid/dpcsup.c
index da9d993..d677b52 100644
--- a/drivers/scsi/aacraid/dpcsup.c
+++ b/drivers/scsi/aacraid/dpcsup.c
@@ -394,7 +394,6 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
fib->callback(fib->callback_data, fib);
} else {
aac_fib_complete(fib);
- aac_fib_free(fib);
}
} else {
unsigned long flagv;
@@ -416,7 +415,6 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
fib->done = 0;
spin_unlock_irqrestore(&fib->event_lock, flagv);
aac_fib_complete(fib);
- aac_fib_free(fib);
}
}
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 76eaa38..129a515 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -454,6 +454,8 @@ static int aac_slave_configure(struct scsi_device *sdev)
} else
scsi_change_queue_depth(sdev, 1);
+ sdev->tagged_supported = 1;
+
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH V4 2/9] aacraid: Fix RRQ overload
2016-01-29 22:45 [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
2016-01-29 22:45 ` [PATCH V4 1/9] aacraid: SCSI blk tag support Raghava Aditya Renukunta
@ 2016-01-29 22:45 ` Raghava Aditya Renukunta
2016-01-29 22:45 ` [PATCH V4 3/9] aacraid: Added EEH support Raghava Aditya Renukunta
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-01-29 22:45 UTC (permalink / raw)
To: James.Bottomley, martin.petersen, linux-scsi
Cc: Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, thenzl, shane.seymour,
RaghavaAditya.Renukunta
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
The driver utilizes an array of atomic variables to keep track of
IO submissions to each vector. To submit an IO multiple threads
iterate through the array to find a vector which has empty slots
to send an IO. The reading and updating of the variable is not atomic,
causing race conditions when a thread uses a full vector to
submit an IO.
Fixed by mapping each FIB to a vector, the submission path then uses
said vector to submit IO thereby removing the possibly of a race
condition.The vector assignment is started from 1 since vector 0 is
reserved for the use of AIF management FIBS.If the number of MSIx
vectors is 1 (MSI or INTx mode) then all the fibs are allocated to
vector 0.
Changes in V2:
Created function aac_fib_vector_assign to assign vector numbers to
fib and to prevent code duplication
Changes in V3:
None
Changes in V4:
None
Fixes: 495c0217 "aacraid: MSI-x support"
Cc: stable@vger.kernel.org # v4.1
Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
---
drivers/scsi/aacraid/aacraid.h | 2 ++
drivers/scsi/aacraid/commsup.c | 28 ++++++++++++++++++++++++++++
drivers/scsi/aacraid/src.c | 30 +++++++-----------------------
3 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index f51f0a0..fff1306 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -944,6 +944,7 @@ struct fib {
*/
struct list_head fiblink;
void *data;
+ u32 vector_no;
struct hw_fib *hw_fib_va; /* Actual shared object */
dma_addr_t hw_fib_pa; /* physical address of hw_fib*/
};
@@ -2113,6 +2114,7 @@ static inline unsigned int cap_to_cyls(sector_t capacity, unsigned divisor)
int aac_acquire_irq(struct aac_dev *dev);
void aac_free_irq(struct aac_dev *dev);
const char *aac_driverinfo(struct Scsi_Host *);
+void aac_fib_vector_assign(struct aac_dev *dev);
struct fib *aac_fib_alloc(struct aac_dev *dev);
struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd);
int aac_fib_setup(struct aac_dev *dev);
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 46a2a2f..07a42a3 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -90,6 +90,28 @@ void aac_fib_map_free(struct aac_dev *dev)
dev->hw_fib_pa = 0;
}
+void aac_fib_vector_assign(struct aac_dev *dev)
+{
+ u32 i = 0;
+ u32 vector = 1;
+ struct fib *fibptr = NULL;
+
+ for (i = 0, fibptr = &dev->fibs[i];
+ i < (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB);
+ i++, fibptr++) {
+ if ((dev->max_msix == 1) ||
+ (i > ((dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB - 1)
+ - dev->vector_cap))) {
+ fibptr->vector_no = 0;
+ } else {
+ fibptr->vector_no = vector;
+ vector++;
+ if (vector == dev->max_msix)
+ vector = 1;
+ }
+ }
+}
+
/**
* aac_fib_setup - setup the fibs
* @dev: Adapter to set up
@@ -152,6 +174,12 @@ int aac_fib_setup(struct aac_dev * dev)
hw_fib_pa = hw_fib_pa +
dev->max_fib_size + sizeof(struct aac_fib_xporthdr);
}
+
+ /*
+ *Assign vector numbers to fibs
+ */
+ aac_fib_vector_assign(dev);
+
/*
* Add the fib chain to the free list
*/
diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 2aa34ea..bc0203f 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -156,8 +156,8 @@ static irqreturn_t aac_src_intr_message(int irq, void *dev_id)
break;
if (dev->msi_enabled && dev->max_msix > 1)
atomic_dec(&dev->rrq_outstanding[vector_no]);
- aac_intr_normal(dev, handle-1, 0, isFastResponse, NULL);
dev->host_rrq[index++] = 0;
+ aac_intr_normal(dev, handle-1, 0, isFastResponse, NULL);
if (index == (vector_no + 1) * dev->vector_cap)
index = vector_no * dev->vector_cap;
dev->host_rrq_idx[vector_no] = index;
@@ -452,36 +452,20 @@ static int aac_src_deliver_message(struct fib *fib)
#endif
u16 hdr_size = le16_to_cpu(fib->hw_fib_va->header.Size);
+ u16 vector_no;
atomic_inc(&q->numpending);
if (dev->msi_enabled && fib->hw_fib_va->header.Command != AifRequest &&
dev->max_msix > 1) {
- u_int16_t vector_no, first_choice = 0xffff;
-
- vector_no = dev->fibs_pushed_no % dev->max_msix;
- do {
- vector_no += 1;
- if (vector_no == dev->max_msix)
- vector_no = 1;
- if (atomic_read(&dev->rrq_outstanding[vector_no]) <
- dev->vector_cap)
- break;
- if (0xffff == first_choice)
- first_choice = vector_no;
- else if (vector_no == first_choice)
- break;
- } while (1);
- if (vector_no == first_choice)
- vector_no = 0;
- atomic_inc(&dev->rrq_outstanding[vector_no]);
- if (dev->fibs_pushed_no == 0xffffffff)
- dev->fibs_pushed_no = 0;
- else
- dev->fibs_pushed_no++;
+ vector_no = fib->vector_no;
fib->hw_fib_va->header.Handle += (vector_no << 16);
+ } else {
+ vector_no = 0;
}
+ atomic_inc(&dev->rrq_outstanding[vector_no]);
+
if (dev->comm_interface == AAC_COMM_MESSAGE_TYPE2) {
/* Calculate the amount to the fibsize bits */
fibsize = (hdr_size + 127) / 128 - 1;
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH V4 3/9] aacraid: Added EEH support
2016-01-29 22:45 [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
2016-01-29 22:45 ` [PATCH V4 1/9] aacraid: SCSI blk tag support Raghava Aditya Renukunta
2016-01-29 22:45 ` [PATCH V4 2/9] aacraid: Fix RRQ overload Raghava Aditya Renukunta
@ 2016-01-29 22:45 ` Raghava Aditya Renukunta
2016-01-30 22:52 ` kbuild test robot
2016-01-29 22:45 ` [PATCH V4 4/9] aacraid: Fix memory leak in aac_fib_map_free Raghava Aditya Renukunta
` (6 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-01-29 22:45 UTC (permalink / raw)
To: James.Bottomley, martin.petersen, linux-scsi
Cc: Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, thenzl, shane.seymour,
RaghavaAditya.Renukunta
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Added support for PCI EEH(extended error handling).
Changes in V2:
Made local functions static
Removed call to aac_fib_free_tag
Set adapter_shutdown flag when PCI error detected
Changes in V3:
None
Changes in V4:
Removed setting of adapter_shutdown flag when \
PCI error detected
Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
drivers/scsi/aacraid/aacraid.h | 1 +
drivers/scsi/aacraid/linit.c | 137 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 138 insertions(+)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index fff1306..2916288 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1235,6 +1235,7 @@ struct aac_dev
struct msix_entry msixentry[AAC_MAX_MSIX];
struct aac_msix_ctx aac_msix[AAC_MAX_MSIX]; /* context */
u8 adapter_shutdown;
+ u32 handle_pci_error;
};
#define aac_adapter_interrupt(dev) \
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 129a515..62ac7be 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -38,6 +38,7 @@
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/pci.h>
+#include <linux/aer.h>
#include <linux/pci-aspm.h>
#include <linux/slab.h>
#include <linux/mutex.h>
@@ -1298,6 +1299,9 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
goto out_deinit;
scsi_scan_host(shost);
+ pci_enable_pcie_error_reporting(pdev);
+ pci_save_state(pdev);
+
return 0;
out_deinit:
@@ -1501,6 +1505,138 @@ static void aac_remove_one(struct pci_dev *pdev)
}
}
+static void aac_flush_ios(struct aac_dev *aac)
+{
+ int i;
+ struct scsi_cmnd *cmd;
+
+ for (i = 0; i < aac->scsi_host_ptr->can_queue; i++) {
+ cmd = (struct scsi_cmnd *)aac->fibs[i].callback_data;
+ if (cmd && (cmd->SCp.phase == AAC_OWNER_FIRMWARE)) {
+ scsi_dma_unmap(cmd);
+
+ if (aac->handle_pci_error)
+ cmd->result = DID_NO_CONNECT << 16;
+ else
+ cmd->result = DID_RESET << 16;
+
+ cmd->scsi_done(cmd);
+ }
+ }
+}
+
+static pci_ers_result_t aac_pci_error_detected(struct pci_dev *pdev,
+ enum pci_channel_state error)
+{
+ struct Scsi_Host *shost = pci_get_drvdata(pdev);
+ struct aac_dev *aac = shost_priv(shost);
+
+ dev_err(&pdev->dev, "aacraid: PCI error detected %x\n", error);
+
+ switch (error) {
+ case pci_channel_io_normal:
+ return PCI_ERS_RESULT_CAN_RECOVER;
+ case pci_channel_io_frozen:
+ aac->handle_pci_error = 1;
+
+ scsi_block_requests(aac->scsi_host_ptr);
+ aac_flush_ios(aac);
+ aac_release_resources(aac);
+
+ pci_disable_pcie_error_reporting(pdev);
+ aac_adapter_ioremap(aac, 0);
+
+ return PCI_ERS_RESULT_NEED_RESET;
+ case pci_channel_io_perm_failure:
+ aac->handle_pci_error = 1;
+
+ aac_flush_ios(aac);
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+
+ return PCI_ERS_RESULT_NEED_RESET;
+}
+
+static pci_ers_result_t aac_pci_mmio_enabled(struct pci_dev *pdev)
+{
+ dev_err(&pdev->dev, "aacraid: PCI error - mmio enabled\n");
+ return PCI_ERS_RESULT_NEED_RESET;
+}
+
+static pci_ers_result_t aac_pci_slot_reset(struct pci_dev *pdev)
+{
+ dev_err(&pdev->dev, "aacraid: PCI error - slot reset\n");
+ pci_restore_state(pdev);
+ if (pci_enable_device(pdev)) {
+ dev_warn(&pdev->dev,
+ "aacraid: failed to enable slave\n");
+ goto fail_device;
+ }
+
+ pci_set_master(pdev);
+
+ if (pci_enable_device_mem(pdev)) {
+ dev_err(&pdev->dev, "pci_enable_device_mem failed\n");
+ goto fail_device;
+ }
+
+ return PCI_ERS_RESULT_RECOVERED;
+
+fail_device:
+ dev_err(&pdev->dev, "aacraid: PCI error - slot reset failed\n");
+ return PCI_ERS_RESULT_DISCONNECT;
+}
+
+
+static void aac_pci_resume(struct pci_dev *pdev)
+{
+ struct Scsi_Host *shost = pci_get_drvdata(pdev);
+ struct scsi_device *sdev = NULL;
+ struct aac_dev *aac = (struct aac_dev *)shost_priv(shost);
+
+ pci_cleanup_aer_uncorrect_error_status(pdev);
+
+ if (aac_adapter_ioremap(aac, aac->base_size)) {
+
+ dev_err(&pdev->dev, "aacraid: ioremap failed\n");
+ /* remap failed, go back ... */
+ aac->comm_interface = AAC_COMM_PRODUCER;
+ if (aac_adapter_ioremap(aac, AAC_MIN_FOOTPRINT_SIZE)) {
+ dev_warn(&pdev->dev,
+ "aacraid: unable to map adapter.\n");
+
+ return;
+ }
+ }
+
+ msleep(10000);
+
+ aac_acquire_resources(aac);
+
+ /*
+ * reset this flag to unblock ioctl() as it was set
+ * at aac_send_shutdown() to block ioctls from upperlayer
+ */
+ aac->adapter_shutdown = 0;
+ aac->handle_pci_error = 0;
+
+ shost_for_each_device(sdev, shost)
+ if (sdev->sdev_state == SDEV_OFFLINE)
+ sdev->sdev_state = SDEV_RUNNING;
+ scsi_unblock_requests(aac->scsi_host_ptr);
+ scsi_scan_host(aac->scsi_host_ptr);
+ pci_save_state(pdev);
+
+ dev_err(&pdev->dev, "aacraid: PCI error - resume\n");
+}
+
+static struct pci_error_handlers aac_pci_err_handler = {
+ .error_detected = aac_pci_error_detected,
+ .mmio_enabled = aac_pci_mmio_enabled,
+ .slot_reset = aac_pci_slot_reset,
+ .resume = aac_pci_resume,
+};
+
static struct pci_driver aac_pci_driver = {
.name = AAC_DRIVERNAME,
.id_table = aac_pci_tbl,
@@ -1511,6 +1647,7 @@ static struct pci_driver aac_pci_driver = {
.resume = aac_resume,
#endif
.shutdown = aac_shutdown,
+ .err_handler = &aac_pci_err_handler,
};
static int __init aac_init(void)
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH V4 3/9] aacraid: Added EEH support
2016-01-29 22:45 ` [PATCH V4 3/9] aacraid: Added EEH support Raghava Aditya Renukunta
@ 2016-01-30 22:52 ` kbuild test robot
0 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2016-01-30 22:52 UTC (permalink / raw)
Cc: kbuild-all, James.Bottomley, martin.petersen, linux-scsi,
Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, thenzl, shane.seymour,
RaghavaAditya.Renukunta
[-- Attachment #1: Type: text/plain, Size: 3808 bytes --]
Hi Raghava,
[auto build test ERROR on scsi/for-next]
[also build test ERROR on v4.5-rc1 next-20160129]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Raghava-Aditya-Renukunta/aacraid-Patchset-for-aacraid-driver-version-41052/20160130-064140
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: tile-allmodconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=tile
All errors (new ones prefixed by >>):
drivers/scsi/aacraid/linit.c: In function 'aac_pci_error_detected':
>> drivers/scsi/aacraid/linit.c:1544:3: error: implicit declaration of function 'aac_release_resources'
drivers/scsi/aacraid/linit.c: In function 'aac_pci_resume':
>> drivers/scsi/aacraid/linit.c:1614:2: error: implicit declaration of function 'aac_acquire_resources'
cc1: some warnings being treated as errors
vim +/aac_release_resources +1544 drivers/scsi/aacraid/linit.c
1538 return PCI_ERS_RESULT_CAN_RECOVER;
1539 case pci_channel_io_frozen:
1540 aac->handle_pci_error = 1;
1541
1542 scsi_block_requests(aac->scsi_host_ptr);
1543 aac_flush_ios(aac);
> 1544 aac_release_resources(aac);
1545
1546 pci_disable_pcie_error_reporting(pdev);
1547 aac_adapter_ioremap(aac, 0);
1548
1549 return PCI_ERS_RESULT_NEED_RESET;
1550 case pci_channel_io_perm_failure:
1551 aac->handle_pci_error = 1;
1552
1553 aac_flush_ios(aac);
1554 return PCI_ERS_RESULT_DISCONNECT;
1555 }
1556
1557 return PCI_ERS_RESULT_NEED_RESET;
1558 }
1559
1560 static pci_ers_result_t aac_pci_mmio_enabled(struct pci_dev *pdev)
1561 {
1562 dev_err(&pdev->dev, "aacraid: PCI error - mmio enabled\n");
1563 return PCI_ERS_RESULT_NEED_RESET;
1564 }
1565
1566 static pci_ers_result_t aac_pci_slot_reset(struct pci_dev *pdev)
1567 {
1568 dev_err(&pdev->dev, "aacraid: PCI error - slot reset\n");
1569 pci_restore_state(pdev);
1570 if (pci_enable_device(pdev)) {
1571 dev_warn(&pdev->dev,
1572 "aacraid: failed to enable slave\n");
1573 goto fail_device;
1574 }
1575
1576 pci_set_master(pdev);
1577
1578 if (pci_enable_device_mem(pdev)) {
1579 dev_err(&pdev->dev, "pci_enable_device_mem failed\n");
1580 goto fail_device;
1581 }
1582
1583 return PCI_ERS_RESULT_RECOVERED;
1584
1585 fail_device:
1586 dev_err(&pdev->dev, "aacraid: PCI error - slot reset failed\n");
1587 return PCI_ERS_RESULT_DISCONNECT;
1588 }
1589
1590
1591 static void aac_pci_resume(struct pci_dev *pdev)
1592 {
1593 struct Scsi_Host *shost = pci_get_drvdata(pdev);
1594 struct scsi_device *sdev = NULL;
1595 struct aac_dev *aac = (struct aac_dev *)shost_priv(shost);
1596
1597 pci_cleanup_aer_uncorrect_error_status(pdev);
1598
1599 if (aac_adapter_ioremap(aac, aac->base_size)) {
1600
1601 dev_err(&pdev->dev, "aacraid: ioremap failed\n");
1602 /* remap failed, go back ... */
1603 aac->comm_interface = AAC_COMM_PRODUCER;
1604 if (aac_adapter_ioremap(aac, AAC_MIN_FOOTPRINT_SIZE)) {
1605 dev_warn(&pdev->dev,
1606 "aacraid: unable to map adapter.\n");
1607
1608 return;
1609 }
1610 }
1611
1612 msleep(10000);
1613
> 1614 aac_acquire_resources(aac);
1615
1616 /*
1617 * reset this flag to unblock ioctl() as it was set
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 42931 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V4 4/9] aacraid: Fix memory leak in aac_fib_map_free
2016-01-29 22:45 [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
` (2 preceding siblings ...)
2016-01-29 22:45 ` [PATCH V4 3/9] aacraid: Added EEH support Raghava Aditya Renukunta
@ 2016-01-29 22:45 ` Raghava Aditya Renukunta
2016-01-29 22:45 ` [PATCH V4 5/9] aacraid: Set correct msix count for EEH recovery Raghava Aditya Renukunta
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-01-29 22:45 UTC (permalink / raw)
To: James.Bottomley, martin.petersen, linux-scsi
Cc: Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, thenzl, shane.seymour,
RaghavaAditya.Renukunta
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
aac_fib_map_free() calls pci_free_consistent() without checking that
dev->hw_fib_va is not NULL and dev->max_fib_size is not zero.If they
are indeed NULL/0, this will result in a hang as pci_free_consistent()
will attempt to invalidate cache for the entire 64-bit address space
(which would take a very long time).
Fixed by adding a check to make sure that dev->hw_fib_va and
dev->max_fib_size are not NULL and 0 respectively.
Changes in V2:
None
Changes in V3:
None
Changes in V4:
None
Fixes: 9ad5204d6 - "[SCSI]aacraid: incorrect dma mapping mask
during blinked recover or user initiated reset"
Cc: stable@vger.kernel.org
Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
---
drivers/scsi/aacraid/commsup.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 07a42a3..511bbc5 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -83,9 +83,12 @@ static int fib_map_alloc(struct aac_dev *dev)
void aac_fib_map_free(struct aac_dev *dev)
{
- pci_free_consistent(dev->pdev,
- dev->max_fib_size * (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB),
- dev->hw_fib_va, dev->hw_fib_pa);
+ if (dev->hw_fib_va && dev->max_fib_size) {
+ pci_free_consistent(dev->pdev,
+ (dev->max_fib_size *
+ (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)),
+ dev->hw_fib_va, dev->hw_fib_pa);
+ }
dev->hw_fib_va = NULL;
dev->hw_fib_pa = 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH V4 5/9] aacraid: Set correct msix count for EEH recovery
2016-01-29 22:45 [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
` (3 preceding siblings ...)
2016-01-29 22:45 ` [PATCH V4 4/9] aacraid: Fix memory leak in aac_fib_map_free Raghava Aditya Renukunta
@ 2016-01-29 22:45 ` Raghava Aditya Renukunta
2016-01-29 22:45 ` [PATCH V4 6/9] aacraid: Fundamental reset support for Series 7 Raghava Aditya Renukunta
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-01-29 22:45 UTC (permalink / raw)
To: James.Bottomley, martin.petersen, linux-scsi
Cc: Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, thenzl, shane.seymour,
RaghavaAditya.Renukunta
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
During EEH recovery number of online CPU's might change thereby changing
the number of MSIx vectors. Since each fib is allocated to a vector,
changes in the number of vectors causes fib to be sent thru invalid
vectors.In addition the correct number of MSIx vectors is not
updated in the INIT struct sent to the controller, when it is
reinitialized.
Fixed by reassigning vectors to fibs based on the updated number of MSIx
vectors and updating the INIT structure before sending to controller.
Changes in V2:
Replaced fib vector allocation code with aac_fib_vector_assign
Changes in V3:
None
Changes in V4:
None
Fixes: MSI-X vector calculation for suspend/resume
Cc: stable@vger.kernel.org
Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
Reviewed-by: Johannes Thumshirn <jthushirn@suse.de>
---
drivers/scsi/aacraid/linit.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 62ac7be..4bc4503 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1410,8 +1410,18 @@ static int aac_acquire_resources(struct aac_dev *dev)
aac_adapter_enable_int(dev);
- if (!dev->sync_mode)
+ /*max msix may change after EEH
+ * Re-assign vectors to fibs
+ */
+ aac_fib_vector_assign(dev);
+
+ if (!dev->sync_mode) {
+ /* After EEH recovery or suspend resume, max_msix count
+ * may change, therfore updating in init as well.
+ */
aac_adapter_start(dev);
+ dev->init->Sa_MSIXVectors = cpu_to_le32(dev->max_msix);
+ }
return 0;
error_iounmap:
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH V4 6/9] aacraid: Fundamental reset support for Series 7
2016-01-29 22:45 [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
` (4 preceding siblings ...)
2016-01-29 22:45 ` [PATCH V4 5/9] aacraid: Set correct msix count for EEH recovery Raghava Aditya Renukunta
@ 2016-01-29 22:45 ` Raghava Aditya Renukunta
2016-01-29 22:45 ` [PATCH V4 7/9] aacraid: Fix AIF triggered IOP_RESET Raghava Aditya Renukunta
` (3 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-01-29 22:45 UTC (permalink / raw)
To: James.Bottomley, martin.petersen, linux-scsi
Cc: Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, thenzl, shane.seymour,
RaghavaAditya.Renukunta
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Series 7 does not support PCI hot reset used by EEH.
Enabled fundamental reset only for Series 7
Changes in V2:
None
Changes in V3:
None
Changes in V4:
None
Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
---
drivers/scsi/aacraid/linit.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 4bc4503..af8974e 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1135,6 +1135,12 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
u64 dmamask;
extern int aac_sync_mode;
+ /*
+ * Only series 7 needs freset.
+ */
+ if (pdev->device == PMC_DEVICE_S7)
+ pdev->needs_freset = 1;
+
list_for_each_entry(aac, &aac_devices, entry) {
if (aac->id > unique_id)
break;
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH V4 7/9] aacraid: Fix AIF triggered IOP_RESET
2016-01-29 22:45 [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
` (5 preceding siblings ...)
2016-01-29 22:45 ` [PATCH V4 6/9] aacraid: Fundamental reset support for Series 7 Raghava Aditya Renukunta
@ 2016-01-29 22:45 ` Raghava Aditya Renukunta
2016-02-01 16:43 ` Tomas Henzl
2016-01-29 22:45 ` [PATCH V4 8/9] aacraid: Fix character device re-initialization Raghava Aditya Renukunta
` (2 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-01-29 22:45 UTC (permalink / raw)
To: James.Bottomley, martin.petersen, linux-scsi
Cc: Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, thenzl, shane.seymour,
RaghavaAditya.Renukunta
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
while driver removal is in progress or PCI shutdown is invoked, driver
kills AIF aacraid thread, but IOCTL requests from the management tools
re-start AIF thread leading to IOP_RESET.
Fixed by setting adapter_shutdown flag when PCI shutdown is invoked.
Changes in V2:
Set adapter_shutdown flag before shutdown command is sent to \
controller
Changes in V3:
Call aac_send_shut_shutdown first thing in __aac_shutdown
Convert adapter_shutdown to atomic_t variable to prevent \
SMP coherency issues(race conditions)
Changes in V4:
Used mutex to protect ioctl path and adapter_shutdown to prevent \
race conditions.
Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
drivers/scsi/aacraid/aacraid.h | 2 +-
drivers/scsi/aacraid/commctrl.c | 3 ++
drivers/scsi/aacraid/comminit.c | 6 ++--
drivers/scsi/aacraid/linit.c | 63 +++++++++++++++++++++++++++++++----------
4 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 2916288..6c55749 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1123,7 +1123,7 @@ struct aac_dev
struct fib *free_fib;
spinlock_t fib_lock;
-
+ struct mutex ioctl_mutex;
struct aac_queue_block *queues;
/*
* The user API will use an IOCTL to register itself to receive
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 54195a1..8d3438c 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -855,6 +855,9 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
{
int status;
+ if (dev->adapter_shutdown)
+ return -EACCES;
+
/*
* HBA gets first crack
*/
diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
index 0e954e3..2b4e753 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -212,8 +212,11 @@ int aac_send_shutdown(struct aac_dev * dev)
return -ENOMEM;
aac_fib_init(fibctx);
- cmd = (struct aac_close *) fib_data(fibctx);
+ mutex_lock(&dev->ioctl_mutex);
+ dev->adapter_shutdown = 1;
+ mutex_unlock(&dev->ioctl_mutex);
+ cmd = (struct aac_close *) fib_data(fibctx);
cmd->command = cpu_to_le32(VM_CloseAll);
cmd->cid = cpu_to_le32(0xfffffffe);
@@ -229,7 +232,6 @@ int aac_send_shutdown(struct aac_dev * dev)
/* FIB should be freed only after getting the response from the F/W */
if (status != -ERESTARTSYS)
aac_fib_free(fibctx);
- dev->adapter_shutdown = 1;
if ((dev->pdev->device == PMC_DEVICE_S7 ||
dev->pdev->device == PMC_DEVICE_S8 ||
dev->pdev->device == PMC_DEVICE_S9) &&
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index af8974e..9453e11 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -524,10 +524,17 @@ static struct device_attribute *aac_dev_attrs[] = {
static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
{
- struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
+ int ret;
+ struct aac_dev *aac = (struct aac_dev *)sdev->host->hostdata;
+
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- return aac_do_ioctl(dev, cmd, arg);
+
+ mutex_lock(&aac->ioctl_mutex);
+ ret = aac_do_ioctl(aac, cmd, arg);
+ mutex_unlock(&aac->ioctl_mutex);
+
+ return ret;
}
static int aac_eh_abort(struct scsi_cmnd* cmd)
@@ -704,13 +711,14 @@ static long aac_cfg_ioctl(struct file *file,
unsigned int cmd, unsigned long arg)
{
int ret;
- struct aac_dev *aac;
- aac = (struct aac_dev *)file->private_data;
- if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
+ struct aac_dev *aac = (struct aac_dev *)file->private_data;
+
+ if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- mutex_lock(&aac_mutex);
- ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
- mutex_unlock(&aac_mutex);
+
+ mutex_lock(&aac->ioctl_mutex);
+ ret = aac_do_ioctl(aac, cmd, (void __user *)arg);
+ mutex_unlock(&aac->ioctl_mutex);
return ret;
}
@@ -719,7 +727,10 @@ static long aac_cfg_ioctl(struct file *file,
static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long arg)
{
long ret;
- mutex_lock(&aac_mutex);
+
+ if (dev->adapter_shutdown)
+ return -EACCES;
+
switch (cmd) {
case FSACTL_MINIPORT_REV_CHECK:
case FSACTL_SENDFIB:
@@ -753,23 +764,37 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
ret = -ENOIOCTLCMD;
break;
}
- mutex_unlock(&aac_mutex);
return ret;
}
static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
{
- struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
+ int ret;
+ struct aac_dev *aac = (struct aac_dev *)sdev->host->hostdata;
+
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- return aac_compat_do_ioctl(dev, cmd, (unsigned long)arg);
+
+ mutex_lock(&aac->ioctl_mutex);
+ ret = aac_compat_do_ioctl(aac, cmd, (unsigned long)arg);
+ mutex_unlock(&aac->ioctl_mutex);
+
+ return ret;
}
static long aac_compat_cfg_ioctl(struct file *file, unsigned cmd, unsigned long arg)
{
+ int ret;
+ struct aac_dev *aac = (struct aac_dev *)file->private_data;
+
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- return aac_compat_do_ioctl(file->private_data, cmd, arg);
+
+ mutex_lock(&aac->ioctl_mutex);
+ ret = aac_compat_do_ioctl(aac, cmd, arg);
+ mutex_unlock(&aac->ioctl_mutex);
+
+ return ret;
}
#endif
@@ -1078,6 +1103,8 @@ static void __aac_shutdown(struct aac_dev * aac)
int i;
int cpu;
+ aac_send_shutdown(aac);
+
if (aac->aif_thread) {
int i;
/* Clear out events first */
@@ -1089,7 +1116,7 @@ static void __aac_shutdown(struct aac_dev * aac)
}
kthread_stop(aac->thread);
}
- aac_send_shutdown(aac);
+
aac_adapter_disable_int(aac);
cpu = cpumask_first(cpu_online_mask);
if (aac->pdev->device == PMC_DEVICE_S6 ||
@@ -1193,7 +1220,7 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
if (!aac->fibs)
goto out_free_host;
spin_lock_init(&aac->fib_lock);
-
+ mutex_init(&aac->ioctl_mutex);
/*
* Map in the registers from the adapter.
*/
@@ -1474,7 +1501,10 @@ static int aac_resume(struct pci_dev *pdev)
* reset this flag to unblock ioctl() as it was set at
* aac_send_shutdown() to block ioctls from upperlayer
*/
+ mutex_lock(&aac->ioctl_mutex);
aac->adapter_shutdown = 0;
+ mutex_unlock(&aac->ioctl_mutex);
+
scsi_unblock_requests(shost);
return 0;
@@ -1633,7 +1663,10 @@ static void aac_pci_resume(struct pci_dev *pdev)
* reset this flag to unblock ioctl() as it was set
* at aac_send_shutdown() to block ioctls from upperlayer
*/
+ mutex_lock(&aac->ioctl_mutex);
aac->adapter_shutdown = 0;
+ mutex_unlock(&aac->ioctl_mutex);
+
aac->handle_pci_error = 0;
shost_for_each_device(sdev, shost)
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH V4 7/9] aacraid: Fix AIF triggered IOP_RESET
2016-01-29 22:45 ` [PATCH V4 7/9] aacraid: Fix AIF triggered IOP_RESET Raghava Aditya Renukunta
@ 2016-02-01 16:43 ` Tomas Henzl
2016-02-02 1:18 ` Raghava Aditya Renukunta
0 siblings, 1 reply; 15+ messages in thread
From: Tomas Henzl @ 2016-02-01 16:43 UTC (permalink / raw)
To: Raghava Aditya Renukunta, James.Bottomley, martin.petersen,
linux-scsi
Cc: Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, shane.seymour
On 29.1.2016 23:45, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
>
> while driver removal is in progress or PCI shutdown is invoked, driver
> kills AIF aacraid thread, but IOCTL requests from the management tools
> re-start AIF thread leading to IOP_RESET.
>
> Fixed by setting adapter_shutdown flag when PCI shutdown is invoked.
>
> Changes in V2:
> Set adapter_shutdown flag before shutdown command is sent to \
> controller
>
> Changes in V3:
> Call aac_send_shut_shutdown first thing in __aac_shutdown
> Convert adapter_shutdown to atomic_t variable to prevent \
> SMP coherency issues(race conditions)
>
> Changes in V4:
> Used mutex to protect ioctl path and adapter_shutdown to prevent \
> race conditions.
>
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
> drivers/scsi/aacraid/aacraid.h | 2 +-
> drivers/scsi/aacraid/commctrl.c | 3 ++
> drivers/scsi/aacraid/comminit.c | 6 ++--
> drivers/scsi/aacraid/linit.c | 63 +++++++++++++++++++++++++++++++----------
> 4 files changed, 56 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 2916288..6c55749 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -1123,7 +1123,7 @@ struct aac_dev
>
> struct fib *free_fib;
> spinlock_t fib_lock;
> -
> + struct mutex ioctl_mutex;
> struct aac_queue_block *queues;
> /*
> * The user API will use an IOCTL to register itself to receive
> diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
> index 54195a1..8d3438c 100644
> --- a/drivers/scsi/aacraid/commctrl.c
> +++ b/drivers/scsi/aacraid/commctrl.c
> @@ -855,6 +855,9 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
> {
> int status;
>
> + if (dev->adapter_shutdown)
> + return -EACCES;
> +
> /*
> * HBA gets first crack
> */
> diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
> index 0e954e3..2b4e753 100644
> --- a/drivers/scsi/aacraid/comminit.c
> +++ b/drivers/scsi/aacraid/comminit.c
> @@ -212,8 +212,11 @@ int aac_send_shutdown(struct aac_dev * dev)
> return -ENOMEM;
> aac_fib_init(fibctx);
>
> - cmd = (struct aac_close *) fib_data(fibctx);
> + mutex_lock(&dev->ioctl_mutex);
> + dev->adapter_shutdown = 1;
> + mutex_unlock(&dev->ioctl_mutex);
>
> + cmd = (struct aac_close *) fib_data(fibctx);
> cmd->command = cpu_to_le32(VM_CloseAll);
> cmd->cid = cpu_to_le32(0xfffffffe);
>
> @@ -229,7 +232,6 @@ int aac_send_shutdown(struct aac_dev * dev)
> /* FIB should be freed only after getting the response from the F/W */
> if (status != -ERESTARTSYS)
> aac_fib_free(fibctx);
> - dev->adapter_shutdown = 1;
> if ((dev->pdev->device == PMC_DEVICE_S7 ||
> dev->pdev->device == PMC_DEVICE_S8 ||
> dev->pdev->device == PMC_DEVICE_S9) &&
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index af8974e..9453e11 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -524,10 +524,17 @@ static struct device_attribute *aac_dev_attrs[] = {
>
> static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
> {
> - struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> + int ret;
> + struct aac_dev *aac = (struct aac_dev *)sdev->host->hostdata;
> +
> if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
> - return aac_do_ioctl(dev, cmd, arg);
> +
> + mutex_lock(&aac->ioctl_mutex);
> + ret = aac_do_ioctl(aac, cmd, arg);
> + mutex_unlock(&aac->ioctl_mutex);
> +
> + return ret;
> }
>
> static int aac_eh_abort(struct scsi_cmnd* cmd)
> @@ -704,13 +711,14 @@ static long aac_cfg_ioctl(struct file *file,
> unsigned int cmd, unsigned long arg)
> {
> int ret;
> - struct aac_dev *aac;
> - aac = (struct aac_dev *)file->private_data;
> - if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
> + struct aac_dev *aac = (struct aac_dev *)file->private_data;
> +
> + if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
> - mutex_lock(&aac_mutex);
> - ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
> - mutex_unlock(&aac_mutex);
> +
> + mutex_lock(&aac->ioctl_mutex);
> + ret = aac_do_ioctl(aac, cmd, (void __user *)arg);
> + mutex_unlock(&aac->ioctl_mutex);
>
> return ret;
> }
> @@ -719,7 +727,10 @@ static long aac_cfg_ioctl(struct file *file,
> static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long arg)
> {
> long ret;
> - mutex_lock(&aac_mutex);
> +
> + if (dev->adapter_shutdown)
> + return -EACCES;
There is another test for the same in aac_do_ioctl, this duplicated test is needless.
> +
> switch (cmd) {
> case FSACTL_MINIPORT_REV_CHECK:
> case FSACTL_SENDFIB:
> @@ -753,23 +764,37 @@ static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long
> ret = -ENOIOCTLCMD;
> break;
> }
> - mutex_unlock(&aac_mutex);
> return ret;
> }
>
> static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
> {
> - struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> + int ret;
> + struct aac_dev *aac = (struct aac_dev *)sdev->host->hostdata;
> +
> if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
> - return aac_compat_do_ioctl(dev, cmd, (unsigned long)arg);
> +
> + mutex_lock(&aac->ioctl_mutex);
> + ret = aac_compat_do_ioctl(aac, cmd, (unsigned long)arg);
> + mutex_unlock(&aac->ioctl_mutex);
> +
> + return ret;
> }
>
> static long aac_compat_cfg_ioctl(struct file *file, unsigned cmd, unsigned long arg)
> {
> + int ret;
> + struct aac_dev *aac = (struct aac_dev *)file->private_data;
> +
> if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
> - return aac_compat_do_ioctl(file->private_data, cmd, arg);
> +
> + mutex_lock(&aac->ioctl_mutex);
> + ret = aac_compat_do_ioctl(aac, cmd, arg);
> + mutex_unlock(&aac->ioctl_mutex);
> +
> + return ret;
> }
> #endif
>
> @@ -1078,6 +1103,8 @@ static void __aac_shutdown(struct aac_dev * aac)
> int i;
> int cpu;
>
> + aac_send_shutdown(aac);
> +
> if (aac->aif_thread) {
> int i;
> /* Clear out events first */
> @@ -1089,7 +1116,7 @@ static void __aac_shutdown(struct aac_dev * aac)
> }
> kthread_stop(aac->thread);
> }
> - aac_send_shutdown(aac);
> +
> aac_adapter_disable_int(aac);
> cpu = cpumask_first(cpu_online_mask);
> if (aac->pdev->device == PMC_DEVICE_S6 ||
> @@ -1193,7 +1220,7 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
> if (!aac->fibs)
> goto out_free_host;
> spin_lock_init(&aac->fib_lock);
> -
> + mutex_init(&aac->ioctl_mutex);
> /*
> * Map in the registers from the adapter.
> */
> @@ -1474,7 +1501,10 @@ static int aac_resume(struct pci_dev *pdev)
> * reset this flag to unblock ioctl() as it was set at
> * aac_send_shutdown() to block ioctls from upperlayer
> */
> + mutex_lock(&aac->ioctl_mutex);
> aac->adapter_shutdown = 0;
> + mutex_unlock(&aac->ioctl_mutex);
A mutex surrounding adapter_shutdown = 0; is needless.
> +
> scsi_unblock_requests(shost);
>
> return 0;
> @@ -1633,7 +1663,10 @@ static void aac_pci_resume(struct pci_dev *pdev)
> * reset this flag to unblock ioctl() as it was set
> * at aac_send_shutdown() to block ioctls from upperlayer
> */
> + mutex_lock(&aac->ioctl_mutex);
> aac->adapter_shutdown = 0;
> + mutex_unlock(&aac->ioctl_mutex);
Same here.
aac_compat_do_ioctl and aac_do_ioctl is surrounded by mutexes - that is fine,
but by moving the mutex down to aac_do_ioctl the code would be easier
and more readable.
With the new mutex, you have changed the functionality so that a new patch
is needed for that - before this patch there was no mutex in the main
.ioctl path.
I have found no functional issues so regardless whether you follow
my comments now, later or never -
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
Cheers
Tomas
> +
> aac->handle_pci_error = 0;
>
> shost_for_each_device(sdev, shost)
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH V4 7/9] aacraid: Fix AIF triggered IOP_RESET
2016-02-01 16:43 ` Tomas Henzl
@ 2016-02-02 1:18 ` Raghava Aditya Renukunta
0 siblings, 0 replies; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-02-02 1:18 UTC (permalink / raw)
To: Tomas Henzl, James.Bottomley@HansenPartnership.com,
martin.petersen@oracle.com, linux-scsi@vger.kernel.org
Cc: Mahesh Rajashekhara, Murthy Bhat, Gana Sridaran,
aacraid@pmc-sierra.com, Scott Benesh, jthumshirn@suse.de,
shane.seymour@hpe.com
Hello Tomas,
> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Monday, February 1, 2016 8:44 AM
> To: Raghava Aditya Renukunta; James.Bottomley@HansenPartnership.com;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> sierra.com; Scott Benesh; jthumshirn@suse.de; shane.seymour@hpe.com
> Subject: Re: [PATCH V4 7/9] aacraid: Fix AIF triggered IOP_RESET
>
> On 29.1.2016 23:45, Raghava Aditya Renukunta wrote:
> > From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> >
> > while driver removal is in progress or PCI shutdown is invoked, driver
> > kills AIF aacraid thread, but IOCTL requests from the management tools
> > re-start AIF thread leading to IOP_RESET.
> >
> > Fixed by setting adapter_shutdown flag when PCI shutdown is invoked.
> >
> > Changes in V2:
> > Set adapter_shutdown flag before shutdown command is sent to \
> > controller
> >
> > Changes in V3:
> > Call aac_send_shut_shutdown first thing in __aac_shutdown
> > Convert adapter_shutdown to atomic_t variable to prevent \
> > SMP coherency issues(race conditions)
> >
> > Changes in V4:
> > Used mutex to protect ioctl path and adapter_shutdown to prevent \
> > race conditions.
> >
> > Signed-off-by: Raghava Aditya Renukunta
> <raghavaaditya.renukunta@pmcs.com>
> > Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
> > Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
> > ---
> > drivers/scsi/aacraid/aacraid.h | 2 +-
> > drivers/scsi/aacraid/commctrl.c | 3 ++
> > drivers/scsi/aacraid/comminit.c | 6 ++--
> > drivers/scsi/aacraid/linit.c | 63 +++++++++++++++++++++++++++++++--
> --------
> > 4 files changed, 56 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> > index 2916288..6c55749 100644
> > --- a/drivers/scsi/aacraid/aacraid.h
> > +++ b/drivers/scsi/aacraid/aacraid.h
> > @@ -1123,7 +1123,7 @@ struct aac_dev
> >
> > struct fib *free_fib;
> > spinlock_t fib_lock;
> > -
> > + struct mutex ioctl_mutex;
> > struct aac_queue_block *queues;
> > /*
> > * The user API will use an IOCTL to register itself to receive
> > diff --git a/drivers/scsi/aacraid/commctrl.c
> b/drivers/scsi/aacraid/commctrl.c
> > index 54195a1..8d3438c 100644
> > --- a/drivers/scsi/aacraid/commctrl.c
> > +++ b/drivers/scsi/aacraid/commctrl.c
> > @@ -855,6 +855,9 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void
> __user *arg)
> > {
> > int status;
> >
> > + if (dev->adapter_shutdown)
> > + return -EACCES;
> > +
> > /*
> > * HBA gets first crack
> > */
> > diff --git a/drivers/scsi/aacraid/comminit.c
> b/drivers/scsi/aacraid/comminit.c
> > index 0e954e3..2b4e753 100644
> > --- a/drivers/scsi/aacraid/comminit.c
> > +++ b/drivers/scsi/aacraid/comminit.c
> > @@ -212,8 +212,11 @@ int aac_send_shutdown(struct aac_dev * dev)
> > return -ENOMEM;
> > aac_fib_init(fibctx);
> >
> > - cmd = (struct aac_close *) fib_data(fibctx);
> > + mutex_lock(&dev->ioctl_mutex);
> > + dev->adapter_shutdown = 1;
> > + mutex_unlock(&dev->ioctl_mutex);
> >
> > + cmd = (struct aac_close *) fib_data(fibctx);
> > cmd->command = cpu_to_le32(VM_CloseAll);
> > cmd->cid = cpu_to_le32(0xfffffffe);
> >
> > @@ -229,7 +232,6 @@ int aac_send_shutdown(struct aac_dev * dev)
> > /* FIB should be freed only after getting the response from the F/W
> */
> > if (status != -ERESTARTSYS)
> > aac_fib_free(fibctx);
> > - dev->adapter_shutdown = 1;
> > if ((dev->pdev->device == PMC_DEVICE_S7 ||
> > dev->pdev->device == PMC_DEVICE_S8 ||
> > dev->pdev->device == PMC_DEVICE_S9) &&
> > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> > index af8974e..9453e11 100644
> > --- a/drivers/scsi/aacraid/linit.c
> > +++ b/drivers/scsi/aacraid/linit.c
> > @@ -524,10 +524,17 @@ static struct device_attribute *aac_dev_attrs[] = {
> >
> > static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg)
> > {
> > - struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> > + int ret;
> > + struct aac_dev *aac = (struct aac_dev *)sdev->host->hostdata;
> > +
> > if (!capable(CAP_SYS_RAWIO))
> > return -EPERM;
> > - return aac_do_ioctl(dev, cmd, arg);
> > +
> > + mutex_lock(&aac->ioctl_mutex);
> > + ret = aac_do_ioctl(aac, cmd, arg);
> > + mutex_unlock(&aac->ioctl_mutex);
> > +
> > + return ret;
> > }
> >
> > static int aac_eh_abort(struct scsi_cmnd* cmd)
> > @@ -704,13 +711,14 @@ static long aac_cfg_ioctl(struct file *file,
> > unsigned int cmd, unsigned long arg)
> > {
> > int ret;
> > - struct aac_dev *aac;
> > - aac = (struct aac_dev *)file->private_data;
> > - if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
> > + struct aac_dev *aac = (struct aac_dev *)file->private_data;
> > +
> > + if (!capable(CAP_SYS_RAWIO))
> > return -EPERM;
> > - mutex_lock(&aac_mutex);
> > - ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
> > - mutex_unlock(&aac_mutex);
> > +
> > + mutex_lock(&aac->ioctl_mutex);
> > + ret = aac_do_ioctl(aac, cmd, (void __user *)arg);
> > + mutex_unlock(&aac->ioctl_mutex);
> >
> > return ret;
> > }
> > @@ -719,7 +727,10 @@ static long aac_cfg_ioctl(struct file *file,
> > static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd,
> unsigned long arg)
> > {
> > long ret;
> > - mutex_lock(&aac_mutex);
> > +
> > + if (dev->adapter_shutdown)
> > + return -EACCES;
>
> There is another test for the same in aac_do_ioctl, this duplicated test is
> needless.
Will remove
> > +
> > switch (cmd) {
> > case FSACTL_MINIPORT_REV_CHECK:
> > case FSACTL_SENDFIB:
> > @@ -753,23 +764,37 @@ static long aac_compat_do_ioctl(struct aac_dev
> *dev, unsigned cmd, unsigned long
> > ret = -ENOIOCTLCMD;
> > break;
> > }
> > - mutex_unlock(&aac_mutex);
> > return ret;
> > }
> >
> > static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user
> *arg)
> > {
> > - struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata;
> > + int ret;
> > + struct aac_dev *aac = (struct aac_dev *)sdev->host->hostdata;
> > +
> > if (!capable(CAP_SYS_RAWIO))
> > return -EPERM;
> > - return aac_compat_do_ioctl(dev, cmd, (unsigned long)arg);
> > +
> > + mutex_lock(&aac->ioctl_mutex);
> > + ret = aac_compat_do_ioctl(aac, cmd, (unsigned long)arg);
> > + mutex_unlock(&aac->ioctl_mutex);
> > +
> > + return ret;
> > }
> >
> > static long aac_compat_cfg_ioctl(struct file *file, unsigned cmd, unsigned
> long arg)
> > {
> > + int ret;
> > + struct aac_dev *aac = (struct aac_dev *)file->private_data;
> > +
> > if (!capable(CAP_SYS_RAWIO))
> > return -EPERM;
> > - return aac_compat_do_ioctl(file->private_data, cmd, arg);
> > +
> > + mutex_lock(&aac->ioctl_mutex);
> > + ret = aac_compat_do_ioctl(aac, cmd, arg);
> > + mutex_unlock(&aac->ioctl_mutex);
> > +
> > + return ret;
> > }
> > #endif
> >
> > @@ -1078,6 +1103,8 @@ static void __aac_shutdown(struct aac_dev * aac)
> > int i;
> > int cpu;
> >
> > + aac_send_shutdown(aac);
> > +
> > if (aac->aif_thread) {
> > int i;
> > /* Clear out events first */
> > @@ -1089,7 +1116,7 @@ static void __aac_shutdown(struct aac_dev * aac)
> > }
> > kthread_stop(aac->thread);
> > }
> > - aac_send_shutdown(aac);
> > +
> > aac_adapter_disable_int(aac);
> > cpu = cpumask_first(cpu_online_mask);
> > if (aac->pdev->device == PMC_DEVICE_S6 ||
> > @@ -1193,7 +1220,7 @@ static int aac_probe_one(struct pci_dev *pdev,
> const struct pci_device_id *id)
> > if (!aac->fibs)
> > goto out_free_host;
> > spin_lock_init(&aac->fib_lock);
> > -
> > + mutex_init(&aac->ioctl_mutex);
> > /*
> > * Map in the registers from the adapter.
> > */
> > @@ -1474,7 +1501,10 @@ static int aac_resume(struct pci_dev *pdev)
> > * reset this flag to unblock ioctl() as it was set at
> > * aac_send_shutdown() to block ioctls from upperlayer
> > */
> > + mutex_lock(&aac->ioctl_mutex);
> > aac->adapter_shutdown = 0;
> > + mutex_unlock(&aac->ioctl_mutex);
>
> A mutex surrounding adapter_shutdown = 0; is needless.
Will Remove
> > +
> > scsi_unblock_requests(shost);
> >
> > return 0;
> > @@ -1633,7 +1663,10 @@ static void aac_pci_resume(struct pci_dev
> *pdev)
> > * reset this flag to unblock ioctl() as it was set
> > * at aac_send_shutdown() to block ioctls from upperlayer
> > */
> > + mutex_lock(&aac->ioctl_mutex);
> > aac->adapter_shutdown = 0;
> > + mutex_unlock(&aac->ioctl_mutex);
>
> Same here.
Will Remove
>
> aac_compat_do_ioctl and aac_do_ioctl is surrounded by mutexes - that is
> fine,
> but by moving the mutex down to aac_do_ioctl the code would be easier
> and more readable.
>
> With the new mutex, you have changed the functionality so that a new patch
> is needed for that - before this patch there was no mutex in the main
> .ioctl path.
>
> I have found no functional issues so regardless whether you follow
> my comments now, later or never -
>
> Reviewed-by: Tomas Henzl <thenzl@redhat.com>
>
> Cheers
> Tomas
I will make the changes that you have suggested ,
Create a new patch for the new mutex and apply this patch on top of it.
Thank you Tomas.
Regards,
Raghava Aditya
>
>
>
>
>
>
>
>
> > +
> > aac->handle_pci_error = 0;
> >
> > shost_for_each_device(sdev, shost)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V4 8/9] aacraid: Fix character device re-initialization
2016-01-29 22:45 [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
` (6 preceding siblings ...)
2016-01-29 22:45 ` [PATCH V4 7/9] aacraid: Fix AIF triggered IOP_RESET Raghava Aditya Renukunta
@ 2016-01-29 22:45 ` Raghava Aditya Renukunta
2016-01-29 22:45 ` [PATCH V4 9/9] aacraid: Update driver version Raghava Aditya Renukunta
2016-02-02 0:56 ` [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Martin K. Petersen
9 siblings, 0 replies; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-01-29 22:45 UTC (permalink / raw)
To: James.Bottomley, martin.petersen, linux-scsi
Cc: Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, thenzl, shane.seymour,
RaghavaAditya.Renukunta
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
During EEH PCI hotplug activity kernel unloads and loads the driver,
causing character device to be unregistered(aac_remove_one).When the
driver is loaded back using aac_probe_one the character device needs
to be registered again for the AIF management tools to work.
Fixed by adding code to register character device in aac_probe_one if
it is unregistered in aac_remove_one.
Changes in V2:
Added macros to track character device state
Changes in V3:
None
Changes in V4:
None
Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
drivers/scsi/aacraid/aacraid.h | 7 +++++++
drivers/scsi/aacraid/linit.c | 21 ++++++++++++++-------
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 6c55749..332e3c9 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -94,6 +94,13 @@ enum {
#define aac_phys_to_logical(x) ((x)+1)
#define aac_logical_to_phys(x) ((x)?(x)-1:0)
+/*
+ * These macros are for keeping track of
+ * character device state.
+ */
+#define AAC_CHARDEV_UNREGISTERED (-1)
+#define AAC_CHARDEV_NEEDS_REINIT (-2)
+
/* #define AAC_DETAILED_STATUS_INFO */
struct diskparm
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 9453e11..a43c1dd 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION);
static DEFINE_MUTEX(aac_mutex);
static LIST_HEAD(aac_devices);
-static int aac_cfg_major = -1;
+static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED;
char aac_driver_version[] = AAC_DRIVER_FULL_VERSION;
/*
@@ -1150,6 +1150,13 @@ static void __aac_shutdown(struct aac_dev * aac)
else if (aac->max_msix > 1)
pci_disable_msix(aac->pdev);
}
+static void aac_init_char(void)
+{
+ aac_cfg_major = register_chrdev(0, "aac", &aac_cfg_fops);
+ if (aac_cfg_major < 0) {
+ pr_err("aacraid: unable to register \"aac\" device.\n");
+ }
+}
static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
{
@@ -1207,6 +1214,9 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
shost->max_cmd_len = 16;
shost->use_cmd_list = 1;
+ if (aac_cfg_major == AAC_CHARDEV_NEEDS_REINIT)
+ aac_init_char();
+
aac = (struct aac_dev *)shost->hostdata;
aac->base_start = pci_resource_start(pdev, 0);
aac->scsi_host_ptr = shost;
@@ -1547,7 +1557,7 @@ static void aac_remove_one(struct pci_dev *pdev)
pci_disable_device(pdev);
if (list_empty(&aac_devices)) {
unregister_chrdev(aac_cfg_major, "aac");
- aac_cfg_major = -1;
+ aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT;
}
}
@@ -1710,11 +1720,8 @@ static int __init aac_init(void)
if (error < 0)
return error;
- aac_cfg_major = register_chrdev( 0, "aac", &aac_cfg_fops);
- if (aac_cfg_major < 0) {
- printk(KERN_WARNING
- "aacraid: unable to register \"aac\" device.\n");
- }
+ aac_init_char();
+
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH V4 9/9] aacraid: Update driver version
2016-01-29 22:45 [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
` (7 preceding siblings ...)
2016-01-29 22:45 ` [PATCH V4 8/9] aacraid: Fix character device re-initialization Raghava Aditya Renukunta
@ 2016-01-29 22:45 ` Raghava Aditya Renukunta
2016-02-02 0:56 ` [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Martin K. Petersen
9 siblings, 0 replies; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-01-29 22:45 UTC (permalink / raw)
To: James.Bottomley, martin.petersen, linux-scsi
Cc: Mahesh.Rajashekhara, Murthy.Bhat, Gana.Sridaran, aacraid,
Scott.Benesh, jthumshirn, thenzl, shane.seymour,
RaghavaAditya.Renukunta
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Updated diver version to 41052
Changes in V2:
None
Changes in V3:
None
Changes in V4:
None
Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
drivers/scsi/aacraid/aacraid.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 332e3c9..b5f4f09 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -62,7 +62,7 @@ enum {
#define PMC_GLOBAL_INT_BIT0 0x00000001
#ifndef AAC_DRIVER_BUILD
-# define AAC_DRIVER_BUILD 41010
+# define AAC_DRIVER_BUILD 41052
# define AAC_DRIVER_BRANCH "-ms"
#endif
#define MAXIMUM_NUM_CONTAINERS 32
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052
2016-01-29 22:45 [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Raghava Aditya Renukunta
` (8 preceding siblings ...)
2016-01-29 22:45 ` [PATCH V4 9/9] aacraid: Update driver version Raghava Aditya Renukunta
@ 2016-02-02 0:56 ` Martin K. Petersen
2016-02-02 1:19 ` Raghava Aditya Renukunta
9 siblings, 1 reply; 15+ messages in thread
From: Martin K. Petersen @ 2016-02-02 0:56 UTC (permalink / raw)
To: Raghava Aditya Renukunta
Cc: James.Bottomley, martin.petersen, linux-scsi, Mahesh.Rajashekhara,
Murthy.Bhat, Gana.Sridaran, aacraid, Scott.Benesh, jthumshirn,
thenzl, shane.seymour
>>>>> "Raghava" == Raghava Aditya Renukunta <RaghavaAditya.Renukunta@pmcs.com> writes:
Raghava,
Raghava> This patchset includes the following changes (bug fixes and new
Raghava> feature support) specific to aacraid driver.
Please fix the build failure in patch 3/9 when CONFIG_PM is not defined
and address Tomas' comments for 7/9.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052
2016-02-02 0:56 ` [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 Martin K. Petersen
@ 2016-02-02 1:19 ` Raghava Aditya Renukunta
0 siblings, 0 replies; 15+ messages in thread
From: Raghava Aditya Renukunta @ 2016-02-02 1:19 UTC (permalink / raw)
To: Martin K. Petersen
Cc: James.Bottomley@HansenPartnership.com, linux-scsi@vger.kernel.org,
Mahesh Rajashekhara, Murthy Bhat, Gana Sridaran,
aacraid@pmc-sierra.com, Scott Benesh, jthumshirn@suse.de,
thenzl@redhat.com, shane.seymour@hpe.com
Hello Martin,
> -----Original Message-----
> From: Martin K. Petersen [mailto:martin.petersen@oracle.com]
> Sent: Monday, February 1, 2016 4:57 PM
> To: Raghava Aditya Renukunta
> Cc: James.Bottomley@HansenPartnership.com;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org; Mahesh
> Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-sierra.com; Scott
> Benesh; jthumshirn@suse.de; thenzl@redhat.com;
> shane.seymour@hpe.com
> Subject: Re: [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052
>
> >>>>> "Raghava" == Raghava Aditya Renukunta
> <RaghavaAditya.Renukunta@pmcs.com> writes:
>
> Raghava,
>
> Raghava> This patchset includes the following changes (bug fixes and new
> Raghava> feature support) specific to aacraid driver.
>
> Please fix the build failure in patch 3/9 when CONFIG_PM is not defined
> and address Tomas' comments for 7/9.
I will create a new patch version fixing these issues shortly.
Thank you Martin.
Regards,
Raghava Aditya
> --
> Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 15+ messages in thread