- 13 Oct, 2023 34 commits
-
-
Martin K. Petersen authored
Mike Christie <michael.christie@oracle.com> says: The following patches were made over Linus's tree but apply over Martin's branches. They allow userspace to configure how fabric drivers submit cmds to backend drivers. Right now loop and vhost use a worker thread, and the other drivers submit from the contexts they receive/process the cmd from. For multiple LUN cases where the target can queue more cmds than the backend can handle then deferring to a worker thread is safest because the backend driver can block when doing things like waiting for a free request/tag. Deferring also helps when the target has to handle transport level requests from the recv context. For cases where the backend devices can queue everything the target sends, then there is no need to defer to a workqueue and you can see a perf boost of up to 26% for small IO workloads. For a nvme device and vhost-scsi I can see with 4K IOs: fio jobs 1 2 4 8 10 -------------------------------------------------- workqueue submit 94K 190K 394K 770K 890K direct submit 128K 252K 488K 950K - Link: https://lore.kernel.org/r/1b1f7a5c-0988-45f9-b103-dfed2c0405b1@oracle.comSigned-off-by:
Martin K. Petersen <martin.petersen@oracle.com>
-
Mike Christie authored
This exports the fabric driver's direct submit settings, so users know what the driver supports. It will be helpful when they are exporting a device through different targets and one doesn't support direct submission. The new files allow the fabric to report what submission types they default to and if they support direct submission: default_submit_type: 1 - TARGET_DIRECT_SUBMIT - If the user has not requested a specific value then the fabric requests direct submission. 2 - TARGET_QUEUE_SUBMIT - If the user has not requested a specific value then the fabric requests queued submission. Note that these fabric values are based on what the fabric driver currently defaults to for compat with exiting setups. direct_submit_supported: 0 - The fabric does not support direct submission. 1 - The fabric supports direct submission. Signed-off-by:Mike Christie <michael.christie@oracle.com> Link: https://lore.kernel.org/r/20230928020907.5730-8-michael.christie@oracle.comSigned-off-by:
-
Mike Christie authored
target_queue_submission() is not called by drivers anymore so unexport it. Signed-off-by:
-
Mike Christie authored
This allows userspace to request the fabric drivers do direct submissions if they support it. With the new device file, submit_type, users can write 0 - 2 to control how commands are submitted to the backend: 0 - TARGET_FABRIC_DEFAULT_SUBMIT - LIO will use the fabric's default submission type. This is the default for compat. 1 - TARGET_DIRECT_SUBMIT - LIO will submit the cmd to the backend from the calling context if the fabric the cmd was received on supports it, else it will use the fabric's default type. 2 - TARGET_QUEUE_SUBMIT - LIO will queue the cmd to the LIO submission workqueue which will pass it to the backend. When using an NVMe drive and vhost-scsi with direct submission we see around a 20% improvement in 4K I/Os: fio jobs 1 2 4 8 10 -------------------------------------------------- defer 94K 190K 394K 770K 890K direct 128K 252K 488K 950K - And when using the queueing mode, we now no longer see issues like where the iSCSI tx thread is blocked in the block layer waiting on a tag so it can't respond to a nop or perform I/Os for other LUs. Signed-off-by: -
Mike Christie authored
Move the code from transport_handle_cdb_direct() to target_submit() and have iSCSI call target_submit(). Signed-off-by:
-
Mike Christie authored
Move the hack to clear some buffers to transport_handle_cdb_direct() so we can eventually merge transport_handle_cdb_direct() and target_submit(). This also fixes up the comment so it's clear it was only for udev and reflects that the referenced function does not exist and we now allow more than 1 page for control CDBs. Signed-off-by:
-
Mike Christie authored
Move core_alua_check_nonop_delay() to transport_handle_cdb_direct() so the iSCSI target driver doesn't have to call as many core functions directly. We will eventually merge transport_handle_cdb_direct and target_submit so iSCSI and the other drivers call a common function. It will also be helpful as preparation for future changes which allow the iSCSI target to defer command submission to the LIO submission workqueue, because we will have a common submission function for that which will be based on transport_handle_cdb_direct()/target_submit(). Signed-off-by:
-
Mike Christie authored
In some cases, like with multiple LUN targets or where the target has to respond to transport level requests from the receiving context it can be better to defer cmd submission to a helper thread. If the backend driver blocks on something like request/tag allocation it can block the entire target submission path and other LUs and transport IO on that session. In other cases like single LUN targets with storage that can support all the commands that the target can queue, then it's best to submit the cmd to the backend from the target's cmd receiving context. Subsequent commits will allow the user to config what they prefer, but drivers like loop can't directly submit because they can be called from a context that can't sleep. And, drivers like vhost-scsi can support direct submission, but need to keep their default behavior of deferring execution to avoid possible regressions where the backend can block. Make the drivers tell LIO core if they support direct submissions and their current default, so we can prevent users from misconfiguring the system and initialize devices correctly. Signed-off-by:
-
Mike Christie authored
Subsequent commits add more on/off type of settings to the target_core_fabric_ops struct so this makes write_pending_must_be_called a bit field instead of a bool to better organize the settings. Signed-off-by:
-
Martin K. Petersen authored
Hannes Reinecke <hare@suse.de> says: Hi all, (taking up an old thread:) here's the first batch of patches for my EH rework. It modifies the reset callbacks for SCSI drivers such that the final conversion to drop the 'struct scsi_cmnd' argument and use the entity in question (host, bus, target, device) as the argument to the SCSI EH callbacks becomes possible. The first part covers drivers which just requires minor tweaks. Link: https://lore.kernel.org/r/20231002154328.43718-1-hare@suse.deSigned-off-by:
-
Hannes Reinecke authored
SCSI EH host reset is the final callback in the escalation chain; once we reach this we need to reset the controller. As such it defeats the purpose to skip controller reset if no I/Os are pending and the RAID device is to be reset; especially after kexec there might be stale commands pending in firmware for which we have no reference whatsoever. So this patch splits off the check for pending I/O into a 'bus_reset' function, and leaves the actual controller reset to the host reset. Signed-off-by:
Hannes Reinecke <hare@suse.de> Link: https://lore.kernel.org/r/20231002154328.43718-19-hare@suse.de Cc: Kashyap Desai <kashyap.desai@broadcom.com> Cc: Sathya Prakash Veerichetty <sathya.prakash@broadcom.com> Cc: Sumit Saxena <sumit.saxena@broadcom.com> Cc: Sreekanth Reddy <sreekanth.reddy@broadcom.com> Reviewed-by:
Christoph Hellwig <hch@lst.de> Reviewed-by:
Johannes Thumshirn <johannes.thumshirn@wdc.com> Signed-off-by:
-
Hannes Reinecke authored
The reset code requires a device to be selected, but we shouldn't rely on the command to provide a device for us. So select the first device on the target when sending down a target reset. Signed-off-by:
-
Hannes Reinecke authored
The reset code requires a device to be selected, but we shouldn't rely on the command to provide a device for us. So select the first device on the bus when sending down a bus reset. Signed-off-by:
-
Hannes Reinecke authored
There's not much in common between host reset and all other error handlers, so use a separate function here. Signed-off-by:
-
Hannes Reinecke authored
Split off the combined abort and device reset handling into distinct functions. And rename the current device reset handler into a target reset handler, seeing that it really is a target reset. Signed-off-by:
-
Hannes Reinecke authored
The current handler does both, bus reset and host reset. So split them off into two distinct functions. Signed-off-by:
-
Hannes Reinecke authored
The code for aborting an outstanding command is a copy of the functionality from command abort. As we already have called this function once we reach host reset there's no point in trying to do so again. Signed-off-by:
-
Hannes Reinecke authored
When calling a host reset we shouldn't rely on the command triggering the reset, so allow megaraid_abort_and_reset() to be called with a NULL scb. And drop the pointless 'bus_reset' and 'target_reset' handlers, which just call the same function as host_reset. Signed-off-by:
-
Hannes Reinecke authored
For target reset we need a device to send the target reset to, so open-code the loop in target reset to send the target reset TMF to the correct device. Signed-off-by:
-
Hannes Reinecke authored
When sending a device reset we should not take a reference to the SCSI command. Signed-off-by:
-
Hannes Reinecke authored
Convert BUILD_SCSIID() into a function and add a scsi_device argument. Signed-off-by:
-
Hannes Reinecke authored
When sending a device reset we should not take a reference to the SCSI command. Signed-off-by:
-
Hannes Reinecke authored
Convert BUILD_SCSIID() into a function and add a scsi_device argument. Reported-by:
kernel test robot <lkp@intel.com> Signed-off-by:
-
Hannes Reinecke authored
When a LUN or target reset is issued, we should not rely on a SCSI command to be present; we'll have to reset the entire device or target anyway. Signed-off-by:
-
Hannes Reinecke authored
When sending a TMF we're only concerned with the rport and the LUN ID, so use struct fc_rport as argument for qedf_initiate_tmf(). Signed-off-by:
-
Hannes Reinecke authored
When calling bus_reset we have potentially several ports to be reset, so this patch open-codes the existing mptfc_block_error_handler() to wait for all ports attached to this bus. Signed-off-by:
-
Hannes Reinecke authored
mptscsih_dev_reset() is _not_ a device reset, but rather a target reset. Nevertheless it's being used for either purpose. This patch adds a correct implementation for mptscsih_dev_reset(), and renames the original function to mptscsih_target_reset(). Signed-off-by:
-
Hannes Reinecke authored
Instead of passing in a function to mptfc_block_error_handler() we can as well return a status and call the function afterwards. Signed-off-by:
-
Nathan Chancellor authored
Clang warns (or errors with CONFIG_WERROR=y) several times along the lines of: drivers/scsi/ibmvscsi/ibmvfc.c:650:17: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion] 650 | vhost->reinit = 1; | ^ ~ A single-bit signed integer bitfield only has possible values of -1 and 0, not 0 and 1 like an unsigned one would. No context appears to check the actual value of these bitfields, just whether or not it is zero. However, it is easy enough to change the type of the fields to 'unsigned int', which keeps the same size in memory and resolves the warning. Fixes: 51449058 ("scsi: ibmvfc: Use a bitfield for boolean flags") Signed-off-by:Nathan Chancellor <nathan@kernel.org> Link: https://lore.kernel.org/r/20231010-ibmvfc-fix-bitfields-type-v1-1-37e95b5a60e5@kernel.orgReviewed-by:
Nick Desaulniers <ndesaulniers@google.com> Signed-off-by:
-
Wenchao Hao authored
fc_lport_ptp_setup() did not check the return value of fc_rport_create() which can return NULL and would cause a NULL pointer dereference. Address this issue by checking return value of fc_rport_create() and log error message on fc_rport_create() failed. Signed-off-by:
Wenchao Hao <haowenchao2@huawei.com> Link: https://lore.kernel.org/r/20231011130350.819571-1-haowenchao2@huawei.comReviewed-by:
Simon Horman <horms@kernel.org> Signed-off-by:
-
Muhammad Muzammil authored
Fix 'generated' typo. Signed-off-by:
Muhammad Muzammil <m.muzzammilashraf@gmail.com> Link: https://lore.kernel.org/r/20231013055121.12310-1-m.muzzammilashraf@gmail.comSigned-off-by:
-
Peter Wang authored
When no active_reqs, devfreq_monitor (thread A) will suspend clock scaling. But it may have racing with clk_scaling.suspend_work (thread B) and actually not suspend clock scaling (requeue after suspend). Next time after polling_ms, devfreq_monitor read clk_scaling.window_start_t = 0 then scale up clock abnormal. Below is racing step: devfreq->work (Thread A) devfreq_monitor update_devfreq ..... ufshcd_devfreq_target queue_work(hba->clk_scaling.workq, 1 &hba->clk_scaling.suspend_work) ..... 5 queue_delayed_work(devfreq_wq, &devfreq->work, msecs_to_jiffies(devfreq->profile->polling_ms)); 2 hba->clk_scaling.suspend_work (Thread B) ufshcd_clk_scaling_suspend_work __ufshcd_suspend_clkscaling devfreq_suspend_device(hba->devfreq); 3 cancel_delayed_work_sync(&devfreq->work); 4 hba->clk_scaling.window_start_t = 0; ..... Signed-off-by:
Peter Wang <peter.wang@mediatek.com> Link: https://lore.kernel.org/r/20230831130826.5592-4-peter.wang@mediatek.comReviewed-by:
Bart Van Assche <bvanassche@acm.org> Signed-off-by:
-
Peter Wang authored
When ufshcd_clk_scaling_suspend_work (thread A) running and new command coming, ufshcd_clk_scaling_start_busy (thread B) may get host_lock after thread A first time release host_lock. Then thread A second time get host_lock will set clk_scaling.window_start_t = 0 which scale up clock abnormal next polling_ms time. Also inlines another __ufshcd_suspend_clkscaling calls. Below is racing step: 1 hba->clk_scaling.suspend_work (Thread A) ufshcd_clk_scaling_suspend_work 2 spin_lock_irqsave(hba->host->host_lock, irq_flags); 3 hba->clk_scaling.is_suspended = true; 4 spin_unlock_irqrestore(hba->host->host_lock, irq_flags); __ufshcd_suspend_clkscaling 7 spin_lock_irqsave(hba->host->host_lock, flags); 8 hba->clk_scaling.window_start_t = 0; 9 spin_unlock_irqrestore(hba->host->host_lock, flags); ufshcd_send_command (Thread B) ufshcd_clk_scaling_start_busy 5 spin_lock_irqsave(hba->host->host_lock, flags); .... 6 spin_unlock_irqrestore(hba->host->host_lock, flags); Signed-off-by:
-
Peter Wang authored
If clock scale up and suspend clock scaling, ufs will keep high performance/power mode but no read/write requests on going. It is logic wrong and have power concern. Signed-off-by:
-
- 10 Oct, 2023 6 commits
-
-
Dan Carpenter authored
The "attr" pointer points to an offset into the "host" struct so it can't be NULL. Delete the if statement and pull the code in a tab. Signed-off-by:
Dan Carpenter <dan.carpenter@linaro.org> Link: https://lore.kernel.org/r/fe3b8fcd-64a7-4887-bddd-32239a88a6a3@moroto.mountainSigned-off-by:
-
Andy Shevchenko authored
The acpi_evaluate_dsm_typed() provides a way to check the type of the object evaluated by _DSM call. Use it instead of open coded variant. Signed-off-by:
Andy Shevchenko <andriy.shevchenko@linux.intel.com> Link: https://lore.kernel.org/r/20231002135125.2602895-1-andriy.shevchenko@linux.intel.comReviewed-by:
Avri Altman <avri.altman@wdc.com> Signed-off-by:
-
Peter Wang authored
If a dev command times out, clk_scaling.active_reqs is not decreased which causes abnormal clock scaling. It is complicated to handle different dev command timeout cases in both legacy mode and MCQ mode. Besides, dev cmds are rarely used and the busy time is short. Remove clock scaling busy window for dev cmds like we do for UIC or TM cmds which don't update busy window either. Signed-off-by:
Stanley Chu <stanley.chu@mediatek.com> Signed-off-by:
-
Justin Stitt authored
strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. The only caller of mptsas_exp_repmanufacture_info() is mptsas_probe_one_phy() which can allocate rphy in either sas_end_device_alloc() or sas_expander_alloc(). Both of which zero-allocate: | rdev = kzalloc(sizeof(*rdev), GFP_KERNEL); ... this is supplied to mptsas_exp_repmanufacture_info() as edev meaning that no future NUL-padding of edev members is needed. Considering the above, a suitable replacement is strscpy() [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. Also use the more idiomatic strscpy() pattern of (dest, src, sizeof(dest)). Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Cc: Kees Cook <keescook@chromium.org> Signed-off-by:
Justin Stitt <justinstitt@google.com> Link: https://lore.kernel.org/r/20231003-strncpy-drivers-message-fusion-mptsas-c-v2-1-5ce07e60bd21@google.comReviewed-by:
Kees Cook <keescook@chromium.org> Signed-off-by:
-
Justin Stitt authored
strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. Since all these structs are copied out to userspace let's keep them NUL-padded by using strscpy_pad() which guarantees NUL-termination of the destination buffer while also providing the NUL-padding behavior that strncpy() has. Let's also opt to use the more idiomatic strscpy() usage of: 'dest, src, sizeof(dest)' in cases where the compiler can determine the size of the destination buffer. Do this for all cases of strscpy...() in this file. To be abundantly sure we don't leak stack data out to user space let's also change a strscpy() to strscpy_pad(). This strscpy() was introduced in commit dbe37c71 ("scsi: message: fusion: Replace all non-returning strlcpy() with strscpy()") Note that since we are creating these structs with a copy_from_user() and modifying fields and then copying back out to the user it is probably OK not to explicitly NUL-pad everything as any data leak is probably just data from the user themselves. If this is too eager, let's opt for strscpy() which is still in the spirit of removing deprecated strncpy() usage treewide. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Cc: Kees Cook <keescook@chromium.org> Signed-off-by:
-
Peter Wang authored
When runtime PM send SSU times out, the SCSI core invokes eh_host_reset_handler, in this case ufshcd_eh_host_reset_handler(), which is then stuck waiting for flush_work(&hba->eh_work). However, ufshcd_err_handler hangs in wait RPM resume. Do link recovery only in this case. The following IO hang stack dump was observed: kworker/4:0 D <ffffffd7d31f6fb4> __switch_to+0x180/0x344 <ffffffd7d31f779c> __schedule+0x5ec/0xa14 <ffffffd7d31f7c3c> schedule+0x78/0xe0 <ffffffd7d31fefbc> schedule_timeout+0xb0/0x15c <ffffffd7d31f8120> io_schedule_timeout+0x48/0x70 <ffffffd7d31f8e40> do_wait_for_common+0x108/0x19c <ffffffd7d31f837c> wait_for_completion_io_timeout+0x50/0x78 <ffffffd7d2876bc0> blk_execute_rq+0x1b8/0x218 <ffffffd7d2b4297c> scsi_execute_cmd+0x148/0x238 <ffffffd7d2da7358> ufshcd_set_dev_pwr_mode+0xe8/0x244 <ffffffd7d2da7e40> __ufshcd_wl_resume+0x1e0/0x45c <ffffffd7d2da7b28> ufshcd_wl_runtime_resume+0x3c/0x174 <ffffffd7d2b4f290> scsi_runtime_resume+0x7c/0xc8 <ffffffd7d2ae1d48> __rpm_callback+0xa0/0x410 <ffffffd7d2ae0128> rpm_resume+0x43c/0x67c <ffffffd7d2ae1e98> __rpm_callback+0x1f0/0x410 <ffffffd7d2ae014c> rpm_resume+0x460/0x67c <ffffffd7d2ae1450> pm_runtime_work+0xa4/0xac <ffffffd7d22e39ac> process_one_work+0x208/0x598 <ffffffd7d22e3fc0> worker_thread+0x228/0x438 <ffffffd7d22eb038> kthread+0x104/0x1d4 <ffffffd7d22171a0> ret_from_fork+0x10/0x20 scsi_eh_0 D <ffffffd7d31f6fb4> __switch_to+0x180/0x344 <ffffffd7d31f779c> __schedule+0x5ec/0xa14 <ffffffd7d31f7c3c> schedule+0x78/0xe0 <ffffffd7d31fef50> schedule_timeout+0x44/0x15c <ffffffd7d31f8e40> do_wait_for_common+0x108/0x19c <ffffffd7d31f8234> wait_for_completion+0x48/0x64 <ffffffd7d22deb88> __flush_work+0x260/0x2d0 <ffffffd7d22de918> flush_work+0x10/0x20 <ffffffd7d2da4728> ufshcd_eh_host_reset_handler+0x88/0xcc <ffffffd7d2b41da4> scsi_try_host_reset+0x48/0xe0 <ffffffd7d2b410fc> scsi_eh_ready_devs+0x934/0xa40 <ffffffd7d2b41618> scsi_error_handler+0x168/0x374 <ffffffd7d22eb038> kthread+0x104/0x1d4 <ffffffd7d22171a0> ret_from_fork+0x10/0x20 kworker/u16:5 D <ffffffd7d31f6fb4> __switch_to+0x180/0x344 <ffffffd7d31f779c> __schedule+0x5ec/0xa14 <ffffffd7d31f7c3c> schedule+0x78/0xe0 <ffffffd7d2adfe00> rpm_resume+0x114/0x67c <ffffffd7d2adfca8> __pm_runtime_resume+0x70/0xb4 <ffffffd7d2d9cf48> ufshcd_err_handler+0x1a0/0xe68 <ffffffd7d22e39ac> process_one_work+0x208/0x598 <ffffffd7d22e3fc0> worker_thread+0x228/0x438 <ffffffd7d22eb038> kthread+0x104/0x1d4 <ffffffd7d22171a0> ret_from_fork+0x10/0x20 Signed-off-by:
-
