diff mbox series

[v5,04/10] scsi: ufshpb: Make eviction depends on region's reads

Message ID 20210302132503.224670-5-avri.altman@wdc.com
State Superseded
Headers show
Series Add Host control mode to HPB | expand

Commit Message

Avri Altman March 2, 2021, 1:24 p.m. UTC
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(-)

Comments

Can Guo March 11, 2021, 7:53 a.m. UTC | #1
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,
Avri Altman March 11, 2021, 8:06 a.m. UTC | #2
> > 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.
Can Guo March 15, 2021, 8:30 a.m. UTC | #3
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,
Avri Altman March 16, 2021, 8:34 a.m. UTC | #4
> >       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 mbox series

Patch

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,