Hello Martin, James, Jonathan, here are some cleanups and small changes for zfcp I'd like to include in 5.9 if possible. One of the changes touches documentation in `Documentation/scsi/`, so I put Jonathan on To, hope that was correct. I hope you can still pull this all in one go to minimize work. IBM did retire some old URLs and content from our public website, so we have to clean that up in the documentation so there are not dead links. I changed these in the hopes we can minimize documentation churn going forward, just to replace URLs. The other changes are mostly small cleanups for our driver that make at least parts more sane than they've been. Most of these have been in our CI for quite a while now - shame on me for delaying sending them upstream so long -, so I am rather confident that they don't break anything for us. As always, feedback and reviews are appreciated :-) Benjamin Block (2): scsi: docs: update outdated link to IBM developerworks scsi: docs: remove invalid link and update text for zfcp kernel config George Spelvin (1): zfcp: use prandom_u32_max() for backoff Julian Wiedmann (4): zfcp: fix an outdated comment for zfcp_qdio_send() zfcp: clean up zfcp_erp_action_ready() zfcp: replace open-coded list move zfcp: avoid benign overflow of the Request Queue's free-level Documentation/scsi/scsi-parameters.rst | 2 +- drivers/s390/scsi/zfcp_ccw.c | 7 +++---- drivers/s390/scsi/zfcp_erp.c | 2 +- drivers/s390/scsi/zfcp_fc.c | 2 +- drivers/s390/scsi/zfcp_qdio.c | 7 +++++-- drivers/scsi/Kconfig | 15 ++++++++++----- 6 files changed, 21 insertions(+), 14 deletions(-) -- 2.26.2
From: George Spelvin <lkml@sdf.org> We don't need crypto-grade random numbers for randomized backoffs. Instead use prandom_u32_max(ep_ro) which generates a pseudo-random number uniformly distributed in the interval [0, ep_ro). Signed-off-by: George Spelvin <lkml@sdf.org> Reviewed-by: Steffen Maier <maier@linux.ibm.com> Signed-off-by: Benjamin Block <bblock@linux.ibm.com> --- drivers/s390/scsi/zfcp_fc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c index b018b61bd168..d24cafe02708 100644 --- a/drivers/s390/scsi/zfcp_fc.c +++ b/drivers/s390/scsi/zfcp_fc.c @@ -48,7 +48,7 @@ unsigned int zfcp_fc_port_scan_backoff(void) { if (!port_scan_backoff) return 0; - return get_random_int() % port_scan_backoff; + return prandom_u32_max(port_scan_backoff); } static void zfcp_fc_port_scan_time(struct zfcp_adapter *adapter) -- 2.26.2
From: Julian Wiedmann <jwi@linux.ibm.com> zfcp no longer uses the qdio PCI flag, update the comment. Fixes: 21ddaa53f92d ("[SCSI] zfcp: Remove PCI flag") Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com> Reviewed-by: Steffen Maier <maier@linux.ibm.com> Reviewed-by: Fedor Loshakov <loshakov@linux.ibm.com> Signed-off-by: Benjamin Block <bblock@linux.ibm.com> --- drivers/s390/scsi/zfcp_qdio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/s390/scsi/zfcp_qdio.c b/drivers/s390/scsi/zfcp_qdio.c index 3a7f3374d10a..d3d110a04884 100644 --- a/drivers/s390/scsi/zfcp_qdio.c +++ b/drivers/s390/scsi/zfcp_qdio.c @@ -246,7 +246,7 @@ int zfcp_qdio_sbal_get(struct zfcp_qdio *qdio) } /** - * zfcp_qdio_send - set PCI flag in first SBALE and send req to QDIO + * zfcp_qdio_send - send req to QDIO * @qdio: pointer to struct zfcp_qdio * @q_req: pointer to struct zfcp_qdio_req * Returns: 0 on success, error otherwise -- 2.26.2
IBM decided to retire a lot of the content that was previously hosted on "developerworks", and so some of the links we've used for documentation are now dead or redirect to some general landing page with no correlation to what the links were meant to provide. The s390-tools package is meanwhile also hosted on github, so we can link to the script directly instead of to the archive. Reviewed-by: Julian Wiedmann <jwi@linux.ibm.com> Reviewed-by: Steffen Maier <maier@linux.ibm.com> Signed-off-by: Benjamin Block <bblock@linux.ibm.com> --- Documentation/scsi/scsi-parameters.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/scsi/scsi-parameters.rst b/Documentation/scsi/scsi-parameters.rst index 9aba897c97ac..e5f68b431f5c 100644 --- a/Documentation/scsi/scsi-parameters.rst +++ b/Documentation/scsi/scsi-parameters.rst @@ -94,7 +94,7 @@ parameters may be changed at runtime by the command (/proc/sys/dev/scsi/logging_level). There is also a nice 'scsi_logging_level' script in the S390-tools package, available for download at - http://www-128.ibm.com/developerworks/linux/linux390/s390-tools-1.5.4.html + https://github.com/ibm-s390-tools/s390-tools/blob/master/scripts/scsi_logging_level scsi_mod.scan= [SCSI] sync (default) scans SCSI busses as they are discovered. async scans them in kernel threads, -- 2.26.2
IBM decided to retire a lot of the content that was previously hosted on "developerworks", and so some of the links we've used for documentation are now dead or redirect to some general landing page with no correlation to what the links were meant to provide. Change the provided link in the Kconfig file for zfcp to rather refer to our device drivers book that we regularly update and publish for free, and whose name hasn't been changed since it was first published. Our hardware is also not called "IBM eServer zSeries" anymore - in fact, it hasn't been called like that since 2006. Use a broader term that covers different server names over time. Lastly, add a short paragraph about how our HBAs are typically named, to have some more tangible references. Reviewed-by: Julian Wiedmann <jwi@linux.ibm.com> Reviewed-by: Steffen Maier <maier@linux.ibm.com> Signed-off-by: Benjamin Block <bblock@linux.ibm.com> --- drivers/scsi/Kconfig | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index e9ff4cd5fbe9..571473a58f98 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -1469,14 +1469,19 @@ config SCSI_SUNESP module will be called sun_esp. config ZFCP - tristate "FCP host bus adapter driver for IBM eServer zSeries" + tristate "FCP host bus adapter driver for IBM mainframes" depends on S390 && QDIO && SCSI depends on SCSI_FC_ATTRS help - If you want to access SCSI devices attached to your IBM eServer - zSeries by means of Fibre Channel interfaces say Y. - For details please refer to the documentation provided by IBM at - <http://oss.software.ibm.com/developerworks/opensource/linux390> + If you want to access SCSI devices attached to your IBM mainframe by + means of Fibre Channel Protocol host bus adapters say Y. + + Supported HBAs include different models of the FICON Express and FCP + Express I/O cards. + + For a more complete list, and for more details about setup and + operation refer to the IBM publication "Device Drivers, Features, and + Commands", SC33-8411. This driver is also available as a module. This module will be called zfcp. If you want to compile it as a module, say M here -- 2.26.2
From: Julian Wiedmann <jwi@linux.ibm.com> We already maintain a pointer to act->adapter. Use it consistently to avoid any confusion about whose ->erp_ready_head and ->erp_ready_wq we are accessing. Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com> Reviewed-by: Steffen Maier <maier@linux.ibm.com> Signed-off-by: Benjamin Block <bblock@linux.ibm.com> --- drivers/s390/scsi/zfcp_erp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/s390/scsi/zfcp_erp.c b/drivers/s390/scsi/zfcp_erp.c index 79f6e8fb03ca..59e662df5774 100644 --- a/drivers/s390/scsi/zfcp_erp.c +++ b/drivers/s390/scsi/zfcp_erp.c @@ -68,7 +68,7 @@ static void zfcp_erp_action_ready(struct zfcp_erp_action *act) { struct zfcp_adapter *adapter = act->adapter; - list_move(&act->list, &act->adapter->erp_ready_head); + list_move(&act->list, &adapter->erp_ready_head); zfcp_dbf_rec_run("erardy1", act); wake_up(&adapter->erp_ready_wq); zfcp_dbf_rec_run("erardy2", act); -- 2.26.2
From: Julian Wiedmann <jwi@linux.ibm.com> Instead of manually moving each element of the unit and port lists into our temporary on-stack lists, splice them over in one go. Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com> Reviewed-by: Steffen Maier <maier@linux.ibm.com> Signed-off-by: Benjamin Block <bblock@linux.ibm.com> --- drivers/s390/scsi/zfcp_ccw.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/s390/scsi/zfcp_ccw.c b/drivers/s390/scsi/zfcp_ccw.c index 49eda141ea43..d9fd0a41da64 100644 --- a/drivers/s390/scsi/zfcp_ccw.c +++ b/drivers/s390/scsi/zfcp_ccw.c @@ -124,13 +124,12 @@ static void zfcp_ccw_remove(struct ccw_device *cdev) return; write_lock_irq(&adapter->port_list_lock); - list_for_each_entry_safe(port, p, &adapter->port_list, list) { + list_for_each_entry(port, &adapter->port_list, list) { write_lock(&port->unit_list_lock); - list_for_each_entry_safe(unit, u, &port->unit_list, list) - list_move(&unit->list, &unit_remove_lh); + list_splice_init(&port->unit_list, &unit_remove_lh); write_unlock(&port->unit_list_lock); - list_move(&port->list, &port_remove_lh); } + list_splice_init(&adapter->port_list, &port_remove_lh); write_unlock_irq(&adapter->port_list_lock); zfcp_ccw_adapter_put(adapter); /* put from zfcp_ccw_adapter_by_cdev */ -- 2.26.2
From: Julian Wiedmann <jwi@linux.ibm.com> zfcp_qdio_send() and zfcp_qdio_int_req() run concurrently, adding and completing SBALs on the Request Queue. There's a theoretical race where zfcp_qdio_int_req() completes a number of SBALs & increments the queue's free-level _before_ zfcp_qdio_send() was able to decrement it. This can cause ->req_q_free to momentarily hold a value larger than QDIO_MAX_BUFFERS_PER_Q. Luckily zfcp_qdio_send() is always called under ->req_q_lock, and all readers of the free-level also take this lock. So we can trust that zfcp_qdio_send() will clean up such a temporary overflow before anyone can actually observe it. But it's still confusing and annoying to worry about. So adjust the code to avoid this race. Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com> Reviewed-by: Steffen Maier <maier@linux.ibm.com> Signed-off-by: Benjamin Block <bblock@linux.ibm.com> --- drivers/s390/scsi/zfcp_qdio.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/s390/scsi/zfcp_qdio.c b/drivers/s390/scsi/zfcp_qdio.c index d3d110a04884..e78d65bd46b1 100644 --- a/drivers/s390/scsi/zfcp_qdio.c +++ b/drivers/s390/scsi/zfcp_qdio.c @@ -260,17 +260,20 @@ int zfcp_qdio_send(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req) zfcp_qdio_account(qdio); spin_unlock(&qdio->stat_lock); + atomic_sub(sbal_number, &qdio->req_q_free); + retval = do_QDIO(qdio->adapter->ccw_device, QDIO_FLAG_SYNC_OUTPUT, 0, q_req->sbal_first, sbal_number); if (unlikely(retval)) { + /* Failed to submit the IO, roll back our modifications. */ + atomic_add(sbal_number, &qdio->req_q_free); zfcp_qdio_zero_sbals(qdio->req_q, q_req->sbal_first, sbal_number); return retval; } /* account for transferred buffers */ - atomic_sub(sbal_number, &qdio->req_q_free); qdio->req_q_idx += sbal_number; qdio->req_q_idx %= QDIO_MAX_BUFFERS_PER_Q; -- 2.26.2
On Fri, 3 Jul 2020 15:19:56 +0200, Benjamin Block wrote: > here are some cleanups and small changes for zfcp I'd like to include in > 5.9 if possible. > > One of the changes touches documentation in `Documentation/scsi/`, so I > put Jonathan on To, hope that was correct. I hope you can still pull > this all in one go to minimize work. IBM did retire some old URLs and > content from our public website, so we have to clean that up in the > documentation so there are not dead links. I changed these in the hopes > we can minimize documentation churn going forward, just to replace URLs. > > [...] Applied to 5.9/scsi-queue, thanks! [1/7] scsi: zfcp: Use prandom_u32_max() for backoff https://git.kernel.org/mkp/scsi/c/0cd0e57ec858 [2/7] scsi: zfcp: Fix an outdated comment for zfcp_qdio_send() https://git.kernel.org/mkp/scsi/c/459ad085d87b [3/7] scsi: docs: Update outdated link to IBM developerworks https://git.kernel.org/mkp/scsi/c/b9789bfbfe9d [4/7] scsi: docs: Remove invalid link and update text for zfcp kernel config https://git.kernel.org/mkp/scsi/c/c06de6e28c9e [5/7] scsi: zfcp: Clean up zfcp_erp_action_ready() https://git.kernel.org/mkp/scsi/c/b43cdb5ac856 [6/7] scsi: zfcp: Replace open-coded list move https://git.kernel.org/mkp/scsi/c/6bcb7c171a0c [7/7] scsi: zfcp: Avoid benign overflow of the Request Queue's free-level https://git.kernel.org/mkp/scsi/c/c3bfffa5ec69 -- Martin K. Petersen Oracle Linux Engineering
On Wed, Jul 08, 2020 at 02:06:45AM -0400, Martin K. Petersen wrote: > On Fri, 3 Jul 2020 15:19:56 +0200, Benjamin Block wrote: > > > here are some cleanups and small changes for zfcp I'd like to include in > > 5.9 if possible. > > > > One of the changes touches documentation in `Documentation/scsi/`, so I > > put Jonathan on To, hope that was correct. I hope you can still pull > > this all in one go to minimize work. IBM did retire some old URLs and > > content from our public website, so we have to clean that up in the > > documentation so there are not dead links. I changed these in the hopes > > we can minimize documentation churn going forward, just to replace URLs. > > > > [...] > > Applied to 5.9/scsi-queue, thanks! > > [1/7] scsi: zfcp: Use prandom_u32_max() for backoff > https://git.kernel.org/mkp/scsi/c/0cd0e57ec858 > [2/7] scsi: zfcp: Fix an outdated comment for zfcp_qdio_send() > https://git.kernel.org/mkp/scsi/c/459ad085d87b > [3/7] scsi: docs: Update outdated link to IBM developerworks > https://git.kernel.org/mkp/scsi/c/b9789bfbfe9d > [4/7] scsi: docs: Remove invalid link and update text for zfcp kernel config > https://git.kernel.org/mkp/scsi/c/c06de6e28c9e > [5/7] scsi: zfcp: Clean up zfcp_erp_action_ready() > https://git.kernel.org/mkp/scsi/c/b43cdb5ac856 > [6/7] scsi: zfcp: Replace open-coded list move > https://git.kernel.org/mkp/scsi/c/6bcb7c171a0c > [7/7] scsi: zfcp: Avoid benign overflow of the Request Queue's free-level > https://git.kernel.org/mkp/scsi/c/c3bfffa5ec69 > Thanks Martin! -- Best Regards, Benjamin Block / Linux on IBM Z Kernel Development / IBM Systems IBM Deutschland Research & Development GmbH / https://www.ibm.com/privacy Vorsitz. AufsR.: Gregor Pillen / Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294