- 10 Jun, 2021 5 commits
-
-
Mike Christie authored
qedi_clear_session_ctx() could race with the in-kernel or userspace driven recovery/removal and we could access a NULL conn or do a double free. We should be using iscsi_host_remove() to start the removal process from the driver. It will start the in-kernel recovery and notify userspace that the driver's scsi_hosts are being removed. iscsid will then drive the session removal like is done when the logout command is run. When the sessions are removed, iscsi_host_remove() will return so qedi can finish knowing there are no running sessions and no new sessions will be allowed. This also fixes an issue where we check for a NULL conn after already accessing it introduced in commit 27e98628 ("scsi: iscsi: Drop suspend calls from ep_disconnect") by just removing the function completely. Link: https://lore.kernel.org/r/20210609192709.5094-1-michael.christie@oracle.com Fixes: 27e98628 ("scsi: iscsi: Drop suspend calls from ep_disconnect") Signed-off-by:
Mike Christie <michael.christie@oracle.com> Signed-off-by:
Martin K. Petersen <martin.petersen@oracle.com>
-
Dan Carpenter authored
The pci_alloc_irq_vectors_affinity() function returns negative error codes or it returns a number between the minimum vectors (1 in this case) and max_vectors. It won't return zero. Because "i" is a u16 then the error handling won't work. And also if it did work the error code was not set. Really "max_vectors" can be an int as well because we're doing a min_t() on int type. The other change is that it's better to remove unnecessary initialization so that static checkers can warn us if there are ever uninitialized variable bugs introduced in the future. I changed the error code from -1 (-EPERM) if the kmalloc() failed to -ENOMEM. And on success path I changed it from "return retval;" to "return 0;" which shouldn't affect the compiled code but makes it more readable. Link: https://lore.kernel.org/r/YMCJcnmSI4kOIyv/@mwanda Fixes: 824a1566 ("scsi: mpi3mr: Base driver code") Signed-off-by:
Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by:
-
Dan Carpenter authored
The "mrioc->intr_info" pointer can't be NULL, but if it could then the second iteration through the loop would Oops. Let's delete the confusing and impossible NULL check. Link: https://lore.kernel.org/r/YMCJKgykDYtyvY44@mwandaSigned-off-by:
-
Tomas Henzl authored
Fix a double free, scsi_tgt_priv_data will be freed in mpi3mr_target_destroy() so remove the kfree() from mpi3mr_target_alloc(). I've also removed few unneeded initialisations. Link: https://lore.kernel.org/r/20210608145712.16386-1-thenzl@redhat.comAcked-by:
Kashyap Desai <kashyap.desai@broadcom.com> Signed-off-by:
Tomas Henzl <thenzl@redhat.com> Signed-off-by:
-
Can Guo authored
In ufshcd_exec_dev_cmd(), if error happens before lrpb is initialized, then we should bail out instead of letting trace record the error. Link: https://lore.kernel.org/r/1623227044-22635-1-git-send-email-cang@codeaurora.org Fixes: a45f9371 ("scsi: ufs: Optimize host lock on transfer requests send/compl paths") Reported-by:
kernel test robot <lkp@intel.com> Reviewed-by:
Stanley Chu <stanley.chu@mediatek.com> Reviewed-by:
Nathan Chancellor <nathan@kernel.org> Signed-off-by:
Can Guo <cang@codeaurora.org> Signed-off-by:
-
- 08 Jun, 2021 14 commits
-
-
Bean Huo authored
Since devman_upiu_cmd() is not COMMAND UPIU, and doesn't have CDB, it is better to use UPIU query trace, which provides more helpful information for issue troubleshooting. Link: https://lore.kernel.org/r/20210531104308.391842-5-huobean@gmail.comReviewed-by:
Bean Huo <beanhuo@micron.com> Signed-off-by:
-
Bean Huo authored
For the query request, we already have query_trace, but in ufshcd_send_command(), there will add two more redundant traces. Since lrbp->cmd is NULL in the query request, the two trace events below provide nothing except the tag and DB. Instead of letting them take up the limited trace ring buffer, it’s better not to print these traces in case of cmd == NULL. ufshcd_command: send_req: ff3b0000.ufs: tag: 28, DB: 0x0, size: -1, IS: 0, LBA: 18446744073709551615, opcode: 0x0 (0x0), group_id: 0x0 ufshcd_command: dev_complete: ff3b0000.ufs: tag: 28, DB: 0x0, size: -1, IS: 0, LBA: 18446744073709551615, opcode: 0x0 (0x0), group_id: 0x0 Link: https://lore.kernel.org/r/20210531104308.391842-4-huobean@gmail.comReviewed-by:
-
Bean Huo authored
The current UPIU completion event trace still prints the COMMAND UPIU header, rather than the RSP UPIU header. This makes UPIU command trace useless in problem shooting in case we receive a trace log from the customer/field. There are two important fields in RSP UPIU: 1. The response field, which indicates the UFS defined overall success or failure of the series of Command, Data and RESPONSE UPIU’s that make up the execution of a task. 2. The Status field, which contains the command set specific status for a specific command issued by the initiator device. Before this commit, the UPIU paired trace events: ufshcd_upiu: send_req: fe3b0000.ufs: HDR:01 20 00 1c 00 00 00 00 00 00 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00 ufshcd_upiu: complete_rsp: fe3b0000.ufs: HDR:01 20 00 1c 00 00 00 00 00 00 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00 After this commit: ufshcd_upiu: send_req: fe3b0000.ufs: HDR:01 20 00 1c 00 00 00 00 00 00 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00 ufshcd_upiu: complete_rsp: fe3b0000.ufs: HDR:21 00 00 1c 00 00 00 00 00 00 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00 Link: https://lore.kernel.org/r/20210531104308.391842-3-huobean@gmail.comReviewed-by: -
Bean Huo authored
To consistent with trace event print, convert the value of the variable 'lba' from a block layer sector address to a logical block adress. Link: https://lore.kernel.org/r/20210531104308.391842-2-huobean@gmail.comSuggested-by:
Bart Van Assche <bvanassche@acm.org> Reviewed-by:
-
Keoseong Park authored
Remove repeated word "for" in comment. Link: https://lore.kernel.org/r/1891546521.01622777101796.JavaMail.epsvc@epcpadp3Signed-off-by:
Keoseong Park <keosung.park@samsung.com> Signed-off-by:
-
Gustavo A. R. Silva authored
In preparation to enable -Wimplicit-fallthrough for Clang, fix a fall-through warning by explicitly adding a break statement instead of just letting the code fall through to the next case. Link: https://github.com/KSPP/linux/issues/115 Link: https://lore.kernel.org/r/20210604023530.GA180997@embeddedorReviewed-by:
Kees Cook <keescook@chromium.org> Signed-off-by:
Gustavo A. R. Silva <gustavoars@kernel.org> Signed-off-by:
-
Gustavo A. R. Silva authored
In preparation to enable -Wimplicit-fallthrough for Clang, fix a fall-through warning by replacing a /* fallthrough */ comment with the new pseudo-keyword macro fallthrough; Link: https://github.com/KSPP/linux/issues/115 Link: https://lore.kernel.org/r/20210604022752.GA168289@embeddedorReviewed-by:
-
Can Guo authored
By reading the UTP Transfer Request List Completion Notification Register, which is added in UFSHCI Ver 3.0, SW can easily get the compeleted transfer requests. Thus, SW can get rid of host lock, which is used to synchronize the tr_doorbell and outstanding_reqs, on transfer requests dispatch and completion paths. This can further benefit random read/write performance. Link: https://lore.kernel.org/r/1621845419-14194-4-git-send-email-cang@codeaurora.org Cc: Stanley Chu <stanley.chu@mediatek.com> Reviewed-by:
Asutosh Das <asutoshd@codeaurora.org> Signed-off-by:
-
Can Guo authored
Current UFS IRQ handler is completely wrapped by host lock, and because ufshcd_send_command() is also protected by host lock, when IRQ handler fires, not only the CPU running the IRQ handler cannot send new requests, the rest CPUs can neither. Move the host lock wrapping the IRQ handler into specific branches, i.e., ufshcd_uic_cmd_compl(), ufshcd_check_errors(), ufshcd_tmc_handler() and ufshcd_transfer_req_compl(). Meanwhile, to further reduce occpuation of host lock in ufshcd_transfer_req_compl(), host lock is no longer required to call __ufshcd_transfer_req_compl(). As per test, the optimization can bring considerable gain to random read/write performance. Link: https://lore.kernel.org/r/1621845419-14194-3-git-send-email-cang@codeaurora.org Cc: Stanley Chu <stanley.chu@mediatek.com> Reported-by:
-
Can Guo authored
ufshcd_host_reset_and_restore() anyways completes all pending requests before starts re-probing, so there is no need to complete the command on the highest bit in tr_doorbell in advance. Link: https://lore.kernel.org/r/1621845419-14194-2-git-send-email-cang@codeaurora.orgReviewed-by:
-
Dan Carpenter authored
The "retval" variable needs to be signed for the error handling to work. Link: https://lore.kernel.org/r/YLjMEAFNxOas1mIp@mwanda Fixes: 7e26e3ea ("scsi: scsi_dh_alua: Check for negative result value") Reviewed-by:
Martin Wilck <mwilck@suse.com> Signed-off-by:
-
Avri Altman authored
Remove all references to the description of __ufshcd_wl_{suspend,resume} as no such description exist. Fixes: b294ff3e (scsi: ufs: core: Enable power management for wlun) Link: https://lore.kernel.org/r/20210603122209.635799-1-avri.altman@wdc.comSigned-off-by:Avri Altman <avri.altman@wdc.com> Signed-off-by:
-
Kees Cook authored
In preparation for FORTIFY_SOURCE performing compile-time and run-time field bounds checking for memcpy() avoid using an inline const buffer argument and instead just statically initialize the destination array directly. Link: https://lore.kernel.org/r/20210602180000.3326448-1-keescook@chromium.orgReviewed-by:
-
Saurav Kashyap authored
host->max_id defines the maximum target id that the SCSI midlayer will attempt to manually scan. The default is 8. Update the value to the max sessions the driver supports. [mkp: applied by hand] Link: https://lore.kernel.org/r/20210602104653.17278-1-jhasan@marvell.comReviewed-by:
Himanshu Madhani <himanshu.madhani@oracle.com> Signed-off-by:
Saurav Kashyap <skashyap@marvell.com> Signed-off-by:
Javed Hasan <jhasan@marvell.com> Signed-off-by:
-
- 03 Jun, 2021 2 commits
-
-
Bart Van Assche authored
Allow the compiler to verify the type of the second argument passed to scsi_host_complete_all_commands(). Link: https://lore.kernel.org/r/20210524025457.11299-4-bvanassche@acm.org Cc: Hannes Reinecke <hare@suse.com> Cc: John Garry <john.garry@huawei.com> Reviewed-by:
-
Bart Van Assche authored
Make it possible for the compiler to verify whether SAM and host status codes are used correctly. [mkp: resolve conflicts with Hannes' SCSI result series] Link: https://lore.kernel.org/r/20210524025457.11299-3-bvanassche@acm.org Cc: Hannes Reinecke <hare@suse.com> Reviewed-by:
John Garry <john.garry@huawei.com> Reviewed-by:
-
- 02 Jun, 2021 19 commits
-
-
Bart Van Assche authored
This patch prepares for converting SAM status codes into an enum. Without this patch converting SAM status codes into an enumeration type would trigger complaints about enum type mismatches for the SAS code. Link: https://lore.kernel.org/r/20210524025457.11299-2-bvanassche@acm.org Cc: Hannes Reinecke <hare@suse.com> Cc: Artur Paszkiewicz <artur.paszkiewicz@intel.com> Cc: Jason Yan <yanaijie@huawei.com> Reviewed-by:
Jack Wang <jinpu.wang@ionos.com> Signed-off-by:
-
Martin K. Petersen authored
Include Hannes' SCSI command result rework in the staging branch. [mkp: remove DRIVER_SENSE from mpi3mr] Signed-off-by: -
Mike Christie authored
If we got a response then we should always wake up the conn. For both the cmd_cleanup_req == 0 or cmd_cleanup_req > 0, we shouldn't dig into iscsi_itt_to_task because we don't know what the upper layers are doing. We can also remove the qedi_clear_task_idx call here because once we signal success libiscsi will loop over the affected commands and end up calling the cleanup_task callout which will release it. Link: https://lore.kernel.org/r/20210525181821.7617-29-michael.christie@oracle.comReviewed-by:
Manish Rangankar <mrangankar@marvell.com> Signed-off-by:
-
Mike Christie authored
We need to make sure that abort and reset completion work has completed before ep_disconnect returns. After ep_disconnect we can't manipulate cmds because libiscsi will call conn_stop and take onwership. We are trying to make sure abort work and reset completion work has completed before we do the cmd clean up in ep_disconnect. The problem is that: 1. the work function sets the QEDI_CONN_FW_CLEANUP bit, so if the work was still pending we would not see the bit set. We need to do this before the work is queued. 2. If we had multiple works queued then we could break from the loop in qedi_ep_disconnect early because when abort work 1 completes it could clear QEDI_CONN_FW_CLEANUP. qedi_ep_disconnect could then see that before work 2 has run. 3. A TMF reset completion work could run after ep_disconnect starts cleaning up cmds via qedi_clearsq. ep_disconnect's call to qedi_clearsq -> qedi_cleanup_all_io would might think it's done cleaning up cmds, but the reset completion work could still be running. We then return from ep_disconnect while still doing cleanup. This replaces the bit with a counter to track the number of queued TMF works, and adds a bool to prevent new works from starting from the completion path once a ep_disconnect starts. Link: https://lore.kernel.org/r/20210525181821.7617-28-michael.christie@oracle.comReviewed-by: -
Mike Christie authored
qedi_abort_work knows what task to abort so just pass it to send_iscsi_tmf. Link: https://lore.kernel.org/r/20210525181821.7617-27-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
Drivers shouldn't be calling block/unblock session for cmd cleanup because the functions can change the session state from under libiscsi. This adds a new a driver level bit so it can block all I/O the host while it drains the card. Link: https://lore.kernel.org/r/20210525181821.7617-26-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
Drivers shouldn't be calling block/unblock session for tmf handling because the functions can change the session state from under libiscsi. iscsi_queuecommand's call to iscsi_prep_scsi_cmd_pdu-> iscsi_check_tmf_restrictions will prevent new cmds from being sent to qedi after we've started handling a TMF. So we don't need to try and block it in the driver, and we can remove these block calls. Link: https://lore.kernel.org/r/20210525181821.7617-25-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
We run from a workqueue with no locks held so use GFP_NOIO. Link: https://lore.kernel.org/r/20210525181821.7617-24-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
qedi_iscsi_abort_work and qedi_tmf_work both allocate a tid then call qedi_send_iscsi_tmf which also allocates a tid. This removes the tid allocation from the callers. Link: https://lore.kernel.org/r/20210525181821.7617-23-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
If qedi_tmf_work's qedi_wait_for_cleanup_request call times out we will also force the clean up of the qedi_work_map but qedi_process_cmd_cleanup_resp could still be accessing the qedi_cmd. To fix this issue we extend where we hold the tmf_work_lock and back_lock so the qedi_process_cmd_cleanup_resp access is serialized with the cleanup done in qedi_tmf_work and any completion handling for the iscsi_task. Link: https://lore.kernel.org/r/20210525181821.7617-22-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
If the SCSI cmd completes after qedi_tmf_work calls iscsi_itt_to_task then the qedi qedi_cmd->task_id could be freed and used for another cmd. If we then call qedi_iscsi_cleanup_task with that task_id we will be cleaning up the wrong cmd. Wait to release the task_id until the last put has been done on the iscsi_task. Because libiscsi grabs a ref to the task when sending the abort, we know that for the non-abort timeout case that the task_id we are referencing is for the cmd that was supposed to be aborted. A latter commit will fix the case where the abort times out while we are running qedi_tmf_work. Link: https://lore.kernel.org/r/20210525181821.7617-21-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
If qedi_process_cmd_cleanup_resp finds the cmd it frees the work and sets list_tmf_work to NULL, so qedi_tmf_work should check if list_tmf_work is non-NULL when it wants to force cleanup. Link: https://lore.kernel.org/r/20210525181821.7617-20-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
This doesn't fix any bugs, but it makes more sense to free the pool after we have removed the session. At that time we know nothing is touching any of the session fields, because all devices have been removed and scans are stopped. Link: https://lore.kernel.org/r/20210525181821.7617-19-michael.christie@oracle.comReviewed-by:
Lee Duncan <lduncan@suse.com> Signed-off-by:
-
Mike Christie authored
For aborts, qedi needs to cleanup the FW then send the TMF from a worker thread. While it's doing these the cmd could complete normally and the TMF could time out. libiscsi would then complete the iscsi_task which will call into the driver to cleanup the driver level resources while it still might be accessing them for the cleanup/abort. This has iscsi_eh_abort keep the iscsi_task ref if the TMF times out, so qedi does not have to worry about if the task is being freed while in use and does not need to get its own ref. Link: https://lore.kernel.org/r/20210525181821.7617-18-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
We set the max_active iSCSI EH works to 1, so all work is going to execute in order by default. However, userspace can now override this in sysfs. If max_active > 1, we can end up with the block_work on CPU1 and iscsi_unblock_session running the unblock_work on CPU2 and the session and target/device state will end up out of sync with each other. This adds a flush of the block_work in iscsi_unblock_session. Link: https://lore.kernel.org/r/20210525181821.7617-17-michael.christie@oracle.com Fixes: 1d726aa6 ("scsi: iscsi: Optimize work queue flush use") Reviewed-by:
-
Mike Christie authored
We have a ref to the task being aborted, so SCp.ptr will never be NULL. We need to use iscsi_task_is_completed to check for the completed state. Link: https://lore.kernel.org/r/20210525181821.7617-16-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
The iscsi offload drivers are setting the shost->max_id to the max number of sessions they support. The problem is that max_id is not the max number of targets but the highest identifier the targets can have. To use it to limit the number of targets we need to set it to max sessions - 1, or we can end up with a session we might not have preallocated resources for. Link: https://lore.kernel.org/r/20210525181821.7617-15-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
If we haven't done a unbind target call we can race where iscsi_conn_teardown wakes up the EH thread and then frees the conn while those threads are still accessing the conn ehwait. We can only do one TMF per session so this just moves the TMF fields from the conn to the session. We can then rely on the iscsi_session_teardown->iscsi_remove_session->__iscsi_unbind_session call to remove the target and it's devices, and know after that point there is no device or scsi-ml callout trying to access the session. Link: https://lore.kernel.org/r/20210525181821.7617-14-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
The comment in iscsi_eh_session_reset is wrong and we don't wait for the EH to complete before tearing down the conn. This has us get a ref to the conn when we are not holding the eh_mutex/frwd_lock so it does not get freed from under us. Link: https://lore.kernel.org/r/20210525181821.7617-13-michael.christie@oracle.comReviewed-by:
-
