RE: RE: [PATCH v4 7/9] scsi: ufshpb: Add "Cold" regions timer
Hi Avri, > > > > Hi Avri, > > > > > +static void ufshpb_read_to_handler(struct work_struct *work) > > > +{ > > > +struct delayed_work *dwork = to_delayed_work(work); > > > +struct ufshpb_lu *hpb; > > > +struct victim_select_info *lru_info; > > > +struct ufshpb_region *rgn; > > > +unsigned long flags; > > > +LIST_HEAD(expired_list); > > > + > > > +hpb = container_of(dwork, struct ufshpb_lu, ufshpb_read_to_work); > > > + > > > +if (test_and_set_bit(TIMEOUT_WORK_PENDING, > > >work_data_bits)) > > > +return; > > > + > > > +spin_lock_irqsave(>rgn_state_lock, flags); > > > + > > > +lru_info = >lru_info; > > > + > > > +list_for_each_entry(rgn, _info->lh_lru_rgn, list_lru_rgn) { > > > +bool timedout = ktime_after(ktime_get(), > > > rgn->read_timeout); > > > + > > > +if (timedout) { > > > > It is important not just to check the timeout, but how much time has passed. > > If the time exceeded is twice the threshold, the read_timeout_expiries > > value should be reduced by 2 instead of 1. > Theoretically this shouldn't happened. > Please note that POLLING_INTERVAL_MS << READ_TO_MS. > Better add appropriate check when making those configurable. OK, I agree. Thanks, Daejun > > Thanks, > Avri > > > > > +rgn->read_timeout_expiries--; > > > > Thanks, > > Daejun > > > >
RE: [PATCH v4 7/9] scsi: ufshpb: Add "Cold" regions timer
Hi Avri, > +static void ufshpb_read_to_handler(struct work_struct *work) > +{ > +struct delayed_work *dwork = to_delayed_work(work); > +struct ufshpb_lu *hpb; > +struct victim_select_info *lru_info; > +struct ufshpb_region *rgn; > +unsigned long flags; > +LIST_HEAD(expired_list); > + > +hpb = container_of(dwork, struct ufshpb_lu, ufshpb_read_to_work); > + > +if (test_and_set_bit(TIMEOUT_WORK_PENDING, >work_data_bits)) > +return; > + > +spin_lock_irqsave(>rgn_state_lock, flags); > + > +lru_info = >lru_info; > + > +list_for_each_entry(rgn, _info->lh_lru_rgn, list_lru_rgn) { > +bool timedout = ktime_after(ktime_get(), rgn->read_timeout); > + > +if (timedout) { It is important not just to check the timeout, but how much time has passed. If the time exceeded is twice the threshold, the read_timeout_expiries value should be reduced by 2 instead of 1. > +rgn->read_timeout_expiries--; Thanks, Daejun
RE: [PATCH v4 7/9] scsi: ufshpb: Add "Cold" regions timer
> > Hi Avri, > > > +static void ufshpb_read_to_handler(struct work_struct *work) > > +{ > > +struct delayed_work *dwork = to_delayed_work(work); > > +struct ufshpb_lu *hpb; > > +struct victim_select_info *lru_info; > > +struct ufshpb_region *rgn; > > +unsigned long flags; > > +LIST_HEAD(expired_list); > > + > > +hpb = container_of(dwork, struct ufshpb_lu, ufshpb_read_to_work); > > + > > +if (test_and_set_bit(TIMEOUT_WORK_PENDING, > >work_data_bits)) > > +return; > > + > > +spin_lock_irqsave(>rgn_state_lock, flags); > > + > > +lru_info = >lru_info; > > + > > +list_for_each_entry(rgn, _info->lh_lru_rgn, list_lru_rgn) { > > +bool timedout = ktime_after(ktime_get(), > > rgn->read_timeout); > > + > > +if (timedout) { > > It is important not just to check the timeout, but how much time has passed. > If the time exceeded is twice the threshold, the read_timeout_expiries > value should be reduced by 2 instead of 1. Theoretically this shouldn't happened. Please note that POLLING_INTERVAL_MS << READ_TO_MS. Better add appropriate check when making those configurable. Thanks, Avri > > > +rgn->read_timeout_expiries--; > > Thanks, > Daejun