On Tue, 2017-08-08 at 13:07 +0530, Sumit Saxena wrote: > > > > -----Original Message----- > > From: Martin K. Petersen [mailto:martin.petersen@xxxxxxxxxx] > > Sent: Monday, August 07, 2017 11:07 PM > > To: Tomas Henzl > > Cc: linux-scsi@xxxxxxxxxxxxxxx; sumit.saxena@xxxxxxxxxxxx; > > kashyap.desai@xxxxxxxxxxxx > > Subject: Re: [PATCH] megaraid_sas: move command counter to correct > > place > > > > > > Tomas, > > > > > > > > the eh reset function returns success when fw_outstanding equals > > > zero, > > > that means that the counter shouldn't be decremented when the > > > driver > > > still owns the command > > > > Applied to 4.13/scsi-fixes. Thank you! > > Just realized that this patch may cause performance regression. > With this patch below scenario may occur- > > -Consider outstanding IOs reaches to controller's Queue depth. > -Driver frees up command and complete it back to SML. > -Since host_busy is decremented, SML can issue one new IO to driver. > -By the time, if "fw_outstanding" is not decremented by driver, then > driver will return newly submitted IO back to SML with return status > SCSI_MLQUEUE_HOST_BUSY because of below code > in megaraid_sas driver's IO submission path- > > if (atomic_inc_return(&instance->fw_outstanding) > > instance->host->can_queue) { > atomic_dec(&instance->fw_outstanding); > return SCSI_MLQUEUE_HOST_BUSY; > } > > This situation will be more evident when RAID1 fastpath IOs are > running as in that case driver will be issuing two IOs to firmware > for single IO issued from SML. Above situation can be avoided, if > this patch is removed. > > Regarding Tomas' concern, there should not be any problem as driver > calls "synchronize_irq" before checking "fw_outstanding". Once > fw_outstanding is decremented and driver frees up command, then only > driver will be checking "fw_outstanding" equals to zero or not so all > this will always fall in a sequence and will not cause the problem > stated by Tomas. > > I am sorry for confusion and would request to revert this patch. OK, I've taken it out of the fixes tree. Martin: this means my fixes branch got rebased; can you rebase your fixes branch on top of mine before we all get a beating from Stephen Rothwell because of a mismerge in linux-next due to the tree differences? Thanks, James
