- 06 Aug, 2021 3 commits
-
-
Bean Huo authored
ufshcd_add_cmd_upiu_trace() will be called later anyway. Simplify code by moving if-statement. Link: https://lore.kernel.org/r/20210802180803.100033-1-huobean@gmail.comReviewed-by:
Avri Altman <avri.altman@wdc.com> Signed-off-by:
Bean Huo <beanhuo@micron.com> Signed-off-by:
Martin K. Petersen <martin.petersen@oracle.com>
-
Colin Ian King authored
The variable num_cnt is being initialized with a value that is never read, it is being updated later on. The assignment is redundant and can be removed. Link: https://lore.kernel.org/r/20210804131344.112635-1-colin.king@canonical.comReviewed-by:
Himanshu Madhani <himanshu.madhani@oracle.com> Signed-off-by:
Colin Ian King <colin.king@canonical.com> Signed-off-by:
-
Dan Carpenter authored
The sp->free(sp); call frees "sp" and then the debug code dereferences it on the next line. Swap the order. Link: https://lore.kernel.org/r/20210803155625.GA22735@kili Fixes: 84318a9f ("scsi: qla2xxx: edif: Add send, receive, and accept for auth_els") Reviewed-by:
Ewan D. Milne <emilne@redhat.com> Reviewed-by:
Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by:
-
- 03 Aug, 2021 22 commits
-
-
David Disseldorp authored
These members are only used for ALUA sense detail propagation, which can just as easily be done via sense_reason_t. Link: https://lore.kernel.org/r/20210728115353.2396-4-ddiss@suse.deReviewed-by:
Mike Christie <michael.christie@oracle.com> Signed-off-by:
David Disseldorp <ddiss@suse.de> Signed-off-by:
-
David Disseldorp authored
The se_cmd scsi_asc and scsi_ascq members are only used for tracking ALUA SCSI sense detail between target_core_alua and translate_sense_reason(), so they're effectively always zero here. Link: https://lore.kernel.org/r/20210728115353.2396-3-ddiss@suse.de Cc: Chris Boot <bootc@bootc.net> Reviewed-by:
-
David Disseldorp authored
core_alua_state_lba_dependent() currently uses lun->lun_tg_pt_gp without holding the lun_tg_pt_gp_lock. The lock is taken in the caller, so obtain the needed tg_pt_gp_id there instead. Link: https://lore.kernel.org/r/20210728115353.2396-2-ddiss@suse.de Cc: Hannes Reinecke <hare@suse.de> Reviewed-by:
-
Bodo Stroesser authored
When running command pipelining for WRITE direction commands (e.g. tape device write), userspace sends cmd completion to cmd ring before processing write data. In that case userspace has to copy data before sending completion, because cmd completion also implicitly releases the data buffer in data area. The new feature KEEP_BUF allows userspace to optionally keep the buffer after completion by setting new bit TCMU_UFLAG_KEEP_BUF in tcmu_cmd_entry_hdr->uflags. In that case buffer has to be released explicitly by writing the cmd_id to new action item free_kept_buf. All kept buffers are released during reset_ring and if userspace closes uio device (tcmu_release). Link: https://lore.kernel.org/r/20210713175021.20103-1-bostroesser@gmail.comReviewed-by:
Bodo Stroesser <bostroesser@gmail.com> Signed-off-by:
-
Bart Van Assche authored
Make it easier to test the UFS error handler and abort handler. Link: https://lore.kernel.org/r/20210722033439.26550-19-bvanassche@acm.orgAcked-by:
Bart Van Assche <bvanassche@acm.org> Signed-off-by:
-
Bart Van Assche authored
Neither SAM nor the UFS standard require that the UFS controller fills in the completion status of commands that have been aborted (LUN RESET aborts pending commands). Hence do not rely on the completion status provided by the UFS controller for aborted commands but instead ask the SCSI core to retry SCSI commands that have been aborted. Link: https://lore.kernel.org/r/20210722033439.26550-18-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Avri Altman <avri.altman@wdc.com> Reviewed-by:
-
Bart Van Assche authored
Use the SCSI error handler instead of a custom error handling strategy. This change reduces the number of potential races in the UFS drivers since the UFS error handler and the SCSI error handler no longer run concurrently. Link: https://lore.kernel.org/r/20210722033439.26550-17-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Avri Altman <avri.altman@wdc.com> Tested-by:
-
Bart Van Assche authored
Clearing a unit attention synchronously from inside the UFS error handler may trigger the following deadlock: - ufshcd_err_handler() calls ufshcd_err_handling_unprepare() and the latter function calls ufshcd_clear_ua_wluns(). - ufshcd_clear_ua_wluns() submits a REQUEST SENSE command and that command activates the SCSI error handler. - The SCSI error handler calls ufshcd_host_reset_and_restore(). - ufshcd_host_reset_and_restore() executes the following code: ufshcd_schedule_eh_work(hba); flush_work(&hba->eh_work); This sequence results in a deadlock (circular wait). Fix this by requesting sense data asynchronously. Link: https://lore.kernel.org/r/20210722033439.26550-16-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Avri Altman <avri.altman@wdc.com> Reviewed-by:
-
Bart Van Assche authored
Make the following changes in ufshcd_abort(): - Return FAILED instead of SUCCESS if the abort handler notices that a SCSI command has already been completed. Returning SUCCESS in this case triggers a use-after-free and may trigger a kernel crash. - Fix the code for aborting SCSI commands submitted to a WLUN. The current approach for aborting SCSI commands that have been submitted to a WLUN and that timed out is as follows: - Report to the SCSI core that the command has completed successfully. Let the block layer free any data buffers associated with the command. - Mark the command as outstanding in 'outstanding_reqs'. - If the block layer tries to reuse the tag associated with the aborted command, busy-wait until the tag is freed. This approach can result in: - Memory corruption if the controller accesses the data buffer after the block layer has freed the associated data buffers. - A race condition if ufshcd_queuecommand() or ufshcd_exec_dev_cmd() checks the bit that corresponds to an aborted command in 'outstanding_reqs' after it has been cleared and before it is reset. - High energy consumption if ufshcd_queuecommand() repeatedly returns SCSI_MLQUEUE_HOST_BUSY. Fix this by reporting to the SCSI error handler that aborting a SCSI command failed if the SCSI command was submitted to a WLUN. Link: https://lore.kernel.org/r/20210722033439.26550-15-bvanassche@acm.org Fixes: 7a7e66c6 ("scsi: ufs: Fix a race condition between ufshcd_abort() and eh_work()") Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Avri Altman <avri.altman@wdc.com> Reviewed-by:
-
Bart Van Assche authored
Use a spinlock to protect hba->outstanding_reqs instead of using atomic operations to update this member variable. This patch is a performance improvement because it reduces the number of atomic operations in the hot path (test_and_clear_bit()) and because it reduces the lock contention on the SCSI host lock. On my test setup this patch improves IOPS by about 1%. Link: https://lore.kernel.org/r/20210722033439.26550-14-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Avri Altman <avri.altman@wdc.com> Reviewed-by:
Daejun Park <daejun7.park@samsung.com> Reviewed-by:
-
Bart Van Assche authored
Reduce the number of times the host lock is taken in the hot path. Additionally, inline ufshcd_vops_setup_xfer_req() because that function is too short to keep it. Link: https://lore.kernel.org/r/20210722033439.26550-13-bvanassche@acm.org Fixes: a45f9371 ("scsi: ufs: Optimize host lock on transfer requests send/compl paths") Cc: Jaegeuk Kim <jaegeuk@kernel.org> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Bean Huo <beanhuo@micron.com> Cc: Asutosh Das <asutoshd@codeaurora.org> Reviewed-by:
-
Bart Van Assche authored
Using the UTRLCNR register involves two MMIO accesses in the hot path while using the doorbell register only involves a single MMIO access. Since MMIO accesses take time, do not use the UTRLCNR register. The spinlock contention on the SCSI host lock that is reintroduced by this commit will be addressed later. This reverts commit 6f715172. Link: https://lore.kernel.org/r/20210722033439.26550-12-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Avri Altman <avri.altman@wdc.com> Tested-by:
-
Bart Van Assche authored
Inline ufshcd_outstanding_req_clear() since it only has one caller and since its body is only one line long. Link: https://lore.kernel.org/r/20210722033439.26550-11-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Reviewed-by:
-
Bart Van Assche authored
From arch/arm/include/asm/io.h #define __iowmb() wmb() [ ... ] #define writel(v,c) ({ __iowmb(); writel_relaxed(v,c); }) From Documentation/memory-barriers.txt: "Note that, when using writel(), a prior wmb() is not needed to guarantee that the cache coherent memory writes have completed before writing to the MMIO region." In other words, calling wmb() before writel() is not necessary. Hence remove the wmb() calls that precede a writel() call. Remove the wmb() calls that precede a ufshcd_send_command() call since the latter function uses writel(). Remove the wmb() call from ufshcd_wait_for_dev_cmd() since the following chain of events guarantees that the CPU will see up-to-date LRB values: - UFS controller writes to host memory. - UFS controller posts completion interrupt after the memory writes from the previous step are visible to the CPU. - complete(hba->dev_cmd.complete) is called from the UFS interrupt handler. - The wait_for_completion(hba->dev_cmd.complete) call in ufshcd_wait_for_dev_cmd() returns. Link: https://lore.kernel.org/r/20210722033439.26550-10-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Avri Altman <avri.altman@wdc.com> Tested-by: -
Bart Van Assche authored
Assign a name to the enumeration type for UFS host controller states and remove the default clause from switch statements on this enumeration type to make the compiler warn about unhandled enumeration labels. Link: https://lore.kernel.org/r/20210722033439.26550-9-bvanassche@acm.org Cc: Can Guo <cang@codeaurora.org> Reviewed-by:
Keoseong Park <keosung.park@samsung.com> Reviewed-by:
-
Bart Van Assche authored
Instead of documenting the locking requirements of the UIC code as comments, use lockdep_assert_held() such that lockdep verifies the lockdep requirements at runtime if lockdep is enabled. Link: https://lore.kernel.org/r/20210722033439.26550-8-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Reviewed-by:
-
Bart Van Assche authored
scsi_add_host() allocates shost->can_queue tags. ufshcd_init() sets shost->can_queue to hba->nutrs. In other words, we know that tag values will less than hba->nutrs. Hence remove the checks that verify that blk_get_request() returns a tag less than hba->nutrs. This check was introduced by commit 14497328 ("scsi: ufs: verify command tag validity"). Keep the tag >= 0 check because it helps to detect use-after-free issues. Link: https://lore.kernel.org/r/20210722033439.26550-7-bvanassche@acm.org CC: Avri Altman <avri.altman@wdc.com> Cc: Alim Akhtar <alim.akhtar@samsung.com> Reviewed-by:
-
Bart Van Assche authored
From Documentation/scheduler/completion.rst: "When a completion is declared as a local variable within a function, then the initialization should always use DECLARE_COMPLETION_ONSTACK() explicitly, not just to make lockdep happy, but also to make it clear that limited scope had been considered and is intentional." Link: https://lore.kernel.org/r/20210722033439.26550-6-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Avri Altman <avri.altman@wdc.com> Reviewed-by:
-
Bart Van Assche authored
Rename the second argument of ufshcd_probe_hba() such that the name of that argument reflects its purpose instead of how the function is called. See also commit 1b9e2141 ("scsi: ufs: Split ufshcd_probe_hba() based on its called flow"). Link: https://lore.kernel.org/r/20210722033439.26550-5-bvanassche@acm.org Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Can Guo <cang@codeaurora.org> Reviewed-by:
Stanley Chu <stanley.chu@mediatek.com> Signed-off-by:
-
Bart Van Assche authored
This patch slightly reduces the UFS driver size if built with power management support disabled. Link: https://lore.kernel.org/r/20210722033439.26550-4-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Reviewed-by:
-
Bart Van Assche authored
Move the dev_get_drvdata() calls into the ufshcd_{system,runtime}_*() functions. Remove ufshcd_runtime_idle() since it is empty. This patch does not change any functionality. Link: https://lore.kernel.org/r/20210722033439.26550-3-bvanassche@acm.org Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Reviewed-by: -
Bart Van Assche authored
If param_offset > buff_len then the memcpy() statement in ufshcd_read_desc_param() corrupts memory since it copies 256 + buff_len - param_offset bytes into a buffer with size buff_len. Since param_offset < 256 this results in writing past the bound of the output buffer. Link: https://lore.kernel.org/r/20210722033439.26550-2-bvanassche@acm.org Fixes: cbe193f6 ("scsi: ufs: Fix potential NULL pointer access during memcpy") Reviewed-by:
-
- 01 Aug, 2021 15 commits
-
-
Avri Altman authored
Elaborate some more on the host control mode logic parameters, explaining what they do and how to configure them. Link: https://lore.kernel.org/r/20210712095039.8093-13-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
Support devices that report they are using host control mode. Link: https://lore.kernel.org/r/20210712095039.8093-12-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
HPB WRITE BUFFER with buffer-id = 0x3h is supported in device control mode only. Link: https://lore.kernel.org/r/20210712095039.8093-11-avri.altman@wdc.comSigned-off-by:
-
Avri Altman authored
In host control mode the host is the originator of map requests. To not flood the device with map requests, use a simple throttling mechanism that limits the number of in-flight map requests. Link: https://lore.kernel.org/r/20210712095039.8093-10-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
In order not to hang on to "cold" regions, we inactivate a region that has had no READ access for a predefined amount of time - READ_TO_MS. For that purpose monitor the active regions list, polling it on every POLLING_INTERVAL_MS. On timeout expiry add the region to the "to-be-inactivated" list unless it is clean and did not exhaust its READ_TO_EXPIRIES - another parameter. None of this applies to pinned regions. Link: https://lore.kernel.org/r/20210712095039.8093-9-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
The spec does not define what the host's recommended response is when the device sends HPB dev reset response (oper 0x2). Update all active HPB regions. Link: https://lore.kernel.org/r/20210712095039.8093-8-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
In host mode, the host is expected to send HPB WRITE BUFFER with buffer-id = 0x1 when it inactivates a region. Use the map-requests pool as there is no point in assigning a designated cache for umap-requests. [mkp: REQ_OP_DRV_*] Link: https://lore.kernel.org/r/20210712095039.8093-7-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
In host mode, eviction is considered an extreme measure. Verify that the entering region has enough reads, and the exiting region has fewer reads. Link: https://lore.kernel.org/r/20210712095039.8093-6-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
In host control mode, reads are the major source of activation trials. Keep track of those reads counters, for both active as well inactive regions. We reset the read counter upon write - we are only interested in "clean" reads. Keep those counters normalized, as we are using those reads as a comparative score, to make various decisions. If during consecutive normalizations an active region has exhaust its reads - inactivate it. While at it, protect the {active,inactive}_count stats by adding them into the applicable handler. Link: https://lore.kernel.org/r/20210712095039.8093-5-avri.altman@wdc.comReviewed-by: -
Avri Altman authored
Given a transfer length, set_dirty meticulously iterates over all the entries, across subregions and regions if needed. Currently its only use is to mark dirty blocks, but HCM may benefit from it as well to manage its read counters. Link: https://lore.kernel.org/r/20210712095039.8093-4-avri.altman@wdc.comReviewed-by:
-
Avri Altman authored
In device control mode, the device may recommend the host to either activate or inactivate a region, and the host should follow. Meaning those are not actually recommendations, but more of instructions. Conversely, in host control mode, the recommendation protocol is slightly changed: a) The device may only recommend the host to update a subregion of an already-active region. And, b) The device may *not* recommend to inactivate a region. Furthermore, in host control mode, the host may choose not to follow any of the device's recommendations. However, in case of a recommendation to update an active and clean subregion, it is better to follow those recommendation because otherwise the host has no other way to know that some internal relocation took place. Link: https://lore.kernel.org/r/20210712095039.8093-3-avri.altman@wdc.comReviewed-by: -
Avri Altman authored
We will use control_mode later when we need to differentiate between device and host control modes. Link: https://lore.kernel.org/r/20210712095039.8093-2-avri.altman@wdc.comReviewed-by:
-
Daejun Park authored
Version 2.0 of HBP supports reads of varying sizes from 4KB to 1MB. A read operation <= 32KB is supported as single HPB read. A read between 36KB and 1MB is supported by a combination of write buffer command and HPB read command to deliver more PPN. The write buffer commands may not be issued immediately due to busy tags. To use HPB read more aggressively, the driver can requeue the write buffer command. The requeue threshold is implemented as timeout and can be modified with requeue_timeout_ms entry in sysfs. [mkp: REQ_OP_DRV_* and blk_rq_is_passthrough()] Link: https://lore.kernel.org/r/20210712090025epcms2p3b3d94f6f1b2cfa394e3d9ba130ca0fa7@epcms2p3Tested-by:
Can Guo <cang@codeaurora.org> Tested-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reviewed-by:
-
Daejun Park authored
If the logical address of a read I/O belongs to an active sub-region, the HPB driver modifies the read I/O command to an HPB read. The driver modifies the UFS UPIU instead of modifying the existing SCSI command. In HPB version 1.0, the maximum read I/O size that can be converted to HPB read is 4KB. The dirty map of the active sub-region prevents an incorrect HPB read that has stale physical page number which is updated by previous write I/O. [mkp: REQ_OP_DRV_* and blk_rq_is_passthrough()] Link: https://lore.kernel.org/r/20210712085936epcms2p4b0ec5c8cecdeea6cc043d684363842b6@epcms2p4Tested-by:
-
Daejun Park authored
Implement L2P map management in HPB. The HPB divides logical addresses into several regions. A region consists of several sub-regions. The sub-region is a basic unit where L2P mapping is managed. The driver loads L2P mapping data of each sub-region. The loaded sub-region is called active-state. The HPB driver unloads L2P mapping data as region unit. The unloaded region is called inactive-state. Sub-region/region candidates to be loaded and unloaded are delivered from the UFS device. The UFS device delivers the recommended active sub-region and inactivate region to the driver using sense data. The HPB module performs L2P mapping management on the host through the delivered information. A pinned region is a preset region on the UFS device that is always in activate-state. The data structures for map data requests and L2P mappings use the mempool API, minimizing allocation overhead while avoiding static allocation. The mininum size of the memory pool used in the HPB is implemented as a module parameter so that it can be configurable by the user. To guarantee a minimum memory pool size of 4MB: ufshpb_host_map_kbytes=4096. The map_work manages active/inactive via 2 "to-do" lists: - hpb->lh_inact_rgn: regions to be inactivated - hpb->lh_act_srgn: subregions to be activated These lists are maintained on I/O completion. [mkp: switch to REQ_OP_DRV_*] Link: https://lore.kernel.org/r/20210712085859epcms2p36e420f19564f6cd0c4a45d54949619eb@epcms2p3Tested-by:
-
