Re: [PATCH 1/2 v2] libsas: remove irq save in sas_ata_qc_issue()
Sebastian,
> Since commit 312d3e56119a ("[SCSI] libsas: remove ata_port.lock
> management duties from lldds") the sas_ata_qc_issue() function unlocks
> the ata_port.lock and disables interrupts before doing so. That lock
> is always taken with disabled interrupts so at this point, the
> interrupts are already disabled. There is no need to disable the
> interrupts before the unlock operation because they are already
> disabled. Restoring the interrupt state later does not change
> anything because they were disabled and remain disabled. Therefore
> remove the operations which do not change the behaviour.
Applied to 4.19/scsi-queue. Thank you!
--
Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi: qla2xxx: Fix compilation warning in qlt_schedule_sess_for_deletion
> Compiler output: > drivers/scsi/qla2xxx/qla_target.c: In function > 'qlt_schedule_sess_for_deletion': > drivers/scsi/qla2xxx/qla_target.c:1227:22: warning: unused variable 'ha' > [-Wunused-variable] > struct qla_hw_data *ha = sess->vha->hw; Applied to 4.18/scsi-fixes. Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 4/8] mpt3sas: Fix kernel-doc warnings
On 06/14/2018 09:49 AM, Bart Van Assche wrote: > This patch avoids that warnings about the kernel headers appear when > building with W=1. > > Signed-off-by: Bart Van Assche > Cc: Sathya Prakash > Cc: Chaitra P B > Cc: Suganath Prabu Subramani > --- > drivers/scsi/mpt3sas/mpt3sas_base.c | 16 +- > drivers/scsi/mpt3sas/mpt3sas_ctl.c | 251 +++- > drivers/scsi/mpt3sas/mpt3sas_scsih.c| 36 +-- > drivers/scsi/mpt3sas/mpt3sas_transport.c| 10 +- > drivers/scsi/mpt3sas/mpt3sas_trigger_diag.c | 18 +- > drivers/scsi/mpt3sas/mpt3sas_warpdrive.c| 3 - > 6 files changed, 186 insertions(+), 148 deletions(-) > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c > b/drivers/scsi/mpt3sas/mpt3sas_base.c > index 9c233ddc5b2b..67e1b603f287 100644 > --- a/drivers/scsi/mpt3sas/mpt3sas_base.c > +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c > @@ -103,7 +103,8 @@ _base_get_ioc_facts(struct MPT3SAS_ADAPTER *ioc); > > /** > * _scsih_set_fwfault_debug - global setting of ioc->fwfault_debug. > - * > + * @val: ? > + * @kp: ? > */ > static int > _scsih_set_fwfault_debug(const char *val, const struct kernel_param *kp) > @@ -197,7 +198,7 @@ _base_clone_to_sys_mem(void __iomem *dst_iomem, void > *src, u32 size) > * @smid: system request message index > * @sge_chain_count: Scatter gather chain count. > * > - * @Return: chain address. > + * Returns the chain address. > */ Hi Bart, Documentation/doc-guide/kernel-doc.rst says that Return info "should be" described in a Return: section: The return value, if any, should be described in a dedicated section named ``Return``. This applies to many instances in this patch. thanks, -- ~Randy
[PATCH 2/8] mpt3sas: Remove set-but-not-used variables
This patch does not change any functionality.
Signed-off-by: Bart Van Assche
Cc: Sathya Prakash
Cc: Chaitra P B
Cc: Suganath Prabu Subramani
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 10 --
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 5 -
2 files changed, 15 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 798ee62c97f1..9c233ddc5b2b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2127,11 +2127,9 @@ base_is_prp_possible(struct MPT3SAS_ADAPTER *ioc,
struct _pcie_device *pcie_device, struct scsi_cmnd *scmd, int sge_count)
{
u32 data_length = 0;
- struct scatterlist *sg_scmd;
bool build_prp = true;
data_length = scsi_bufflen(scmd);
- sg_scmd = scsi_sglist(scmd);
/* If Datalenth is <= 16K and number of SGE’s entries are <= 2
* we built IEEE SGL
@@ -2162,11 +2160,9 @@ _base_check_pcie_native_sgl(struct MPT3SAS_ADAPTER *ioc,
Mpi25SCSIIORequest_t *mpi_request, u16 smid, struct scsi_cmnd *scmd,
struct _pcie_device *pcie_device)
{
- struct scatterlist *sg_scmd;
int sges_left;
/* Get the SG list pointer and info. */
- sg_scmd = scsi_sglist(scmd);
sges_left = scsi_dma_map(scmd);
if (sges_left < 0) {
sdev_printk(KERN_ERR, scmd->device,
@@ -3472,11 +3468,8 @@ mpt3sas_base_put_smid_hi_priority(struct MPT3SAS_ADAPTER
*ioc, u16 smid,
u64 *request;
if (ioc->is_mcpu_endpoint) {
- MPI2RequestHeader_t *request_hdr;
-
__le32 *mfp = (__le32 *)mpt3sas_base_get_msg_frame(ioc, smid);
- request_hdr = (MPI2RequestHeader_t *)mfp;
/* TBD 256 is offset within sys register. */
mpi_req_iomem = (void __force *)ioc->chip
+ MPI_FRAME_START_OFFSET
@@ -3539,13 +3532,10 @@ mpt3sas_base_put_smid_default(struct MPT3SAS_ADAPTER
*ioc, u16 smid)
Mpi2RequestDescriptorUnion_t descriptor;
void *mpi_req_iomem;
u64 *request;
- MPI2RequestHeader_t *request_hdr;
if (ioc->is_mcpu_endpoint) {
__le32 *mfp = (__le32 *)mpt3sas_base_get_msg_frame(ioc, smid);
- request_hdr = (MPI2RequestHeader_t *)mfp;
-
_clone_sg_entries(ioc, (void *) mfp, smid);
/* TBD 256 is offset within sys register */
mpi_req_iomem = (void __force *)ioc->chip +
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 42a8d79be7ec..cac9a264e152 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -1653,7 +1653,6 @@ scsih_target_destroy(struct scsi_target *starget)
struct _raid_device *raid_device;
struct _pcie_device *pcie_device;
unsigned long flags;
- struct sas_rphy *rphy;
sas_target_priv_data = starget->hostdata;
if (!sas_target_priv_data)
@@ -1693,7 +1692,6 @@ scsih_target_destroy(struct scsi_target *starget)
}
spin_lock_irqsave(>sas_device_lock, flags);
- rphy = dev_to_rphy(starget->dev.parent);
sas_device = __mpt3sas_get_sdev_from_target(ioc, sas_target_priv_data);
if (sas_device && (sas_device->starget == starget) &&
(sas_device->id == starget->id) &&
@@ -6873,7 +6871,6 @@ _scsih_pcie_add_device(struct MPT3SAS_ADAPTER *ioc, u16
handle)
Mpi2ConfigReply_t mpi_reply;
struct _pcie_device *pcie_device;
struct _enclosure_node *enclosure_dev;
- u32 pcie_device_type;
u32 ioc_status;
u64 wwid;
@@ -6935,8 +6932,6 @@ _scsih_pcie_add_device(struct MPT3SAS_ADAPTER *ioc, u16
handle)
pcie_device->port_num = pcie_device_pg0.PortNum;
pcie_device->fast_path = (le32_to_cpu(pcie_device_pg0.Flags) &
MPI26_PCIEDEV0_FLAGS_FAST_PATH_CAPABLE) ? 1 : 0;
- pcie_device_type = pcie_device->device_info &
- MPI26_PCIE_DEVINFO_MASK_DEVICE_TYPE;
pcie_device->enclosure_handle =
le16_to_cpu(pcie_device_pg0.EnclosureHandle);
--
2.17.0
[PATCH 5/8] mpt3sas: Introduce struct mpt3sas_nvme_cmd
Make _base_build_nvme_prp() easier to read by introducing a structure
to access NVMe command fields.
Signed-off-by: Bart Van Assche
Cc: Sathya Prakash
Cc: Chaitra P B
Cc: Suganath Prabu Subramani
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 13 -
drivers/scsi/mpt3sas/mpt3sas_base.h | 7 +--
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 67e1b603f287..d681e4f2691a 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1834,6 +1834,8 @@ _base_build_nvme_prp(struct MPT3SAS_ADAPTER *ioc, u16
smid,
u32 offset, entry_len;
u32 page_mask_result, page_mask;
size_t length;
+ struct mpt3sas_nvme_cmd *nvme_cmd =
+ (void *)nvme_encap_request->NVMe_Command;
/*
* Not all commands require a data transfer. If no data, just return
@@ -1841,15 +1843,8 @@ _base_build_nvme_prp(struct MPT3SAS_ADAPTER *ioc, u16
smid,
*/
if (!data_in_sz && !data_out_sz)
return;
- /*
-* Set pointers to PRP1 and PRP2, which are in the NVMe command.
-* PRP1 is located at a 24 byte offset from the start of the NVMe
-* command. Then set the current PRP entry pointer to PRP1.
-*/
- prp1_entry = (__le64 *)(nvme_encap_request->NVMe_Command +
- NVME_CMD_PRP1_OFFSET);
- prp2_entry = (__le64 *)(nvme_encap_request->NVMe_Command +
- NVME_CMD_PRP2_OFFSET);
+ prp1_entry = _cmd->prp1;
+ prp2_entry = _cmd->prp2;
prp_entry = prp1_entry;
/*
* For the PRP entries, use the specially allocated buffer of
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h
b/drivers/scsi/mpt3sas/mpt3sas_base.h
index f02974c0be4a..c6c36750deaa 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -143,14 +143,17 @@
* NVMe defines
*/
#defineNVME_PRP_SIZE 8 /* PRP size */
-#defineNVME_CMD_PRP1_OFFSET24 /* PRP1 offset in NVMe
cmd */
-#defineNVME_CMD_PRP2_OFFSET32 /* PRP2 offset in NVMe
cmd */
#defineNVME_ERROR_RESPONSE_SIZE16 /* Max NVME Error
Response */
#define NVME_TASK_ABORT_MIN_TIMEOUT6
#define NVME_TASK_ABORT_MAX_TIMEOUT60
#define NVME_TASK_MNGT_CUSTOM_MASK (0x0010)
#defineNVME_PRP_PAGE_SIZE 4096/* Page size */
+struct mpt3sas_nvme_cmd {
+ u8 rsvd[24];
+ __le64 prp1;
+ __le64 prp2;
+};
/*
* reset phases
--
2.17.0
[PATCH 7/8] mpt3sas: Fix a race condition in mpt3sas_base_hard_reset_handler()
Since ioc->shost_recovery is set after ioc->reset_in_progress_mutex
is obtained, if concurrent resets are issued there is a short time
during which ioc->reset_in_progress_mutex is locked and
ioc->shost_recovery == 0. Avoid that this can cause trouble by
unconditionally locking ioc->shost_recovery.
Signed-off-by: Bart Van Assche
Cc: Sathya Prakash
Cc: Chaitra P B
Cc: Suganath Prabu Subramani
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 10 +-
drivers/scsi/mpt3sas/mpt3sas_base.h | 1 -
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index d681e4f2691a..c5cb1b2333c0 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -6932,14 +6932,7 @@ mpt3sas_base_hard_reset_handler(struct MPT3SAS_ADAPTER
*ioc,
mpt3sas_halt_firmware(ioc);
/* wait for an active reset in progress to complete */
- if (!mutex_trylock(>reset_in_progress_mutex)) {
- do {
- ssleep(1);
- } while (ioc->shost_recovery == 1);
- dtmprintk(ioc, pr_info(MPT3SAS_FMT "%s: exit\n", ioc->name,
- __func__));
- return ioc->ioc_reset_in_progress_status;
- }
+ mutex_lock(>reset_in_progress_mutex);
spin_lock_irqsave(>ioc_reset_in_progress_lock, flags);
ioc->shost_recovery = 1;
@@ -6988,7 +6981,6 @@ mpt3sas_base_hard_reset_handler(struct MPT3SAS_ADAPTER
*ioc,
ioc->name, __func__, ((r == 0) ? "SUCCESS" : "FAILED")));
spin_lock_irqsave(>ioc_reset_in_progress_lock, flags);
- ioc->ioc_reset_in_progress_status = r;
ioc->shost_recovery = 0;
spin_unlock_irqrestore(>ioc_reset_in_progress_lock, flags);
ioc->ioc_reset_count++;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h
b/drivers/scsi/mpt3sas/mpt3sas_base.h
index c6c36750deaa..a23ae2eb9c09 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1165,7 +1165,6 @@ struct MPT3SAS_ADAPTER {
struct mutexreset_in_progress_mutex;
spinlock_t ioc_reset_in_progress_lock;
u8 ioc_link_reset_in_progress;
- u8 ioc_reset_in_progress_status;
u8 ignore_loginfos;
u8 remove_host;
--
2.17.0
[PATCH 4/8] mpt3sas: Fix kernel-doc warnings
This patch avoids that warnings about the kernel headers appear when
building with W=1.
Signed-off-by: Bart Van Assche
Cc: Sathya Prakash
Cc: Chaitra P B
Cc: Suganath Prabu Subramani
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 16 +-
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 251 +++-
drivers/scsi/mpt3sas/mpt3sas_scsih.c| 36 +--
drivers/scsi/mpt3sas/mpt3sas_transport.c| 10 +-
drivers/scsi/mpt3sas/mpt3sas_trigger_diag.c | 18 +-
drivers/scsi/mpt3sas/mpt3sas_warpdrive.c| 3 -
6 files changed, 186 insertions(+), 148 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 9c233ddc5b2b..67e1b603f287 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -103,7 +103,8 @@ _base_get_ioc_facts(struct MPT3SAS_ADAPTER *ioc);
/**
* _scsih_set_fwfault_debug - global setting of ioc->fwfault_debug.
- *
+ * @val: ?
+ * @kp: ?
*/
static int
_scsih_set_fwfault_debug(const char *val, const struct kernel_param *kp)
@@ -197,7 +198,7 @@ _base_clone_to_sys_mem(void __iomem *dst_iomem, void *src,
u32 size)
* @smid: system request message index
* @sge_chain_count: Scatter gather chain count.
*
- * @Return: chain address.
+ * Returns the chain address.
*/
static inline void __iomem*
_base_get_chain(struct MPT3SAS_ADAPTER *ioc, u16 smid,
@@ -322,8 +323,6 @@ _base_get_chain_buffer_dma_to_chain_buffer(struct
MPT3SAS_ADAPTER *ioc,
* @ioc: per adapter object.
* @mpi_request: mf request pointer.
* @smid: system request message index.
- *
- * @Returns: Nothing.
*/
static void _clone_sg_entries(struct MPT3SAS_ADAPTER *ioc,
void *mpi_request, u16 smid)
@@ -1358,7 +1357,6 @@ union reply_descriptor {
* _base_interrupt - MPT adapter (IOC) specific interrupt handler.
* @irq: irq number (not used)
* @bus_id: bus identifier cookie == pointer to MPT_ADAPTER structure
- * @r: pt_regs pointer (not used)
*
* Return IRQ_HANDLE if processed, else IRQ_NONE.
*/
@@ -1777,7 +1775,7 @@ _base_build_sg(struct MPT3SAS_ADAPTER *ioc, void *psge,
* describes the first data memory segment, and PRP2 contains a pointer to a
PRP
* list located elsewhere in memory to describe the remaining data memory
* segments. The PRP list will be contiguous.
-
+ *
* The native SGL for NVMe devices is a Physical Region Page (PRP). A PRP
* consists of a list of PRP entries to describe a number of noncontigous
* physical memory segments as a single memory buffer, just as a SGL does.
Note
@@ -3350,7 +3348,6 @@ _base_mpi_ep_writeq(__u64 b, volatile void __iomem *addr,
/**
* _base_writeq - 64 bit write to MMIO
- * @ioc: per adapter object
* @b: data payload
* @addr: address in MMIO space
* @writeq_lock: spin lock
@@ -4980,6 +4977,7 @@ mpt3sas_base_get_iocstate(struct MPT3SAS_ADAPTER *ioc,
int cooked)
/**
* _base_wait_on_iocstate - waiting on a particular ioc state
+ * @ioc: ?
* @ioc_state: controller state { READY, OPERATIONAL, or RESET }
* @timeout: timeout in second
*
@@ -5011,7 +5009,6 @@ _base_wait_on_iocstate(struct MPT3SAS_ADAPTER *ioc, u32
ioc_state, int timeout)
* _base_wait_for_doorbell_int - waiting for controller interrupt(generated by
* a write to the doorbell)
* @ioc: per adapter object
- * @timeout: timeout in second
*
* Returns 0 for success, non-zero for failure.
*
@@ -5529,6 +5526,7 @@ mpt3sas_base_scsi_enclosure_processor(struct
MPT3SAS_ADAPTER *ioc,
/**
* _base_get_port_facts - obtain port facts reply and save in ioc
* @ioc: per adapter object
+ * @port: ?
*
* Returns 0 for success, non-zero for failure.
*/
@@ -6109,7 +6107,7 @@ _base_event_notification(struct MPT3SAS_ADAPTER *ioc)
/**
* mpt3sas_base_validate_event_type - validating event types
* @ioc: per adapter object
- * @event: firmware event
+ * @event_type: firmware event
*
* This will turn on firmware event notification when application
* ask for that event. We don't mask events that are already enabled.
diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index 07d28b7d5f1c..f84ffa07f525 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -412,7 +412,7 @@ mpt3sas_ctl_event_callback(struct MPT3SAS_ADAPTER *ioc, u8
msix_index,
/**
* _ctl_verify_adapter - validates ioc_number passed from application
- * @ioc: per adapter object
+ * @ioc_number: ?
* @iocpp: The ioc pointer is returned in this.
* @mpi_version: will be MPI2_VERSION for mpt2ctl ioctl device &
* MPI25_VERSION | MPI26_VERSION for mpt3ctl ioctl device.
@@ -516,9 +516,9 @@ mpt3sas_ctl_reset_handler(struct MPT3SAS_ADAPTER *ioc, int
reset_phase)
/**
* _ctl_fasync -
- * @fd -
- * @filep -
- * @mode -
+ * @fd: ?
+ * @filep: ?
+ * @mode: ?
*
* Called when application request fasyn callback handler.
*/
@@ -530,8 +530,8 @@ _ctl_fasync(int fd, struct file *filep, int mode)
/**
*
[PATCH 8/8] mpt3sas: Split _base_reset_handler(), mpt3sas_scsih_reset_handler() and mpt3sas_ctl_reset_handler()
Split each of these functions in three functions - one function
per reset phase. This patch does not change any functionality but
makes the code easier to read.
Signed-off-by: Bart Van Assche
Cc: Sathya Prakash
Cc: Chaitra P B
Cc: Suganath Prabu Subramani
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 118 ++-
drivers/scsi/mpt3sas/mpt3sas_base.h | 15 ++--
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 91 +++--
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 94 +++--
4 files changed, 168 insertions(+), 150 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index c5cb1b2333c0..395c7ddb0be1 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -6813,65 +6813,69 @@ mpt3sas_base_detach(struct MPT3SAS_ADAPTER *ioc)
}
/**
- * _base_reset_handler - reset callback handler (for base)
+ * _base_pre_reset_handler - pre reset handler
* @ioc: per adapter object
- * @reset_phase: phase
- *
- * The handler for doing any required cleanup or initialization.
- *
- * The reset phase can be MPT3_IOC_PRE_RESET, MPT3_IOC_AFTER_RESET,
- * MPT3_IOC_DONE_RESET
- *
- * Return nothing.
*/
-static void
-_base_reset_handler(struct MPT3SAS_ADAPTER *ioc, int reset_phase)
+static void _base_pre_reset_handler(struct MPT3SAS_ADAPTER *ioc)
{
- mpt3sas_scsih_reset_handler(ioc, reset_phase);
- mpt3sas_ctl_reset_handler(ioc, reset_phase);
- switch (reset_phase) {
- case MPT3_IOC_PRE_RESET:
- dtmprintk(ioc, pr_info(MPT3SAS_FMT
- "%s: MPT3_IOC_PRE_RESET\n", ioc->name, __func__));
- break;
- case MPT3_IOC_AFTER_RESET:
- dtmprintk(ioc, pr_info(MPT3SAS_FMT
- "%s: MPT3_IOC_AFTER_RESET\n", ioc->name, __func__));
- if (ioc->transport_cmds.status & MPT3_CMD_PENDING) {
- ioc->transport_cmds.status |= MPT3_CMD_RESET;
- mpt3sas_base_free_smid(ioc, ioc->transport_cmds.smid);
- complete(>transport_cmds.done);
- }
- if (ioc->base_cmds.status & MPT3_CMD_PENDING) {
- ioc->base_cmds.status |= MPT3_CMD_RESET;
- mpt3sas_base_free_smid(ioc, ioc->base_cmds.smid);
- complete(>base_cmds.done);
- }
- if (ioc->port_enable_cmds.status & MPT3_CMD_PENDING) {
- ioc->port_enable_failed = 1;
- ioc->port_enable_cmds.status |= MPT3_CMD_RESET;
- mpt3sas_base_free_smid(ioc, ioc->port_enable_cmds.smid);
- if (ioc->is_driver_loading) {
- ioc->start_scan_failed =
- MPI2_IOCSTATUS_INTERNAL_ERROR;
- ioc->start_scan = 0;
- ioc->port_enable_cmds.status =
- MPT3_CMD_NOT_USED;
- } else
- complete(>port_enable_cmds.done);
- }
- if (ioc->config_cmds.status & MPT3_CMD_PENDING) {
- ioc->config_cmds.status |= MPT3_CMD_RESET;
- mpt3sas_base_free_smid(ioc, ioc->config_cmds.smid);
- ioc->config_cmds.smid = USHRT_MAX;
- complete(>config_cmds.done);
+ mpt3sas_scsih_pre_reset_handler(ioc);
+ mpt3sas_ctl_pre_reset_handler(ioc);
+ dtmprintk(ioc, pr_info(MPT3SAS_FMT
+ "%s: MPT3_IOC_PRE_RESET\n", ioc->name, __func__));
+}
+
+/**
+ * _base_after_reset_handler - after reset handler
+ * @ioc: per adapter object
+ */
+static void _base_after_reset_handler(struct MPT3SAS_ADAPTER *ioc)
+{
+ mpt3sas_scsih_after_reset_handler(ioc);
+ mpt3sas_ctl_after_reset_handler(ioc);
+ dtmprintk(ioc, pr_info(MPT3SAS_FMT
+ "%s: MPT3_IOC_AFTER_RESET\n", ioc->name, __func__));
+ if (ioc->transport_cmds.status & MPT3_CMD_PENDING) {
+ ioc->transport_cmds.status |= MPT3_CMD_RESET;
+ mpt3sas_base_free_smid(ioc, ioc->transport_cmds.smid);
+ complete(>transport_cmds.done);
+ }
+ if (ioc->base_cmds.status & MPT3_CMD_PENDING) {
+ ioc->base_cmds.status |= MPT3_CMD_RESET;
+ mpt3sas_base_free_smid(ioc, ioc->base_cmds.smid);
+ complete(>base_cmds.done);
+ }
+ if (ioc->port_enable_cmds.status & MPT3_CMD_PENDING) {
+ ioc->port_enable_failed = 1;
+ ioc->port_enable_cmds.status |= MPT3_CMD_RESET;
+ mpt3sas_base_free_smid(ioc, ioc->port_enable_cmds.smid);
+ if (ioc->is_driver_loading) {
+ ioc->start_scan_failed =
+ MPI2_IOCSTATUS_INTERNAL_ERROR;
+ ioc->start_scan = 0;
+
[PATCH 6/8] mpt3sas: Fix _transport_smp_handler() error path
This patch avoids that smatch complains about a double unlock on
ioc->transport_cmds.mutex.
Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for SMP
passthrough")
Signed-off-by: Bart Van Assche
Cc: Christoph Hellwig
Cc: Sathya Prakash
Cc: Chaitra P B
Cc: Suganath Prabu Subramani
Cc: sta...@vger.kernel.org
---
drivers/scsi/mpt3sas/mpt3sas_transport.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c
b/drivers/scsi/mpt3sas/mpt3sas_transport.c
index 77a8ec401461..53224d97f6c1 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_transport.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_transport.c
@@ -1938,12 +1938,12 @@ _transport_smp_handler(struct bsg_job *job, struct
Scsi_Host *shost,
pr_info(MPT3SAS_FMT "%s: host reset in progress!\n",
__func__, ioc->name);
rc = -EFAULT;
- goto out;
+ goto job_done;
}
rc = mutex_lock_interruptible(>transport_cmds.mutex);
if (rc)
- goto out;
+ goto job_done;
if (ioc->transport_cmds.status != MPT3_CMD_NOT_USED) {
pr_err(MPT3SAS_FMT "%s: transport_cmds in use\n", ioc->name,
@@ -2068,6 +2068,7 @@ _transport_smp_handler(struct bsg_job *job, struct
Scsi_Host *shost,
out:
ioc->transport_cmds.status = MPT3_CMD_NOT_USED;
mutex_unlock(>transport_cmds.mutex);
+job_done:
bsg_job_done(job, rc, reslen);
}
--
2.17.0
[PATCH 3/8] mpt3sas: Annotate switch/case fall-through
This patch avoids that gcc complains about switch/case fall-through when building with W=1. Signed-off-by: Bart Van Assche Cc: Sathya Prakash Cc: Chaitra P B Cc: Suganath Prabu Subramani --- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 1 + drivers/scsi/mpt3sas/mpt3sas_scsih.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c index 3269ef43f07e..07d28b7d5f1c 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c @@ -970,6 +970,7 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg, } /* drop to default case for posting the request */ } + /* fall through */ default: ioc->build_sg_mpi(ioc, psge, data_out_dma, data_out_sz, data_in_dma, data_in_sz); diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index cac9a264e152..f1a748081fe4 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -5414,6 +5414,7 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply) case MPI2_IOCSTATUS_SCSI_DATA_OVERRUN: scsi_set_resid(scmd, 0); + /* fall through */ case MPI2_IOCSTATUS_SCSI_RECOVERED_ERROR: case MPI2_IOCSTATUS_SUCCESS: scmd->result = (DID_OK << 16) | scsi_status; @@ -6444,6 +6445,7 @@ _scsih_sas_topology_change_event(struct MPT3SAS_ADAPTER *ioc, if (!test_bit(handle, ioc->pend_os_device_add)) break; + /* fall through */ case MPI2_EVENT_SAS_TOPO_RC_TARG_ADDED: @@ -7160,6 +7162,7 @@ _scsih_pcie_topology_change_event(struct MPT3SAS_ADAPTER *ioc, event_data->PortEntry[i].PortStatus &= 0xF0; event_data->PortEntry[i].PortStatus |= MPI26_EVENT_PCIE_TOPO_PS_DEV_ADDED; + /* fall through */ case MPI26_EVENT_PCIE_TOPO_PS_DEV_ADDED: if (ioc->shost_recovery) break; -- 2.17.0
[PATCH 0/8] mpt3sas: Fix static source code checker complaints
Hello Martin, This patch series addresses the complaints reported by multiple static source code analysis tools about the mpt3sas source code. Please consider these patches for kernel v4.19. Thanks, Bart. Bart Van Assche (8): mpt3sas: Fix indentation mpt3sas: Remove set-but-not-used variables mpt3sas: Annotate switch/case fall-through mpt3sas: Fix kernel-doc warnings mpt3sas: Introduce struct mpt3sas_nvme_cmd mpt3sas: Fix _transport_smp_handler() error path mpt3sas: Fix a race condition in mpt3sas_base_hard_reset_handler() mpt3sas: Split _base_reset_handler(), mpt3sas_scsih_reset_handler() and mpt3sas_ctl_reset_handler() drivers/scsi/mpt3sas/mpt3sas_base.c | 169 +- drivers/scsi/mpt3sas/mpt3sas_base.h | 23 +- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 343 +++- drivers/scsi/mpt3sas/mpt3sas_scsih.c| 178 +- drivers/scsi/mpt3sas/mpt3sas_transport.c| 17 +- drivers/scsi/mpt3sas/mpt3sas_trigger_diag.c | 18 +- drivers/scsi/mpt3sas/mpt3sas_warpdrive.c| 3 - 7 files changed, 393 insertions(+), 358 deletions(-) -- 2.17.0
[PATCH 1/8] mpt3sas: Fix indentation
Modify the indentation such that smatch no longer complains about
inconsistent indenting.
Signed-off-by: Bart Van Assche
Cc: Sathya Prakash
Cc: Chaitra P B
Cc: Suganath Prabu Subramani
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 2 +-
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 40
drivers/scsi/mpt3sas/mpt3sas_transport.c | 2 +-
3 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 569392d0d4c9..798ee62c97f1 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -633,7 +633,7 @@ mpt3sas_base_start_watchdog(struct MPT3SAS_ADAPTER *ioc)
if (!ioc->fault_reset_work_q) {
pr_err(MPT3SAS_FMT "%s: failed (line=%d)\n",
ioc->name, __func__, __LINE__);
- return;
+ return;
}
spin_lock_irqsave(>ioc_reset_in_progress_lock, flags);
if (ioc->fault_reset_work_q)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index b8d131a455d0..42a8d79be7ec 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2256,7 +2256,7 @@ scsih_slave_configure(struct scsi_device *sdev)
ds = "SSP";
} else {
qdepth = MPT3SAS_SATA_QUEUE_DEPTH;
-if (raid_device->device_info &
+ if (raid_device->device_info &
MPI2_SAS_DEVICE_INFO_SATA_DEVICE)
ds = "SATA";
else
@@ -4004,19 +4004,19 @@ _scsih_issue_delayed_event_ack(struct MPT3SAS_ADAPTER
*ioc, u16 smid, U16 event,
static void
_scsih_issue_delayed_sas_io_unit_ctrl(struct MPT3SAS_ADAPTER *ioc,
u16 smid, u16 handle)
- {
- Mpi2SasIoUnitControlRequest_t *mpi_request;
- u32 ioc_state;
- int i = smid - ioc->internal_smid;
- unsigned long flags;
+{
+ Mpi2SasIoUnitControlRequest_t *mpi_request;
+ u32 ioc_state;
+ int i = smid - ioc->internal_smid;
+ unsigned long flags;
- if (ioc->remove_host) {
- dewtprintk(ioc, pr_info(MPT3SAS_FMT
+ if (ioc->remove_host) {
+ dewtprintk(ioc, pr_info(MPT3SAS_FMT
"%s: host has been removed\n",
__func__, ioc->name));
- return;
- } else if (ioc->pci_error_recovery) {
- dewtprintk(ioc, pr_info(MPT3SAS_FMT
+ return;
+ } else if (ioc->pci_error_recovery) {
+ dewtprintk(ioc, pr_info(MPT3SAS_FMT
"%s: host in pci error recovery\n",
__func__, ioc->name));
return;
@@ -4674,19 +4674,19 @@ scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd
*scmd)
}
- /* host recovery or link resets sent via IOCTLs */
- if (ioc->shost_recovery || ioc->ioc_link_reset_in_progress)
+ if (ioc->shost_recovery || ioc->ioc_link_reset_in_progress) {
+ /* host recovery or link resets sent via IOCTLs */
return SCSI_MLQUEUE_HOST_BUSY;
-
- /* device has been deleted */
- else if (sas_target_priv_data->deleted) {
+ } else if (sas_target_priv_data->deleted) {
+ /* device has been deleted */
scmd->result = DID_NO_CONNECT << 16;
scmd->scsi_done(scmd);
return 0;
- /* device busy with task management */
} else if (sas_target_priv_data->tm_busy ||
- sas_device_priv_data->block)
+ sas_device_priv_data->block) {
+ /* device busy with task management */
return SCSI_MLQUEUE_DEVICE_BUSY;
+ }
/*
* Bug work around for firmware SATL handling. The loop
@@ -5812,7 +5812,7 @@ _scsih_expander_add(struct MPT3SAS_ADAPTER *ioc, u16
handle)
}
_scsih_expander_node_add(ioc, sas_expander);
-return 0;
+ return 0;
out_fail:
@@ -9519,7 +9519,7 @@ _mpt3sas_fw_work(struct MPT3SAS_ADAPTER *ioc, struct
fw_event_work *fw_event)
break;
case MPT3SAS_PORT_ENABLE_COMPLETE:
ioc->start_scan = 0;
- if (missing_delay[0] != -1 && missing_delay[1] != -1)
+ if (missing_delay[0] != -1 && missing_delay[1] != -1)
mpt3sas_base_update_missing_delay(ioc, missing_delay[0],
missing_delay[1]);
dewtprintk(ioc, pr_info(MPT3SAS_FMT
diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c
b/drivers/scsi/mpt3sas/mpt3sas_transport.c
index 3a143bb5ca72..05d506d78c66 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_transport.c
+++
Re: [PATCH 1/2 v2] libsas: remove irq save in sas_ata_qc_issue()
nit:
scsi: libsas: remove irq save in sas_ata_qc_issue()
On 14/06/2018 17:18, Sebastian Andrzej Siewior wrote:
Since commit 312d3e56119a ("[SCSI] libsas: remove ata_port.lock
management duties from lldds") the sas_ata_qc_issue() function unlocks
the ata_port.lock and disables interrupts before doing so.
That lock is always taken with disabled interrupts so at this point, the
interrupts are already disabled. There is no need to disable the
interrupts before the unlock operation because they are already
disabled.
Restoring the interrupt state later does not change anything because
they were disabled and remain disabled. Therefore remove the operations
which do not change the behaviour.
Signed-off-by: Sebastian Andrzej Siewior
Reviewed-by: John Garry
Re: [PATCH 1/2 v2] libsas: remove irq save in sas_ata_qc_issue()
On Thu, Jun 14, 2018 at 9:18 AM, Sebastian Andrzej Siewior
wrote:
> Since commit 312d3e56119a ("[SCSI] libsas: remove ata_port.lock
> management duties from lldds") the sas_ata_qc_issue() function unlocks
> the ata_port.lock and disables interrupts before doing so.
> That lock is always taken with disabled interrupts so at this point, the
> interrupts are already disabled. There is no need to disable the
> interrupts before the unlock operation because they are already
> disabled.
> Restoring the interrupt state later does not change anything because
> they were disabled and remain disabled. Therefore remove the operations
> which do not change the behaviour.
>
> Signed-off-by: Sebastian Andrzej Siewior
Acked-by: Dan Williams
[PATCH 1/2 v2] libsas: remove irq save in sas_ata_qc_issue()
Since commit 312d3e56119a ("[SCSI] libsas: remove ata_port.lock
management duties from lldds") the sas_ata_qc_issue() function unlocks
the ata_port.lock and disables interrupts before doing so.
That lock is always taken with disabled interrupts so at this point, the
interrupts are already disabled. There is no need to disable the
interrupts before the unlock operation because they are already
disabled.
Restoring the interrupt state later does not change anything because
they were disabled and remain disabled. Therefore remove the operations
which do not change the behaviour.
Signed-off-by: Sebastian Andrzej Siewior
---
v1…v2: Updating comment as suggested by Dan Williams.
drivers/scsi/libsas/sas_ata.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index ff1d612f6fb9..2ac7395112b4 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -176,7 +176,6 @@ static void sas_ata_task_done(struct sas_task *task)
static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc)
{
- unsigned long flags;
struct sas_task *task;
struct scatterlist *sg;
int ret = AC_ERR_SYSTEM;
@@ -187,10 +186,7 @@ static unsigned int sas_ata_qc_issue(struct ata_queued_cmd
*qc)
struct Scsi_Host *host = sas_ha->core.shost;
struct sas_internal *i = to_sas_internal(host->transportt);
- /* TODO: audit callers to ensure they are ready for qc_issue to
-* unconditionally re-enable interrupts
-*/
- local_irq_save(flags);
+ /* TODO: we should try to remove that unlock */
spin_unlock(ap->lock);
/* If the device fell off, no sense in issuing commands */
@@ -252,7 +248,6 @@ static unsigned int sas_ata_qc_issue(struct ata_queued_cmd
*qc)
out:
spin_lock(ap->lock);
- local_irq_restore(flags);
return ret;
}
--
2.17.1
Re: [PATCH] mpt3sas: Fix calltrace observed while running IO & host reset
On Thu, 2018-06-14 at 15:56 +0530, Chaitra Basappa wrote: > Bart, > Please see my replies inline. Hello Chaitra, Please don't use inline replies. That makes it very hard to follow the conversation. BTW, in the headers of your e-mail I found the following: "X-Mailer: Microsoft Outlook 14.0". Please use another e-mail client that is better suited for collaboration on open source mailing lists. If outgoing e-mails have to pass through an Exchange server, the following e-mail clients support Exchange servers and are better suited for open source collaboration: * Evolution (Linux only). * Thunderbird + ExQuilla plugin. * If IMAP has been enabled on the Exchange server that you are using then that means that you can choose from the many open source e-mail clients that support IMAP. Thanks, Bart.
Re: [PATCH v1] mpt3sas: Fix calltrace observed while running IO & host reset
On Thu, 2018-06-14 at 06:25 -0400, Chaitra P B wrote:
> As a part of host reset operation, driver will flushout all IOs
> outstanding at driver level with "DID_RESET" result.
> To find which are all commands outstanding at the driver level,
> driver loops with smid starting from one to HBA queue depth and
> calls mpt3sas_scsih_scsi_lookup_get() to get scmd as shown below
>
> for (smid = 1; smid <= ioc->scsiio_depth; smid++) {
> scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
> if (!scmd)
> continue;
>
> But in mpt3sas_scsih_scsi_lookup_get() function, driver returns some
> scsi cmnds which are not outstanding at the driver level (possibly request
> is constructed at block layer since QUEUE_FLAG_QUIESCED is not set. Even if
> driver uses scsi_block_requests and scsi_unblock_requests, issue still
> persists as they will be just blocking further IO from scsi layer and
> not from block layer) and these commands are flushed with
> DID_RESET host bytes thus resulting into above kernel BUG.
Have you considered to call scsi_target_block() before flushing out pending
I/O and to call scsi_target_unblock() after flushing out pending I/O has
finished? I think that would be a much cleaner solution than the proposed
patch. The effect of scsi_target_block() is explained above
scsi_internal_device_block():
/**
* scsi_internal_device_block - try to transition to the SDEV_BLOCK state
* @sdev: device to block
*
* Pause SCSI command processing on the specified device and wait until all
* ongoing scsi_request_fn() / scsi_queue_rq() calls have finished. May sleep.
*
* Returns zero if successful or a negative error code upon failure.
*
* Note:
* This routine transitions the device to the SDEV_BLOCK state (which must be
* a legal transition). When the device is in this state, command processing
* is paused until the device leaves the SDEV_BLOCK state. See also
* scsi_internal_device_unblock().
*
* To do: avoid that scsi_send_eh_cmnd() calls queuecommand() after
* scsi_internal_device_block() has blocked a SCSI device and also
* remove the rport mutex lock and unlock calls from srp_queuecommand().
*/
Thanks,
Bart.
Re: [PATCH 0/2 REPOST] remove unneded irq save
On Thu, Jun 14, 2018 at 7:30 AM, Sebastian Andrzej Siewior wrote: > On 2018-06-12 08:46:38 [-0700], Dan Williams wrote: >> On Tue, Jun 12, 2018 at 8:04 AM, John Garry wrote: >> >> We had this comment for 6 years or so and nothing happend. What makes >> >> you think that an updated version of that comment will motivate someone >> >> to make change here in the near future? >> > >> > Updating the comment is not in itself something to motivate someone to >> > change, but we should keep the comment reasonably accurate or get rid. >> > >> >> It looks to me like a stale comment which won't change a thing because >> >> it does not point out the benefit of doing so (re-enabling interrupts >> >> while dropping the lock) and the price, that is paid for not doing so >> >> (keeping the code as it is) is small enough to not bother. >> >> >> >> So if updating the comment as suggested instead of keeping it as-is or >> >> removing it is the blocker *here* then I can send an updated version. >> >> Any comments? >> > >> > >> > I'd prefer an updated comment. >> > >> >> I think we should try to remove the unlock completely. I agree with >> Sebastian that the audit is never coming. As it is libsas is the only >> ata_port_operations implementation that drops the host_lock while >> running ->qc_issue(). > > Dan, so in meantime I update the comment in patch #1 [0] to say "we > should try to remove unlock completely"? > > [0] https://lkml.kernel.org/r/20180611144053.18294-2-bige...@linutronix.de > Up to you, I'd ack that patch either way... I just wanted to propose the idea that unlock/relock flows tend to have hidden bugs and would be worthwhile to revisit why libsas needs that arrangement.
Re: [PATCH 0/2 REPOST] remove unneded irq save
On 2018-06-12 08:46:38 [-0700], Dan Williams wrote: > On Tue, Jun 12, 2018 at 8:04 AM, John Garry wrote: > >> We had this comment for 6 years or so and nothing happend. What makes > >> you think that an updated version of that comment will motivate someone > >> to make change here in the near future? > > > > Updating the comment is not in itself something to motivate someone to > > change, but we should keep the comment reasonably accurate or get rid. > > > >> It looks to me like a stale comment which won't change a thing because > >> it does not point out the benefit of doing so (re-enabling interrupts > >> while dropping the lock) and the price, that is paid for not doing so > >> (keeping the code as it is) is small enough to not bother. > >> > >> So if updating the comment as suggested instead of keeping it as-is or > >> removing it is the blocker *here* then I can send an updated version. > >> Any comments? > > > > > > I'd prefer an updated comment. > > > > I think we should try to remove the unlock completely. I agree with > Sebastian that the audit is never coming. As it is libsas is the only > ata_port_operations implementation that drops the host_lock while > running ->qc_issue(). Dan, so in meantime I update the comment in patch #1 [0] to say "we should try to remove unlock completely"? [0] https://lkml.kernel.org/r/20180611144053.18294-2-bige...@linutronix.de Sebastian
Re: I will definately enlighten you more as soon as I hear from you.
Good Morning, Although, I am not comfortable discussing the content of my mail on the Internet owing to lots of unsolicited/Spam mails on the net nowadays. However, I have made up my mind to will my late Husband's funds to you so you can use it for charity duties and good work to humanity. The amounts are relatively huge, but please get back to me for further information. I will definately enlighten you more as soon as I hear from you. Best of wishes to you, Mrs. Rania Hassya
[PATCH V3 0/3] Add ufs provisioning support in driver
This change adds a new API ufshcd_do_config_device() to write configuration descriptor with the provisioning data. Configfs support is added in driver to trigger ufs provisioning at runtime. Provisioning data is parsed from vendor specific provisioning file. This parsed data is passed as a buffer via configfs to provision ufs device. Changes since V2: Added configfs support for ufs provisioning and removed sysfs support. Changes since V1: Added device tree entry to parse reference clock frequency instead of hardcoding 19.2 MHz, as it can vary for different vendors. Also removed setting ref_clk again during runtime provisioning as it will be already set during probe. Used get_unaligned_be*/put_unaligned_be* where applicable. Changes since RFC: Added check to avoid ufs runtime provisioning if Configuration decriptor lock attribute is set to one. Instead of parsing ref_clk frequency via device tree, used correct enum ref_clk_freq value(19.2 Mhz for proviosioning). Added config_descriptor sysfs entry to provision ufs and also updated documentation for its correct usage. Added more protection against bad data handling in sysfs code. Sayali Lokhande (2): scsi: ufs: Add ufs provisioning support scsi: ufs: Add configfs support for ufs provisioning Subhash Jadavani (1): scsi: ufs: set the device reference clock setting Documentation/ABI/testing/configfs-driver-ufs | 14 ++ .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 7 + drivers/scsi/ufs/Makefile | 1 + drivers/scsi/ufs/ufs-configfs.c| 191 + drivers/scsi/ufs/ufs.h | 39 drivers/scsi/ufs/ufshcd-pltfrm.c | 24 +++ drivers/scsi/ufs/ufshcd.c | 234 + drivers/scsi/ufs/ufshcd.h | 10 + 8 files changed, 520 insertions(+) create mode 100644 Documentation/ABI/testing/configfs-driver-ufs create mode 100644 drivers/scsi/ufs/ufs-configfs.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: RAID6: "Bad block number requested"
Dear James, dear all!
Am 11.06.2018 um 17:06 schrieb James Bottomley
:
> This means that somehow, something sent a non 4k aligned 4k sized
> request. SCSI here is just the messenger. However, if you apply this
> patch, it will capture the stack trace of what above it triggered this,
> which may help us in debugging. It could be we may also want to see
> what the values of block and blk_rq_sectors(rq) actually are, but lets
> begin with the stack trace.
> ---
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 9421d9877730..ac865e048533 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1109,6 +1109,7 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd
> *SCpnt)
> if ((block & 7) || (blk_rq_sectors(rq) & 7)) {
> scmd_printk(KERN_ERR, SCpnt,
> "Bad block number requested\n");
> + WARN_ON_ONCE(1);
> goto out;
> } else {
> block = block >> 3;
I'll give that a try. But don't expect to hear from me soon, I'll need to build
a test system for that. The error occurred in a production system, which I am
very hesitant to re-boot, let alone insert drives that cause error messages.
Yours sincerely,
Sebastian
[PATCH] scsi: qla2xxx: Fix compilation warning in qlt_schedule_sess_for_deletion
From: Mikhail Malygin
Compiler output:
drivers/scsi/qla2xxx/qla_target.c: In function 'qlt_schedule_sess_for_deletion':
drivers/scsi/qla2xxx/qla_target.c:1227:22: warning: unused variable 'ha'
[-Wunused-variable]
struct qla_hw_data *ha = sess->vha->hw;
Signed-off-by: Mikhail Malygin
Reported-by: Stephen Rothwell
---
drivers/scsi/qla2xxx/qla_target.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c
b/drivers/scsi/qla2xxx/qla_target.c
index 032897e22e78..0266c4d07bc9 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1230,7 +1230,6 @@ static void qla24xx_chk_fcp_state(struct fc_port *sess)
void qlt_schedule_sess_for_deletion(struct fc_port *sess)
{
struct qla_tgt *tgt = sess->tgt;
- struct qla_hw_data *ha = sess->vha->hw;
unsigned long flags;
if (sess->disc_state == DSC_DELETE_PEND)
--
2.15.2
RE: [PATCH] mpt3sas: Fix calltrace observed while running IO & host reset
Bart, Please see my replies inline. Thanks, Chaitra -Original Message- From: Bart Van Assche [mailto:bart.vanass...@wdc.com] Sent: Wednesday, June 13, 2018 9:22 PM To: chaitra.basa...@broadcom.com; linux-scsi@vger.kernel.org Cc: sathya.prak...@broadcom.com; suganath-prabu.subram...@broadcom.com; sreekanth.re...@broadcom.com Subject: Re: [PATCH] mpt3sas: Fix calltrace observed while running IO & host reset On Wed, 2018-06-13 at 15:46 +0530, Chaitra Basappa wrote: > When host reset is issued from application, through ioctl reset handler > _ctl_do_reset() -> mpt3sas_base_hard_reset_handler() sets > “ioc->shost_recovery” flag. > If “ioc->shost_recovery” flag is set then driver will return all the > incoming SCSI cmds with “SCSI_MLQUEUE_HOST_BUSY” in the scsih_qcmd(). And > hence no new request gets processed by the driver until the reset > completes, > which guarantees that the smid won't change. Hello Chaitra, The patch at the start of this e-mail thread checks whether st->smid is zero. That check could only be useful if there would be code in the mpt3sas driver that clears that field upon command completion. However, I haven't found any such code in the mpt3sas driver. [Chaitra] Before starting the host reset operation, driver will set "ioc->shost_recovery" flag to one, so during host reset time if driver receives any IO commands then below check in scsih_qcmd() returns these scsi commands with host busy status and hence these commands are not issued to the HBA FW. So these scsi commands will not be outstanding at the driver level, hence smid for these scsi commands will be zero and no need to flush out these commands during host reset time. /* host recovery or link resets sent via IOCTLs */ if (ioc->shost_recovery || ioc->ioc_link_reset_in_progress) return SCSI_MLQUEUE_HOST_BUSY; As a part of host reset operation, driver will flush out all the scsi commands which are outstanding at the driver level with "DID_RESET" result. To determine whether scsi cmnds are outstanding at the driver level while looping from 'tag' value zero to hba queue depth, driver will check for below two fields from the scsiio_tracker 1. cb_idx == 0xFF : this means that scsi cmnd has completed from the driver, so this command is not outstanding at the driver level. So this check itself is enough to determine that scsi cmnd is completedfrom the driver and no need reset smid to zero. But any way it is better to reset the smid field also to zero along with cb_idx setting to 0xff. And hence we will re-post this patch with setting of smid field in scsiio_tracker to zero upon completion of the scsi cmnd by the driver. 2. smid == 0 (zero): this means that scsi cmnd has not issued to the HBA firmware, so this command is not outstanding at the driver level. (current driver was not checking this case and hence we are observing this issue. In this patch we have added this check to fix this issue) If cd_idx != 0xff && smid != 0 , this means that scsi cmnd is outstanding at the driver level and Driver will flush this scsi cmnd with "DID_RESET" during diag reset time. Another concern is that setting ioc->shost_recovery prevents new calls of scsih_qcmd() to submit any commands. But I don't think that setting that flag prevents any scsih_qcmd() calls that had already been started to submit a new command. [Chaitra] If scsi cmnd has already crossed the check for "ioc->shost_recovery" flag (it means that scmd has been issued just before starting of host reset operation) then such commands will be processed by driver , which assigns valid 'smid' whose value b/w 1 and <= ioc->scsiio_depth (i.e. scsi cmnd's tag value + 1) thus these commands will be outstanding at driver level and hence will be flushed out with "DID_RESET" during reset operation. In other words, I don't think that checking whether or not st->smid == 0 is sufficient to fix the reported race. Bart.
[PATCH v1] mpt3sas: Fix calltrace observed while running IO & host reset
Below kernel BUG was observed while running IOs with host reset (issued from application), mpt3sas_cm0: diag reset: SUCCESS [ cut here ] WARNING: CPU: 12 PID: 4336 at drivers/scsi/mpt3sas/mpt3sas_base.c:3282 mpt3sas_base_clear_st+0x3d/0x40 [mpt3sas] Modules linked in: macsec tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag binfmt_misc fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun devlink ip6t_rpfilter ipt_REJECT nf_reject_ipv4 ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter sunrpc vfat fat sb_edac intel_powerclamp coretemp intel_rapl iosf_mbi kvm_intel kvm irqbypass crc32_pclmul ghash_clmulni_intel aesni_intel lrw gf128mul glue_helper ablk_helper cryptd iTCO_wdt iTCO_vendor_support dcdbas pcspkr joydev ipmi_ssif ses enclosure sg ipmi_devintf acpi_pad ipmi_msghandler acpi_power_meter mei_me lpc_ich wmi mei shpchp ip_tables xfs libcrc32c sd_mod crc_t10dif crct10dif_generic ata_generic pata_acpi uas usb_storage mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm ata_piix mpt3sas libata crct10dif_pclmul crct10dif_common tg3 crc32c_intel i2c_core raid_class ptp scsi_transport_sas pps_core dm_mirror dm_region_hash dm_log dm_mod CPU: 12 PID: 4336 Comm: python Kdump: loaded Tainted: GW 3.10.0-875.el7.brdc.x86_64 #1 Hardware name: Dell Inc. PowerEdge R820/0YWR73, BIOS 1.5.0 03/08/2013 Call Trace: [] dump_stack+0x19/0x1b [] __warn+0xd8/0x100 [] warn_slowpath_null+0x1d/0x20 [] mpt3sas_base_clear_st+0x3d/0x40 [mpt3sas] [] _scsih_flush_running_cmds+0x92/0xe0 [mpt3sas] [] mpt3sas_scsih_reset_handler+0x43b/0xaf0 [mpt3sas] [] ? vprintk_default+0x29/0x40 [] ? printk+0x60/0x77 [] ? _base_diag_reset+0x238/0x340 [mpt3sas] [] mpt3sas_base_hard_reset_handler+0x1ad/0x420 [mpt3sas] [] _ctl_ioctl_main.isra.12+0x11b9/0x1200 [mpt3sas] [] ? xfs_file_aio_write+0x155/0x1b0 [xfs] [] ? do_sync_write+0x93/0xe0 [] _ctl_ioctl+0x1a/0x20 [mpt3sas] [] do_vfs_ioctl+0x350/0x560 [] ? __sb_end_write+0x31/0x60 [] SyS_ioctl+0xa1/0xc0 [] ? system_call_after_swapgs+0xa2/0x146 [] system_call_fastpath+0x1c/0x21 [] ? system_call_after_swapgs+0xae/0x146 ---[ end trace 5dac5b98d89aaa3c ]--- [ cut here ] kernel BUG at block/blk-core.c:1476! invalid opcode: [#1] SMP Modules linked in: macsec tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag binfmt_misc fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun devlink ip6t_rpfilter ipt_REJECT nf_reject_ipv4 ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter sunrpc vfat fat sb_edac intel_powerclamp coretemp intel_rapl iosf_mbi kvm_intel kvm irqbypass crc32_pclmul ghash_clmulni_intel aesni_intel lrw gf128mul glue_helper ablk_helper cryptd iTCO_wdt iTCO_vendor_support dcdbas pcspkr joydev ipmi_ssif ses enclosure sg ipmi_devintf acpi_pad ipmi_msghandler acpi_power_meter mei_me lpc_ich wmi mei shpchp ip_tables xfs libcrc32c sd_mod crc_t10dif crct10dif_generic ata_generic pata_acpi uas usb_storage mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm ata_piix mpt3sas libata crct10dif_pclmul crct10dif_common tg3 crc32c_intel i2c_core raid_class ptp scsi_transport_sas pps_core dm_mirror dm_region_hash dm_log dm_mod CPU: 12 PID: 4336 Comm: python Kdump: loaded Tainted: GW 3.10.0-875.el7.brdc.x86_64 #1 Hardware name: Dell Inc. PowerEdge R820/0YWR73, BIOS 1.5.0 03/08/2013 task: 903fc96e0fd0 ti: 903fb1eec000 task.ti: 903fb1eec000 RIP: 0010:[] [] blk_requeue_request+0x90/0xa0 RSP: 0018:903c6b783dc0 EFLAGS: 00010087 RAX: 903bb67026d0 RBX: 903b7d6a6140 RCX: dead0200 RDX: 903bb67026d0 RSI: 903bb6702580 RDI: 903bb67026d0 RBP: 903c6b783dd8 R08: 903bb67026d0 R09: d97e8000 R10: 903c658bac00 R11: R12: 903bb6702580 R13: 903fa9a292f0 R14: 0246 R15: 1057 FS: 7f7026f5b740() GS:903c6b78() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7f298877c004 CR3: caf36000 CR4: 000607e0 Call Trace: [] __scsi_queue_insert+0xbf/0x110 [] scsi_io_completion+0x5da/0x6a0 [] scsi_finish_command+0xdc/0x140 []
