Messages in this thread | | | Date | Mon, 15 Mar 2021 14:40:56 +0800 | From | Can Guo <> | Subject | Re: [PATCH v5 06/10] scsi: ufshpb: Add hpb dev reset response |
| |
On 2021-03-15 09:34, Can Guo wrote: > On 2021-03-02 21:24, Avri Altman wrote: >> The spec does not define what is the host's recommended response when >> the device send hpb dev reset response (oper 0x2). >> >> We will update all active hpb regions: mark them and do that on the >> next >> read. >> >> Signed-off-by: Avri Altman <avri.altman@wdc.com> >> --- >> drivers/scsi/ufs/ufshpb.c | 47 >> ++++++++++++++++++++++++++++++++++++--- >> drivers/scsi/ufs/ufshpb.h | 2 ++ >> 2 files changed, 46 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c >> index 0744feb4d484..0034fa03fdc6 100644 >> --- a/drivers/scsi/ufs/ufshpb.c >> +++ b/drivers/scsi/ufs/ufshpb.c >> @@ -642,7 +642,8 @@ int ufshpb_prep(struct ufs_hba *hba, struct >> ufshcd_lrb *lrbp) >> if (rgn->reads == ACTIVATION_THRESHOLD) >> activate = true; >> spin_unlock_irqrestore(&rgn->rgn_lock, flags); >> - if (activate) { >> + if (activate || >> + test_and_clear_bit(RGN_FLAG_UPDATE, &rgn->rgn_flags)) { >> spin_lock_irqsave(&hpb->rsp_list_lock, flags); >> ufshpb_update_active_info(hpb, rgn_idx, srgn_idx); >> hpb->stats.rb_active_cnt++; >> @@ -1480,6 +1481,20 @@ void ufshpb_rsp_upiu(struct ufs_hba *hba, >> struct ufshcd_lrb *lrbp) >> case HPB_RSP_DEV_RESET: >> dev_warn(&hpb->sdev_ufs_lu->sdev_dev, >> "UFS device lost HPB information during PM.\n"); >> + >> + if (hpb->is_hcm) { >> + struct scsi_device *sdev; > bool need_reset = false; >> + >> + __shost_for_each_device(sdev, hba->host) { >> + struct ufshpb_lu *h = sdev->hostdata; >> + >> + if (!h) >> + continue; >> + >> + need_reset = true; >> + } > > if (need_reset) > schedule_work(&hpb->ufshpb_lun_reset_work); > > At last, scheduling only one reset work shall be enough, otherwise > multiple > reset work can be flying in parallel, so maybe above changes?
Forget about this one, I misunderstood it - reset work is for each ufshpb_lu...
Regards, Can Guo.
> >> + } >> + >> break; >> default: >> dev_notice(&hpb->sdev_ufs_lu->sdev_dev, >> @@ -1594,6 +1609,25 @@ static void >> ufshpb_run_inactive_region_list(struct ufshpb_lu *hpb) >> spin_unlock_irqrestore(&hpb->rsp_list_lock, flags); >> } >> >> +static void ufshpb_reset_work_handler(struct work_struct *work) >> +{ >> + struct ufshpb_lu *hpb; > > struct ufshpb_lu *hpb = container_of(work, struct ufshpb_lu, > ufshpb_lun_reset_work); > >> + struct victim_select_info *lru_info; > > struct victim_select_info *lru_info = &hpb->lru_info; > > This can save some lines. > > Thanks, > Can Guo. > >> + struct ufshpb_region *rgn; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&hpb->rgn_state_lock, flags); >> + >> + list_for_each_entry(rgn, &lru_info->lh_lru_rgn, list_lru_rgn) >> + set_bit(RGN_FLAG_UPDATE, &rgn->rgn_flags); >> + >> + spin_unlock_irqrestore(&hpb->rgn_state_lock, flags); >> +} >> + >> static void ufshpb_normalization_work_handler(struct work_struct >> *work) >> { >> struct ufshpb_lu *hpb; >> @@ -1798,6 +1832,8 @@ static int ufshpb_alloc_region_tbl(struct >> ufs_hba *hba, struct ufshpb_lu *hpb) >> } else { >> rgn->rgn_state = HPB_RGN_INACTIVE; >> } >> + >> + rgn->rgn_flags = 0; >> } >> >> return 0; >> @@ -2012,9 +2048,12 @@ static int ufshpb_lu_hpb_init(struct ufs_hba >> *hba, struct ufshpb_lu *hpb) >> INIT_LIST_HEAD(&hpb->list_hpb_lu); >> >> INIT_WORK(&hpb->map_work, ufshpb_map_work_handler); >> - if (hpb->is_hcm) >> + if (hpb->is_hcm) { >> INIT_WORK(&hpb->ufshpb_normalization_work, >> ufshpb_normalization_work_handler); >> + INIT_WORK(&hpb->ufshpb_lun_reset_work, >> + ufshpb_reset_work_handler); >> + } >> >> hpb->map_req_cache = kmem_cache_create("ufshpb_req_cache", >> sizeof(struct ufshpb_req), 0, 0, NULL); >> @@ -2114,8 +2153,10 @@ static void ufshpb_discard_rsp_lists(struct >> ufshpb_lu *hpb) >> >> static void ufshpb_cancel_jobs(struct ufshpb_lu *hpb) >> { >> - if (hpb->is_hcm) >> + if (hpb->is_hcm) { >> + cancel_work_sync(&hpb->ufshpb_lun_reset_work); >> cancel_work_sync(&hpb->ufshpb_normalization_work); >> + } >> cancel_work_sync(&hpb->map_work); >> } >> >> diff --git a/drivers/scsi/ufs/ufshpb.h b/drivers/scsi/ufs/ufshpb.h >> index 84598a317897..37c1b0ea0c0a 100644 >> --- a/drivers/scsi/ufs/ufshpb.h >> +++ b/drivers/scsi/ufs/ufshpb.h >> @@ -121,6 +121,7 @@ struct ufshpb_region { >> struct list_head list_lru_rgn; >> unsigned long rgn_flags; >> #define RGN_FLAG_DIRTY 0 >> +#define RGN_FLAG_UPDATE 1 >> >> /* region reads - for host mode */ >> spinlock_t rgn_lock; >> @@ -217,6 +218,7 @@ struct ufshpb_lu { >> /* for selecting victim */ >> struct victim_select_info lru_info; >> struct work_struct ufshpb_normalization_work; >> + struct work_struct ufshpb_lun_reset_work; >> >> /* pinned region information */ >> u32 lu_pinned_start;
| |