> On Mar 26, 2021, at 12:46 PM, Quinn Tran <qutran@xxxxxxxxxxx> wrote: > > > >> On Mar 23, 2021, at 11:31 AM, Bart Van Assche <bvanassche@xxxxxxx> wrote: >> >> On 3/22/21 9:42 PM, Nilesh Javali wrote: >>> diff --git a/drivers/scsi/qla2xxx/qla_target.c >>> b/drivers/scsi/qla2xxx/qla_target.c >>> index c48daf52725d..fa8c4dae8dce 100644 >>> --- a/drivers/scsi/qla2xxx/qla_target.c >>> +++ b/drivers/scsi/qla2xxx/qla_target.c >>> @@ -1029,7 +1029,7 @@ void qlt_free_session_done(struct work_struct *work) >>> } >>> msleep(100); >>> cnt++; >>> - if (cnt > 200) >>> + if (cnt > 230) >>> break; >>> } >> >> One magic constant is changed into another magic constant and that is >> sufficient to fix a bug? Please add a comment that explains the >> meaning of that constant. >> > > Agree with Bart here. > > How did you come up with this new count value? Some more details (either in commit message or actual comment in code) would definitely help understand. If you have some log message snippet or stack trace add that to commit message. > > QT: FW timeout is 20seconds (cnt=200). Driver time out is set at 22 seconds (220) to monitor the logout (2 seconds pad for worst case). Changing from 200 -> 230 to allow the logout thread to finish its sequence before advancing the state. Previous setting at 200/20s create a race condition where driver was allow to advance to relogin, while the logout completion straddles behind and modifies fields that interfere with the relogin. This led to session being stuck. > Would you add this as a comment above the if() statement for the future would be nice. You can add my R-B to the patch > -- Himanshu Madhani Oracle Linux Engineering
