From: Uma Krishnan <hidden> Date: 2018-05-11 19:04:00
This patch series adds few improvements to the cxlflash driver and
it also contains couple of bug fixes.
This patch series is intended for 4.18 and is bisectable.
Matthew R. Ochs (1):
cxlflash: Use local mutex for AFU serialization
Uma Krishnan (6):
cxlflash: Yield to active send threads
cxlflash: Limit the debug logs in the IO path
cxlflash: Acquire semaphore before invoking ioctl services
cxlflash: Add include guards to backend.h
cxlflash: Abstract hardware dependent assignments
cxlflash: Isolate external module dependencies
drivers/scsi/cxlflash/Kconfig | 2 +-
drivers/scsi/cxlflash/Makefile | 4 +++-
drivers/scsi/cxlflash/backend.h | 5 +++++
drivers/scsi/cxlflash/common.h | 1 +
drivers/scsi/cxlflash/lunmgt.c | 4 +++-
drivers/scsi/cxlflash/main.c | 21 +++++++++------------
drivers/scsi/cxlflash/main.h | 20 ++++++++++++++++++++
drivers/scsi/cxlflash/superpipe.c | 9 ++++++++-
drivers/scsi/cxlflash/vlun.c | 3 ++-
9 files changed, 52 insertions(+), 17 deletions(-)
--
2.1.0
From: Uma Krishnan <hidden> Date: 2018-05-11 19:04:46
The following Oops may be encountered if the device is reset, i.e. EEH
recovery, while there is heavy I/O traffic:
59:mon> t
[c000200db64bb680] c008000009264c40 cxlflash_queuecommand+0x3b8/0x500
[cxlflash]
[c000200db64bb770] c00000000090d3b0 scsi_dispatch_cmd+0x130/0x2f0
[c000200db64bb7f0] c00000000090fdd8 scsi_request_fn+0x3c8/0x8d0
[c000200db64bb900] c00000000067f528 __blk_run_queue+0x68/0xb0
[c000200db64bb930] c00000000067ab80 __elv_add_request+0x140/0x3c0
[c000200db64bb9b0] c00000000068daac blk_execute_rq_nowait+0xec/0x1a0
[c000200db64bba00] c00000000068dbb0 blk_execute_rq+0x50/0xe0
[c000200db64bba50] c0000000006b2040 sg_io+0x1f0/0x520
[c000200db64bbaf0] c0000000006b2e94 scsi_cmd_ioctl+0x534/0x610
[c000200db64bbc20] c000000000926208 sd_ioctl+0x118/0x280
[c000200db64bbcc0] c00000000069f7ac blkdev_ioctl+0x7fc/0xe30
[c000200db64bbd20] c000000000439204 block_ioctl+0x84/0xa0
[c000200db64bbd40] c0000000003f8514 do_vfs_ioctl+0xd4/0xa00
[c000200db64bbde0] c0000000003f8f04 SyS_ioctl+0xc4/0x130
[c000200db64bbe30] c00000000000b184 system_call+0x58/0x6c
When there is no room to send the I/O request, the cached room is refreshed
by reading the memory mapped command room value from the AFU. The AFU
register mapping is refreshed during a reset, creating a race condition
that can lead to the Oops above.
During a device reset, the AFU should not be unmapped until all the active
send threads quiesce. An atomic counter, cmds_active, is currently used to
track internal AFU commands and quiesce during reset. This same counter can
also be used for the active send threads.
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/main.c | 2 ++
1 file changed, 2 insertions(+)
From: Uma Krishnan <hidden> Date: 2018-05-11 19:05:08
The kernel log can get filled with debug messages from send_cmd_ioarrin()
when dynamic debug is enabled for the cxlflash module and there is a lot
of legacy I/O traffic.
While these messages are necessary to debug issues that involve command
tracking, the abundance of data can overwrite other useful data in the
log. The best option available is to limit the messages that should
serve most of the common use cases.
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Uma Krishnan <hidden> Date: 2018-05-11 19:05:22
When a superpipe process that makes use of virtual LUNs is terminated or
killed abruptly, there is a possibility that the cxlflash driver could
hang and deprive other operations on the adapter.
The release fop registered to be invoked on a context close, detaches
every LUN associated with the context. The underlying service to detach
the LUN assumes it has been called with the read semaphore held, and
releases the semaphore before any operation that could be time consuming.
When invoked without holding the read semaphore, an opportunity is created
for the semaphore's count to become negative when it is temporarily
released during one of these potential lengthy operations. This negative
count results in subsequent acquisition attempts taking forever, leading
to the hang.
To support the current design point of holding the semaphore on the
ioctl() paths, the release fop should acquire it before invoking any
ioctl services.
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -988,6 +988,10 @@ static int cxlflash_disk_detach(struct scsi_device *sdev,*theoreticallyneveroccur),everycallintothisroutineresults*inacompletefreeingofacontext.*+*DetachingtheLUNistypicallyanioctl()operationandtheunderlying+*codeassumesthatioctl_rwsemhasbeenacquiredasareader.Tosupport+*thatdesignpoint,thesemaphoreisacquiredandreleasedarounddetach.+**Return:0onsuccess*/staticintcxlflash_cxl_release(structinode*inode,structfile*file)
@@ -1026,9 +1030,11 @@ static int cxlflash_cxl_release(struct inode *inode, struct file *file)dev_dbg(dev,"%s: close for ctxid=%d\n",__func__,ctxid);+down_read(&cfg->ioctl_rwsem);detach.context_id=ctxi->ctxid;list_for_each_entry_safe(lun_access,t,&ctxi->luns,list)_cxlflash_disk_detach(lun_access->sdev,ctxi,&detach);+up_read(&cfg->ioctl_rwsem);out_release:cfg->ops->fd_release(inode,file);out:
From: Uma Krishnan <hidden> Date: 2018-05-11 19:05:37
From: "Matthew R. Ochs" <redacted>
AFUs can only process a single AFU command at a time. This is enforced
with a global mutex situated within the AFU send routine. As this mutex
has a global scope, it has the potential to unnecessarily block commands
destined for other AFUs.
Instead of using a global mutex, transition the mutex to be per-AFU. This
will allow commands to only be blocked by siblings of the same AFU.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/common.h | 1 +
drivers/scsi/cxlflash/main.c | 6 +++---
2 files changed, 4 insertions(+), 3 deletions(-)
@@ -240,6 +240,7 @@ struct afu {structcxlflash_afu_map__iomem*afu_map;/* entire MMIO map */atomic_tcmds_active;/* Number of currently active AFU commands */+structmutexsync_active;/* Mutex to serialize AFU commands */u64hb;u32internal_lun;/* User-desired LUN mode for this AFU */
From: Uma Krishnan <hidden> Date: 2018-05-11 19:05:51
The new header file, backend.h, that was recently added is missing
the include guards. This commit adds the guards.
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/backend.h | 5 +++++
1 file changed, 5 insertions(+)
From: Uma Krishnan <hidden> Date: 2018-05-11 19:06:05
As a staging cleanup to support transport specific builds of the cxlflash
module, relocate device dependent assignments to header files. This will
avoid littering the core driver with conditional compilation logic.
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/main.c | 7 ++-----
drivers/scsi/cxlflash/main.h | 15 +++++++++++++++
2 files changed, 17 insertions(+), 5 deletions(-)
From: Uma Krishnan <hidden> Date: 2018-05-11 19:06:19
Depending on the underlying transport, cxlflash has a dependency on either
the CXL or OCXL drivers, which are enabled via their Kconfig option.
Instead of having a module wide dependency on these config options, it is
better to isolate the object modules that are dependent on the CXL and OCXL
drivers and adjust the module dependencies accordingly.
This commit isolates the object files that are dependent on CXL and/or
OCXL. The cxl/ocxl fops used in the core driver are tucked under an ifdef
to avoid compilation errors.
Signed-off-by: Uma Krishnan <redacted>
---
drivers/scsi/cxlflash/Kconfig | 2 +-
drivers/scsi/cxlflash/Makefile | 4 +++-
drivers/scsi/cxlflash/lunmgt.c | 4 +++-
drivers/scsi/cxlflash/main.c | 2 --
drivers/scsi/cxlflash/main.h | 5 +++++
drivers/scsi/cxlflash/superpipe.c | 3 ++-
drivers/scsi/cxlflash/vlun.c | 3 ++-
7 files changed, 16 insertions(+), 7 deletions(-)
@@ -4,7 +4,7 @@configCXLFLASHtristate"Support for IBM CAPI Flash"-depends onPCI&&SCSI&&CXL&&OCXL&&EEH+depends onPCI&&SCSI&&(CXL||OCXL)&&EEHselectIRQ_POLLdefaultmhelp
From: Matthew R. Ochs <hidden> Date: 2018-05-16 15:09:28
On Fri, May 11, 2018 at 02:04:46PM -0500, Uma Krishnan wrote:
The following Oops may be encountered if the device is reset, i.e. EEH
recovery, while there is heavy I/O traffic:
59:mon> t
[c000200db64bb680] c008000009264c40 cxlflash_queuecommand+0x3b8/0x500
[cxlflash]
[c000200db64bb770] c00000000090d3b0 scsi_dispatch_cmd+0x130/0x2f0
[c000200db64bb7f0] c00000000090fdd8 scsi_request_fn+0x3c8/0x8d0
[c000200db64bb900] c00000000067f528 __blk_run_queue+0x68/0xb0
[c000200db64bb930] c00000000067ab80 __elv_add_request+0x140/0x3c0
[c000200db64bb9b0] c00000000068daac blk_execute_rq_nowait+0xec/0x1a0
[c000200db64bba00] c00000000068dbb0 blk_execute_rq+0x50/0xe0
[c000200db64bba50] c0000000006b2040 sg_io+0x1f0/0x520
[c000200db64bbaf0] c0000000006b2e94 scsi_cmd_ioctl+0x534/0x610
[c000200db64bbc20] c000000000926208 sd_ioctl+0x118/0x280
[c000200db64bbcc0] c00000000069f7ac blkdev_ioctl+0x7fc/0xe30
[c000200db64bbd20] c000000000439204 block_ioctl+0x84/0xa0
[c000200db64bbd40] c0000000003f8514 do_vfs_ioctl+0xd4/0xa00
[c000200db64bbde0] c0000000003f8f04 SyS_ioctl+0xc4/0x130
[c000200db64bbe30] c00000000000b184 system_call+0x58/0x6c
When there is no room to send the I/O request, the cached room is refreshed
by reading the memory mapped command room value from the AFU. The AFU
register mapping is refreshed during a reset, creating a race condition
that can lead to the Oops above.
During a device reset, the AFU should not be unmapped until all the active
send threads quiesce. An atomic counter, cmds_active, is currently used to
track internal AFU commands and quiesce during reset. This same counter can
also be used for the active send threads.
Signed-off-by: Uma Krishnan <redacted>
From: Matthew R. Ochs <hidden> Date: 2018-05-16 15:53:45
On Fri, May 11, 2018 at 02:05:08PM -0500, Uma Krishnan wrote:
The kernel log can get filled with debug messages from send_cmd_ioarrin()
when dynamic debug is enabled for the cxlflash module and there is a lot
of legacy I/O traffic.
While these messages are necessary to debug issues that involve command
tracking, the abundance of data can overwrite other useful data in the
log. The best option available is to limit the messages that should
serve most of the common use cases.
Signed-off-by: Uma Krishnan <redacted>
From: Matthew R. Ochs <hidden> Date: 2018-05-16 15:54:35
On Fri, May 11, 2018 at 02:05:22PM -0500, Uma Krishnan wrote:
When a superpipe process that makes use of virtual LUNs is terminated or
killed abruptly, there is a possibility that the cxlflash driver could
hang and deprive other operations on the adapter.
The release fop registered to be invoked on a context close, detaches
every LUN associated with the context. The underlying service to detach
the LUN assumes it has been called with the read semaphore held, and
releases the semaphore before any operation that could be time consuming.
When invoked without holding the read semaphore, an opportunity is created
for the semaphore's count to become negative when it is temporarily
released during one of these potential lengthy operations. This negative
count results in subsequent acquisition attempts taking forever, leading
to the hang.
To support the current design point of holding the semaphore on the
ioctl() paths, the release fop should acquire it before invoking any
ioctl services.
Signed-off-by: Uma Krishnan <redacted>
From: Matthew R. Ochs <hidden> Date: 2018-05-16 15:58:24
On Fri, May 11, 2018 at 02:05:51PM -0500, Uma Krishnan wrote:
The new header file, backend.h, that was recently added is missing
the include guards. This commit adds the guards.
Signed-off-by: Uma Krishnan <redacted>
From: Matthew R. Ochs <hidden> Date: 2018-05-16 15:59:17
On Fri, May 11, 2018 at 02:06:05PM -0500, Uma Krishnan wrote:
As a staging cleanup to support transport specific builds of the cxlflash
module, relocate device dependent assignments to header files. This will
avoid littering the core driver with conditional compilation logic.
Signed-off-by: Uma Krishnan <redacted>
From: Matthew R. Ochs <hidden> Date: 2018-05-16 16:13:16
On Fri, May 11, 2018 at 02:06:19PM -0500, Uma Krishnan wrote:
Depending on the underlying transport, cxlflash has a dependency on either
the CXL or OCXL drivers, which are enabled via their Kconfig option.
Instead of having a module wide dependency on these config options, it is
better to isolate the object modules that are dependent on the CXL and OCXL
drivers and adjust the module dependencies accordingly.
This commit isolates the object files that are dependent on CXL and/or
OCXL. The cxl/ocxl fops used in the core driver are tucked under an ifdef
to avoid compilation errors.
Signed-off-by: Uma Krishnan <redacted>