- 02 Mar, 2022 16 commits
-
-
Mike Christie authored
When the workqueue code was created it didn't allow variable args so we have been using a temp buffer. Drop that. Link: https://lore.kernel.org/r/20220226230435.38733-7-michael.christie@oracle.comReviewed-by:
Chris Leech <cleech@redhat.com> Reviewed-by:
Lee Duncan <lduncan@suse.com> Signed-off-by:
Mike Christie <michael.christie@oracle.com> Signed-off-by:
Martin K. Petersen <martin.petersen@oracle.com>
-
Mike Christie authored
Use the session workqueue for recovery and unbinding. If there are delays during device blocking/cleanup then it will no longer affect other sessions. Link: https://lore.kernel.org/r/20220226230435.38733-6-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
We currently allocate a workqueue per host and only use it for removing the target. For the session per host case we could be using this workqueue to be able to do recoveries (block, unblock, timeout handling) in parallel. To also allow offload drivers to do their session recoveries in parallel, this drops the per host workqueue and replaces it with a per session one. Link: https://lore.kernel.org/r/20220226230435.38733-5-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
qla4xxx does not use iscsi_scan_finished() anymore so remove it. Link: https://lore.kernel.org/r/20220226230435.38733-4-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
When the iSCSI class was added upstream, blocking a queue was fast because it just set some flag bits and didn't handle I/O that was in the process of being sent to the driver. That's no longer the case so blocking a queue is expensive and we can end up with a backlog of blocks by the time we have relogged in and are trying to start the queues. For the session unblock case, this has try to cancel the block and recovery work in case they are still queued so we can avoid unneeded queue manipulations. For removal, we also now try to cancel all the recovery related works since a couple lines down we will set the session and device state so running those functions are not necessary. Link: https://lore.kernel.org/r/20220226230435.38733-3-michael.christie@oracle.comReviewed-by:
-
Mike Christie authored
If the user sets the iscsi_eh_timer_workq/iscsi_eh workqueue's max_active to greater than 1, the recovery_work could be running when __iscsi_unblock_session() runs. The cancel_delayed_work() will then not wait for the running work and we can race where we end up with the wrong session state and scsi_device state set. This replaces the cancel_delayed_work() with the sync version. Link: https://lore.kernel.org/r/20220226230435.38733-2-michael.christie@oracle.comReviewed-by:
-
James Smart authored
In the original FPIN commit, stats were incremented by the event_count. Event_count is the minimum # of events that must occur before an FPIN is sent. Thus, its not the actual number of events, and could be significantly off (too low) as it doesn't reflect anything not reported. Rather than attempt to count events, have the statistic count how many FPINS cross the threshold and were reported. Link: https://lore.kernel.org/r/20220301175536.60250-1-jsmart2021@gmail.com Fixes: 3dcfe0de ("scsi: fc: Parse FPIN packets and update statistics") Cc: <stable@vger.kernel.org> # v5.11+ Cc: Shyam Sundar <ssundar@marvell.com> Cc: Nilesh Javali <njavali@marvell.com> Reviewed-by:
Himanshu Madhani <himanshu.madhani@oracle.com> Signed-off-by:
James Smart <jsmart2021@gmail.com> Signed-off-by:
-
Damien Le Moal authored
Sparse throws a warning about context imbalance ("different lock contexts for basic block") in sas_form_port() as it gets confused with the fact that a port is locked within one of the two search loops and unlocked afterward outside of the search loops once the phy is added to the port. Since this code is not easy to follow, improve it by factoring out the code adding the phy to the port once the port is locked into the helper function sas_form_port_add_phy(). This helper can then be called directly within the port search loops, avoiding confusion and clearing the sparse warning. Link: https://lore.kernel.org/r/20220228094857.557329-1-damien.lemoal@opensource.wdc.comReviewed-by:John Garry <john.garry@huawei.com> Signed-off-by:
Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by:
-
Christoph Hellwig authored
This header is empty now except for an include of <linux/blk-mq.h>, so remove it. Link: https://lore.kernel.org/r/20220224175552.988286-9-hch@lst.deReviewed-by:
Bart Van Assche <bvanassche@acm.org> Reviewed-by:
Christoph Hellwig <hch@lst.de> Signed-off-by:
-
Christoph Hellwig authored
Let submitters initialize the scmd->allowed field directly instead of indirecting through struct scsi_request and remove the now superfluous structure. Link: https://lore.kernel.org/r/20220224175552.988286-8-hch@lst.deReviewed-by:
-
Christoph Hellwig authored
Prepare for removing the scsi_request structure by moving the result field to struct scsi_cmnd. Link: https://lore.kernel.org/r/20220224175552.988286-7-hch@lst.deReviewed-by:
-
Christoph Hellwig authored
Prepare for removing the scsi_request structure by moving the resid_len field to struct scsi_cmnd. Link: https://lore.kernel.org/r/20220224175552.988286-6-hch@lst.deReviewed-by:
-
Christoph Hellwig authored
Just use the sense_buffer field in struct scsi_cmnd for the sense data and move the sense_len field over to struct scsi_cmnd. Link: https://lore.kernel.org/r/20220224175552.988286-5-hch@lst.deReviewed-by:
-
Christoph Hellwig authored
Now that each scsi_request is backed by a scsi_cmnd, there is no need to indirect the CDB storage. Change all submitters of SCSI passthrough requests to store the CDB information directly in the scsi_cmnd, and while doing so allocate the full 32 bytes that cover all Linux supported SCSI hosts instead of requiring dynamic allocation for > 16 byte CDBs. On 64-bit systems this does not change the size of the scsi_cmnd at all, while on 32-bit systems it slightly increases it for now, but that increase will be made up by the removal of the remaining scsi_request fields. Link: https://lore.kernel.org/r/20220224175552.988286-4-hch@lst.deReviewed-by:
-
Christoph Hellwig authored
Replace the big fat memset that requires saving and restoring various fields with just initializing those fields that need initialization. All the clearing to 0 is moved to scsi_prepare_cmd() as scsi_ioctl_reset() alreadly uses kzalloc() to allocate a pre-zeroed command. This is still conservative and can probably be optimized further. Link: https://lore.kernel.org/r/20220224175552.988286-3-hch@lst.deReviewed-by:
-
Christoph Hellwig authored
Copy directly from the se_cmd CDB to the one in the scsi_request. This temporarily limits the pscsi backend to supporting only up to 16 byte CDBs, but this restriction will be lifted later in this series. Link: https://lore.kernel.org/r/20220224175552.988286-2-hch@lst.deSigned-off-by:
-
- 28 Feb, 2022 16 commits
-
-
John Garry authored
Function queue_work() returns a bool, so use a bool to hold this value for the return code from callers, which should make the code a tiny bit more clear. Also take this opportunity to condense the code of the those callers, such as sas_queue_work(), as suggested by Damien. Link: https://lore.kernel.org/r/1645786656-221630-3-git-send-email-john.garry@huawei.comReviewed-by:
-
John Garry authored
Nobody checks the return codes, so make them return void. Indeed, if the LLDD cannot send an event, nothing much can be done in the LLDD about it. Also remove prototype for sas_notify_phy_event() in sas_internal.h, which should not be there. Link: https://lore.kernel.org/r/1645786656-221630-2-git-send-email-john.garry@huawei.comReviewed-by:
Xiang Chen <chenxiang66@hisilicon.com> Reviewed-by:
-
Xingui Yang authored
In case of SSP underflow allow the response frame IU to be examined for setting the response stat value rather than always setting SAS_DATA_UNDERRUN. This will mean that we call sas_ssp_task_response() in those scenarios and may send sense data to upper layer. Such a condition would be for bad blocks were we just reporting an underflow error to upper layer, but now the sense data will tell immediately that the media is faulty. Link: https://lore.kernel.org/r/1645703489-87194-7-git-send-email-john.garry@huawei.comSigned-off-by:
Xingui Yang <yangxingui@huawei.com> Signed-off-by:
Qi Liu <liuqi115@huawei.com> Signed-off-by:
-
Xiang Chen authored
Add a file operation for "cnt" file under bist directory, so users can only read "cnt" or clear "cnt" to zero, but cannot randomly modify. Link: https://lore.kernel.org/r/1645703489-87194-6-git-send-email-john.garry@huawei.comSigned-off-by:
-
Qi Liu authored
To avoid doubt, rename the error labels to indicate the action they will take. Link: https://lore.kernel.org/r/1645703489-87194-5-git-send-email-john.garry@huawei.comSigned-off-by:
-
Qi Liu authored
If the driver probe fails to request the channel IRQ or fatal IRQ, the driver will free the IRQ vectors before freeing the IRQs in free_irq(), and this will cause a kernel BUG like this: ------------[ cut here ]------------ kernel BUG at drivers/pci/msi.c:369! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP Call trace: free_msi_irqs+0x118/0x13c pci_disable_msi+0xfc/0x120 pci_free_irq_vectors+0x24/0x3c hisi_sas_v3_probe+0x360/0x9d0 [hisi_sas_v3_hw] local_pci_probe+0x44/0xb0 work_for_cpu_fn+0x20/0x34 process_one_work+0x1d0/0x340 worker_thread+0x2e0/0x460 kthread+0x180/0x190 ret_from_fork+0x10/0x20 ---[ end trace b88990335b610c11 ]--- So we use devm_add_action() to control the order in which we free the vectors. Link: https://lore.kernel.org/r/1645703489-87194-4-git-send-email-john.garry@huawei.comSigned-off-by:
-
Xiang Chen authored
The time of phyup not only depends on the controller but also the type of disk connected. As an example, from experience, for some SATA disks the amount of time from reset/power-on to receive the D2H FIS for phyup can take upto and more than 10s sometimes. According to the specification of some SATA disks such as ST14000NM0018, the max time from power-on to ready is 30s. Based on this the current timeout of phyup at 2s which is not enough. So set the value as HISI_SAS_WAIT_PHYUP_TIMEOUT (30s) in hisi_sas_control_phy(). For v3 hw there is a pre-existing workaround for a HW bug, being that we issue a link reset when the OOB occurs but the phyup does not. The current phyup timeout is HISI_SAS_WAIT_PHYUP_TIMEOUT. So if this does occur from when issuing a phy enable or similar via hisi_sas_control_phy(), the subsequent HW workaround linkreset processing calls hisi_sas_control_phy(), but this will pend the original phy reset timing out, so it is safe. Link: https://lore.kernel.org/r/1645703489-87194-3-git-send-email-john.garry@huawei.comSigned-off-by:
-
Xiang Chen authored
Currently the permission of parameter prot_mask is 0x0, which means that the member does not appear in sysfs. Change it as other module parameters to 0444 for world-readable. [mkp: s/v3/v2/] Link: https://lore.kernel.org/r/1645703489-87194-2-git-send-email-john.garry@huawei.com Fixes: d6a9000b ("scsi: hisi_sas: Add support for DIF feature for v2 hw") Reported-by:
Yihang Li <liyihang6@hisilicon.com> Signed-off-by:
-
Changcheng Deng authored
Remove unneeded variable used to store return value. Link: https://lore.kernel.org/r/20220224090735.1967816-1-deng.changcheng@zte.com.cnReported-by:
Zeal Robot <zealci@zte.com.cn> Signed-off-by:
Changcheng Deng <deng.changcheng@zte.com.cn> Signed-off-by:
-
Randy Dunlap authored
__setup() handlers should return 1 if the command line option is handled and 0 if not (or maybe never return 0; doing so just pollutes init's environment with strings that are not init arguments/parameters). Return 1 from aha152x_setup() to indicate that the boot option has been handled. Link: lore.kernel.org/r/64644a2f-4a20-bab3-1e15-3b2cdd0defe3@omprussia.ru Link: https://lore.kernel.org/r/20220223000623.5920-1-rdunlap@infradead.org Cc: "Juergen E. Fischer" <fischer@norbit.de> Cc: "James E.J. Bottomley" <jejb@linux.ibm.com> Cc: "Martin K. Petersen" <martin.petersen@oracle.com> Reported-by:
Igor Zhbanov <i.zhbanov@omprussia.ru> Signed-off-by:
Randy Dunlap <rdunlap@infradead.org> Signed-off-by:
-
Ajish Koshy authored
Firmware expects host driver to clear scratchpad rsvd 0 register after non-fatal error is found. This is done when firmware raises fatal error interrupt and indicates non-fatal error. At this point firmware updates scratchpad rsvd 0 register with non-fatal error value. Here host has to clear the register after reading it during non-fatal errors. Rename: - MSGU_HOST_SCRATCH_PAD_6 to MSGU_SCRATCH_PAD_RSVD_0 - MSGU_HOST_SCRATCH_PAD_7 to MSGU_SCRATCH_PAD_RSVD_1 Link: https://lore.kernel.org/r/20220222092618.108198-1-Ajish.Koshy@microchip.comReviewed-by:
Jack Wang <jinpu.wang@ionos.com> Signed-off-by:
Ajish Koshy <Ajish.Koshy@microchip.com> Signed-off-by:
Viswas G <Viswas.G@microchip.com> Signed-off-by:
-
Finn Thain authored
This driver doesn't use SCp.ptr to save a SCSI command data pointer which means "scsi pointer" is a complete misnomer here. Only a few members of struct scsi_pointer are used and the rest waste memory. Avoid the "struct foo { struct bar; };" silliness. Link: https://lore.kernel.org/r/3529a59873a7de8455a27af2528341afe5069adc.1645484982.git.fthain@linux-m68k.orgReviewed-by:Finn Thain <fthain@linux-m68k.org> Signed-off-by:
-
Finn Thain authored
This driver doesn't use SCp.ptr to save a SCSI command data pointer which means "scsi pointer" is a complete misnomer here. Only a few members of struct scsi_pointer are used and the rest waste memory. Avoid the "struct foo { struct bar; };" silliness. Link: https://lore.kernel.org/r/fbf930e64af5b15ca028dfe25b00fe933951f19b.1645484982.git.fthain@linux-m68k.orgReviewed-by: -
Khazhismel Kumykov authored
Most importantly: eh_timed_out() is not limited by scmd->allowed, and can reset timer forever. Fixes: c829c394 ("[SCSI] FC transport : Avoid device offline cases by stalling aborts until device unblocked") Link: https://lore.kernel.org/r/20220219001601.3534043-1-khazhy@google.comReviewed-by:
Khazhismel Kumykov <khazhy@google.com> Signed-off-by:
-
Christophe JAILLET authored
Just a few lines below this kzalloc() we have a mutex_lock() which can sleep. Moreover, the only way to call this function is when a delayed work is schedule. And delayed work can sleep: INIT_DELAYED_WORK(&fw_event->work, mptsas_firmware_event_work); --> mptsas_firmware_event_work() --> mptsas_send_link_status_event() --> mptsas_expander_add() So there is really no good reason to use GFP_ATOMIC here. Change it to GFP_KERNEL to give more opportunities to the kernel. Link: https://lore.kernel.org/r/eccb2179ce800529851ed4fabc9d3f95fbbf7d7f.1644906731.git.christophe.jaillet@wanadoo.frSigned-off-by:Christophe JAILLET <christophe.jaillet@wanadoo.fr> Signed-off-by:
-
Gustavo A. R. Silva authored
Use flexible-array members in struct fc_fdmi_attr_entry and fs_fdmi_attrs instead of one-element arrays, and refactor the code accordingly. Also, this helps with the ongoing efforts to globally enable -Warray-bounds and get us closer to being able to tighten the FORTIFY_SOURCE routines on memcpy(). https://github.com/KSPP/linux/issues/79 https://github.com/ClangBuiltLinux/linux/issues/1590 Link: https://lore.kernel.org/r/20220214223903.GA859464@embeddedorSigned-off-by:
Gustavo A. R. Silva <gustavoars@kernel.org> Signed-off-by:
-
- 23 Feb, 2022 8 commits
-
-
Damien Le Moal authored
Make the driver messages more readable by adding a space after the message prefix ":" and removing the extra space between function name and line number. Link: https://lore.kernel.org/r/20220220031810.738362-32-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
Both pm8001_send_abort_all() and pm80xx_send_abort_all() are called only for a non null device with the NCQ_READ_LOG_FLAG set, so remove the device check on entry of these functions. Furthermore, setting the NCQ_ABORT_ALL_FLAG device id flag and clearing the NCQ_READ_LOG_FLAG is always done before calling these functions. Move these operations inside the functions. Link: https://lore.kernel.org/r/20220220031810.738362-31-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
The task argument of the pm8001_ccb_task_free() function can be inferred from the ccb argument ccb_task field. So there is no need to have this argument. Likewise, the ccb_index argument is always equal to the ccb tag field and is not needed either. Remove both arguments and update all call sites. The pm8001_ccb_task_free_done() helper is also modified to match this change. Link: https://lore.kernel.org/r/20220220031810.738362-30-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
The main part of the pm8001_task_exec() function uses a do {} while(0) loop that is useless and only makes the code harder to read. Remove this loop. The unnecessary local variable t is also removed. Additionally, avoid repeatedly declaring "struct task_status_struct *ts" to handle error cases by declaring this variable for the entire function scope. This allows simplifying the error cases, and together with the addition of blank lines make the code more readable. Finally, handling of the running_req counter is fixed to avoid decrementing it without a corresponding incrementation in the case of an invalid task protocol. Link: https://lore.kernel.org/r/20220220031810.738362-29-damien.lemoal@opensource.wdc.comReviewed-by: -
Damien Le Moal authored
There is no need to pass a pointer to a struct inbound_queue_table to pm8001_mpi_build_cmd(). Passing the start index in the inbound queue table of the adapter is enough. This change allows avoiding the declaration of a struct inbound_queue_table pointer (circularQ variables) in many functions, simplifying the code. While at it, blank lines are added i(e.g. after local variable declarations) to make the code more readable. Link: https://lore.kernel.org/r/20220220031810.738362-28-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
Introduce the pm8001_ccb_alloc() and pm8001_ccb_free() helpers to replace the typical code patterns: res = pm8001_tag_alloc(pm8001_ha, &ccb_tag); if (res) ... ccb = &pm8001_ha->ccb_info[ccb_tag]; ccb->device = pm8001_ha_dev; ccb->ccb_tag = ccb_tag; ccb->task = task; ccb->n_elem = 0; and ccb->task = NULL; ccb->ccb_tag = PM8001_INVALID_TAG; pm8001_tag_free(pm8001_ha, tag); With the simpler function calls: ccb = pm8001_ccb_alloc(pm8001_ha, pm8001_ha_dev, task); if (!ccb) ... and pm8001_ccb_free(pm8001_ha, ccb); The pm8001_ccb_alloc() helper ensures that all fields of the ccb info structure for the newly allocated tag are all initialized, except the buf_prd field. The pm8001_ccb_free() helper clears the initialized fields and the ccb tag to ensure that iteration over the adapter ccb_info array detects ccbs that are in use. All call site of the pm8001_tag_alloc() function that use a ccb info associated with an allocated tag are converted to use the new helpers. Link: https://lore.kernel.org/r/20220220031810.738362-27-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
To detect if a command is NCQ, there is no need to test all possible NCQ command codes. Instead, use ata_is_ncq() to test the command protocol. Link: https://lore.kernel.org/r/20220220031810.738362-26-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
Replace the goto statement in the for loop with "break" and remove the ex_err label. Also fix long lines, identation and blank lines to make the code more readable. Link: https://lore.kernel.org/r/20220220031810.738362-25-damien.lemoal@opensource.wdc.comSigned-off-by:
-
