| Message ID | 20210302132503.224670-5-avri.altman@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Host control mode to HPB | expand |
Hi Avri, On 2021-03-02 21:24, Avri Altman wrote: > In host mode, eviction is considered an extreme measure. > verify that the entering region has enough reads, and the exiting > region has much less reads. > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > --- > drivers/scsi/ufs/ufshpb.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c > index a8f8d13af21a..6f4fd22eaf2f 100644 > --- a/drivers/scsi/ufs/ufshpb.c > +++ b/drivers/scsi/ufs/ufshpb.c > @@ -17,6 +17,7 @@ > #include "../sd.h" > > #define ACTIVATION_THRESHOLD 4 /* 4 IOs */ > +#define EVICTION_THRESHOLD (ACTIVATION_THRESHOLD << 6) /* 256 IOs */ Same here, can this be added as a sysfs entry? Thanks, Can Guo. > > /* memory management */ > static struct kmem_cache *ufshpb_mctx_cache; > @@ -1050,6 +1051,13 @@ static struct ufshpb_region > *ufshpb_victim_lru_info(struct ufshpb_lu *hpb) > if (ufshpb_check_srgns_issue_state(hpb, rgn)) > continue; > > + /* > + * in host control mode, verify that the exiting region > + * has less reads > + */ > + if (hpb->is_hcm && rgn->reads > (EVICTION_THRESHOLD >> 1)) > + continue; > + > victim_rgn = rgn; > break; > } > @@ -1235,7 +1243,7 @@ static int ufshpb_issue_map_req(struct ufshpb_lu > *hpb, > > static int ufshpb_add_region(struct ufshpb_lu *hpb, struct > ufshpb_region *rgn) > { > - struct ufshpb_region *victim_rgn; > + struct ufshpb_region *victim_rgn = NULL; > struct victim_select_info *lru_info = &hpb->lru_info; > unsigned long flags; > int ret = 0; > @@ -1263,6 +1271,16 @@ static int ufshpb_add_region(struct ufshpb_lu > *hpb, struct ufshpb_region *rgn) > * because the device could detect this region > * by not issuing HPB_READ > */ > + > + /* > + * in host control mode, verify that the entering > + * region has enough reads > + */ > + if (hpb->is_hcm && rgn->reads < EVICTION_THRESHOLD) { > + ret = -EACCES; > + goto out; > + } > + > victim_rgn = ufshpb_victim_lru_info(hpb); > if (!victim_rgn) { > dev_warn(&hpb->sdev_ufs_lu->sdev_dev,
> > diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c > > index a8f8d13af21a..6f4fd22eaf2f 100644 > > --- a/drivers/scsi/ufs/ufshpb.c > > +++ b/drivers/scsi/ufs/ufshpb.c > > @@ -17,6 +17,7 @@ > > #include "../sd.h" > > > > #define ACTIVATION_THRESHOLD 4 /* 4 IOs */ > > +#define EVICTION_THRESHOLD (ACTIVATION_THRESHOLD << 6) /* 256 IOs > */ > > Same here, can this be added as a sysfs entry? Yes. Please see last patch. Thanks, Avri > > Thanks, > Can Guo.
On 2021-03-02 21:24, Avri Altman wrote: > In host mode, eviction is considered an extreme measure. > verify that the entering region has enough reads, and the exiting > region has much less reads. > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > --- > drivers/scsi/ufs/ufshpb.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c > index a8f8d13af21a..6f4fd22eaf2f 100644 > --- a/drivers/scsi/ufs/ufshpb.c > +++ b/drivers/scsi/ufs/ufshpb.c > @@ -17,6 +17,7 @@ > #include "../sd.h" > > #define ACTIVATION_THRESHOLD 4 /* 4 IOs */ > +#define EVICTION_THRESHOLD (ACTIVATION_THRESHOLD << 6) /* 256 IOs */ > > /* memory management */ > static struct kmem_cache *ufshpb_mctx_cache; > @@ -1050,6 +1051,13 @@ static struct ufshpb_region > *ufshpb_victim_lru_info(struct ufshpb_lu *hpb) > if (ufshpb_check_srgns_issue_state(hpb, rgn)) > continue; > > + /* > + * in host control mode, verify that the exiting region > + * has less reads > + */ > + if (hpb->is_hcm && rgn->reads > (EVICTION_THRESHOLD >> 1)) > + continue; > + > victim_rgn = rgn; > break; > } > @@ -1235,7 +1243,7 @@ static int ufshpb_issue_map_req(struct ufshpb_lu > *hpb, > > static int ufshpb_add_region(struct ufshpb_lu *hpb, struct > ufshpb_region *rgn) > { > - struct ufshpb_region *victim_rgn; > + struct ufshpb_region *victim_rgn = NULL; > struct victim_select_info *lru_info = &hpb->lru_info; > unsigned long flags; > int ret = 0; > @@ -1263,6 +1271,16 @@ static int ufshpb_add_region(struct ufshpb_lu > *hpb, struct ufshpb_region *rgn) > * because the device could detect this region > * by not issuing HPB_READ > */ > + > + /* > + * in host control mode, verify that the entering > + * region has enough reads > + */ Maybe merge the new comments with the original comments above? Thanks, Can Guo. > + if (hpb->is_hcm && rgn->reads < EVICTION_THRESHOLD) { > + ret = -EACCES; > + goto out; > + } > + > victim_rgn = ufshpb_victim_lru_info(hpb); > if (!victim_rgn) { > dev_warn(&hpb->sdev_ufs_lu->sdev_dev,
> > int ret = 0; > > @@ -1263,6 +1271,16 @@ static int ufshpb_add_region(struct ufshpb_lu > > *hpb, struct ufshpb_region *rgn) > > * because the device could detect this region > > * by not issuing HPB_READ > > */ > > + > > + /* > > + * in host control mode, verify that the entering > > + * region has enough reads > > + */ > > Maybe merge the new comments with the original comments above? Done. Thanks, Avri > Thanks, > Can Guo.
diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c index a8f8d13af21a..6f4fd22eaf2f 100644 --- a/drivers/scsi/ufs/ufshpb.c +++ b/drivers/scsi/ufs/ufshpb.c @@ -17,6 +17,7 @@ #include "../sd.h" #define ACTIVATION_THRESHOLD 4 /* 4 IOs */ +#define EVICTION_THRESHOLD (ACTIVATION_THRESHOLD << 6) /* 256 IOs */ /* memory management */ static struct kmem_cache *ufshpb_mctx_cache; @@ -1050,6 +1051,13 @@ static struct ufshpb_region *ufshpb_victim_lru_info(struct ufshpb_lu *hpb) if (ufshpb_check_srgns_issue_state(hpb, rgn)) continue; + /* + * in host control mode, verify that the exiting region + * has less reads + */ + if (hpb->is_hcm && rgn->reads > (EVICTION_THRESHOLD >> 1)) + continue; + victim_rgn = rgn; break; } @@ -1235,7 +1243,7 @@ static int ufshpb_issue_map_req(struct ufshpb_lu *hpb, static int ufshpb_add_region(struct ufshpb_lu *hpb, struct ufshpb_region *rgn) { - struct ufshpb_region *victim_rgn; + struct ufshpb_region *victim_rgn = NULL; struct victim_select_info *lru_info = &hpb->lru_info; unsigned long flags; int ret = 0; @@ -1263,6 +1271,16 @@ static int ufshpb_add_region(struct ufshpb_lu *hpb, struct ufshpb_region *rgn) * because the device could detect this region * by not issuing HPB_READ */ + + /* + * in host control mode, verify that the entering + * region has enough reads + */ + if (hpb->is_hcm && rgn->reads < EVICTION_THRESHOLD) { + ret = -EACCES; + goto out; + } + victim_rgn = ufshpb_victim_lru_info(hpb); if (!victim_rgn) { dev_warn(&hpb->sdev_ufs_lu->sdev_dev,
In host mode, eviction is considered an extreme measure. verify that the entering region has enough reads, and the exiting region has much less reads. Signed-off-by: Avri Altman <avri.altman@wdc.com> --- drivers/scsi/ufs/ufshpb.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)