From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:24:44
This patch set contains various fixes and corrections for issues that
were found during test and code review. The series is based upon the
code upstreamed in 4.3 and is intended for the rc phase. The entire
set is bisectable. Please reference the changelog below for details
on what has been altered from previous versions of this patch set.
v2 Changes:
- Incorporate comments from Ian Munsie
- Rework commit messages to be more descriptive
- Add state change serialization patch
Manoj Kumar (3):
cxlflash: Fix to avoid invalid port_sel value
cxlflash: Replace magic numbers with literals
cxlflash: Fix read capacity timeout
Matthew R. Ochs (27):
cxlflash: Fix potential oops following LUN removal
cxlflash: Fix data corruption when vLUN used over multiple cards
cxlflash: Fix to avoid sizeof(bool)
cxlflash: Fix context encode mask width
cxlflash: Fix to avoid CXL services during EEH
cxlflash: Check for removal when processing interrupt
cxlflash: Correct naming of limbo state and waitq
cxlflash: Make functions static
cxlflash: Refine host/device attributes
cxlflash: Fix to avoid spamming the kernel log
cxlflash: Fix to avoid stall while waiting on TMF
cxlflash: Fix location of setting resid
cxlflash: Fix host link up event handling
cxlflash: Fix async interrupt bypass logic
cxlflash: Remove dual port online dependency
cxlflash: Fix AFU version access/storage and add check
cxlflash: Correct usage of scsi_host_put()
cxlflash: Fix to prevent workq from accessing freed memory
cxlflash: Correct behavior in device reset handler following EEH
cxlflash: Remove unnecessary scsi_block_requests
cxlflash: Fix function prolog parameters and return codes
cxlflash: Fix MMIO and endianness errors
cxlflash: Fix to prevent EEH recovery failure
cxlflash: Correct spelling, grammar, and alignment mistakes
cxlflash: Fix to prevent stale AFU RRQ
cxlflash: Fix to avoid state change collision
MAINTAINERS: Add cxlflash driver
MAINTAINERS | 9 +
drivers/scsi/cxlflash/common.h | 29 +-
drivers/scsi/cxlflash/lunmgt.c | 9 +-
drivers/scsi/cxlflash/main.c | 1575 ++++++++++++++++++++-----------------
drivers/scsi/cxlflash/main.h | 1 +
drivers/scsi/cxlflash/sislite.h | 8 +-
drivers/scsi/cxlflash/superpipe.c | 177 +++--
drivers/scsi/cxlflash/superpipe.h | 11 +-
drivers/scsi/cxlflash/vlun.c | 39 +-
9 files changed, 1036 insertions(+), 822 deletions(-)
--
2.1.0
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:26:42
From: Manoj Kumar <redacted>
If two concurrent MANAGE_LUN ioctls are issued with the same
WWID parameter, it would result in an incorrect value of port_sel.
This is because port_sel is modified without any locks being
held. If the first caller stalls after the return from
find_and_create_lun(), the value of port_sel will be set
incorrectly to indicate a single port, though in this case
it should have been set to both ports.
To fix, use the global mutex to serialize the lookup of the
WWID and the subsequent modification of port_sel.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/lunmgt.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:27:41
From: Manoj Kumar <redacted>
Magic numbers are not meaningful and can create confusion. As a
remedy, replace them with descriptive literals.
Replace 512 with literal MAX_SECTOR_UNIT.
Replace 5 with literal CMD_RETRIES.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 6 ++++--
drivers/scsi/cxlflash/superpipe.h | 3 +++
drivers/scsi/cxlflash/vlun.c | 3 ++-
3 files changed, 9 insertions(+), 3 deletions(-)
@@ -315,7 +315,8 @@ retry:retry_cnt?"re":"",scsi_cmd[0]);result=scsi_execute(sdev,scsi_cmd,DMA_FROM_DEVICE,cmd_buf,-CMD_BUFSIZE,sense_buf,tout,5,0,NULL);+CMD_BUFSIZE,sense_buf,tout,CMD_RETRIES,+0,NULL);if(driver_byte(result)==DRIVER_SENSE){result&=~(0xFF<<24);/* DRIVER_SENSE is not an error */
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:28:12
When a LUN is removed, the sdev that is associated with the LUN
remains intact until its reference count drops to 0. In order
to prevent an sdev from being removed while a context is still
associated with it, obtain an additional reference per-context
for each LUN attached to the context.
This resolves a potential Oops in the release handler when a
dealing with a LUN that has already been removed.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
@@ -880,6 +880,9 @@ static int _cxlflash_disk_detach(struct scsi_device *sdev,sys_close(lfd);}+/* Release the sdev reference that bound this LUN to the context */+scsi_device_put(sdev);+out:if(put_ctx)put_context(ctxi);
@@ -1287,11 +1290,18 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,}}+rc=scsi_device_get(sdev);+if(unlikely(rc)){+dev_err(dev,"%s: Unable to get sdev reference!\n",__func__);+gotoout;+}+lun_access=kzalloc(sizeof(*lun_access),GFP_KERNEL);if(unlikely(!lun_access)){dev_err(dev,"%s: Unable to allocate lun_access!\n",__func__);+scsi_device_put(sdev);rc=-ENOMEM;-gotoout;+gotoerr0;}lun_access->lli=lli;
@@ -1311,21 +1321,21 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,dev_err(dev,"%s: Could not initialize context %p\n",__func__,ctx);rc=-ENODEV;-gotoerr0;+gotoerr1;}ctxid=cxl_process_element(ctx);if(unlikely((ctxid>MAX_CONTEXT)||(ctxid<0))){dev_err(dev,"%s: ctxid (%d) invalid!\n",__func__,ctxid);rc=-EPERM;-gotoerr1;+gotoerr2;}file=cxl_get_fd(ctx,&cfg->cxl_fops,&fd);if(unlikely(fd<0)){rc=-ENODEV;dev_err(dev,"%s: Could not get file descriptor\n",__func__);-gotoerr1;+gotoerr2;}/* Translate read/write O_* flags from fcntl.h to AFU permission bits */
@@ -1335,7 +1345,7 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,if(unlikely(!ctxi)){dev_err(dev,"%s: Failed to create context! (%d)\n",__func__,ctxid);-gotoerr2;+gotoerr3;}work=&ctxi->work;
@@ -1346,13 +1356,13 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,if(unlikely(rc)){dev_dbg(dev,"%s: Could not start context rc=%d\n",__func__,rc);-gotoerr3;+gotoerr4;}rc=afu_attach(cfg,ctxi);if(unlikely(rc)){dev_err(dev,"%s: Could not attach AFU rc %d\n",__func__,rc);-gotoerr4;+gotoerr5;}/*
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:28:13
From: Manoj Kumar <redacted>
The timeout value for read capacity is too small. Certain devices
may take longer to respond and thus the command may prematurely
timeout. Additionally the literal used for the timeout is stale.
Update the timeout to 30 seconds (matches the value used in sd.c)
and rework the timeout literal to a more appropriate description.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 9 ++++-----
drivers/scsi/cxlflash/superpipe.h | 2 +-
drivers/scsi/cxlflash/vlun.c | 4 ++--
3 files changed, 7 insertions(+), 8 deletions(-)
@@ -315,8 +315,7 @@ retry:retry_cnt?"re":"",scsi_cmd[0]);result=scsi_execute(sdev,scsi_cmd,DMA_FROM_DEVICE,cmd_buf,-CMD_BUFSIZE,sense_buf,tout,CMD_RETRIES,-0,NULL);+CMD_BUFSIZE,sense_buf,to,CMD_RETRIES,0,NULL);if(driver_byte(result)==DRIVER_SENSE){result&=~(0xFF<<24);/* DRIVER_SENSE is not an error */
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:28:15
If the same virtual LUN is accessed over multiple cards, only accesses
made over the first card will be valid. Accesses made over the second
card will go to the wrong LUN causing data corruption.
This is because the global LUN's mode word was being used to determine
whether the LUN table for that card needs to be programmed. The mode
word would be setup by the first card, causing the LUN table for the
second card to not be programmed.
By unconditionally initializing the LUN table (not depending on the
mode word), the problem is avoided.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/vlun.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
@@ -915,16 +915,9 @@ int cxlflash_disk_virtual_open(struct scsi_device *sdev, void *arg)pr_debug("%s: ctxid=%llu ls=0x%llx\n",__func__,ctxid,lun_size);+/* Setup the LUNs block allocator on first call */mutex_lock(&gli->mutex);if(gli->mode==MODE_NONE){-/* Setup the LUN table and block allocator on first call */-rc=init_luntable(cfg,lli);-if(rc){-dev_err(dev,"%s: call to init_luntable failed "-"rc=%d!\n",__func__,rc);-gotoerr0;-}-rc=init_vlun(lli);if(rc){dev_err(dev,"%s: call to init_vlun failed rc=%d!\n",
@@ -942,6 +935,13 @@ int cxlflash_disk_virtual_open(struct scsi_device *sdev, void *arg)}mutex_unlock(&gli->mutex);+rc=init_luntable(cfg,lli);+if(rc){+dev_err(dev,"%s: call to init_luntable failed rc=%d!\n",+__func__,rc);+gotoerr1;+}+ctxi=get_context(cfg,rctxid,lli,0);if(unlikely(!ctxi)){dev_err(dev,"%s: Bad context! (%llu)\n",__func__,ctxid);
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:28:16
Using sizeof(bool) is considered poor form for various reasons and
sparse warns us of that. Correct by changing type from bool to u8.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 2 +-
drivers/scsi/cxlflash/superpipe.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
@@ -97,7 +97,7 @@ struct ctx_info {u32rht_out;/* Number of checked out RHT entries */u32rht_perms;/* User-defined permissions for RHT entries */structllun_info**rht_lun;/* Mapping of RHT entries to LUNs */-bool*rht_needs_ws;/* User-desired write-same function per RHTE */+u8*rht_needs_ws;/* User-desired write-same function per RHTE */structcxl_ioctl_start_workwork;u64ctxid;
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:28:21
The context encode mask covers more than 32-bits, making it
a long integer. This should be noted by appending the ULL
width suffix to the mask.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/superpipe.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:28:27
Interrupt processing can run in parallel to a remove operation. This
can lead to a condition where the interrupt handler is processing with
memory that has been freed.
To avoid processing an interrupt while memory may be yanked, check for
removal while in the interrupt handler. Bail when removal is imminent.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/common.h | 2 ++
drivers/scsi/cxlflash/main.c | 21 +++++++++++++++------
2 files changed, 17 insertions(+), 6 deletions(-)
@@ -1421,8 +1427,10 @@ static irqreturn_t cxlflash_rrq_irq(int irq, void *data)/* Process however many RRQ entries that are ready */while(true){-entry=*hrrq_curr;+if(atomic_read(&cfg->remove_active))+gotoout;+entry=*hrrq_curr;if((entry&SISL_RESP_HANDLE_T_BIT)!=toggle)break;
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:28:27
During an EEH freeze event, certain CXL services should not be
called until after the hardware reset has taken place. Doing so
can result in unnecessary failures and possibly cause other ill
effects by triggering hardware accesses. This translates to a
requirement to quiesce all threads that may potentially use CXL
runtime service during this window. In particular, multiple ioctls
make use of the CXL services when acting on contexts on behalf of
the user. Thus, it is essential to 'drain' running ioctls _before_
proceeding with handling the EEH freeze event.
Create the ability to drain ioctls by wrapping the ioctl handler
call in a read semaphore and then implementing a small routine that
obtains the write semaphore, effectively creating a wait point for
all currently executing ioctls.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/common.h | 2 +
drivers/scsi/cxlflash/main.c | 18 +++++--
drivers/scsi/cxlflash/superpipe.c | 104 +++++++++++++++++++++++---------------
3 files changed, 81 insertions(+), 43 deletions(-)
@@ -2383,16 +2397,14 @@ static pci_ers_result_t cxlflash_pci_error_detected(struct pci_dev *pdev,switch(state){casepci_channel_io_frozen:cfg->state=STATE_LIMBO;--/* Turn off legacy I/O */scsi_block_requests(cfg->host);+drain_ioctls(cfg);rc=cxlflash_mark_contexts_error(cfg);if(unlikely(rc))dev_err(dev,"%s: Failed to mark user contexts!(%d)\n",__func__,rc);term_mc(cfg,UNDO_START);stop_afu(cfg);-returnPCI_ERS_RESULT_NEED_RESET;casepci_channel_io_perm_failure:cfg->state=STATE_FAILTERM;
@@ -1524,41 +1566,6 @@ err1:}/**-*check_state()-checksandrespondstothecurrentadapterstate-*@cfg:Internalstructureassociatedwiththehost.-*-*Thisroutinecanblockandshouldonlybeusedonprocesscontext.-*Notethatwhenwakingupfromwaitinginlimbo,thestateisunknown-*andmustbecheckedagainbeforeproceeding.-*-*Return:0onsuccess,-errnoonfailure-*/-staticintcheck_state(structcxlflash_cfg*cfg)-{-structdevice*dev=&cfg->dev->dev;-intrc=0;--retry:-switch(cfg->state){-caseSTATE_LIMBO:-dev_dbg(dev,"%s: Limbo, going to wait...\n",__func__);-rc=wait_event_interruptible(cfg->limbo_waitq,-cfg->state!=STATE_LIMBO);-if(unlikely(rc))-break;-gotoretry;-caseSTATE_FAILTERM:-dev_dbg(dev,"%s: Failed/Terminating!\n",__func__);-rc=-ENODEV;-break;-default:-break;-}--returnrc;-}--/***cxlflash_afu_recover()-initiatesAFUrecovery*@sdev:SCSIdeviceassociatedwithLUN.*@recover:Recoverioctldatastructure.
@@ -1647,12 +1654,17 @@ retry_recover:/* Test if in error state */reg=readq_be(&afu->ctrl_map->mbox_r);if(reg==-1){-dev_dbg(dev,"%s: MMIO read fail! Wait for recovery...\n",-__func__);-mutex_unlock(&ctxi->mutex);+dev_dbg(dev,"%s: MMIO fail, wait for recovery.\n",__func__);++/*+*Beforecheckingthestate,putbackthecontextobtainedwith+*get_context()asitisnolongerneededandsleepforashort+*periodoftime(seeprolognotes).+*/+put_context(ctxi);ctxi=NULL;ssleep(1);-rc=check_state(cfg);+rc=check_state(cfg,true);if(unlikely(rc))gotoout;gotoretry;
@@ -1946,7 +1958,7 @@ static int ioctl_common(struct scsi_device *sdev, int cmd)gotoout;}-rc=check_state(cfg);+rc=check_state(cfg,true);if(unlikely(rc)&&(cfg->state==STATE_FAILTERM)){switch(cmd){caseDK_CXLFLASH_VLUN_RESIZE:
@@ -2002,6 +2022,9 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg){sizeof(structdk_cxlflash_clone),(sioctl)cxlflash_disk_clone},};+/* Hold read semaphore so we can drain if needed */+down_read(&cfg->ioctl_rwsem);+/* Restrict command set to physical support only for internal LUN */if(afu->internal_lun)switch(cmd){
@@ -2083,6 +2106,7 @@ int cxlflash_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)/* fall through to exit */cxlflash_ioctl_exit:+up_read(&cfg->ioctl_rwsem);if(unlikely(rc&&known_ioctl))dev_err(dev,"%s: ioctl %s (%08X) on dev(%d/%d/%d/%llu) ""returned rc %d\n",__func__,
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:28:32
Limbo is not an accurate representation of this state and is
also not consistent with the terminology that other drivers
use to represent this concept. Rename the state and and its
associated waitq to 'reset'.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/common.h | 4 ++--
drivers/scsi/cxlflash/main.c | 26 +++++++++++++-------------
drivers/scsi/cxlflash/superpipe.c | 14 +++++++-------
3 files changed, 22 insertions(+), 22 deletions(-)
@@ -382,8 +382,8 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp)spin_unlock_irqrestore(&cfg->tmf_waitq.lock,lock_flags);switch(cfg->state){-caseSTATE_LIMBO:-dev_dbg_ratelimited(&cfg->dev->dev,"%s: device in limbo!\n",+caseSTATE_RESET:+dev_dbg_ratelimited(&cfg->dev->dev,"%s: device is in reset!\n",__func__);rc=SCSI_MLQUEUE_HOST_BUSY;gotoout;
@@ -479,8 +479,8 @@ static int cxlflash_eh_device_reset_handler(struct scsi_cmnd *scp)if(unlikely(rcr))rc=FAILED;break;-caseSTATE_LIMBO:-wait_event(cfg->limbo_waitq,cfg->state!=STATE_LIMBO);+caseSTATE_RESET:+wait_event(cfg->reset_waitq,cfg->state!=STATE_RESET);if(cfg->state==STATE_NORMAL)break;/* fall through */
@@ -519,7 +519,7 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)switch(cfg->state){caseSTATE_NORMAL:-cfg->state=STATE_LIMBO;+cfg->state=STATE_RESET;scsi_block_requests(cfg->host);cxlflash_mark_contexts_error(cfg);rcr=cxlflash_afu_reset(cfg);
@@ -528,11 +528,11 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)cfg->state=STATE_FAILTERM;}elsecfg->state=STATE_NORMAL;-wake_up_all(&cfg->limbo_waitq);+wake_up_all(&cfg->reset_waitq);scsi_unblock_requests(cfg->host);break;-caseSTATE_LIMBO:-wait_event(cfg->limbo_waitq,cfg->state!=STATE_LIMBO);+caseSTATE_RESET:+wait_event(cfg->reset_waitq,cfg->state!=STATE_RESET);if(cfg->state==STATE_NORMAL)break;/* fall through */
@@ -100,7 +100,7 @@ void cxlflash_stop_term_user_contexts(struct cxlflash_cfg *cfg)dev_dbg(dev,"%s: Wait for user contexts to quiesce...\n",__func__);-wake_up_all(&cfg->limbo_waitq);+wake_up_all(&cfg->reset_waitq);ssleep(1);}}
@@ -1233,12 +1233,12 @@ static int check_state(struct cxlflash_cfg *cfg, bool ioctl)retry:switch(cfg->state){-caseSTATE_LIMBO:-dev_dbg(dev,"%s: Limbo state, going to wait...\n",__func__);+caseSTATE_RESET:+dev_dbg(dev,"%s: Reset state, going to wait...\n",__func__);if(ioctl)up_read(&cfg->ioctl_rwsem);-rc=wait_event_interruptible(cfg->limbo_waitq,-cfg->state!=STATE_LIMBO);+rc=wait_event_interruptible(cfg->reset_waitq,+cfg->state!=STATE_RESET);if(ioctl)down_read(&cfg->ioctl_rwsem);if(unlikely(rc))
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:29:31
Found during code inspection, that the following functions are not
being used outside of the file where they are defined. Make them static.
int cxlflash_send_cmd(struct afu *, struct afu_cmd *);
void cxlflash_wait_resp(struct afu *, struct afu_cmd *);
int cxlflash_afu_reset(struct cxlflash_cfg *);
struct afu_cmd *cxlflash_cmd_checkout(struct afu *);
void cxlflash_cmd_checkin(struct afu_cmd *);
void init_pcr(struct cxlflash_cfg *);
int init_global(struct cxlflash_cfg *);
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/common.h | 5 -
drivers/scsi/cxlflash/main.c | 1018 ++++++++++++++++++++--------------------
2 files changed, 509 insertions(+), 514 deletions(-)
@@ -238,7 +238,7 @@ static void cmd_complete(struct afu_cmd *cmd)resid=cmd->sa.resid;cmd_is_tmf=cmd->cmd_tmf;-cxlflash_cmd_checkin(cmd);/* Don't use cmd after here */+cmd_checkin(cmd);/* Don't use cmd after here */pr_debug("%s: calling scsi_set_resid, scp=%p ""result=%X resid=%d\n",__func__,
@@ -260,6 +260,146 @@ static void cmd_complete(struct afu_cmd *cmd)}/**+*context_reset()-timeouthandlerforAFUcommands+*@cmd:AFUcommandthattimedout.+*+*SendsaresettotheAFU.+*/+staticvoidcontext_reset(structafu_cmd*cmd)+{+intnretry=0;+u64rrin=0x1;+u64room=0;+structafu*afu=cmd->parent;+ulonglock_flags;++pr_debug("%s: cmd=%p\n",__func__,cmd);++spin_lock_irqsave(&cmd->slock,lock_flags);++/* Already completed? */+if(cmd->sa.host_use_b[0]&B_DONE){+spin_unlock_irqrestore(&cmd->slock,lock_flags);+return;+}++cmd->sa.host_use_b[0]|=(B_DONE|B_ERROR|B_TIMEOUT);+spin_unlock_irqrestore(&cmd->slock,lock_flags);++/*+*Wereallywanttosendthisresetatallcosts,sospread+*outwaittimeonsuccessiveretriesforavailableroom.+*/+do{+room=readq_be(&afu->host_map->cmd_room);+atomic64_set(&afu->room,room);+if(room)+gotowrite_rrin;+udelay(nretry);+}while(nretry++<MC_ROOM_RETRY_CNT);++pr_err("%s: no cmd_room to send reset\n",__func__);+return;++write_rrin:+nretry=0;+writeq_be(rrin,&afu->host_map->ioarrin);+do{+rrin=readq_be(&afu->host_map->ioarrin);+if(rrin!=0x1)+break;+/* Double delay each time */+udelay(2^nretry);+}while(nretry++<MC_ROOM_RETRY_CNT);+}++/**+*send_cmd()-sendsanAFUcommand+*@afu:AFUassociatedwiththehost.+*@cmd:AFUcommandtosend.+*+*Return:+*0onsuccessorSCSI_MLQUEUE_HOST_BUSY+*/+staticintsend_cmd(structafu*afu,structafu_cmd*cmd)+{+structcxlflash_cfg*cfg=afu->parent;+structdevice*dev=&cfg->dev->dev;+intnretry=0;+intrc=0;+u64room;+longnewval;++/*+*ThisroutineisusedbycriticaluserssuchanAFUsyncandto+*sendataskmanagementfunction(TMF).Thuswewanttoretrya+*bitbeforereturninganerror.Toavoidtheperformancepenalty+*ofMMIO,wespreadtheupdateof'room'overmultiplecommands.+*/+retry:+newval=atomic64_dec_if_positive(&afu->room);+if(!newval){+do{+room=readq_be(&afu->host_map->cmd_room);+atomic64_set(&afu->room,room);+if(room)+gotowrite_ioarrin;+udelay(nretry);+}while(nretry++<MC_ROOM_RETRY_CNT);++dev_err(dev,"%s: no cmd_room to send 0x%X\n",+__func__,cmd->rcb.cdb[0]);++gotono_room;+}elseif(unlikely(newval<0)){+/* This should be rare. i.e. Only if two threads race and+*decrementbeforetheMMIOreadisdone.Inthiscase+*justbenefitfromtheotherthreadhavingupdated+*afu->room.+*/+if(nretry++<MC_ROOM_RETRY_CNT){+udelay(nretry);+gotoretry;+}++gotono_room;+}++write_ioarrin:+writeq_be((u64)&cmd->rcb,&afu->host_map->ioarrin);+out:+pr_devel("%s: cmd=%p len=%d ea=%p rc=%d\n",__func__,cmd,+cmd->rcb.data_len,(void*)cmd->rcb.data_ea,rc);+returnrc;++no_room:+afu->read_room=true;+schedule_work(&cfg->work_q);+rc=SCSI_MLQUEUE_HOST_BUSY;+gotoout;+}++/**+*wait_resp()-pollsforaresponseortimeouttoasentAFUcommand+*@afu:AFUassociatedwiththehost.+*@cmd:AFUcommandthatwassent.+*/+staticvoidwait_resp(structafu*afu,structafu_cmd*cmd)+{+ulongtimeout=msecs_to_jiffies(cmd->rcb.timeout*2*1000);++timeout=wait_for_completion_timeout(&cmd->cevent,timeout);+if(!timeout)+context_reset(cmd);++if(unlikely(cmd->sa.ioasc!=0))+pr_err("%s: CMD 0x%X failed, IOASC: flags 0x%X, afu_rc 0x%X, "+"scsi_rc 0x%X, fc_rc 0x%X\n",__func__,cmd->rcb.cdb[0],+cmd->sa.rc.flags,cmd->sa.rc.afu_rc,cmd->sa.rc.scsi_rc,+cmd->sa.rc.fc_rc);+}++/***send_tmf()-sendsaTaskManagementFunction(TMF)*@afu:AFUtocheckoutfrom.*@scp:SCSIcommandfromstack.
@@ -280,7 +420,7 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)ulonglock_flags;intrc=0;-cmd=cxlflash_cmd_checkout(afu);+cmd=cmd_checkout(afu);if(unlikely(!cmd)){pr_err("%s: could not get a free command\n",__func__);rc=SCSI_MLQUEUE_HOST_BUSY;
@@ -313,9 +453,9 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)memcpy(cmd->rcb.cdb,&tmfcmd,sizeof(tmfcmd));/* Send the command */-rc=cxlflash_send_cmd(afu,cmd);+rc=send_cmd(afu,cmd);if(unlikely(rc)){-cxlflash_cmd_checkin(cmd);+cmd_checkin(cmd);spin_lock_irqsave(&cfg->tmf_waitq.lock,lock_flags);cfg->tmf_active=false;spin_unlock_irqrestore(&cfg->tmf_waitq.lock,lock_flags);
@@ -398,7 +538,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp)break;}-cmd=cxlflash_cmd_checkout(afu);+cmd=cmd_checkout(afu);if(unlikely(!cmd)){pr_err("%s: could not get a free command\n",__func__);rc=SCSI_MLQUEUE_HOST_BUSY;
@@ -438,9 +578,9 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp)memcpy(cmd->rcb.cdb,scp->cmnd,sizeof(cmd->rcb.cdb));/* Send the command */-rc=cxlflash_send_cmd(afu,cmd);+rc=send_cmd(afu,cmd);if(unlikely(rc)){-cxlflash_cmd_checkin(cmd);+cmd_checkin(cmd);scsi_dma_unmap(scp);}
@@ -449,369 +589,55 @@ out:}/**-*cxlflash_eh_device_reset_handler()-resetasingleLUN-*@scp:SCSIcommandtosend.-*-*Return:-*SUCCESSasdefinedinscsi/scsi.h-*FAILEDasdefinedinscsi/scsi.h+*cxlflash_wait_for_pci_err_recovery()-waitforerrorrecoveryduringprobe+*@cxlflash:Internalstructureassociatedwiththehost.*/-staticintcxlflash_eh_device_reset_handler(structscsi_cmnd*scp)+staticvoidcxlflash_wait_for_pci_err_recovery(structcxlflash_cfg*cfg){-intrc=SUCCESS;-structScsi_Host*host=scp->device->host;-structcxlflash_cfg*cfg=(structcxlflash_cfg*)host->hostdata;-structafu*afu=cfg->afu;-intrcr=0;--pr_debug("%s: (scp=%p) %d/%d/%d/%llu "-"cdb=(%08X-%08X-%08X-%08X)\n",__func__,scp,-host->host_no,scp->device->channel,-scp->device->id,scp->device->lun,-get_unaligned_be32(&((u32*)scp->cmnd)[0]),-get_unaligned_be32(&((u32*)scp->cmnd)[1]),-get_unaligned_be32(&((u32*)scp->cmnd)[2]),-get_unaligned_be32(&((u32*)scp->cmnd)[3]));--switch(cfg->state){-caseSTATE_NORMAL:-rcr=send_tmf(afu,scp,TMF_LUN_RESET);-if(unlikely(rcr))-rc=FAILED;-break;-caseSTATE_RESET:-wait_event(cfg->reset_waitq,cfg->state!=STATE_RESET);-if(cfg->state==STATE_NORMAL)-break;-/* fall through */-default:-rc=FAILED;-break;-}+structpci_dev*pdev=cfg->dev;-pr_debug("%s: returning rc=%d\n",__func__,rc);-returnrc;+if(pci_channel_offline(pdev))+wait_event_timeout(cfg->reset_waitq,+!pci_channel_offline(pdev),+CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT);}/**-*cxlflash_eh_host_reset_handler()-resetthehostadapter-*@scp:SCSIcommandfromstackidentifyinghost.-*-*Return:-*SUCCESSasdefinedinscsi/scsi.h-*FAILEDasdefinedinscsi/scsi.h+*free_mem()-freememoryassociatedwiththeAFU+*@cxlflash:Internalstructureassociatedwiththehost.*/-staticintcxlflash_eh_host_reset_handler(structscsi_cmnd*scp)+staticvoidfree_mem(structcxlflash_cfg*cfg){-intrc=SUCCESS;-intrcr=0;-structScsi_Host*host=scp->device->host;-structcxlflash_cfg*cfg=(structcxlflash_cfg*)host->hostdata;+inti;+char*buf=NULL;+structafu*afu=cfg->afu;-pr_debug("%s: (scp=%p) %d/%d/%d/%llu "-"cdb=(%08X-%08X-%08X-%08X)\n",__func__,scp,-host->host_no,scp->device->channel,-scp->device->id,scp->device->lun,-get_unaligned_be32(&((u32*)scp->cmnd)[0]),-get_unaligned_be32(&((u32*)scp->cmnd)[1]),-get_unaligned_be32(&((u32*)scp->cmnd)[2]),-get_unaligned_be32(&((u32*)scp->cmnd)[3]));+if(cfg->afu){+for(i=0;i<CXLFLASH_NUM_CMDS;i++){+buf=afu->cmd[i].buf;+if(!((u64)buf&(PAGE_SIZE-1)))+free_page((ulong)buf);+}-switch(cfg->state){-caseSTATE_NORMAL:-cfg->state=STATE_RESET;-scsi_block_requests(cfg->host);-cxlflash_mark_contexts_error(cfg);-rcr=cxlflash_afu_reset(cfg);-if(rcr){-rc=FAILED;-cfg->state=STATE_FAILTERM;-}else-cfg->state=STATE_NORMAL;-wake_up_all(&cfg->reset_waitq);-scsi_unblock_requests(cfg->host);-break;-caseSTATE_RESET:-wait_event(cfg->reset_waitq,cfg->state!=STATE_RESET);-if(cfg->state==STATE_NORMAL)-break;-/* fall through */-default:-rc=FAILED;-break;+free_pages((ulong)afu,get_order(sizeof(structafu)));+cfg->afu=NULL;}--pr_debug("%s: returning rc=%d\n",__func__,rc);-returnrc;}/**-*cxlflash_change_queue_depth()-changethequeuedepthforthedevice-*@sdev:SCSIdevicedestinedforqueuedepthchange.-*@qdepth:Requestedqueuedepthvaluetoset.-*-*Therequestedqueuedepthiscappedtothemaximumsupportedvalue.+*stop_afu()-stopstheAFUcommandtimersandunmapstheMMIOspace+*@cxlflash:Internalstructureassociatedwiththehost.*-*Return:Theactualqueuedepthset.+*SafetocallwithAFUinapartiallyallocated/initializedstate.*/-staticintcxlflash_change_queue_depth(structscsi_device*sdev,intqdepth)+staticvoidstop_afu(structcxlflash_cfg*cfg){+inti;+structafu*afu=cfg->afu;-if(qdepth>CXLFLASH_MAX_CMDS_PER_LUN)-qdepth=CXLFLASH_MAX_CMDS_PER_LUN;--scsi_change_queue_depth(sdev,qdepth);-returnsdev->queue_depth;-}--/**-*cxlflash_show_port_status()-queriesandpresentsthecurrentportstatus-*@dev:Genericdeviceassociatedwiththehostowningtheport.-*@attr:Deviceattributerepresentingtheport.-*@buf:BufferoflengthPAGE_SIZEtoreportbackportstatusinASCII.-*-*Return:ThesizeoftheASCIIstringreturnedin@buf.-*/-staticssize_tcxlflash_show_port_status(structdevice*dev,-structdevice_attribute*attr,-char*buf)-{-structScsi_Host*shost=class_to_shost(dev);-structcxlflash_cfg*cfg=(structcxlflash_cfg*)shost->hostdata;-structafu*afu=cfg->afu;--char*disp_status;-intrc;-u32port;-u64status;-u64*fc_regs;--rc=kstrtouint((attr->attr.name+4),10,&port);-if(rc||(port>=NUM_FC_PORTS))-return0;--fc_regs=&afu->afu_map->global.fc_regs[port][0];-status=-(readq_be(&fc_regs[FC_MTIP_STATUS/8])&FC_MTIP_STATUS_MASK);--if(status==FC_MTIP_STATUS_ONLINE)-disp_status="online";-elseif(status==FC_MTIP_STATUS_OFFLINE)-disp_status="offline";-else-disp_status="unknown";--returnsnprintf(buf,PAGE_SIZE,"%s\n",disp_status);-}--/**-*cxlflash_show_lun_mode()-presentsthecurrentLUNmodeofthehost-*@dev:Genericdeviceassociatedwiththehost.-*@attr:Deviceattributerepresentingthelunmode.-*@buf:BufferoflengthPAGE_SIZEtoreportbacktheLUNmodeinASCII.-*-*Return:ThesizeoftheASCIIstringreturnedin@buf.-*/-staticssize_tcxlflash_show_lun_mode(structdevice*dev,-structdevice_attribute*attr,char*buf)-{-structScsi_Host*shost=class_to_shost(dev);-structcxlflash_cfg*cfg=(structcxlflash_cfg*)shost->hostdata;-structafu*afu=cfg->afu;--returnsnprintf(buf,PAGE_SIZE,"%u\n",afu->internal_lun);-}--/**-*cxlflash_store_lun_mode()-setstheLUNmodeofthehost-*@dev:Genericdeviceassociatedwiththehost.-*@attr:Deviceattributerepresentingthelunmode.-*@buf:BufferoflengthPAGE_SIZEcontainingtheLUNmodeinASCII.-*@count:Lengthofdataresizingin@buf.-*-*TheCXLFlashAFUsupportsadummyLUNmodewheretheexternal-*linksandstoragearenotrequired.SpaceontheFPGAisused-*tocreate1or2smallLUNswhicharepresentedtothesystem-*asiftheywereanormalstoragedevice.Thisfeatureisuseful-*duringdevelopmentandalsoprovidesmanufacturingwithaway-*totesttheAFUwithoutanactualdevice.-*-*0=externalLUN[s](default)-*1=internalLUN(1x64K,512Bblocks,id0)-*2=internalLUN(1x64K,4Kblocks,id0)-*3=internalLUN(2x32K,512Bblocks,ids0,1)-*4=internalLUN(2x32K,4Kblocks,ids0,1)-*-*Return:ThesizeoftheASCIIstringreturnedin@buf.-*/-staticssize_tcxlflash_store_lun_mode(structdevice*dev,-structdevice_attribute*attr,-constchar*buf,size_tcount)-{-structScsi_Host*shost=class_to_shost(dev);-structcxlflash_cfg*cfg=(structcxlflash_cfg*)shost->hostdata;-structafu*afu=cfg->afu;-intrc;-u32lun_mode;--rc=kstrtouint(buf,10,&lun_mode);-if(!rc&&(lun_mode<5)&&(lun_mode!=afu->internal_lun)){-afu->internal_lun=lun_mode;-cxlflash_afu_reset(cfg);-scsi_scan_host(cfg->host);-}--returncount;-}--/**-*cxlflash_show_ioctl_version()-presentsthecurrentioctlversionofthehost-*@dev:Genericdeviceassociatedwiththehost.-*@attr:Deviceattributerepresentingtheioctlversion.-*@buf:BufferoflengthPAGE_SIZEtoreportbacktheioctlversion.-*-*Return:ThesizeoftheASCIIstringreturnedin@buf.-*/-staticssize_tcxlflash_show_ioctl_version(structdevice*dev,-structdevice_attribute*attr,-char*buf)-{-returnscnprintf(buf,PAGE_SIZE,"%u\n",DK_CXLFLASH_VERSION_0);-}--/**-*cxlflash_show_dev_mode()-presentsthecurrentmodeofthedevice-*@dev:Genericdeviceassociatedwiththedevice.-*@attr:Deviceattributerepresentingthedevicemode.-*@buf:BufferoflengthPAGE_SIZEtoreportbackthedevmodeinASCII.-*-*Return:ThesizeoftheASCIIstringreturnedin@buf.-*/-staticssize_tcxlflash_show_dev_mode(structdevice*dev,-structdevice_attribute*attr,char*buf)-{-structscsi_device*sdev=to_scsi_device(dev);--returnsnprintf(buf,PAGE_SIZE,"%s\n",-sdev->hostdata?"superpipe":"legacy");-}--/**-*cxlflash_wait_for_pci_err_recovery()-waitforerrorrecoveryduringprobe-*@cxlflash:Internalstructureassociatedwiththehost.-*/-staticvoidcxlflash_wait_for_pci_err_recovery(structcxlflash_cfg*cfg)-{-structpci_dev*pdev=cfg->dev;--if(pci_channel_offline(pdev))-wait_event_timeout(cfg->reset_waitq,-!pci_channel_offline(pdev),-CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT);-}--/*-*Hostattributes-*/-staticDEVICE_ATTR(port0,S_IRUGO,cxlflash_show_port_status,NULL);-staticDEVICE_ATTR(port1,S_IRUGO,cxlflash_show_port_status,NULL);-staticDEVICE_ATTR(lun_mode,S_IRUGO|S_IWUSR,cxlflash_show_lun_mode,-cxlflash_store_lun_mode);-staticDEVICE_ATTR(ioctl_version,S_IRUGO,cxlflash_show_ioctl_version,NULL);--staticstructdevice_attribute*cxlflash_host_attrs[]={-&dev_attr_port0,-&dev_attr_port1,-&dev_attr_lun_mode,-&dev_attr_ioctl_version,-NULL-};--/*-*Deviceattributes-*/-staticDEVICE_ATTR(mode,S_IRUGO,cxlflash_show_dev_mode,NULL);--staticstructdevice_attribute*cxlflash_dev_attrs[]={-&dev_attr_mode,-NULL-};--/*-*Hosttemplate-*/-staticstructscsi_host_templatedriver_template={-.module=THIS_MODULE,-.name=CXLFLASH_ADAPTER_NAME,-.info=cxlflash_driver_info,-.ioctl=cxlflash_ioctl,-.proc_name=CXLFLASH_NAME,-.queuecommand=cxlflash_queuecommand,-.eh_device_reset_handler=cxlflash_eh_device_reset_handler,-.eh_host_reset_handler=cxlflash_eh_host_reset_handler,-.change_queue_depth=cxlflash_change_queue_depth,-.cmd_per_lun=16,-.can_queue=CXLFLASH_MAX_CMDS,-.this_id=-1,-.sg_tablesize=SG_NONE,/* No scatter gather support. */-.max_sectors=CXLFLASH_MAX_SECTORS,-.use_clustering=ENABLE_CLUSTERING,-.shost_attrs=cxlflash_host_attrs,-.sdev_attrs=cxlflash_dev_attrs,-};--/*-*Devicedependentvalues-*/-staticstructdev_dependent_valsdev_corsa_vals={CXLFLASH_MAX_SECTORS};--/*-*PCIdevicebindingtable-*/-staticstructpci_device_idcxlflash_pci_table[]={-{PCI_VENDOR_ID_IBM,PCI_DEVICE_ID_IBM_CORSA,-PCI_ANY_ID,PCI_ANY_ID,0,0,(kernel_ulong_t)&dev_corsa_vals},-{}-};--MODULE_DEVICE_TABLE(pci,cxlflash_pci_table);--/**-*free_mem()-freememoryassociatedwiththeAFU-*@cxlflash:Internalstructureassociatedwiththehost.-*/-staticvoidfree_mem(structcxlflash_cfg*cfg)-{-inti;-char*buf=NULL;-structafu*afu=cfg->afu;--if(cfg->afu){-for(i=0;i<CXLFLASH_NUM_CMDS;i++){-buf=afu->cmd[i].buf;-if(!((u64)buf&(PAGE_SIZE-1)))-free_page((ulong)buf);-}--free_pages((ulong)afu,get_order(sizeof(structafu)));-cfg->afu=NULL;-}-}--/**-*stop_afu()-stopstheAFUcommandtimersandunmapstheMMIOspace-*@cxlflash:Internalstructureassociatedwiththehost.-*-*SafetocallwithAFUinapartiallyallocated/initializedstate.-*/-staticvoidstop_afu(structcxlflash_cfg*cfg)-{-inti;-structafu*afu=cfg->afu;--if(likely(afu)){-for(i=0;i<CXLFLASH_NUM_CMDS;i++)-complete(&afu->cmd[i].cevent);+if(likely(afu)){+for(i=0;i<CXLFLASH_NUM_CMDS;i++)+complete(&afu->cmd[i].cevent);if(likely(afu->afu_map)){cxl_psa_unmap((void*)afu->afu_map);
@@ -1640,67 +1466,13 @@ out:}/**-*cxlflash_context_reset()-timeouthandlerforAFUcommands-*@cmd:AFUcommandthattimedout.+*init_pcr()-initializetheprovisioningandcontrolregisters+*@cxlflash:Internalstructureassociatedwiththehost.*-*SendsaresettotheAFU.+*AlsosetsupfastaccesstothemappedregistersandinitializesAFU+*commandfieldsthatneverchange.*/-voidcxlflash_context_reset(structafu_cmd*cmd)-{-intnretry=0;-u64rrin=0x1;-u64room=0;-structafu*afu=cmd->parent;-ulonglock_flags;--pr_debug("%s: cmd=%p\n",__func__,cmd);--spin_lock_irqsave(&cmd->slock,lock_flags);--/* Already completed? */-if(cmd->sa.host_use_b[0]&B_DONE){-spin_unlock_irqrestore(&cmd->slock,lock_flags);-return;-}--cmd->sa.host_use_b[0]|=(B_DONE|B_ERROR|B_TIMEOUT);-spin_unlock_irqrestore(&cmd->slock,lock_flags);--/*-*Wereallywanttosendthisresetatallcosts,sospread-*outwaittimeonsuccessiveretriesforavailableroom.-*/-do{-room=readq_be(&afu->host_map->cmd_room);-atomic64_set(&afu->room,room);-if(room)-gotowrite_rrin;-udelay(nretry);-}while(nretry++<MC_ROOM_RETRY_CNT);--pr_err("%s: no cmd_room to send reset\n",__func__);-return;--write_rrin:-nretry=0;-writeq_be(rrin,&afu->host_map->ioarrin);-do{-rrin=readq_be(&afu->host_map->ioarrin);-if(rrin!=0x1)-break;-/* Double delay each time */-udelay(2^nretry);-}while(nretry++<MC_ROOM_RETRY_CNT);-}--/**-*init_pcr()-initializetheprovisioningandcontrolregisters-*@cxlflash:Internalstructureassociatedwiththehost.-*-*AlsosetsupfastaccesstothemappedregistersandinitializesAFU-*commandfieldsthatneverchange.-*/-voidinit_pcr(structcxlflash_cfg*cfg)+staticvoidinit_pcr(structcxlflash_cfg*cfg){structafu*afu=cfg->afu;structsisl_ctrl_map*ctrl_map;
@@ -2007,92 +1779,6 @@ err1:}/**-*cxlflash_send_cmd()-sendsanAFUcommand-*@afu:AFUassociatedwiththehost.-*@cmd:AFUcommandtosend.-*-*Return:-*0onsuccess-*-1onfailure-*/-intcxlflash_send_cmd(structafu*afu,structafu_cmd*cmd)-{-structcxlflash_cfg*cfg=afu->parent;-intnretry=0;-intrc=0;-u64room;-longnewval;--/*-*ThisroutineisusedbycriticaluserssuchanAFUsyncandto-*sendataskmanagementfunction(TMF).Thuswewanttoretrya-*bitbeforereturninganerror.Toavoidtheperformancepenalty-*ofMMIO,wespreadtheupdateof'room'overmultiplecommands.-*/-retry:-newval=atomic64_dec_if_positive(&afu->room);-if(!newval){-do{-room=readq_be(&afu->host_map->cmd_room);-atomic64_set(&afu->room,room);-if(room)-gotowrite_ioarrin;-udelay(nretry);-}while(nretry++<MC_ROOM_RETRY_CNT);--pr_err("%s: no cmd_room to send 0x%X\n",-__func__,cmd->rcb.cdb[0]);--gotono_room;-}elseif(unlikely(newval<0)){-/* This should be rare. i.e. Only if two threads race and-*decrementbeforetheMMIOreadisdone.Inthiscase-*justbenefitfromtheotherthreadhavingupdated-*afu->room.-*/-if(nretry++<MC_ROOM_RETRY_CNT){-udelay(nretry);-gotoretry;-}--gotono_room;-}--write_ioarrin:-writeq_be((u64)&cmd->rcb,&afu->host_map->ioarrin);-out:-pr_debug("%s: cmd=%p len=%d ea=%p rc=%d\n",__func__,cmd,-cmd->rcb.data_len,(void*)cmd->rcb.data_ea,rc);-returnrc;--no_room:-afu->read_room=true;-schedule_work(&cfg->work_q);-rc=SCSI_MLQUEUE_HOST_BUSY;-gotoout;-}--/**-*cxlflash_wait_resp()-pollsforaresponseortimeouttoasentAFUcommand-*@afu:AFUassociatedwiththehost.-*@cmd:AFUcommandthatwassent.-*/-voidcxlflash_wait_resp(structafu*afu,structafu_cmd*cmd)-{-ulongtimeout=jiffies+(cmd->rcb.timeout*2*HZ);--timeout=wait_for_completion_timeout(&cmd->cevent,timeout);-if(!timeout)-cxlflash_context_reset(cmd);--if(unlikely(cmd->sa.ioasc!=0))-pr_err("%s: CMD 0x%X failed, IOASC: flags 0x%X, afu_rc 0x%X, "-"scsi_rc 0x%X, fc_rc 0x%X\n",__func__,cmd->rcb.cdb[0],-cmd->sa.rc.flags,cmd->sa.rc.afu_rc,cmd->sa.rc.scsi_rc,-cmd->sa.rc.fc_rc);-}--/***cxlflash_afu_sync()-buildsandsendsanAFUsynccommand*@afu:AFUassociatedwiththehost.*@ctx_hndl_u:Identifiescontextrequestingsync.
@@ -2130,7 +1816,7 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t ctx_hndl_u,mutex_lock(&sync_active);retry:-cmd=cxlflash_cmd_checkout(afu);+cmd=cmd_checkout(afu);if(unlikely(!cmd)){retry_cnt++;udelay(1000*retry_cnt);
@@ -2159,11 +1845,11 @@ retry:*((u16*)&cmd->rcb.cdb[2])=swab16(ctx_hndl_u);*((u32*)&cmd->rcb.cdb[4])=swab32(res_hndl_u);-rc=cxlflash_send_cmd(afu,cmd);+rc=send_cmd(afu,cmd);if(unlikely(rc))gotoout;-cxlflash_wait_resp(afu,cmd);+wait_resp(afu,cmd);/* set on timeout */if(unlikely((cmd->sa.ioasc!=0)||
@@ -2172,20 +1858,20 @@ retry:out:mutex_unlock(&sync_active);if(cmd)-cxlflash_cmd_checkin(cmd);+cmd_checkin(cmd);pr_debug("%s: returning rc=%d\n",__func__,rc);returnrc;}/**-*cxlflash_afu_reset()-resetstheAFU-*@cxlflash:Internalstructureassociatedwiththehost.+*afu_reset()-resetstheAFU+*@cfg:Internalstructureassociatedwiththehost.**Return:*0onsuccess*Afailurevaluefrominternalservices.*/-intcxlflash_afu_reset(structcxlflash_cfg*cfg)+staticintafu_reset(structcxlflash_cfg*cfg){intrc=0;/* Stop the context before the reset. Since the context is
@@ -2201,6 +1887,320 @@ int cxlflash_afu_reset(struct cxlflash_cfg *cfg)}/**+*cxlflash_eh_device_reset_handler()-resetasingleLUN+*@scp:SCSIcommandtosend.+*+*Return:+*SUCCESSasdefinedinscsi/scsi.h+*FAILEDasdefinedinscsi/scsi.h+*/+staticintcxlflash_eh_device_reset_handler(structscsi_cmnd*scp)+{+intrc=SUCCESS;+structScsi_Host*host=scp->device->host;+structcxlflash_cfg*cfg=(structcxlflash_cfg*)host->hostdata;+structafu*afu=cfg->afu;+intrcr=0;++pr_debug("%s: (scp=%p) %d/%d/%d/%llu "+"cdb=(%08X-%08X-%08X-%08X)\n",__func__,scp,+host->host_no,scp->device->channel,+scp->device->id,scp->device->lun,+get_unaligned_be32(&((u32*)scp->cmnd)[0]),+get_unaligned_be32(&((u32*)scp->cmnd)[1]),+get_unaligned_be32(&((u32*)scp->cmnd)[2]),+get_unaligned_be32(&((u32*)scp->cmnd)[3]));++switch(cfg->state){+caseSTATE_NORMAL:+rcr=send_tmf(afu,scp,TMF_LUN_RESET);+if(unlikely(rcr))+rc=FAILED;+break;+caseSTATE_RESET:+wait_event(cfg->reset_waitq,cfg->state!=STATE_RESET);+if(cfg->state==STATE_NORMAL)+break;+/* fall through */+default:+rc=FAILED;+break;+}++pr_debug("%s: returning rc=%d\n",__func__,rc);+returnrc;+}++/**+*cxlflash_eh_host_reset_handler()-resetthehostadapter+*@scp:SCSIcommandfromstackidentifyinghost.+*+*Return:+*SUCCESSasdefinedinscsi/scsi.h+*FAILEDasdefinedinscsi/scsi.h+*/+staticintcxlflash_eh_host_reset_handler(structscsi_cmnd*scp)+{+intrc=SUCCESS;+intrcr=0;+structScsi_Host*host=scp->device->host;+structcxlflash_cfg*cfg=(structcxlflash_cfg*)host->hostdata;++pr_debug("%s: (scp=%p) %d/%d/%d/%llu "+"cdb=(%08X-%08X-%08X-%08X)\n",__func__,scp,+host->host_no,scp->device->channel,+scp->device->id,scp->device->lun,+get_unaligned_be32(&((u32*)scp->cmnd)[0]),+get_unaligned_be32(&((u32*)scp->cmnd)[1]),+get_unaligned_be32(&((u32*)scp->cmnd)[2]),+get_unaligned_be32(&((u32*)scp->cmnd)[3]));++switch(cfg->state){+caseSTATE_NORMAL:+cfg->state=STATE_RESET;+scsi_block_requests(cfg->host);+cxlflash_mark_contexts_error(cfg);+rcr=afu_reset(cfg);+if(rcr){+rc=FAILED;+cfg->state=STATE_FAILTERM;+}else+cfg->state=STATE_NORMAL;+wake_up_all(&cfg->reset_waitq);+scsi_unblock_requests(cfg->host);+break;+caseSTATE_RESET:+wait_event(cfg->reset_waitq,cfg->state!=STATE_RESET);+if(cfg->state==STATE_NORMAL)+break;+/* fall through */+default:+rc=FAILED;+break;+}++pr_debug("%s: returning rc=%d\n",__func__,rc);+returnrc;+}++/**+*cxlflash_change_queue_depth()-changethequeuedepthforthedevice+*@sdev:SCSIdevicedestinedforqueuedepthchange.+*@qdepth:Requestedqueuedepthvaluetoset.+*+*Therequestedqueuedepthiscappedtothemaximumsupportedvalue.+*+*Return:Theactualqueuedepthset.+*/+staticintcxlflash_change_queue_depth(structscsi_device*sdev,intqdepth)+{++if(qdepth>CXLFLASH_MAX_CMDS_PER_LUN)+qdepth=CXLFLASH_MAX_CMDS_PER_LUN;++scsi_change_queue_depth(sdev,qdepth);+returnsdev->queue_depth;+}++/**+*cxlflash_show_port_status()-queriesandpresentsthecurrentportstatus+*@dev:Genericdeviceassociatedwiththehostowningtheport.+*@attr:Deviceattributerepresentingtheport.+*@buf:BufferoflengthPAGE_SIZEtoreportbackportstatusinASCII.+*+*Return:ThesizeoftheASCIIstringreturnedin@buf.+*/+staticssize_tcxlflash_show_port_status(structdevice*dev,+structdevice_attribute*attr,+char*buf)+{+structScsi_Host*shost=class_to_shost(dev);+structcxlflash_cfg*cfg=(structcxlflash_cfg*)shost->hostdata;+structafu*afu=cfg->afu;++char*disp_status;+intrc;+u32port;+u64status;+u64*fc_regs;++rc=kstrtouint((attr->attr.name+4),10,&port);+if(rc||(port>=NUM_FC_PORTS))+return0;++fc_regs=&afu->afu_map->global.fc_regs[port][0];+status=+(readq_be(&fc_regs[FC_MTIP_STATUS/8])&FC_MTIP_STATUS_MASK);++if(status==FC_MTIP_STATUS_ONLINE)+disp_status="online";+elseif(status==FC_MTIP_STATUS_OFFLINE)+disp_status="offline";+else+disp_status="unknown";++returnsnprintf(buf,PAGE_SIZE,"%s\n",disp_status);+}++/**+*cxlflash_show_lun_mode()-presentsthecurrentLUNmodeofthehost+*@dev:Genericdeviceassociatedwiththehost.+*@attr:Deviceattributerepresentingthelunmode.+*@buf:BufferoflengthPAGE_SIZEtoreportbacktheLUNmodeinASCII.+*+*Return:ThesizeoftheASCIIstringreturnedin@buf.+*/+staticssize_tcxlflash_show_lun_mode(structdevice*dev,+structdevice_attribute*attr,char*buf)+{+structScsi_Host*shost=class_to_shost(dev);+structcxlflash_cfg*cfg=(structcxlflash_cfg*)shost->hostdata;+structafu*afu=cfg->afu;++returnsnprintf(buf,PAGE_SIZE,"%u\n",afu->internal_lun);+}++/**+*cxlflash_store_lun_mode()-setstheLUNmodeofthehost+*@dev:Genericdeviceassociatedwiththehost.+*@attr:Deviceattributerepresentingthelunmode.+*@buf:BufferoflengthPAGE_SIZEcontainingtheLUNmodeinASCII.+*@count:Lengthofdataresizingin@buf.+*+*TheCXLFlashAFUsupportsadummyLUNmodewheretheexternal+*linksandstoragearenotrequired.SpaceontheFPGAisused+*tocreate1or2smallLUNswhicharepresentedtothesystem+*asiftheywereanormalstoragedevice.Thisfeatureisuseful+*duringdevelopmentandalsoprovidesmanufacturingwithaway+*totesttheAFUwithoutanactualdevice.+*+*0=externalLUN[s](default)+*1=internalLUN(1x64K,512Bblocks,id0)+*2=internalLUN(1x64K,4Kblocks,id0)+*3=internalLUN(2x32K,512Bblocks,ids0,1)+*4=internalLUN(2x32K,4Kblocks,ids0,1)+*+*Return:ThesizeoftheASCIIstringreturnedin@buf.+*/+staticssize_tcxlflash_store_lun_mode(structdevice*dev,+structdevice_attribute*attr,+constchar*buf,size_tcount)+{+structScsi_Host*shost=class_to_shost(dev);+structcxlflash_cfg*cfg=(structcxlflash_cfg*)shost->hostdata;+structafu*afu=cfg->afu;+intrc;+u32lun_mode;++rc=kstrtouint(buf,10,&lun_mode);+if(!rc&&(lun_mode<5)&&(lun_mode!=afu->internal_lun)){+afu->internal_lun=lun_mode;+afu_reset(cfg);+scsi_scan_host(cfg->host);+}++returncount;+}++/**+*cxlflash_show_ioctl_version()-presentsthehostscurrentioctlversion+*@dev:Genericdeviceassociatedwiththehost.+*@attr:Deviceattributerepresentingtheioctlversion.+*@buf:BufferoflengthPAGE_SIZEtoreportbacktheioctlversion.+*+*Return:ThesizeoftheASCIIstringreturnedin@buf.+*/+staticssize_tcxlflash_show_ioctl_version(structdevice*dev,+structdevice_attribute*attr,+char*buf)+{+returnscnprintf(buf,PAGE_SIZE,"%u\n",DK_CXLFLASH_VERSION_0);+}++/**+*cxlflash_show_dev_mode()-presentsthecurrentmodeofthedevice+*@dev:Genericdeviceassociatedwiththedevice.+*@attr:Deviceattributerepresentingthedevicemode.+*@buf:BufferoflengthPAGE_SIZEtoreportbackthedevmodeinASCII.+*+*Return:ThesizeoftheASCIIstringreturnedin@buf.+*/+staticssize_tcxlflash_show_dev_mode(structdevice*dev,+structdevice_attribute*attr,char*buf)+{+structscsi_device*sdev=to_scsi_device(dev);++returnsnprintf(buf,PAGE_SIZE,"%s\n",+sdev->hostdata?"superpipe":"legacy");+}++/*+*Hostattributes+*/+staticDEVICE_ATTR(port0,S_IRUGO,cxlflash_show_port_status,NULL);+staticDEVICE_ATTR(port1,S_IRUGO,cxlflash_show_port_status,NULL);+staticDEVICE_ATTR(lun_mode,S_IRUGO|S_IWUSR,cxlflash_show_lun_mode,+cxlflash_store_lun_mode);+staticDEVICE_ATTR(ioctl_version,S_IRUGO,cxlflash_show_ioctl_version,NULL);++staticstructdevice_attribute*cxlflash_host_attrs[]={+&dev_attr_port0,+&dev_attr_port1,+&dev_attr_lun_mode,+&dev_attr_ioctl_version,+NULL+};++/*+*Deviceattributes+*/+staticDEVICE_ATTR(mode,S_IRUGO,cxlflash_show_dev_mode,NULL);++staticstructdevice_attribute*cxlflash_dev_attrs[]={+&dev_attr_mode,+NULL+};++/*+*Hosttemplate+*/+staticstructscsi_host_templatedriver_template={+.module=THIS_MODULE,+.name=CXLFLASH_ADAPTER_NAME,+.info=cxlflash_driver_info,+.ioctl=cxlflash_ioctl,+.proc_name=CXLFLASH_NAME,+.queuecommand=cxlflash_queuecommand,+.eh_device_reset_handler=cxlflash_eh_device_reset_handler,+.eh_host_reset_handler=cxlflash_eh_host_reset_handler,+.change_queue_depth=cxlflash_change_queue_depth,+.cmd_per_lun=16,+.can_queue=CXLFLASH_MAX_CMDS,+.this_id=-1,+.sg_tablesize=SG_NONE,/* No scatter gather support. */+.max_sectors=CXLFLASH_MAX_SECTORS,+.use_clustering=ENABLE_CLUSTERING,+.shost_attrs=cxlflash_host_attrs,+.sdev_attrs=cxlflash_dev_attrs,+};++/*+*Devicedependentvalues+*/+staticstructdev_dependent_valsdev_corsa_vals={CXLFLASH_MAX_SECTORS};++/*+*PCIdevicebindingtable+*/+staticstructpci_device_idcxlflash_pci_table[]={+{PCI_VENDOR_ID_IBM,PCI_DEVICE_ID_IBM_CORSA,+PCI_ANY_ID,PCI_ANY_ID,0,0,(kernel_ulong_t)&dev_corsa_vals},+{}+};++MODULE_DEVICE_TABLE(pci,cxlflash_pci_table);++/***cxlflash_worker_thread()-workthreadhandlerfortheAFU*@work:Workstructurecontainedwithincxlflashassociatedwithhost.*
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:30:40
Implement the following suggestions and add two new attributes
to allow for debugging the port LUN table.
- use scnprintf() instead of snprintf()
- use DEVICE_ATTR_RO and DEVICE_ATTR_RW
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Shane Seymour <redacted>
---
drivers/scsi/cxlflash/main.c | 180 +++++++++++++++++++++++++++++++++----------
1 file changed, 138 insertions(+), 42 deletions(-)
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:30:46
During run-time the driver can be very chatty and spam the system
kernel log. Various print statements can be limited and/or moved
to development-only mode. Additionally, numerous prints can be
converted to trace the corresponding device.
The following changes were made:
- pr_debug to pr_devel
- pr_debug to pr_debug_ratelimited
- pr_err to dev_err
- pr_debug to dev_dbg
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 105 ++++++++++++++++++++++++-------------------
1 file changed, 58 insertions(+), 47 deletions(-)
@@ -240,9 +240,9 @@ static void cmd_complete(struct afu_cmd *cmd)cmd_is_tmf=cmd->cmd_tmf;cmd_checkin(cmd);/* Don't use cmd after here */-pr_debug("%s: calling scsi_set_resid, scp=%p "-"result=%X resid=%d\n",__func__,-scp,scp->result,resid);+pr_debug_ratelimited("%s: calling scsi_done scp=%p result=%X "+"ioasc=%d\n",__func__,scp,scp->result,+cmd->sa.ioasc);scsi_set_resid(scp,resid);scsi_dma_unmap(scp);
@@ -417,12 +417,13 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)shortlflag=0;structScsi_Host*host=scp->device->host;structcxlflash_cfg*cfg=(structcxlflash_cfg*)host->hostdata;+structdevice*dev=&cfg->dev->dev;ulonglock_flags;intrc=0;cmd=cmd_checkout(afu);if(unlikely(!cmd)){-pr_err("%s: could not get a free command\n",__func__);+dev_err(dev,"%s: could not get a free command\n",__func__);rc=SCSI_MLQUEUE_HOST_BUSY;gotoout;}
@@ -502,13 +503,14 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp)shortlflag=0;intrc=0;-pr_debug("%s: (scp=%p) %d/%d/%d/%llu cdb=(%08X-%08X-%08X-%08X)\n",-__func__,scp,host->host_no,scp->device->channel,-scp->device->id,scp->device->lun,-get_unaligned_be32(&((u32*)scp->cmnd)[0]),-get_unaligned_be32(&((u32*)scp->cmnd)[1]),-get_unaligned_be32(&((u32*)scp->cmnd)[2]),-get_unaligned_be32(&((u32*)scp->cmnd)[3]));+dev_dbg_ratelimited(dev,"%s: (scp=%p) %d/%d/%d/%llu "+"cdb=(%08X-%08X-%08X-%08X)\n",+__func__,scp,host->host_no,scp->device->channel,+scp->device->id,scp->device->lun,+get_unaligned_be32(&((u32*)scp->cmnd)[0]),+get_unaligned_be32(&((u32*)scp->cmnd)[1]),+get_unaligned_be32(&((u32*)scp->cmnd)[2]),+get_unaligned_be32(&((u32*)scp->cmnd)[3]));/* If a Task Management Function is active, wait for it to complete*beforecontinuingwithregularcommands.
@@ -523,13 +525,11 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp)switch(cfg->state){caseSTATE_RESET:-dev_dbg_ratelimited(&cfg->dev->dev,"%s: device is in reset!\n",-__func__);+dev_dbg_ratelimited(dev,"%s: device is in reset!\n",__func__);rc=SCSI_MLQUEUE_HOST_BUSY;gotoout;caseSTATE_FAILTERM:-dev_dbg_ratelimited(&cfg->dev->dev,"%s: device has failed!\n",-__func__);+dev_dbg_ratelimited(dev,"%s: device has failed!\n",__func__);scp->result=(DID_NO_CONNECT<<16);scp->scsi_done(scp);rc=0;
@@ -540,7 +540,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp)cmd=cmd_checkout(afu);if(unlikely(!cmd)){-pr_err("%s: could not get a free command\n",__func__);+dev_err(dev,"%s: could not get a free command\n",__func__);rc=SCSI_MLQUEUE_HOST_BUSY;gotoout;}
@@ -657,9 +658,10 @@ static void term_mc(struct cxlflash_cfg *cfg, enum undo_level level){intrc=0;structafu*afu=cfg->afu;+structdevice*dev=&cfg->dev->dev;if(!afu||!cfg->mcctx){-pr_err("%s: returning from term_mc with NULL afu or MC\n",+dev_err(dev,"%s: returning from term_mc with NULL afu or MC\n",__func__);return;}
@@ -756,6 +758,7 @@ static int alloc_mem(struct cxlflash_cfg *cfg)intrc=0;inti;char*buf=NULL;+structdevice*dev=&cfg->dev->dev;/* This allocation is about 12K, i.e. only 1 64k page*andupto44kpages
@@ -763,8 +766,8 @@ static int alloc_mem(struct cxlflash_cfg *cfg)cfg->afu=(void*)__get_free_pages(GFP_KERNEL|__GFP_ZERO,get_order(sizeof(structafu)));if(unlikely(!cfg->afu)){-pr_err("%s: cannot get %d free pages\n",-__func__,get_order(sizeof(structafu)));+dev_err(dev,"%s: cannot get %d free pages\n",+__func__,get_order(sizeof(structafu)));rc=-ENOMEM;gotoout;}
@@ -1319,8 +1324,8 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void *data)port=info->port;-pr_err("%s: FC Port %d -> %s, fc_status 0x%08llX\n",-__func__,port,info->desc,+dev_err(dev,"%s: FC Port %d -> %s, fc_status 0x%08llX\n",+__func__,port,info->desc,readq_be(&global->fc_regs[port][FC_STATUS/8]));/*
@@ -1328,8 +1333,8 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void *data)*againifclearedbeforeorw/oareset*/if(info->action&LINK_RESET){-pr_err("%s: FC Port %d: resetting link\n",-__func__,port);+dev_err(dev,"%s: FC Port %d: resetting link\n",+__func__,port);cfg->lr_state=LINK_RESET_REQUIRED;cfg->lr_port=port;schedule_work(&cfg->work_q);
@@ -1343,8 +1348,8 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void *data)*shouldbethesameandtracingoneissufficient.*/-pr_err("%s: fc %d: clearing fc_error 0x%08llX\n",-__func__,port,reg);+dev_err(dev,"%s: fc %d: clearing fc_error 0x%08llX\n",+__func__,port,reg);writeq_be(reg,&global->fc_regs[port][FC_ERROR/8]);writeq_be(0,&global->fc_regs[port][FC_ERRCAP/8]);
@@ -1396,7 +1401,7 @@ static int read_vpd(struct cxlflash_cfg *cfg, u64 wwpn[])/* Get the VPD data from the device */vpd_size=pci_read_vpd(dev,0,sizeof(vpd_data),vpd_data);if(unlikely(vpd_size<=0)){-pr_err("%s: Unable to read VPD (size = %ld)\n",+dev_err(&dev->dev,"%s: Unable to read VPD (size = %ld)\n",__func__,vpd_size);rc=-ENODEV;gotoout;
@@ -1406,7 +1411,8 @@ static int read_vpd(struct cxlflash_cfg *cfg, u64 wwpn[])ro_start=pci_vpd_find_tag(vpd_data,0,vpd_size,PCI_VPD_LRDT_RO_DATA);if(unlikely(ro_start<0)){-pr_err("%s: VPD Read-only data not found\n",__func__);+dev_err(&dev->dev,"%s: VPD Read-only data not found\n",+__func__);rc=-ENODEV;gotoout;}
@@ -1435,8 +1441,8 @@ static int read_vpd(struct cxlflash_cfg *cfg, u64 wwpn[])i=pci_vpd_find_info_keyword(vpd_data,i,j,wwpn_vpd_tags[k]);if(unlikely(i<0)){-pr_err("%s: Port %d WWPN not found in VPD\n",-__func__,k);+dev_err(&dev->dev,"%s: Port %d WWPN not found "+"in VPD\n",__func__,k);rc=-ENODEV;gotoout;}
@@ -1444,7 +1450,8 @@ static int read_vpd(struct cxlflash_cfg *cfg, u64 wwpn[])j=pci_vpd_info_field_size(&vpd_data[i]);i+=PCI_VPD_INFO_FLD_HDR_SIZE;if(unlikely((i+j>vpd_size)||(j!=WWPN_LEN))){-pr_err("%s: Port %d WWPN incomplete or VPD corrupt\n",+dev_err(&dev->dev,"%s: Port %d WWPN incomplete or "+"VPD corrupt\n",__func__,k);rc=-ENODEV;gotoout;
@@ -1453,8 +1460,8 @@ static int read_vpd(struct cxlflash_cfg *cfg, u64 wwpn[])memcpy(tmp_buf,&vpd_data[i],WWPN_LEN);rc=kstrtoul(tmp_buf,WWPN_LEN,(ulong*)&wwpn[k]);if(unlikely(rc)){-pr_err("%s: Fail to convert port %d WWPN to integer\n",-__func__,k);+dev_err(&dev->dev,"%s: Fail to convert port %d WWPN "+"to integer\n",__func__,k);rc=-ENODEV;gotoout;}
@@ -1518,7 +1526,7 @@ static int init_global(struct cxlflash_cfg *cfg)rc=read_vpd(cfg,&wwpn[0]);if(rc){-pr_err("%s: could not read vpd rc=%d\n",__func__,rc);+dev_err(dev,"%s: could not read vpd rc=%d\n",__func__,rc);gotoout;}
@@ -1561,7 +1569,7 @@ static int init_global(struct cxlflash_cfg *cfg)afu_set_wwpn(afu,i,&afu->afu_map->global.fc_regs[i][0],wwpn[i])){-pr_err("%s: failed to set WWPN on port %d\n",+dev_err(dev,"%s: failed to set WWPN on port %d\n",__func__,i);rc=-EIO;gotoout;
@@ -1804,6 +1812,7 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t ctx_hndl_u,res_hndl_tres_hndl_u,u8mode){structcxlflash_cfg*cfg=afu->parent;+structdevice*dev=&cfg->dev->dev;structafu_cmd*cmd=NULL;intrc=0;intretry_cnt=0;
@@ -1822,7 +1831,7 @@ retry:udelay(1000*retry_cnt);if(retry_cnt<MC_RETRY_CNT)gotoretry;-pr_err("%s: could not get a free command\n",__func__);+dev_err(dev,"%s: could not get a free command\n",__func__);rc=-1;gotoout;}
@@ -2323,7 +2333,8 @@ static void cxlflash_worker_thread(struct work_struct *work)if(cfg->lr_state==LINK_RESET_REQUIRED){port=cfg->lr_port;if(port<0)-pr_err("%s: invalid port index %d\n",__func__,port);+dev_err(dev,"%s: invalid port index %d\n",+__func__,port);else{spin_unlock_irqrestore(cfg->host->host_lock,lock_flags);
@@ -2428,7 +2439,7 @@ static int cxlflash_probe(struct pci_dev *pdev,*/phys_dev=cxl_get_phys_dev(pdev);if(!dev_is_pci(phys_dev)){-pr_err("%s: not a pci dev\n",__func__);+dev_err(&pdev->dev,"%s: not a pci dev\n",__func__);rc=-ENODEV;gotoout_remove;}
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:30:53
Borrowing the TMF waitq's spinlock causes a stall condition when
waiting for the TMF to complete. To remedy, introduce our own spin
lock to serialize TMF and use the appropriate wait services.
Also add a timeout while waiting for a TMF completion. When a TMF
times out, report back a failure such that a bigger hammer reset
can occur.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/common.h | 1 +
drivers/scsi/cxlflash/main.c | 55 +++++++++++++++++++++++++-----------------
2 files changed, 34 insertions(+), 22 deletions(-)
@@ -428,15 +428,15 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)gotoout;}-/* If a Task Management Function is active, do not send one more.-*/-spin_lock_irqsave(&cfg->tmf_waitq.lock,lock_flags);+/* When Task Management Function is active do not send another */+spin_lock_irqsave(&cfg->tmf_slock,lock_flags);if(cfg->tmf_active)-wait_event_interruptible_locked_irq(cfg->tmf_waitq,-!cfg->tmf_active);+wait_event_interruptible_lock_irq(cfg->tmf_waitq,+!cfg->tmf_active,+cfg->tmf_slock);cfg->tmf_active=true;cmd->cmd_tmf=true;-spin_unlock_irqrestore(&cfg->tmf_waitq.lock,lock_flags);+spin_unlock_irqrestore(&cfg->tmf_slock,lock_flags);cmd->rcb.ctx_id=afu->ctx_hndl;cmd->rcb.port_sel=port_sel;
@@ -512,16 +521,17 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp)get_unaligned_be32(&((u32*)scp->cmnd)[2]),get_unaligned_be32(&((u32*)scp->cmnd)[3]));-/* If a Task Management Function is active, wait for it to complete+/*+*IfaTaskManagementFunctionisactive,waitforittocomplete*beforecontinuingwithregularcommands.*/-spin_lock_irqsave(&cfg->tmf_waitq.lock,lock_flags);+spin_lock_irqsave(&cfg->tmf_slock,lock_flags);if(cfg->tmf_active){-spin_unlock_irqrestore(&cfg->tmf_waitq.lock,lock_flags);+spin_unlock_irqrestore(&cfg->tmf_slock,lock_flags);rc=SCSI_MLQUEUE_HOST_BUSY;gotoout;}-spin_unlock_irqrestore(&cfg->tmf_waitq.lock,lock_flags);+spin_unlock_irqrestore(&cfg->tmf_slock,lock_flags);switch(cfg->state){caseSTATE_RESET:
@@ -713,11 +723,12 @@ static void cxlflash_remove(struct pci_dev *pdev)/* If a Task Management Function is active, wait for it to complete*beforecontinuingwithremove.*/-spin_lock_irqsave(&cfg->tmf_waitq.lock,lock_flags);+spin_lock_irqsave(&cfg->tmf_slock,lock_flags);if(cfg->tmf_active)-wait_event_interruptible_locked_irq(cfg->tmf_waitq,-!cfg->tmf_active);-spin_unlock_irqrestore(&cfg->tmf_waitq.lock,lock_flags);+wait_event_interruptible_lock_irq(cfg->tmf_waitq,+!cfg->tmf_active,+cfg->tmf_slock);+spin_unlock_irqrestore(&cfg->tmf_slock,lock_flags);cfg->state=STATE_FAILTERM;atomic_inc(&cfg->remove_active);
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:30:54
The resid is incorrectly set which can lead to unnecessary retry
attempts by the stack. This is due to resid _always_ being set
using a value returned from the adapter. Instead, the value
should only be interpreted and set when in an underrun scenario.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
@@ -158,8 +160,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct scsi_cmnd *scp)/* If the SISL_RC_FLAGS_OVERRUN flag was set,*thenwewillhandlethiserrorelsewhere.*Ifnotthenwemusthandleithere.-*ThisisprobablyanAFUbug.Wewill-*attemptaretrytoseeifthatresolvesit.+*ThisisprobablyanAFUbug.*/scp->result=(DID_ERROR<<16);}
@@ -183,7 +184,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct scsi_cmnd *scp)/* We have an AFU error */switch(ioasa->rc.afu_rc){caseSISL_AFU_RC_NO_CHANNELS:-scp->result=(DID_MEDIUM_ERROR<<16);+scp->result=(DID_NO_CONNECT<<16);break;caseSISL_AFU_RC_DATA_DMA_ERR:switch(ioasa->afu_extra){
@@ -229,14 +229,11 @@ static void cmd_complete(struct afu_cmd *cmd)if(cmd->rcb.scp){scp=cmd->rcb.scp;-if(unlikely(cmd->sa.rc.afu_rc||-cmd->sa.rc.scsi_rc||-cmd->sa.rc.fc_rc))+if(unlikely(cmd->sa.ioasc))process_cmd_err(cmd,scp);elsescp->result=(DID_OK<<16);-resid=cmd->sa.resid;cmd_is_tmf=cmd->cmd_tmf;cmd_checkin(cmd);/* Don't use cmd after here */
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:30:58
Following a link up event, the LUNs available to the host may
have changed. Without rescanning the host, the LUN topology is
unknown to the user. In such a state, the user would be unable
to locate provisioned resources.
To remedy, the host should be rescanned after a link up event.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/common.h | 2 +-
drivers/scsi/cxlflash/main.c | 17 +++++++++++++----
drivers/scsi/cxlflash/main.h | 1 +
3 files changed, 15 insertions(+), 5 deletions(-)
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:31:04
A bug was introduced earlier in the development cycle when cleaning
up logic statements. Instead of skipping bits that are not set, set
bits are skipped, causing async interrupts to not be handled correctly.
To fix, simply add back in the proper evaluation for an unset bit.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -1326,7 +1326,7 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void *data)/* check each bit that is on */for(i=0;reg_unmasked;i++,reg_unmasked=(reg_unmasked>>1)){info=find_ainfo(1ULL<<i);-if((reg_unmasked&0x1)||!info)+if(((reg_unmasked&0x1)==0)||!info)continue;port=info->port;
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:31:09
At present, both ports must be online for the device to
configure properly. Remove this dependency and the unnecessary
internal LUN override logic as well. Additionally, as a refactoring
measure, change the return code variable name to match that used
throughout the driver.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
@@ -1039,33 +1039,26 @@ static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn)FC_PORT_STATUS_RETRY_CNT)){pr_debug("%s: wait on port %d to go offline timed out\n",__func__,port);-ret=-1;/* but continue on to leave the port back online */+rc=-1;/* but continue on to leave the port back online */}-if(ret==0)+if(rc==0)writeq_be(wwpn,&fc_regs[FC_PNAME/8]);+/* Always return success after programming WWPN */+rc=0;+set_port_online(fc_regs);if(!wait_port_online(fc_regs,FC_PORT_STATUS_RETRY_INTERVAL_US,FC_PORT_STATUS_RETRY_CNT)){pr_debug("%s: wait on port %d to go online timed out\n",__func__,port);-ret=-1;--/*-*Overrideforinternallun!!!-*/-if(afu->internal_lun){-pr_debug("%s: Overriding port %d online timeout!!!\n",-__func__,port);-ret=0;-}}-pr_debug("%s: returning rc=%d\n",__func__,ret);+pr_debug("%s: returning rc=%d\n",__func__,rc);-returnret;+returnrc;}/**
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:31:14
The AFU version is stored as a non-terminated string of bytes within
a 64-bit little-endian register. Presently the value is read directly
(no MMIO accessor) and is stored in a buffer that is not big enough
to contain a NULL terminator. Additionally the version obtained is not
evaluated against a known value to prevent usage with unsupported AFUs.
All of these deficiencies can lead to a variety of problems.
To remedy, use the correct MMIO accessor to read the version value into
a null-terminated buffer and add a check to prevent an incompatible AFU
from being used with this driver.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/common.h | 2 +-
drivers/scsi/cxlflash/main.c | 18 ++++++++++++------
drivers/scsi/cxlflash/sislite.h | 2 +-
3 files changed, 14 insertions(+), 8 deletions(-)
@@ -180,7 +180,7 @@ struct afu {u32cmd_couts;/* Number of command checkouts */u32internal_lun;/* User-desired LUN mode for this AFU */-charversion[8];+charversion[16];u64interface_version;structcxlflash_cfg*parent;/* Pointer back to parent cxlflash_cfg */
@@ -1762,14 +1762,20 @@ static int init_afu(struct cxlflash_cfg *cfg)gotoerr1;}-/* don't byte reverse on reading afu_version, else the string form */-/* will be backwards */-reg=afu->afu_map->global.regs.afu_version;-memcpy(afu->version,®,8);+/* No byte reverse on reading afu_version or string will be backwards */+reg=readq(&afu->afu_map->global.regs.afu_version);+memcpy(afu->version,®,sizeof(reg));afu->interface_version=readq_be(&afu->afu_map->global.regs.interface_version);-pr_debug("%s: afu version %s, interface version 0x%llX\n",-__func__,afu->version,afu->interface_version);+if((afu->interface_version+1)==0){+pr_err("Back level AFU, please upgrade. AFU version %s "+"interface version 0x%llx\n",afu->version,+afu->interface_version);+rc=-EINVAL;+gotoerr1;+}else+pr_debug("%s: afu version %s, interface version 0x%llX\n",+__func__,afu->version,afu->interface_version);rc=start_afu(cfg);if(rc){
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:32:14
Currently, scsi_host_put() is being called prematurely in the
remove path and is missing entirely in an error cleanup path.
The former can lead to memory being freed too early with
subsequent access potentially corrupting data whilst the former
would result in a memory leak.
Move the usage on remove to be the last cleanup action taken
and introduce a call to scsi_host_put() in the one initialization
error path that does not use remove to cleanup.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:32:15
The workq can process work in parallel with a remove event, leading
to a condition where the workq handler can access freed memory.
To remedy, the workq should be terminated prior to freeing memory. Move
the termination call earlier in remove and use cancel_work_sync() instead
of flush_work() as there is not a need to process any scheduled work when
shutting down.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:32:18
When the device reset handler is entered while a reset operation
is taking place, the handler exits without actually sending a
reset (TMF) to the targeted device. This behavior is incorrect
as the device is not reset. Further complicating matters is the
fact that a success is returned even when the TMF was not sent.
To fix, the state is rechecked after coming out of the reset
state. When the state is normal, a TMF will be sent out.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
@@ -1931,6 +1931,7 @@ static int cxlflash_eh_device_reset_handler(struct scsi_cmnd *scp)get_unaligned_be32(&((u32*)scp->cmnd)[2]),get_unaligned_be32(&((u32*)scp->cmnd)[3]));+retry:switch(cfg->state){caseSTATE_NORMAL:rcr=send_tmf(afu,scp,TMF_LUN_RESET);
@@ -1939,9 +1940,7 @@ static int cxlflash_eh_device_reset_handler(struct scsi_cmnd *scp)break;caseSTATE_RESET:wait_event(cfg->reset_waitq,cfg->state!=STATE_RESET);-if(cfg->state==STATE_NORMAL)-break;-/* fall through */+gotoretry;default:rc=FAILED;break;
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:32:22
The host reset handler is called with I/O already blocked, thus
there is no need to explicitly block and unblock I/O in the handler.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/main.c | 2 --
1 file changed, 2 deletions(-)
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:32:24
Several function prologs have incorrect parameter names and return
code descriptions. This can lead to confusion when reviewing the
source and creates inaccurate documentation.
To remedy, update the function prologs to properly reflect parameter
names and return codes.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 68 ++++++++++++++++----------------------------
1 file changed, 25 insertions(+), 43 deletions(-)
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:32:27
Sparse uncovered several errors with MMIO operations (accessing
directly) and handling endianness. These can cause issues when
running in different environments.
Introduce __iomem and proper endianness tags/swaps where
appropriate to make driver sparse clean.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/common.h | 10 +++++-----
drivers/scsi/cxlflash/main.c | 25 +++++++++++++------------
drivers/scsi/cxlflash/superpipe.c | 6 +++---
drivers/scsi/cxlflash/superpipe.h | 2 +-
drivers/scsi/cxlflash/vlun.c | 4 ++--
5 files changed, 24 insertions(+), 23 deletions(-)
@@ -1766,7 +1767,7 @@ static int init_afu(struct cxlflash_cfg *cfg)dev_err(dev,"%s: call to start_afu failed, rc=%d!\n",__func__,rc);term_mc(cfg,UNDO_START);-cxl_psa_unmap((void*)afu->afu_map);+cxl_psa_unmap((void__iomem*)afu->afu_map);afu->afu_map=NULL;gotoerr1;}
@@ -1846,8 +1847,8 @@ retry:cmd->rcb.cdb[1]=mode;/* The cdb is aligned, no unaligned accessors required */-*((u16*)&cmd->rcb.cdb[2])=swab16(ctx_hndl_u);-*((u32*)&cmd->rcb.cdb[4])=swab32(res_hndl_u);+*((__be16*)&cmd->rcb.cdb[2])=cpu_to_be16(ctx_hndl_u);+*((__be32*)&cmd->rcb.cdb[4])=cpu_to_be32(res_hndl_u);rc=send_cmd(afu,cmd);if(unlikely(rc))
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:32:32
The process_sense() routine can perform a read capacity which
can take some time to complete. If an EEH occurs while waiting
on the read capacity, the EEH handler is unable to obtain the
context's mutex in order to put the context in an error state.
The EEH handler will sit and wait until the context is free,
but this wait can last longer than the EEH handler tolerates,
leading to a failed recovery.
To address this issue, make the context unavailable to new,
non-system owned threads and release the context while calling
into process_sense(). After returning from process_sense() the
context mutex is reacquired and the context is made available
again. The context can be safely moved to the error state if
needed during the unavailable window as no other threads will
hold its reference.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 9 +++++++++
1 file changed, 9 insertions(+)
@@ -1790,12 +1790,21 @@ static int cxlflash_disk_verify(struct scsi_device *sdev,*inquiry(i.e.theUnitattentionisduetotheWWNchanging).*/if(verify->hint&DK_CXLFLASH_VERIFY_HINT_SENSE){+/* Can't hold mutex across process_sense/read_cap16,+*sincewecouldhaveaninterveningEEHevent.+*/+ctxi->unavail=true;+mutex_unlock(&ctxi->mutex);rc=process_sense(sdev,verify);if(unlikely(rc)){dev_err(dev,"%s: Failed to validate sense data (%d)\n",__func__,rc);+mutex_lock(&ctxi->mutex);+ctxi->unavail=false;gotoout;}+mutex_lock(&ctxi->mutex);+ctxi->unavail=false;}switch(gli->mode){
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:32:34
There are several spelling and grammar mistakes throughout the
driver. Additionally there are a handful of places where there
are extra lines and unnecessary variables/statements. These are
a nuisance and pollute the driver.
Fix spelling and grammar issues. Update some comments for clarity and
consistency. Remove extra lines and a few unneeded variables/statements.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/common.h | 2 --
drivers/scsi/cxlflash/main.c | 62 +++++++++++++++++----------------------
drivers/scsi/cxlflash/sislite.h | 6 ++--
drivers/scsi/cxlflash/superpipe.c | 2 +-
drivers/scsi/cxlflash/vlun.c | 14 ++++-----
5 files changed, 38 insertions(+), 48 deletions(-)
@@ -34,7 +34,6 @@ MODULE_AUTHOR("Manoj N. Kumar <manoj@linux.vnet.ibm.com>");MODULE_AUTHOR("Matthew R. Ochs <mrochs@linux.vnet.ibm.com>");MODULE_LICENSE("GPL");-/***cmd_checkout()-checksoutanAFUcommand*@afu:AFUtocheckoutfrom.
@@ -731,7 +730,7 @@ static void cxlflash_remove(struct pci_dev *pdev)caseINIT_STATE_SCSI:cxlflash_term_local_luns(cfg);scsi_remove_host(cfg->host);-/* Fall through */+/* fall through */caseINIT_STATE_AFU:cancel_work_sync(&cfg->work_q);term_afu(cfg);
@@ -764,9 +763,7 @@ static int alloc_mem(struct cxlflash_cfg *cfg)char*buf=NULL;structdevice*dev=&cfg->dev->dev;-/* This allocation is about 12K, i.e. only 1 64k page-*andupto44kpages-*/+/* AFU is ~12k, i.e. only one 64k page or up to four 4k pages */cfg->afu=(void*)__get_free_pages(GFP_KERNEL|__GFP_ZERO,get_order(sizeof(structafu)));if(unlikely(!cfg->afu)){
@@ -1306,10 +1303,10 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void *data)gotoout;}-/* it is OK to clear AFU status before FC_ERROR */+/* FYI, it is 'okay' to clear AFU status before FC_ERROR */writeq_be(reg_unmasked,&global->regs.aintr_clear);-/* check each bit that is on */+/* Check each bit that is on */for(i=0;reg_unmasked;i++,reg_unmasked=(reg_unmasked>>1)){info=find_ainfo(1ULL<<i);if(((reg_unmasked&0x1)==0)||!info)
@@ -1483,23 +1480,22 @@ static void init_pcr(struct cxlflash_cfg *cfg)for(i=0;i<MAX_CONTEXT;i++){ctrl_map=&afu->afu_map->ctrls[i].ctrl;-/* disrupt any clients that could be running */-/* e. g. clients that survived a master restart */+/* Disrupt any clients that could be running */+/* e.g. clients that survived a master restart */writeq_be(0,&ctrl_map->rht_start);writeq_be(0,&ctrl_map->rht_cnt_id);writeq_be(0,&ctrl_map->ctx_cap);}-/* copy frequently used fields into afu */+/* Copy frequently used fields into afu */afu->ctx_hndl=(u16)cxl_process_element(cfg->mcctx);-/* ctx_hndl is 16 bits in CAIA */afu->host_map=&afu->afu_map->hosts[afu->ctx_hndl].host;afu->ctrl_map=&afu->afu_map->ctrls[afu->ctx_hndl].ctrl;/* Program the Endian Control for the master context */writeq_be(SISL_ENDIAN_CTRL,&afu->host_map->endian_ctrl);-/* initialize cmd fields that never change */+/* Initialize cmd fields that never change */for(i=0;i<CXLFLASH_NUM_CMDS;i++){afu->cmd[i].rcb.ctx_id=afu->ctx_hndl;afu->cmd[i].rcb.msi=SISL_MSI_RRQ_UPDATED;
@@ -1528,7 +1524,7 @@ static int init_global(struct cxlflash_cfg *cfg)pr_debug("%s: wwpn0=0x%llX wwpn1=0x%llX\n",__func__,wwpn[0],wwpn[1]);-/* set up RRQ in AFU for master issued cmds */+/* Set up RRQ in AFU for master issued cmds */writeq_be((u64)afu->hrrq_start,&afu->host_map->rrq_start);writeq_be((u64)afu->hrrq_end,&afu->host_map->rrq_end);
@@ -1541,9 +1537,9 @@ static int init_global(struct cxlflash_cfg *cfg)/* checker on if dual afu */writeq_be(reg,&afu->afu_map->global.regs.afu_config);-/* global port select: select either port */+/* Global port select: select either port */if(afu->internal_lun){-/* only use port 0 */+/* Only use port 0 */writeq_be(PORT0,&afu->afu_map->global.regs.afu_port_sel);num_ports=NUM_FC_PORTS-1;}else{
@@ -1552,15 +1548,15 @@ static int init_global(struct cxlflash_cfg *cfg)}for(i=0;i<num_ports;i++){-/* unmask all errors (but they are still masked at AFU) */+/* Unmask all errors (but they are still masked at AFU) */writeq_be(0,&afu->afu_map->global.fc_regs[i][FC_ERRMSK/8]);-/* clear CRC error cnt & set a threshold */+/* Clear CRC error cnt & set a threshold */(void)readq_be(&afu->afu_map->global.fc_regs[i][FC_CNT_CRCERR/8]);writeq_be(MC_CRC_THRESH,&afu->afu_map->global.fc_regs[i][FC_CRC_THRESH/8]);-/* set WWPNs. If already programmed, wwpn[i] is 0 */+/* Set WWPNs. If already programmed, wwpn[i] is 0 */if(wwpn[i]!=0&&afu_set_wwpn(afu,i,&afu->afu_map->global.fc_regs[i][0],
@@ -1574,18 +1570,17 @@ static int init_global(struct cxlflash_cfg *cfg)*offline/onlinetransitionsandaPLOGI*/msleep(100);-}-/* set up master's own CTX_CAP to allow real mode, host translation */-/* tbls, afu cmds and read/write GSCSI cmds. */+/* Set up master's own CTX_CAP to allow real mode, host translation */+/* tables, afu cmds and read/write GSCSI cmds. *//* First, unlock ctx_cap write by reading mbox */(void)readq_be(&afu->ctrl_map->mbox_r);/* unlock ctx_cap */writeq_be((SISL_CTX_CAP_REAL_MODE|SISL_CTX_CAP_HOST_XLATE|SISL_CTX_CAP_READ_CMD|SISL_CTX_CAP_WRITE_CMD|SISL_CTX_CAP_AFU_CMD|SISL_CTX_CAP_GSCSI_CMD),&afu->ctrl_map->ctx_cap);-/* init heartbeat */+/* Initialize heartbeat */afu->hb=readq_be(&afu->afu_map->global.regs.afu_hb);out:
@@ -1737,8 +1732,7 @@ static int init_afu(struct cxlflash_cfg *cfg)gotoerr1;}-/* Map the entire MMIO space of the AFU.-*/+/* Map the entire MMIO space of the AFU */afu->afu_map=cxl_psa_map(cfg->mcctx);if(!afu->afu_map){rc=-ENOMEM;
@@ -1856,7 +1850,7 @@ retry:wait_resp(afu,cmd);-/* set on timeout */+/* Set on timeout */if(unlikely((cmd->sa.ioasc!=0)||(cmd->sa.host_use_b[0]&B_ERROR)))rc=-1;
@@ -2273,7 +2267,7 @@ static struct scsi_host_template driver_template = {.cmd_per_lun=16,.can_queue=CXLFLASH_MAX_CMDS,.this_id=-1,-.sg_tablesize=SG_NONE,/* No scatter gather support. */+.sg_tablesize=SG_NONE,/* No scatter gather support */.max_sectors=CXLFLASH_MAX_SECTORS,.use_clustering=ENABLE_CLUSTERING,.shost_attrs=cxlflash_host_attrs,
@@ -2333,8 +2327,7 @@ static void cxlflash_worker_thread(struct work_struct *work)/* The reset can block... */afu_link_reset(afu,port,-&afu->afu_map->-global.fc_regs[port][0]);+&afu->afu_map->global.fc_regs[port][0]);spin_lock_irqsave(cfg->host->host_lock,lock_flags);}
@@ -2413,7 +2406,6 @@ static int cxlflash_probe(struct pci_dev *pdev,cfg->last_lun_index[1]=CXLFLASH_NUM_VLUNS/2-1;cfg->dev_id=(structpci_device_id*)dev_id;-cfg->mcctx=NULL;init_waitqueue_head(&cfg->tmf_waitq);init_waitqueue_head(&cfg->reset_waitq);
@@ -2429,7 +2421,8 @@ static int cxlflash_probe(struct pci_dev *pdev,pci_set_drvdata(pdev,cfg);-/* Use the special service provided to look up the physical+/*+*Usethespecialserviceprovidedtolookupthephysical*PCIdevice,sincewearecalledontheprobeofthevirtual*PCIhostbus(vphb)*/
@@ -2459,7 +2452,6 @@ static int cxlflash_probe(struct pci_dev *pdev,}cfg->init_state=INIT_STATE_AFU;-rc=init_scsi(cfg);if(rc){dev_err(&pdev->dev,"%s: call to init_scsi "
@@ -146,7 +146,7 @@ struct sisl_rc {#define SISL_FC_RC_ABORTFAIL 0x59 /* pending abort completed w/fail */#define SISL_FC_RC_RESID 0x5A /* ioasa underrun/overrun flags set */#define SISL_FC_RC_RESIDERR 0x5B /* actual data len does not match SCSI-reportedlen,possblyduetodropped+reportedlen,possiblyduetodroppedframes*/#define SISL_FC_RC_TGTABORT 0x5C /* command aborted by target */};
@@ -258,7 +258,7 @@ struct sisl_host_map {__be64rrq_start;/* start & end are both inclusive */__be64rrq_end;/* write sequence: start followed by end */__be64cmd_room;-__be64ctx_ctrl;/* least signiifcant byte or b56:63 is LISN# */+__be64ctx_ctrl;/* least significant byte or b56:63 is LISN# */__be64mbox_w;/* restricted use */};
@@ -132,7 +132,7 @@ static int ba_init(struct ba_lun *ba_lun)return-ENOMEM;}-/* Pass the allocated lun info as a handle to the user */+/* Pass the allocated LUN info as a handle to the user */ba_lun->ba_lun_handle=bali;pr_debug("%s: Successfully initialized the LUN: "
@@ -165,7 +165,7 @@ static int find_free_range(u32 low,num_bits=(sizeof(*lam)*BITS_PER_BYTE);bit_pos=find_first_bit(lam,num_bits);-pr_devel("%s: Found free bit %llX in lun "+pr_devel("%s: Found free bit %llX in LUN ""map entry %llX at bitmap index = %X\n",__func__,bit_pos,bali->lun_alloc_map[i],i);
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:33:34
Following an adapter reset, the AFU RRQ that resides in host memory
holds stale data. This can lead to a condition where the RRQ interrupt
handler tries to process stale entries and/or endlessly loops due to an
out of sync generation bit.
To fix, the AFU RRQ in host memory needs to be cleared after each reset.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 3 +++
1 file changed, 3 insertions(+)
@@ -1609,6 +1609,9 @@ static int start_afu(struct cxlflash_cfg *cfg)init_pcr(cfg);+/* After an AFU reset, RRQ entries are stale, clear them */+memset(&afu->rrq_entry,0,sizeof(afu->rrq_entry));+/* Initialize RRQ pointers */afu->hrrq_start=&afu->rrq_entry[0];afu->hrrq_end=&afu->rrq_entry[NUM_RRQ_ENTRY-1];
From: Matthew R. Ochs <hidden> Date: 2015-09-16 21:34:01
The adapter state machine is susceptible to missing and/or
corrupting state updates at runtime. This can lead to a variety
of unintended issues and is due to the lack of a serialization
mechanism to protect the adapter state.
Use an adapter-wide mutex to serialize state changes.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/common.h | 1 +
drivers/scsi/cxlflash/main.c | 40 +++++++++++++++++++++++++++++++++------
drivers/scsi/cxlflash/superpipe.c | 7 ++++++-
3 files changed, 41 insertions(+), 7 deletions(-)
@@ -1912,7 +1922,11 @@ static int cxlflash_eh_device_reset_handler(struct scsi_cmnd *scp)get_unaligned_be32(&((u32*)scp->cmnd)[3]));retry:-switch(cfg->state){+mutex_lock(&cfg->mutex);+state=cfg->state;+mutex_unlock(&cfg->mutex);++switch(state){caseSTATE_NORMAL:rcr=send_tmf(afu,scp,TMF_LUN_RESET);if(unlikely(rcr))
@@ -1954,6 +1968,7 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)get_unaligned_be32(&((u32*)scp->cmnd)[2]),get_unaligned_be32(&((u32*)scp->cmnd)[3]));+mutex_lock(&cfg->mutex);switch(cfg->state){caseSTATE_NORMAL:cfg->state=STATE_RESET;
@@ -1967,7 +1982,9 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)wake_up_all(&cfg->reset_waitq);break;caseSTATE_RESET:+mutex_unlock(&cfg->mutex);wait_event(cfg->reset_waitq,cfg->state!=STATE_RESET);+mutex_lock(&cfg->mutex);if(cfg->state==STATE_NORMAL)break;/* fall through */
@@ -2312,10 +2330,11 @@ static void cxlflash_worker_thread(struct work_struct *work)intport;ulonglock_flags;-/* Avoid MMIO if the device has failed */+mutex_lock(&cfg->mutex);+/* Avoid MMIO if the device has failed */if(cfg->state!=STATE_NORMAL)-return;+gotoout;spin_lock_irqsave(cfg->host->host_lock,lock_flags);
From: Matthew R. Ochs <hidden> Date: 2015-09-17 16:55:34
On Sep 17, 2015, at 6:58 AM, David Laight [off-list ref] =
wrote:
=20
From: Linuxppc-dev Matthew R. Ochs
quoted
Sent: 16 September 2015 22:28
Interrupt processing can run in parallel to a remove operation. This
can lead to a condition where the interrupt handler is processing =
with
quoted
memory that has been freed.
=20
To avoid processing an interrupt while memory may be yanked, check =
for
quoted
removal while in the interrupt handler. Bail when removal is =
imminent.
=20
On the face of it this just reduces the size of the window somewhat.
Agreed.
=20
What happens if the interrupt routine reads the flag just before it is =
set
(so is processing the entry that is being removed) and is then (say)
interrupted by a higher priority interrupt that takes longer to =
execute than
the remove code?
Understood. To completely close we'd need to either introduce a lock or =
a
reciprocal flag/count such that the remove doesn't make forward progress
until after interrupt processing has completed. I can look at =
introducing such
a mechanism in a later patch to fully remove the exposure.
-matt
This change and the one above are not really part of the patch. Not a big deal, but
in future would be good to either call out the fact that there are a couple of unrelated
formatting changes, or keep them out and stick them in a separate cleanup patch.
Reviewed-by: Brian King <redacted>
--
Brian King
Power Linux I/O
IBM Linux Technology Center
From: Brian King <hidden> Date: 2015-09-18 01:27:17
On 09/16/2015 04:27 PM, Matthew R. Ochs wrote:
quoted hunk
When a LUN is removed, the sdev that is associated with the LUN
remains intact until its reference count drops to 0. In order
to prevent an sdev from being removed while a context is still
associated with it, obtain an additional reference per-context
for each LUN attached to the context.
This resolves a potential Oops in the release handler when a
dealing with a LUN that has already been removed.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
@@ -880,6 +880,9 @@ static int _cxlflash_disk_detach(struct scsi_device *sdev,sys_close(lfd);}+/* Release the sdev reference that bound this LUN to the context */+scsi_device_put(sdev);+out:if(put_ctx)put_context(ctxi);
@@ -1287,11 +1290,18 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,}}+rc=scsi_device_get(sdev);+if(unlikely(rc)){+dev_err(dev,"%s: Unable to get sdev reference!\n",__func__);+gotoout;+}+lun_access=kzalloc(sizeof(*lun_access),GFP_KERNEL);if(unlikely(!lun_access)){dev_err(dev,"%s: Unable to allocate lun_access!\n",__func__);+scsi_device_put(sdev);
Looks like you've got a double scsi_device_put in this path, since there is another put
in the the err0 path.
@@ -1311,21 +1321,21 @@ static int cxlflash_disk_attach(struct scsi_device *sdev, dev_err(dev, "%s: Could not initialize context %p\n", __func__, ctx); rc = -ENODEV;- goto err0;+ goto err1; } ctxid = cxl_process_element(ctx); if (unlikely((ctxid > MAX_CONTEXT) || (ctxid < 0))) { dev_err(dev, "%s: ctxid (%d) invalid!\n", __func__, ctxid); rc = -EPERM;- goto err1;+ goto err2; } file = cxl_get_fd(ctx, &cfg->cxl_fops, &fd); if (unlikely(fd < 0)) { rc = -ENODEV; dev_err(dev, "%s: Could not get file descriptor\n", __func__);- goto err1;+ goto err2; } /* Translate read/write O_* flags from fcntl.h to AFU permission bits */
@@ -1335,7 +1345,7 @@ static int cxlflash_disk_attach(struct scsi_device *sdev, if (unlikely(!ctxi)) { dev_err(dev, "%s: Failed to create context! (%d)\n", __func__, ctxid);- goto err2;+ goto err3; } work = &ctxi->work;
@@ -1346,13 +1356,13 @@ static int cxlflash_disk_attach(struct scsi_device *sdev, if (unlikely(rc)) { dev_dbg(dev, "%s: Could not start context rc=%d\n", __func__, rc);- goto err3;+ goto err4; } rc = afu_attach(cfg, ctxi); if (unlikely(rc)) { dev_err(dev, "%s: Could not attach AFU rc %d\n", __func__, rc);- goto err4;+ goto err5; } /*
@@ -1388,13 +1398,13 @@ out: __func__, ctxid, fd, attach->block_size, rc, attach->last_lba); return rc;-err4:+err5: cxl_stop_context(ctx);-err3:+err4: put_context(ctxi); destroy_context(cfg, ctxi); ctxi = NULL;-err2:+err3: /* * Here, we're overriding the fops with a dummy all-NULL fops because * fput() calls the release fop, which will cause us to mistakenly
So, what kicks any outstanding ioctls back? Let's assume you are in the middle of disk_attach
and you've sent the READ_CAP16 to the device. It appears as if what would happen here is we'd
sit here in cxlflash_pci_error_detected. Eventually, the READ_CAP16 would timeout. This would
wake the SCSI error handler, and end up calling your eh_device_reset handler, which would see that
we are in STATE_LIMBO, where it would then do a wait_event, waiting for us to get out of STATE_LIMBO,
and we would end up in a deadlock.
Rather than implementing a rw semaphore, would it be better to simply make the ioctls check the
state we are in and either wait to get out of EEH state or fail themselves?
quoted hunk
rc = cxlflash_mark_contexts_error(cfg);
if (unlikely(rc))
dev_err(dev, "%s: Failed to mark user contexts!(%d)\n",
__func__, rc);
term_mc(cfg, UNDO_START);
stop_afu(cfg);
-
return PCI_ERS_RESULT_NEED_RESET;
case pci_channel_io_perm_failure:
cfg->state = STATE_FAILTERM;
From: Brian King <hidden> Date: 2015-09-18 21:34:48
On 09/16/2015 04:29 PM, Matthew R. Ochs wrote:
Implement the following suggestions and add two new attributes
to allow for debugging the port LUN table.
- use scnprintf() instead of snprintf()
- use DEVICE_ATTR_RO and DEVICE_ATTR_RW
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Shane Seymour <redacted>
---
drivers/scsi/cxlflash/main.c | 180 +++++++++++++++++++++++++++++++++----------
1 file changed, 138 insertions(+), 42 deletions(-)
/**
- * cxlflash_show_dev_mode() - presents the current mode of the device
+ * cxlflash_show_port_lun_table() - queries and presents the port LUN table
+ * @port: Desired port for status reporting.
+ * @afu: AFU owning the specified port.
+ * @buf: Buffer of length PAGE_SIZE to report back port status in ASCII.
+ *
+ * Return: The size of the ASCII string returned in @buf.
+ */
+static ssize_t cxlflash_show_port_lun_table(u32 port,
+ struct afu *afu,
+ char *buf)
+{
+ int i;
+ ssize_t bytes = 0;
+ __be64 __iomem *fc_port;
+
+ if (port >= NUM_FC_PORTS)
+ return 0;
+
+ fc_port = &afu->afu_map->global.fc_port[port][0];
+
+ for (i = 0; i < CXLFLASH_NUM_VLUNS; i++, buf += 22)
Rather than this bug prone hard coded 22, how about never incrementing buf and do something
similar to this:
+ bytes += scnprintf(buf, PAGE_SIZE, "%03d: %016llX\n",
+ i, readq_be(&fc_port[i]));
bytes += scnprintf(&buf[bytes], PAGE_SIZE, "%03d: %016llX\n",
i, readq_be(&fc_port[i]));
+ return bytes;
+}
+
--
Brian King
Power Linux I/O
IBM Linux Technology Center
switch (state) {
case pci_channel_io_frozen:
cfg->state =3D STATE_LIMBO;
-
- /* Turn off legacy I/O */
scsi_block_requests(cfg->host);
+ drain_ioctls(cfg);
=20
So, what kicks any outstanding ioctls back? Let's assume you are in =
the middle of disk_attach
and you've sent the READ_CAP16 to the device. It appears as if what =
would happen here is we'd
sit here in cxlflash_pci_error_detected. Eventually, the READ_CAP16 =
would timeout. This would
wake the SCSI error handler, and end up calling your eh_device_reset =
handler, which would see that
we are in STATE_LIMBO, where it would then do a wait_event, waiting =
for us to get out of STATE_LIMBO,
and we would end up in a deadlock.
=20
Rather than implementing a rw semaphore, would it be better to simply =
make the ioctls check the
state we are in and either wait to get out of EEH state or fail =
themselves?
We do have the ioctls check the state and wait for EEH to complete
or fail completely in the event that the device is terminating (see
ioctl_common()). The drain exists to create a wait point for ioctls that
have already passed the state check and are active. The CXL services
cannot be called during the recovery window (maybe this requirement
will go away in a future release?), thus the reason for this 'drain'.
To handle it I considered 3 options:
- add state check wraps to all CXL service calls
- create a "running ioctls" count that could be evaluated
- wrap the ioctl in read semaphore and obtain write access to 'wait'
I started with the first option and it quickly made the code very nasty. =
I then
began implementing the second option and as I was writing the code to =
wrap
the ioctl with increment/decrement statements, the third option entered =
my
mind and seemed like a much cleaner solution. Therefore I went with that
approach and did not look back.
With regard to your example, you bring up a good point and we'll need to
do something about that. One thought that comes to mind would be for us
to drop the semaphore before making this type of call (I believe there =
are
only 2 places like this), reacquiring it when we return, and then =
checking
the state to make sure we're not in a reset situation.
=20
quoted
rc =3D cxlflash_mark_contexts_error(cfg);
if (unlikely(rc))
dev_err(dev, "%s: Failed to mark user =
};
=20
/**
+ * check_state() - checks and responds to the current adapter state
+ * @cfg: Internal structure associated with the host.
+ * @ioctl: Indicates if on an ioctl thread.
+ *
+ * This routine can block and should only be used on process =
context.
quoted
+ * When blocking on an ioctl thread, the ioctl read semaphore should =
be
quoted
+ * let up to allow for draining actively running ioctls. Also note =
that
quoted
+ * when waking up from waiting in reset, the state is unknown and =
must
quoted
+ * be checked again before proceeding.
+ *
+ * Return: 0 on success, -errno on failure
+ */
+static int check_state(struct cxlflash_cfg *cfg, bool ioctl)
=20
All your callers appear to set the second parameter to true, so why =
bother having it?
That's a good point. I originally had a case where there was a need for =
this
but have since removed it. I can fix that in v3.
From: Tomas Henzl <hidden> Date: 2015-09-21 11:36:36
On 16.9.2015 23:26, Matthew R. Ochs wrote:
quoted hunk
From: Manoj Kumar <redacted>
The timeout value for read capacity is too small. Certain devices
may take longer to respond and thus the command may prematurely
timeout. Additionally the literal used for the timeout is stale.
Update the timeout to 30 seconds (matches the value used in sd.c)
and rework the timeout literal to a more appropriate description.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 9 ++++-----
drivers/scsi/cxlflash/superpipe.h | 2 +-
drivers/scsi/cxlflash/vlun.c | 4 ++--
3 files changed, 7 insertions(+), 8 deletions(-)
From: Tomas Henzl <hidden> Date: 2015-09-21 12:11:31
On 16.9.2015 23:27, Matthew R. Ochs wrote:
quoted hunk
When a LUN is removed, the sdev that is associated with the LUN
remains intact until its reference count drops to 0. In order
to prevent an sdev from being removed while a context is still
associated with it, obtain an additional reference per-context
for each LUN attached to the context.
This resolves a potential Oops in the release handler when a
dealing with a LUN that has already been removed.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
@@ -880,6 +880,9 @@ static int _cxlflash_disk_detach(struct scsi_device *sdev,sys_close(lfd);}+/* Release the sdev reference that bound this LUN to the context */+scsi_device_put(sdev);+
I'm not sure here with the use if scsi_device_get+put, also I don't quite well
understand what you are going to fix here and how can it happen.
The scsi_device_get takes an additional module reference, so if used from
a module it shouldn't be held for a long time.
Is it possible for a user to rmmod the czlflash module
after the disk attach function is called?
Cheers,
--tm
quoted hunk
out:
if (put_ctx)
put_context(ctxi);
@@ -1287,11 +1290,18 @@ static int cxlflash_disk_attach(struct scsi_device *sdev, } }+ rc = scsi_device_get(sdev);+ if (unlikely(rc)) {+ dev_err(dev, "%s: Unable to get sdev reference!\n", __func__);+ goto out;+ }+ lun_access = kzalloc(sizeof(*lun_access), GFP_KERNEL); if (unlikely(!lun_access)) { dev_err(dev, "%s: Unable to allocate lun_access!\n", __func__);+ scsi_device_put(sdev); rc = -ENOMEM;- goto out;+ goto err0; } lun_access->lli = lli;
@@ -1311,21 +1321,21 @@ static int cxlflash_disk_attach(struct scsi_device *sdev, dev_err(dev, "%s: Could not initialize context %p\n", __func__, ctx); rc = -ENODEV;- goto err0;+ goto err1; } ctxid = cxl_process_element(ctx); if (unlikely((ctxid > MAX_CONTEXT) || (ctxid < 0))) { dev_err(dev, "%s: ctxid (%d) invalid!\n", __func__, ctxid); rc = -EPERM;- goto err1;+ goto err2; } file = cxl_get_fd(ctx, &cfg->cxl_fops, &fd); if (unlikely(fd < 0)) { rc = -ENODEV; dev_err(dev, "%s: Could not get file descriptor\n", __func__);- goto err1;+ goto err2; } /* Translate read/write O_* flags from fcntl.h to AFU permission bits */
@@ -1335,7 +1345,7 @@ static int cxlflash_disk_attach(struct scsi_device *sdev, if (unlikely(!ctxi)) { dev_err(dev, "%s: Failed to create context! (%d)\n", __func__, ctxid);- goto err2;+ goto err3; } work = &ctxi->work;
@@ -1346,13 +1356,13 @@ static int cxlflash_disk_attach(struct scsi_device *sdev, if (unlikely(rc)) { dev_dbg(dev, "%s: Could not start context rc=%d\n", __func__, rc);- goto err3;+ goto err4; } rc = afu_attach(cfg, ctxi); if (unlikely(rc)) { dev_err(dev, "%s: Could not attach AFU rc %d\n", __func__, rc);- goto err4;+ goto err5; } /*
@@ -1388,13 +1398,13 @@ out: __func__, ctxid, fd, attach->block_size, rc, attach->last_lba); return rc;-err4:+err5: cxl_stop_context(ctx);-err3:+err4: put_context(ctxi); destroy_context(cfg, ctxi); ctxi = NULL;-err2:+err3: /* * Here, we're overriding the fops with a dummy all-NULL fops because * fput() calls the release fop, which will cause us to mistakenly
From: Tomas Henzl <hidden> Date: 2015-09-21 12:18:05
On 16.9.2015 23:28, Matthew R. Ochs wrote:
quoted hunk
Found during code inspection, that the following functions are not
being used outside of the file where they are defined. Make them static.
int cxlflash_send_cmd(struct afu *, struct afu_cmd *);
void cxlflash_wait_resp(struct afu *, struct afu_cmd *);
int cxlflash_afu_reset(struct cxlflash_cfg *);
struct afu_cmd *cxlflash_cmd_checkout(struct afu *);
void cxlflash_cmd_checkin(struct afu_cmd *);
void init_pcr(struct cxlflash_cfg *);
int init_global(struct cxlflash_cfg *);
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/common.h | 5 -
drivers/scsi/cxlflash/main.c | 1018 ++++++++++++++++++++--------------------
2 files changed, 509 insertions(+), 514 deletions(-)
@@ -238,7 +238,7 @@ static void cmd_complete(struct afu_cmd *cmd)resid=cmd->sa.resid;cmd_is_tmf=cmd->cmd_tmf;-cxlflash_cmd_checkin(cmd);/* Don't use cmd after here */+cmd_checkin(cmd);/* Don't use cmd after here */pr_debug("%s: calling scsi_set_resid, scp=%p ""result=%X resid=%d\n",__func__,
@@ -260,6 +260,146 @@ static void cmd_complete(struct afu_cmd *cmd)}/**+*context_reset()-timeouthandlerforAFUcommands+*@cmd:AFUcommandthattimedout.+*+*SendsaresettotheAFU.+*/+staticvoidcontext_reset(structafu_cmd*cmd)+{+intnretry=0;+u64rrin=0x1;+u64room=0;+structafu*afu=cmd->parent;+ulonglock_flags;++pr_debug("%s: cmd=%p\n",__func__,cmd);++spin_lock_irqsave(&cmd->slock,lock_flags);++/* Already completed? */+if(cmd->sa.host_use_b[0]&B_DONE){+spin_unlock_irqrestore(&cmd->slock,lock_flags);+return;+}++cmd->sa.host_use_b[0]|=(B_DONE|B_ERROR|B_TIMEOUT);+spin_unlock_irqrestore(&cmd->slock,lock_flags);++/*+*Wereallywanttosendthisresetatallcosts,sospread+*outwaittimeonsuccessiveretriesforavailableroom.+*/+do{+room=readq_be(&afu->host_map->cmd_room);+atomic64_set(&afu->room,room);+if(room)+gotowrite_rrin;+udelay(nretry);+}while(nretry++<MC_ROOM_RETRY_CNT);++pr_err("%s: no cmd_room to send reset\n",__func__);+return;++write_rrin:+nretry=0;+writeq_be(rrin,&afu->host_map->ioarrin);+do{+rrin=readq_be(&afu->host_map->ioarrin);+if(rrin!=0x1)+break;+/* Double delay each time */+udelay(2^nretry);
Double delay - isn't another operator needed?
If so, pleas add a new patch for this.
--tm
quoted hunk
+ } while (nretry++ < MC_ROOM_RETRY_CNT);
+}
+
+/**
+ * send_cmd() - sends an AFU command
+ * @afu: AFU associated with the host.
+ * @cmd: AFU command to send.
+ *
+ * Return:
+ * 0 on success or SCSI_MLQUEUE_HOST_BUSY
+ */
+static int send_cmd(struct afu *afu, struct afu_cmd *cmd)
+{
+ struct cxlflash_cfg *cfg = afu->parent;
+ struct device *dev = &cfg->dev->dev;
+ int nretry = 0;
+ int rc = 0;
+ u64 room;
+ long newval;
+
+ /*
+ * This routine is used by critical users such an AFU sync and to
+ * send a task management function (TMF). Thus we want to retry a
+ * bit before returning an error. To avoid the performance penalty
+ * of MMIO, we spread the update of 'room' over multiple commands.
+ */
+retry:
+ newval = atomic64_dec_if_positive(&afu->room);
+ if (!newval) {
+ do {
+ room = readq_be(&afu->host_map->cmd_room);
+ atomic64_set(&afu->room, room);
+ if (room)
+ goto write_ioarrin;
+ udelay(nretry);
+ } while (nretry++ < MC_ROOM_RETRY_CNT);
+
+ dev_err(dev, "%s: no cmd_room to send 0x%X\n",
+ __func__, cmd->rcb.cdb[0]);
+
+ goto no_room;
+ } else if (unlikely(newval < 0)) {
+ /* This should be rare. i.e. Only if two threads race and
+ * decrement before the MMIO read is done. In this case
+ * just benefit from the other thread having updated
+ * afu->room.
+ */
+ if (nretry++ < MC_ROOM_RETRY_CNT) {
+ udelay(nretry);
+ goto retry;
+ }
+
+ goto no_room;
+ }
+
+write_ioarrin:
+ writeq_be((u64)&cmd->rcb, &afu->host_map->ioarrin);
+out:
+ pr_devel("%s: cmd=%p len=%d ea=%p rc=%d\n", __func__, cmd,
+ cmd->rcb.data_len, (void *)cmd->rcb.data_ea, rc);
+ return rc;
+
+no_room:
+ afu->read_room = true;
+ schedule_work(&cfg->work_q);
+ rc = SCSI_MLQUEUE_HOST_BUSY;
+ goto out;
+}
+
+/**
+ * wait_resp() - polls for a response or timeout to a sent AFU command
+ * @afu: AFU associated with the host.
+ * @cmd: AFU command that was sent.
+ */
+static void wait_resp(struct afu *afu, struct afu_cmd *cmd)
+{
+ ulong timeout = msecs_to_jiffies(cmd->rcb.timeout * 2 * 1000);
+
+ timeout = wait_for_completion_timeout(&cmd->cevent, timeout);
+ if (!timeout)
+ context_reset(cmd);
+
+ if (unlikely(cmd->sa.ioasc != 0))
+ pr_err("%s: CMD 0x%X failed, IOASC: flags 0x%X, afu_rc 0x%X, "
+ "scsi_rc 0x%X, fc_rc 0x%X\n", __func__, cmd->rcb.cdb[0],
+ cmd->sa.rc.flags, cmd->sa.rc.afu_rc, cmd->sa.rc.scsi_rc,
+ cmd->sa.rc.fc_rc);
+}
+
+/**
* send_tmf() - sends a Task Management Function (TMF)
* @afu: AFU to checkout from.
* @scp: SCSI command from stack.
@@ -280,7 +420,7 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd) ulong lock_flags; int rc = 0;- cmd = cxlflash_cmd_checkout(afu);+ cmd = cmd_checkout(afu); if (unlikely(!cmd)) { pr_err("%s: could not get a free command\n", __func__); rc = SCSI_MLQUEUE_HOST_BUSY;
@@ -449,369 +589,55 @@ out: } /**- * cxlflash_eh_device_reset_handler() - reset a single LUN- * @scp: SCSI command to send.- *- * Return:- * SUCCESS as defined in scsi/scsi.h- * FAILED as defined in scsi/scsi.h+ * cxlflash_wait_for_pci_err_recovery() - wait for error recovery during probe+ * @cxlflash: Internal structure associated with the host. */-static int cxlflash_eh_device_reset_handler(struct scsi_cmnd *scp)+static void cxlflash_wait_for_pci_err_recovery(struct cxlflash_cfg *cfg) {- int rc = SUCCESS;- struct Scsi_Host *host = scp->device->host;- struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;- struct afu *afu = cfg->afu;- int rcr = 0;-- pr_debug("%s: (scp=%p) %d/%d/%d/%llu "- "cdb=(%08X-%08X-%08X-%08X)\n", __func__, scp,- host->host_no, scp->device->channel,- scp->device->id, scp->device->lun,- get_unaligned_be32(&((u32 *)scp->cmnd)[0]),- get_unaligned_be32(&((u32 *)scp->cmnd)[1]),- get_unaligned_be32(&((u32 *)scp->cmnd)[2]),- get_unaligned_be32(&((u32 *)scp->cmnd)[3]));-- switch (cfg->state) {- case STATE_NORMAL:- rcr = send_tmf(afu, scp, TMF_LUN_RESET);- if (unlikely(rcr))- rc = FAILED;- break;- case STATE_RESET:- wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);- if (cfg->state == STATE_NORMAL)- break;- /* fall through */- default:- rc = FAILED;- break;- }+ struct pci_dev *pdev = cfg->dev;- pr_debug("%s: returning rc=%d\n", __func__, rc);- return rc;+ if (pci_channel_offline(pdev))+ wait_event_timeout(cfg->reset_waitq,+ !pci_channel_offline(pdev),+ CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT); } /**- * cxlflash_eh_host_reset_handler() - reset the host adapter- * @scp: SCSI command from stack identifying host.- *- * Return:- * SUCCESS as defined in scsi/scsi.h- * FAILED as defined in scsi/scsi.h+ * free_mem() - free memory associated with the AFU+ * @cxlflash: Internal structure associated with the host. */-static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)+static void free_mem(struct cxlflash_cfg *cfg) {- int rc = SUCCESS;- int rcr = 0;- struct Scsi_Host *host = scp->device->host;- struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;+ int i;+ char *buf = NULL;+ struct afu *afu = cfg->afu;- pr_debug("%s: (scp=%p) %d/%d/%d/%llu "- "cdb=(%08X-%08X-%08X-%08X)\n", __func__, scp,- host->host_no, scp->device->channel,- scp->device->id, scp->device->lun,- get_unaligned_be32(&((u32 *)scp->cmnd)[0]),- get_unaligned_be32(&((u32 *)scp->cmnd)[1]),- get_unaligned_be32(&((u32 *)scp->cmnd)[2]),- get_unaligned_be32(&((u32 *)scp->cmnd)[3]));+ if (cfg->afu) {+ for (i = 0; i < CXLFLASH_NUM_CMDS; i++) {+ buf = afu->cmd[i].buf;+ if (!((u64)buf & (PAGE_SIZE - 1)))+ free_page((ulong)buf);+ }- switch (cfg->state) {- case STATE_NORMAL:- cfg->state = STATE_RESET;- scsi_block_requests(cfg->host);- cxlflash_mark_contexts_error(cfg);- rcr = cxlflash_afu_reset(cfg);- if (rcr) {- rc = FAILED;- cfg->state = STATE_FAILTERM;- } else- cfg->state = STATE_NORMAL;- wake_up_all(&cfg->reset_waitq);- scsi_unblock_requests(cfg->host);- break;- case STATE_RESET:- wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);- if (cfg->state == STATE_NORMAL)- break;- /* fall through */- default:- rc = FAILED;- break;+ free_pages((ulong)afu, get_order(sizeof(struct afu)));+ cfg->afu = NULL; }-- pr_debug("%s: returning rc=%d\n", __func__, rc);- return rc; } /**- * cxlflash_change_queue_depth() - change the queue depth for the device- * @sdev: SCSI device destined for queue depth change.- * @qdepth: Requested queue depth value to set.- *- * The requested queue depth is capped to the maximum supported value.+ * stop_afu() - stops the AFU command timers and unmaps the MMIO space+ * @cxlflash: Internal structure associated with the host. *- * Return: The actual queue depth set.+ * Safe to call with AFU in a partially allocated/initialized state. */-static int cxlflash_change_queue_depth(struct scsi_device *sdev, int qdepth)+static void stop_afu(struct cxlflash_cfg *cfg) {+ int i;+ struct afu *afu = cfg->afu;- if (qdepth > CXLFLASH_MAX_CMDS_PER_LUN)- qdepth = CXLFLASH_MAX_CMDS_PER_LUN;-- scsi_change_queue_depth(sdev, qdepth);- return sdev->queue_depth;-}--/**- * cxlflash_show_port_status() - queries and presents the current port status- * @dev: Generic device associated with the host owning the port.- * @attr: Device attribute representing the port.- * @buf: Buffer of length PAGE_SIZE to report back port status in ASCII.- *- * Return: The size of the ASCII string returned in @buf.- */-static ssize_t cxlflash_show_port_status(struct device *dev,- struct device_attribute *attr,- char *buf)-{- struct Scsi_Host *shost = class_to_shost(dev);- struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;- struct afu *afu = cfg->afu;-- char *disp_status;- int rc;- u32 port;- u64 status;- u64 *fc_regs;-- rc = kstrtouint((attr->attr.name + 4), 10, &port);- if (rc || (port >= NUM_FC_PORTS))- return 0;-- fc_regs = &afu->afu_map->global.fc_regs[port][0];- status =- (readq_be(&fc_regs[FC_MTIP_STATUS / 8]) & FC_MTIP_STATUS_MASK);-- if (status == FC_MTIP_STATUS_ONLINE)- disp_status = "online";- else if (status == FC_MTIP_STATUS_OFFLINE)- disp_status = "offline";- else- disp_status = "unknown";-- return snprintf(buf, PAGE_SIZE, "%s\n", disp_status);-}--/**- * cxlflash_show_lun_mode() - presents the current LUN mode of the host- * @dev: Generic device associated with the host.- * @attr: Device attribute representing the lun mode.- * @buf: Buffer of length PAGE_SIZE to report back the LUN mode in ASCII.- *- * Return: The size of the ASCII string returned in @buf.- */-static ssize_t cxlflash_show_lun_mode(struct device *dev,- struct device_attribute *attr, char *buf)-{- struct Scsi_Host *shost = class_to_shost(dev);- struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;- struct afu *afu = cfg->afu;-- return snprintf(buf, PAGE_SIZE, "%u\n", afu->internal_lun);-}--/**- * cxlflash_store_lun_mode() - sets the LUN mode of the host- * @dev: Generic device associated with the host.- * @attr: Device attribute representing the lun mode.- * @buf: Buffer of length PAGE_SIZE containing the LUN mode in ASCII.- * @count: Length of data resizing in @buf.- *- * The CXL Flash AFU supports a dummy LUN mode where the external- * links and storage are not required. Space on the FPGA is used- * to create 1 or 2 small LUNs which are presented to the system- * as if they were a normal storage device. This feature is useful- * during development and also provides manufacturing with a way- * to test the AFU without an actual device.- *- * 0 = external LUN[s] (default)- * 1 = internal LUN (1 x 64K, 512B blocks, id 0)- * 2 = internal LUN (1 x 64K, 4K blocks, id 0)- * 3 = internal LUN (2 x 32K, 512B blocks, ids 0,1)- * 4 = internal LUN (2 x 32K, 4K blocks, ids 0,1)- *- * Return: The size of the ASCII string returned in @buf.- */-static ssize_t cxlflash_store_lun_mode(struct device *dev,- struct device_attribute *attr,- const char *buf, size_t count)-{- struct Scsi_Host *shost = class_to_shost(dev);- struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;- struct afu *afu = cfg->afu;- int rc;- u32 lun_mode;-- rc = kstrtouint(buf, 10, &lun_mode);- if (!rc && (lun_mode < 5) && (lun_mode != afu->internal_lun)) {- afu->internal_lun = lun_mode;- cxlflash_afu_reset(cfg);- scsi_scan_host(cfg->host);- }-- return count;-}--/**- * cxlflash_show_ioctl_version() - presents the current ioctl version of the host- * @dev: Generic device associated with the host.- * @attr: Device attribute representing the ioctl version.- * @buf: Buffer of length PAGE_SIZE to report back the ioctl version.- *- * Return: The size of the ASCII string returned in @buf.- */-static ssize_t cxlflash_show_ioctl_version(struct device *dev,- struct device_attribute *attr,- char *buf)-{- return scnprintf(buf, PAGE_SIZE, "%u\n", DK_CXLFLASH_VERSION_0);-}--/**- * cxlflash_show_dev_mode() - presents the current mode of the device- * @dev: Generic device associated with the device.- * @attr: Device attribute representing the device mode.- * @buf: Buffer of length PAGE_SIZE to report back the dev mode in ASCII.- *- * Return: The size of the ASCII string returned in @buf.- */-static ssize_t cxlflash_show_dev_mode(struct device *dev,- struct device_attribute *attr, char *buf)-{- struct scsi_device *sdev = to_scsi_device(dev);-- return snprintf(buf, PAGE_SIZE, "%s\n",- sdev->hostdata ? "superpipe" : "legacy");-}--/**- * cxlflash_wait_for_pci_err_recovery() - wait for error recovery during probe- * @cxlflash: Internal structure associated with the host.- */-static void cxlflash_wait_for_pci_err_recovery(struct cxlflash_cfg *cfg)-{- struct pci_dev *pdev = cfg->dev;-- if (pci_channel_offline(pdev))- wait_event_timeout(cfg->reset_waitq,- !pci_channel_offline(pdev),- CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT);-}--/*- * Host attributes- */-static DEVICE_ATTR(port0, S_IRUGO, cxlflash_show_port_status, NULL);-static DEVICE_ATTR(port1, S_IRUGO, cxlflash_show_port_status, NULL);-static DEVICE_ATTR(lun_mode, S_IRUGO | S_IWUSR, cxlflash_show_lun_mode,- cxlflash_store_lun_mode);-static DEVICE_ATTR(ioctl_version, S_IRUGO, cxlflash_show_ioctl_version, NULL);--static struct device_attribute *cxlflash_host_attrs[] = {- &dev_attr_port0,- &dev_attr_port1,- &dev_attr_lun_mode,- &dev_attr_ioctl_version,- NULL-};--/*- * Device attributes- */-static DEVICE_ATTR(mode, S_IRUGO, cxlflash_show_dev_mode, NULL);--static struct device_attribute *cxlflash_dev_attrs[] = {- &dev_attr_mode,- NULL-};--/*- * Host template- */-static struct scsi_host_template driver_template = {- .module = THIS_MODULE,- .name = CXLFLASH_ADAPTER_NAME,- .info = cxlflash_driver_info,- .ioctl = cxlflash_ioctl,- .proc_name = CXLFLASH_NAME,- .queuecommand = cxlflash_queuecommand,- .eh_device_reset_handler = cxlflash_eh_device_reset_handler,- .eh_host_reset_handler = cxlflash_eh_host_reset_handler,- .change_queue_depth = cxlflash_change_queue_depth,- .cmd_per_lun = 16,- .can_queue = CXLFLASH_MAX_CMDS,- .this_id = -1,- .sg_tablesize = SG_NONE, /* No scatter gather support. */- .max_sectors = CXLFLASH_MAX_SECTORS,- .use_clustering = ENABLE_CLUSTERING,- .shost_attrs = cxlflash_host_attrs,- .sdev_attrs = cxlflash_dev_attrs,-};--/*- * Device dependent values- */-static struct dev_dependent_vals dev_corsa_vals = { CXLFLASH_MAX_SECTORS };--/*- * PCI device binding table- */-static struct pci_device_id cxlflash_pci_table[] = {- {PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CORSA,- PCI_ANY_ID, PCI_ANY_ID, 0, 0, (kernel_ulong_t)&dev_corsa_vals},- {}-};--MODULE_DEVICE_TABLE(pci, cxlflash_pci_table);--/**- * free_mem() - free memory associated with the AFU- * @cxlflash: Internal structure associated with the host.- */-static void free_mem(struct cxlflash_cfg *cfg)-{- int i;- char *buf = NULL;- struct afu *afu = cfg->afu;-- if (cfg->afu) {- for (i = 0; i < CXLFLASH_NUM_CMDS; i++) {- buf = afu->cmd[i].buf;- if (!((u64)buf & (PAGE_SIZE - 1)))- free_page((ulong)buf);- }-- free_pages((ulong)afu, get_order(sizeof(struct afu)));- cfg->afu = NULL;- }-}--/**- * stop_afu() - stops the AFU command timers and unmaps the MMIO space- * @cxlflash: Internal structure associated with the host.- *- * Safe to call with AFU in a partially allocated/initialized state.- */-static void stop_afu(struct cxlflash_cfg *cfg)-{- int i;- struct afu *afu = cfg->afu;-- if (likely(afu)) {- for (i = 0; i < CXLFLASH_NUM_CMDS; i++)- complete(&afu->cmd[i].cevent);+ if (likely(afu)) {+ for (i = 0; i < CXLFLASH_NUM_CMDS; i++)+ complete(&afu->cmd[i].cevent); if (likely(afu->afu_map)) { cxl_psa_unmap((void *)afu->afu_map);
@@ -1640,67 +1466,13 @@ out: } /**- * cxlflash_context_reset() - timeout handler for AFU commands- * @cmd: AFU command that timed out.+ * init_pcr() - initialize the provisioning and control registers+ * @cxlflash: Internal structure associated with the host. *- * Sends a reset to the AFU.+ * Also sets up fast access to the mapped registers and initializes AFU+ * command fields that never change. */-void cxlflash_context_reset(struct afu_cmd *cmd)-{- int nretry = 0;- u64 rrin = 0x1;- u64 room = 0;- struct afu *afu = cmd->parent;- ulong lock_flags;-- pr_debug("%s: cmd=%p\n", __func__, cmd);-- spin_lock_irqsave(&cmd->slock, lock_flags);-- /* Already completed? */- if (cmd->sa.host_use_b[0] & B_DONE) {- spin_unlock_irqrestore(&cmd->slock, lock_flags);- return;- }-- cmd->sa.host_use_b[0] |= (B_DONE | B_ERROR | B_TIMEOUT);- spin_unlock_irqrestore(&cmd->slock, lock_flags);-- /*- * We really want to send this reset at all costs, so spread- * out wait time on successive retries for available room.- */- do {- room = readq_be(&afu->host_map->cmd_room);- atomic64_set(&afu->room, room);- if (room)- goto write_rrin;- udelay(nretry);- } while (nretry++ < MC_ROOM_RETRY_CNT);-- pr_err("%s: no cmd_room to send reset\n", __func__);- return;--write_rrin:- nretry = 0;- writeq_be(rrin, &afu->host_map->ioarrin);- do {- rrin = readq_be(&afu->host_map->ioarrin);- if (rrin != 0x1)- break;- /* Double delay each time */- udelay(2 ^ nretry);- } while (nretry++ < MC_ROOM_RETRY_CNT);-}--/**- * init_pcr() - initialize the provisioning and control registers- * @cxlflash: Internal structure associated with the host.- *- * Also sets up fast access to the mapped registers and initializes AFU- * command fields that never change.- */-void init_pcr(struct cxlflash_cfg *cfg)+static void init_pcr(struct cxlflash_cfg *cfg) { struct afu *afu = cfg->afu; struct sisl_ctrl_map *ctrl_map;
@@ -1736,7 +1508,7 @@ void init_pcr(struct cxlflash_cfg *cfg) * init_global() - initialize AFU global registers * @cxlflash: Internal structure associated with the host. */-int init_global(struct cxlflash_cfg *cfg)+static int init_global(struct cxlflash_cfg *cfg) { struct afu *afu = cfg->afu; u64 wwpn[NUM_FC_PORTS]; /* wwpn of AFU ports */
@@ -2007,92 +1779,6 @@ err1: } /**- * cxlflash_send_cmd() - sends an AFU command- * @afu: AFU associated with the host.- * @cmd: AFU command to send.- *- * Return:- * 0 on success- * -1 on failure- */-int cxlflash_send_cmd(struct afu *afu, struct afu_cmd *cmd)-{- struct cxlflash_cfg *cfg = afu->parent;- int nretry = 0;- int rc = 0;- u64 room;- long newval;-- /*- * This routine is used by critical users such an AFU sync and to- * send a task management function (TMF). Thus we want to retry a- * bit before returning an error. To avoid the performance penalty- * of MMIO, we spread the update of 'room' over multiple commands.- */-retry:- newval = atomic64_dec_if_positive(&afu->room);- if (!newval) {- do {- room = readq_be(&afu->host_map->cmd_room);- atomic64_set(&afu->room, room);- if (room)- goto write_ioarrin;- udelay(nretry);- } while (nretry++ < MC_ROOM_RETRY_CNT);-- pr_err("%s: no cmd_room to send 0x%X\n",- __func__, cmd->rcb.cdb[0]);-- goto no_room;- } else if (unlikely(newval < 0)) {- /* This should be rare. i.e. Only if two threads race and- * decrement before the MMIO read is done. In this case- * just benefit from the other thread having updated- * afu->room.- */- if (nretry++ < MC_ROOM_RETRY_CNT) {- udelay(nretry);- goto retry;- }-- goto no_room;- }--write_ioarrin:- writeq_be((u64)&cmd->rcb, &afu->host_map->ioarrin);-out:- pr_debug("%s: cmd=%p len=%d ea=%p rc=%d\n", __func__, cmd,- cmd->rcb.data_len, (void *)cmd->rcb.data_ea, rc);- return rc;--no_room:- afu->read_room = true;- schedule_work(&cfg->work_q);- rc = SCSI_MLQUEUE_HOST_BUSY;- goto out;-}--/**- * cxlflash_wait_resp() - polls for a response or timeout to a sent AFU command- * @afu: AFU associated with the host.- * @cmd: AFU command that was sent.- */-void cxlflash_wait_resp(struct afu *afu, struct afu_cmd *cmd)-{- ulong timeout = jiffies + (cmd->rcb.timeout * 2 * HZ);-- timeout = wait_for_completion_timeout(&cmd->cevent, timeout);- if (!timeout)- cxlflash_context_reset(cmd);-- if (unlikely(cmd->sa.ioasc != 0))- pr_err("%s: CMD 0x%X failed, IOASC: flags 0x%X, afu_rc 0x%X, "- "scsi_rc 0x%X, fc_rc 0x%X\n", __func__, cmd->rcb.cdb[0],- cmd->sa.rc.flags, cmd->sa.rc.afu_rc, cmd->sa.rc.scsi_rc,- cmd->sa.rc.fc_rc);-}--/** * cxlflash_afu_sync() - builds and sends an AFU sync command * @afu: AFU associated with the host. * @ctx_hndl_u: Identifies context requesting sync.
@@ -2159,11 +1845,11 @@ retry: *((u16 *)&cmd->rcb.cdb[2]) = swab16(ctx_hndl_u); *((u32 *)&cmd->rcb.cdb[4]) = swab32(res_hndl_u);- rc = cxlflash_send_cmd(afu, cmd);+ rc = send_cmd(afu, cmd); if (unlikely(rc)) goto out;- cxlflash_wait_resp(afu, cmd);+ wait_resp(afu, cmd); /* set on timeout */ if (unlikely((cmd->sa.ioasc != 0) ||
@@ -2172,20 +1858,20 @@ retry: out: mutex_unlock(&sync_active); if (cmd)- cxlflash_cmd_checkin(cmd);+ cmd_checkin(cmd); pr_debug("%s: returning rc=%d\n", __func__, rc); return rc; } /**- * cxlflash_afu_reset() - resets the AFU- * @cxlflash: Internal structure associated with the host.+ * afu_reset() - resets the AFU+ * @cfg: Internal structure associated with the host. * * Return: * 0 on success * A failure value from internal services. */-int cxlflash_afu_reset(struct cxlflash_cfg *cfg)+static int afu_reset(struct cxlflash_cfg *cfg) { int rc = 0; /* Stop the context before the reset. Since the context is
@@ -2201,6 +1887,320 @@ int cxlflash_afu_reset(struct cxlflash_cfg *cfg) } /**+ * cxlflash_eh_device_reset_handler() - reset a single LUN+ * @scp: SCSI command to send.+ *+ * Return:+ * SUCCESS as defined in scsi/scsi.h+ * FAILED as defined in scsi/scsi.h+ */+static int cxlflash_eh_device_reset_handler(struct scsi_cmnd *scp)+{+ int rc = SUCCESS;+ struct Scsi_Host *host = scp->device->host;+ struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;+ struct afu *afu = cfg->afu;+ int rcr = 0;++ pr_debug("%s: (scp=%p) %d/%d/%d/%llu "+ "cdb=(%08X-%08X-%08X-%08X)\n", __func__, scp,+ host->host_no, scp->device->channel,+ scp->device->id, scp->device->lun,+ get_unaligned_be32(&((u32 *)scp->cmnd)[0]),+ get_unaligned_be32(&((u32 *)scp->cmnd)[1]),+ get_unaligned_be32(&((u32 *)scp->cmnd)[2]),+ get_unaligned_be32(&((u32 *)scp->cmnd)[3]));++ switch (cfg->state) {+ case STATE_NORMAL:+ rcr = send_tmf(afu, scp, TMF_LUN_RESET);+ if (unlikely(rcr))+ rc = FAILED;+ break;+ case STATE_RESET:+ wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);+ if (cfg->state == STATE_NORMAL)+ break;+ /* fall through */+ default:+ rc = FAILED;+ break;+ }++ pr_debug("%s: returning rc=%d\n", __func__, rc);+ return rc;+}++/**+ * cxlflash_eh_host_reset_handler() - reset the host adapter+ * @scp: SCSI command from stack identifying host.+ *+ * Return:+ * SUCCESS as defined in scsi/scsi.h+ * FAILED as defined in scsi/scsi.h+ */+static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)+{+ int rc = SUCCESS;+ int rcr = 0;+ struct Scsi_Host *host = scp->device->host;+ struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;++ pr_debug("%s: (scp=%p) %d/%d/%d/%llu "+ "cdb=(%08X-%08X-%08X-%08X)\n", __func__, scp,+ host->host_no, scp->device->channel,+ scp->device->id, scp->device->lun,+ get_unaligned_be32(&((u32 *)scp->cmnd)[0]),+ get_unaligned_be32(&((u32 *)scp->cmnd)[1]),+ get_unaligned_be32(&((u32 *)scp->cmnd)[2]),+ get_unaligned_be32(&((u32 *)scp->cmnd)[3]));++ switch (cfg->state) {+ case STATE_NORMAL:+ cfg->state = STATE_RESET;+ scsi_block_requests(cfg->host);+ cxlflash_mark_contexts_error(cfg);+ rcr = afu_reset(cfg);+ if (rcr) {+ rc = FAILED;+ cfg->state = STATE_FAILTERM;+ } else+ cfg->state = STATE_NORMAL;+ wake_up_all(&cfg->reset_waitq);+ scsi_unblock_requests(cfg->host);+ break;+ case STATE_RESET:+ wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);+ if (cfg->state == STATE_NORMAL)+ break;+ /* fall through */+ default:+ rc = FAILED;+ break;+ }++ pr_debug("%s: returning rc=%d\n", __func__, rc);+ return rc;+}++/**+ * cxlflash_change_queue_depth() - change the queue depth for the device+ * @sdev: SCSI device destined for queue depth change.+ * @qdepth: Requested queue depth value to set.+ *+ * The requested queue depth is capped to the maximum supported value.+ *+ * Return: The actual queue depth set.+ */+static int cxlflash_change_queue_depth(struct scsi_device *sdev, int qdepth)+{++ if (qdepth > CXLFLASH_MAX_CMDS_PER_LUN)+ qdepth = CXLFLASH_MAX_CMDS_PER_LUN;++ scsi_change_queue_depth(sdev, qdepth);+ return sdev->queue_depth;+}++/**+ * cxlflash_show_port_status() - queries and presents the current port status+ * @dev: Generic device associated with the host owning the port.+ * @attr: Device attribute representing the port.+ * @buf: Buffer of length PAGE_SIZE to report back port status in ASCII.+ *+ * Return: The size of the ASCII string returned in @buf.+ */+static ssize_t cxlflash_show_port_status(struct device *dev,+ struct device_attribute *attr,+ char *buf)+{+ struct Scsi_Host *shost = class_to_shost(dev);+ struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;+ struct afu *afu = cfg->afu;++ char *disp_status;+ int rc;+ u32 port;+ u64 status;+ u64 *fc_regs;++ rc = kstrtouint((attr->attr.name + 4), 10, &port);+ if (rc || (port >= NUM_FC_PORTS))+ return 0;++ fc_regs = &afu->afu_map->global.fc_regs[port][0];+ status =+ (readq_be(&fc_regs[FC_MTIP_STATUS / 8]) & FC_MTIP_STATUS_MASK);++ if (status == FC_MTIP_STATUS_ONLINE)+ disp_status = "online";+ else if (status == FC_MTIP_STATUS_OFFLINE)+ disp_status = "offline";+ else+ disp_status = "unknown";++ return snprintf(buf, PAGE_SIZE, "%s\n", disp_status);+}++/**+ * cxlflash_show_lun_mode() - presents the current LUN mode of the host+ * @dev: Generic device associated with the host.+ * @attr: Device attribute representing the lun mode.+ * @buf: Buffer of length PAGE_SIZE to report back the LUN mode in ASCII.+ *+ * Return: The size of the ASCII string returned in @buf.+ */+static ssize_t cxlflash_show_lun_mode(struct device *dev,+ struct device_attribute *attr, char *buf)+{+ struct Scsi_Host *shost = class_to_shost(dev);+ struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;+ struct afu *afu = cfg->afu;++ return snprintf(buf, PAGE_SIZE, "%u\n", afu->internal_lun);+}++/**+ * cxlflash_store_lun_mode() - sets the LUN mode of the host+ * @dev: Generic device associated with the host.+ * @attr: Device attribute representing the lun mode.+ * @buf: Buffer of length PAGE_SIZE containing the LUN mode in ASCII.+ * @count: Length of data resizing in @buf.+ *+ * The CXL Flash AFU supports a dummy LUN mode where the external+ * links and storage are not required. Space on the FPGA is used+ * to create 1 or 2 small LUNs which are presented to the system+ * as if they were a normal storage device. This feature is useful+ * during development and also provides manufacturing with a way+ * to test the AFU without an actual device.+ *+ * 0 = external LUN[s] (default)+ * 1 = internal LUN (1 x 64K, 512B blocks, id 0)+ * 2 = internal LUN (1 x 64K, 4K blocks, id 0)+ * 3 = internal LUN (2 x 32K, 512B blocks, ids 0,1)+ * 4 = internal LUN (2 x 32K, 4K blocks, ids 0,1)+ *+ * Return: The size of the ASCII string returned in @buf.+ */+static ssize_t cxlflash_store_lun_mode(struct device *dev,+ struct device_attribute *attr,+ const char *buf, size_t count)+{+ struct Scsi_Host *shost = class_to_shost(dev);+ struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;+ struct afu *afu = cfg->afu;+ int rc;+ u32 lun_mode;++ rc = kstrtouint(buf, 10, &lun_mode);+ if (!rc && (lun_mode < 5) && (lun_mode != afu->internal_lun)) {+ afu->internal_lun = lun_mode;+ afu_reset(cfg);+ scsi_scan_host(cfg->host);+ }++ return count;+}++/**+ * cxlflash_show_ioctl_version() - presents the hosts current ioctl version+ * @dev: Generic device associated with the host.+ * @attr: Device attribute representing the ioctl version.+ * @buf: Buffer of length PAGE_SIZE to report back the ioctl version.+ *+ * Return: The size of the ASCII string returned in @buf.+ */+static ssize_t cxlflash_show_ioctl_version(struct device *dev,+ struct device_attribute *attr,+ char *buf)+{+ return scnprintf(buf, PAGE_SIZE, "%u\n", DK_CXLFLASH_VERSION_0);+}++/**+ * cxlflash_show_dev_mode() - presents the current mode of the device+ * @dev: Generic device associated with the device.+ * @attr: Device attribute representing the device mode.+ * @buf: Buffer of length PAGE_SIZE to report back the dev mode in ASCII.+ *+ * Return: The size of the ASCII string returned in @buf.+ */+static ssize_t cxlflash_show_dev_mode(struct device *dev,+ struct device_attribute *attr, char *buf)+{+ struct scsi_device *sdev = to_scsi_device(dev);++ return snprintf(buf, PAGE_SIZE, "%s\n",+ sdev->hostdata ? "superpipe" : "legacy");+}++/*+ * Host attributes+ */+static DEVICE_ATTR(port0, S_IRUGO, cxlflash_show_port_status, NULL);+static DEVICE_ATTR(port1, S_IRUGO, cxlflash_show_port_status, NULL);+static DEVICE_ATTR(lun_mode, S_IRUGO | S_IWUSR, cxlflash_show_lun_mode,+ cxlflash_store_lun_mode);+static DEVICE_ATTR(ioctl_version, S_IRUGO, cxlflash_show_ioctl_version, NULL);++static struct device_attribute *cxlflash_host_attrs[] = {+ &dev_attr_port0,+ &dev_attr_port1,+ &dev_attr_lun_mode,+ &dev_attr_ioctl_version,+ NULL+};++/*+ * Device attributes+ */+static DEVICE_ATTR(mode, S_IRUGO, cxlflash_show_dev_mode, NULL);++static struct device_attribute *cxlflash_dev_attrs[] = {+ &dev_attr_mode,+ NULL+};++/*+ * Host template+ */+static struct scsi_host_template driver_template = {+ .module = THIS_MODULE,+ .name = CXLFLASH_ADAPTER_NAME,+ .info = cxlflash_driver_info,+ .ioctl = cxlflash_ioctl,+ .proc_name = CXLFLASH_NAME,+ .queuecommand = cxlflash_queuecommand,+ .eh_device_reset_handler = cxlflash_eh_device_reset_handler,+ .eh_host_reset_handler = cxlflash_eh_host_reset_handler,+ .change_queue_depth = cxlflash_change_queue_depth,+ .cmd_per_lun = 16,+ .can_queue = CXLFLASH_MAX_CMDS,+ .this_id = -1,+ .sg_tablesize = SG_NONE, /* No scatter gather support. */+ .max_sectors = CXLFLASH_MAX_SECTORS,+ .use_clustering = ENABLE_CLUSTERING,+ .shost_attrs = cxlflash_host_attrs,+ .sdev_attrs = cxlflash_dev_attrs,+};++/*+ * Device dependent values+ */+static struct dev_dependent_vals dev_corsa_vals = { CXLFLASH_MAX_SECTORS };++/*+ * PCI device binding table+ */+static struct pci_device_id cxlflash_pci_table[] = {+ {PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CORSA,+ PCI_ANY_ID, PCI_ANY_ID, 0, 0, (kernel_ulong_t)&dev_corsa_vals},+ {}+};++MODULE_DEVICE_TABLE(pci, cxlflash_pci_table);++/** * cxlflash_worker_thread() - work thread handler for the AFU * @work: Work structure contained within cxlflash associated with host. *
From: Tomas Henzl <hidden> Date: 2015-09-21 12:25:45
On 16.9.2015 23:31, Matthew R. Ochs wrote:
quoted hunk
The workq can process work in parallel with a remove event, leading
to a condition where the workq handler can access freed memory.
To remedy, the workq should be terminated prior to freeing memory. Move
the termination call earlier in remove and use cancel_work_sync() instead
of flush_work() as there is not a need to process any scheduled work when
shutting down.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -736,12 +736,12 @@ static void cxlflash_remove(struct pci_dev *pdev)scsi_remove_host(cfg->host);/* Fall through */caseINIT_STATE_AFU:+cancel_work_sync(&cfg->work_q);term_afu(cfg);
You disable irqs after a call to cancel_work_sync.
That means a late int could trigger the workqueue again?
Please disable irqs earlier - as described in Documentation/PCI/pci.txt
case INIT_STATE_PCI:
pci_release_regions(cfg->dev);
pci_disable_device(pdev);
case INIT_STATE_NONE:
- flush_work(&cfg->work_q);
free_mem(cfg);
scsi_host_put(cfg->host);
break;
From: Tomas Henzl <hidden> Date: 2015-09-21 12:44:37
On 16.9.2015 23:32, Matthew R. Ochs wrote:
The adapter state machine is susceptible to missing and/or
corrupting state updates at runtime. This can lead to a variety
of unintended issues and is due to the lack of a serialization
mechanism to protect the adapter state.
Use an adapter-wide mutex to serialize state changes.
I've just briefly looked into your code, but it seems to me that
an atomic variable would serve your needs also and might be
more effective resulting in a faster code execution?
If you keep the mutex way you don't need two mutexes
in cxlflash_afu_sync - you should remove the mutex &sync_active
--tm
quoted hunk
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/common.h | 1 +
drivers/scsi/cxlflash/main.c | 40 +++++++++++++++++++++++++++++++++------
drivers/scsi/cxlflash/superpipe.c | 7 ++++++-
3 files changed, 41 insertions(+), 7 deletions(-)
@@ -1912,7 +1922,11 @@ static int cxlflash_eh_device_reset_handler(struct scsi_cmnd *scp)get_unaligned_be32(&((u32*)scp->cmnd)[3]));retry:-switch(cfg->state){+mutex_lock(&cfg->mutex);+state=cfg->state;+mutex_unlock(&cfg->mutex);++switch(state){caseSTATE_NORMAL:rcr=send_tmf(afu,scp,TMF_LUN_RESET);if(unlikely(rcr))
@@ -1954,6 +1968,7 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)get_unaligned_be32(&((u32*)scp->cmnd)[2]),get_unaligned_be32(&((u32*)scp->cmnd)[3]));+mutex_lock(&cfg->mutex);switch(cfg->state){caseSTATE_NORMAL:cfg->state=STATE_RESET;
@@ -1967,7 +1982,9 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)wake_up_all(&cfg->reset_waitq);break;caseSTATE_RESET:+mutex_unlock(&cfg->mutex);wait_event(cfg->reset_waitq,cfg->state!=STATE_RESET);+mutex_lock(&cfg->mutex);if(cfg->state==STATE_NORMAL)break;/* fall through */
@@ -2312,10 +2330,11 @@ static void cxlflash_worker_thread(struct work_struct *work)intport;ulonglock_flags;-/* Avoid MMIO if the device has failed */+mutex_lock(&cfg->mutex);+/* Avoid MMIO if the device has failed */if(cfg->state!=STATE_NORMAL)-return;+gotoout;spin_lock_irqsave(cfg->host->host_lock,lock_flags);
From: Brian King <hidden> Date: 2015-09-21 18:25:22
On 09/16/2015 04:30 PM, Matthew R. Ochs wrote:
Borrowing the TMF waitq's spinlock causes a stall condition when
waiting for the TMF to complete. To remedy, introduce our own spin
lock to serialize TMF and use the appropriate wait services.
Can you clarify what stall condition you were seeing. Its not obvious
to me what this fixes. Do you have softlockup logs from the failure?
-Brian
--
Brian King
Power Linux I/O
IBM Linux Technology Center
From: Brian King <hidden> Date: 2015-09-21 18:28:40
On 09/16/2015 04:30 PM, Matthew R. Ochs wrote:
quoted hunk
@@ -158,8 +160,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct scsi_cmnd *scp) /* If the SISL_RC_FLAGS_OVERRUN flag was set, * then we will handle this error else where. * If not then we must handle it here.- * This is probably an AFU bug. We will- * attempt a retry to see if that resolves it.+ * This is probably an AFU bug.
I would tend to agree with this statement. ioasa->resid should be zero in an overrun case.
*/
scp->result = (DID_ERROR << 16);
}
Reviewed-by: Brian King <redacted>
--
Brian King
Power Linux I/O
IBM Linux Technology Center
From: Brian King <hidden> Date: 2015-09-21 22:02:46
On 09/16/2015 04:30 PM, Matthew R. Ochs wrote:
At present, both ports must be online for the device to
configure properly. Remove this dependency and the unnecessary
internal LUN override logic as well. Additionally, as a refactoring
measure, change the return code variable name to match that used
throughout the driver.
Doesn't this also change the behavior to no longer fail init_afu even
if BOTH ports fail to go offline in the reconfig case. Is that OK?
-Brian
quoted hunk
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
@@ -1039,33 +1039,26 @@ static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn)FC_PORT_STATUS_RETRY_CNT)){pr_debug("%s: wait on port %d to go offline timed out\n",__func__,port);-ret=-1;/* but continue on to leave the port back online */+rc=-1;/* but continue on to leave the port back online */}-if(ret==0)+if(rc==0)writeq_be(wwpn,&fc_regs[FC_PNAME/8]);+/* Always return success after programming WWPN */+rc=0;+set_port_online(fc_regs);if(!wait_port_online(fc_regs,FC_PORT_STATUS_RETRY_INTERVAL_US,FC_PORT_STATUS_RETRY_CNT)){pr_debug("%s: wait on port %d to go online timed out\n",__func__,port);-ret=-1;--/*-*Overrideforinternallun!!!-*/-if(afu->internal_lun){-pr_debug("%s: Overriding port %d online timeout!!!\n",-__func__,port);-ret=0;-}}-pr_debug("%s: returning rc=%d\n",__func__,ret);+pr_debug("%s: returning rc=%d\n",__func__,rc);-returnret;+returnrc;}/**
--
Brian King
Power Linux I/O
IBM Linux Technology Center
From: Matthew R. Ochs <hidden> Date: 2015-09-21 22:12:01
On Sep 21, 2015, at 6:36 AM, Tomas Henzl [off-list ref] wrote:
On 16.9.2015 23:26, Matthew R. Ochs wrote:
quoted
From: Manoj Kumar <redacted>
=20
The timeout value for read capacity is too small. Certain devices
may take longer to respond and thus the command may prematurely
timeout. Additionally the literal used for the timeout is stale.
=20
Update the timeout to 30 seconds (matches the value used in sd.c)
and rework the timeout literal to a more appropriate description.
=20
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 9 ++++-----
drivers/scsi/cxlflash/superpipe.h | 2 +-
drivers/scsi/cxlflash/vlun.c | 4 ++--
3 files changed, 7 insertions(+), 8 deletions(-)
=20
From: Matthew R. Ochs <hidden> Date: 2015-09-21 22:32:48
On Sep 21, 2015, at 7:11 AM, Tomas Henzl [off-list ref] wrote:
On 16.9.2015 23:27, Matthew R. Ochs wrote:
quoted
When a LUN is removed, the sdev that is associated with the LUN
remains intact until its reference count drops to 0. In order
to prevent an sdev from being removed while a context is still
associated with it, obtain an additional reference per-context
for each LUN attached to the context.
=20
This resolves a potential Oops in the release handler when a
dealing with a LUN that has already been removed.
=20
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
Suggested-by: Brian King <redacted>
---
drivers/scsi/cxlflash/superpipe.c | 36 =
@@ -880,6 +880,9 @@ static int _cxlflash_disk_detach(struct =
scsi_device *sdev,
quoted
sys_close(lfd);
}
=20
+ /* Release the sdev reference that bound this LUN to the context =
*/
quoted
+ scsi_device_put(sdev);
+
=20
I'm not sure here with the use if scsi_device_get+put, also I don't =
quite well
understand what you are going to fix here and how can it happen.
The scsi_device_get takes an additional module reference, so if used =
from
a module it shouldn't be held for a long time.
The issue here is that the user context needs to be bound to the device =
so that
in the event that device goes away, it doesn't completely go away until =
the user
context is done using it. Without it, it is possible to crash when the =
context is
being freed.
Essentially this is the same as incrementing the count when an open is =
performed
on the device. The device can be removed (and is hidden upon doing so) =
but is
not actually freed until the reference is resolved (close()).
Is it possible for a user to rmmod the czlflash module
after the disk attach function is called?
From: Matthew R. Ochs <hidden> Date: 2015-09-21 22:44:44
On Sep 21, 2015, at 7:25 AM, Tomas Henzl [off-list ref] wrote:
On 16.9.2015 23:31, Matthew R. Ochs wrote:
quoted
The workq can process work in parallel with a remove event, leading
to a condition where the workq handler can access freed memory.
To remedy, the workq should be terminated prior to freeing memory. Move
the termination call earlier in remove and use cancel_work_sync() instead
of flush_work() as there is not a need to process any scheduled work when
shutting down.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
scsi_remove_host(cfg->host);
/* Fall through */
case INIT_STATE_AFU:
+ cancel_work_sync(&cfg->work_q);
term_afu(cfg);
You disable irqs after a call to cancel_work_sync.
That means a late int could trigger the workqueue again?
Please disable irqs earlier - as described in Documentation/PCI/pci.txt
I'll change the order here such that the work is cancelled after
term_afu() is called.
-matt
From: Matthew R. Ochs <hidden> Date: 2015-09-21 22:59:49
On Sep 21, 2015, at 7:44 AM, Tomas Henzl [off-list ref] wrote:
On 16.9.2015 23:32, Matthew R. Ochs wrote:
quoted
The adapter state machine is susceptible to missing and/or
corrupting state updates at runtime. This can lead to a variety
of unintended issues and is due to the lack of a serialization
mechanism to protect the adapter state.
Use an adapter-wide mutex to serialize state changes.
I've just briefly looked into your code, but it seems to me that
an atomic variable would serve your needs also and might be
more effective resulting in a faster code execution?
Will keep this in mind.
If you keep the mutex way you don't need two mutexes
in cxlflash_afu_sync - you should remove the mutex &sync_active
From: Matthew R. Ochs <hidden> Date: 2015-09-21 23:05:54
On Sep 21, 2015, at 1:24 PM, Brian King [off-list ref] wrote:
On 09/16/2015 04:30 PM, Matthew R. Ochs wrote:
quoted
Borrowing the TMF waitq's spinlock causes a stall condition when
waiting for the TMF to complete. To remedy, introduce our own spin
lock to serialize TMF and use the appropriate wait services.
Can you clarify what stall condition you were seeing. Its not obvious
to me what this fixes. Do you have soft lockup logs from the failure?
I believe we saw cascading RCU stalls.
I couldn't find any more details in my notes or development commits.
Unfortunately the logs are long gone as this was fixed in June.
From: Matthew R. Ochs <hidden> Date: 2015-09-22 20:44:41
On Sep 21, 2015, at 5:02 PM, Brian King [off-list ref] wrote:
On 09/16/2015 04:30 PM, Matthew R. Ochs wrote:
quoted
At present, both ports must be online for the device to
configure properly. Remove this dependency and the unnecessary
internal LUN override logic as well. Additionally, as a refactoring
measure, change the return code variable name to match that used
throughout the driver.
Doesn't this also change the behavior to no longer fail init_afu even
if BOTH ports fail to go offline in the reconfig case. Is that OK?
Correct, there is a change in behavior but it is not an issue.
From: Brian King <hidden> Date: 2015-09-22 20:50:12
On 09/22/2015 03:44 PM, Matthew R. Ochs wrote:
quoted
On Sep 21, 2015, at 5:02 PM, Brian King [off-list ref] wrote:
On 09/16/2015 04:30 PM, Matthew R. Ochs wrote:
quoted
At present, both ports must be online for the device to
configure properly. Remove this dependency and the unnecessary
internal LUN override logic as well. Additionally, as a refactoring
measure, change the return code variable name to match that used
throughout the driver.
Doesn't this also change the behavior to no longer fail init_afu even
if BOTH ports fail to go offline in the reconfig case. Is that OK?
Correct, there is a change in behavior but it is not an issue.
ok.
Reviewed-by: Brian King <redacted>
--
Brian King
Power Linux I/O
IBM Linux Technology Center
From: Brian King <hidden> Date: 2015-09-22 20:53:13
On 09/16/2015 04:30 PM, Matthew R. Ochs wrote:
quoted hunk
Currently, scsi_host_put() is being called prematurely in the
remove path and is missing entirely in an error cleanup path.
The former can lead to memory being freed too early with
subsequent access potentially corrupting data whilst the former
would result in a memory leak.
Move the usage on remove to be the last cleanup action taken
and introduce a call to scsi_host_put() in the one initialization
error path that does not use remove to cleanup.
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Matthew R. Ochs <hidden> Date: 2015-09-22 21:50:03
On Sep 22, 2015, at 3:53 PM, Brian King [off-list ref] =
wrote:
On 09/16/2015 04:30 PM, Matthew R. Ochs wrote:
quoted
Currently, scsi_host_put() is being called prematurely in the
remove path and is missing entirely in an error cleanup path.
The former can lead to memory being freed too early with
subsequent access potentially corrupting data whilst the former
would result in a memory leak.
=20
Move the usage on remove to be the last cleanup action taken
and introduce a call to scsi_host_put() in the one initialization
error path that does not use remove to cleanup.
=20
Signed-off-by: Matthew R. Ochs <redacted>
Signed-off-by: Manoj N. Kumar <redacted>
---
drivers/scsi/cxlflash/main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
=20
case INIT_STATE_SCSI:
cxlflash_term_local_luns(cfg);
scsi_remove_host(cfg->host);
- scsi_host_put(cfg->host);
/* Fall through */
case INIT_STATE_AFU:
term_afu(cfg);