Hi Martin,
This patch series include several hpb fixes, most of them host mode.
Please consider this patch series for kernel v5.15.
Thanks,
Avri
Avri Altman (4):
scsi: ufshpb: re-wind the read timeout on every read
scsi: ufshpb: Use a correct max multi chunk
scsi: ufshpb: Verify that num_inflight_map_req is non-negative
scsi: ufshpb: Do not report victim error in HCM
drivers/scsi/ufs/ufshpb.c | 29 +++++++++++++++++++++++++----
drivers/scsi/ufs/ufshpb.h | 6 ++++--
2 files changed, 29 insertions(+), 6 deletions(-)
--
2.17.1
The "cold"-timer purpose is not to hang-on to active regions with no
reads. Therefore the read-timeout should be re-wind on every read, and
not just when the region is activated.
Fixes: 13c044e91678 (scsi: ufs: ufshpb: Add "cold" regions timer)
Signed-off-by: Avri Altman <[email protected]>
---
drivers/scsi/ufs/ufshpb.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
index d0eb14be47a3..8e92c61ed9d4 100644
--- a/drivers/scsi/ufs/ufshpb.c
+++ b/drivers/scsi/ufs/ufshpb.c
@@ -178,9 +178,19 @@ static void ufshpb_iterate_rgn(struct ufshpb_lu *hpb, int rgn_idx, int srgn_idx,
set_bit_len = cnt;
spin_lock_irqsave(&hpb->rgn_state_lock, flags);
- if (set_dirty && rgn->rgn_state != HPB_RGN_INACTIVE &&
- srgn->srgn_state == HPB_SRGN_VALID)
- bitmap_set(srgn->mctx->ppn_dirty, srgn_offset, set_bit_len);
+ if (rgn->rgn_state != HPB_RGN_INACTIVE) {
+ if (set_dirty) {
+ if (srgn->srgn_state == HPB_SRGN_VALID)
+ bitmap_set(srgn->mctx->ppn_dirty, srgn_offset,
+ set_bit_len);
+ } else if (hpb->is_hcm) {
+ /* rewind the read timer for lru regions */
+ rgn->read_timeout = ktime_add_ms(ktime_get(),
+ rgn->hpb->params.read_timeout_ms);
+ rgn->read_timeout_expiries =
+ rgn->hpb->params.read_timeout_expiries;
+ }
+ }
spin_unlock_irqrestore(&hpb->rgn_state_lock, flags);
if (hpb->is_hcm && prev_srgn != srgn) {
--
2.17.1
num_inflight_map_req should not be negative. It is incremented and
decremented without any protection, allowing it theoretically to be
negative, should some weird unbalanced count occur.
Verify that the those calls are properly serialized.
Fixes: 33845a2d844b (scsi: ufs: ufshpb: Limit the number of in-flight map requests)
Signed-off-by: Avri Altman <[email protected]>
---
drivers/scsi/ufs/ufshpb.c | 10 ++++++++++
drivers/scsi/ufs/ufshpb.h | 4 +++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
index 8e92c61ed9d4..cd48367f94cc 100644
--- a/drivers/scsi/ufs/ufshpb.c
+++ b/drivers/scsi/ufs/ufshpb.c
@@ -756,6 +756,7 @@ static struct ufshpb_req *ufshpb_get_map_req(struct ufshpb_lu *hpb,
{
struct ufshpb_req *map_req;
struct bio *bio;
+ unsigned long flags;
if (hpb->is_hcm &&
hpb->num_inflight_map_req >= hpb->params.inflight_map_req) {
@@ -780,7 +781,10 @@ static struct ufshpb_req *ufshpb_get_map_req(struct ufshpb_lu *hpb,
map_req->rb.srgn_idx = srgn->srgn_idx;
map_req->rb.mctx = srgn->mctx;
+
+ spin_lock_irqsave(&hpb->param_lock, flags);
hpb->num_inflight_map_req++;
+ spin_unlock_irqrestore(&hpb->param_lock, flags);
return map_req;
}
@@ -788,9 +792,14 @@ static struct ufshpb_req *ufshpb_get_map_req(struct ufshpb_lu *hpb,
static void ufshpb_put_map_req(struct ufshpb_lu *hpb,
struct ufshpb_req *map_req)
{
+ unsigned long flags;
+
bio_put(map_req->bio);
ufshpb_put_req(hpb, map_req);
+
+ spin_lock_irqsave(&hpb->param_lock, flags);
hpb->num_inflight_map_req--;
+ spin_unlock_irqrestore(&hpb->param_lock, flags);
}
static int ufshpb_clear_dirty_bitmap(struct ufshpb_lu *hpb,
@@ -2387,6 +2396,7 @@ static int ufshpb_lu_hpb_init(struct ufs_hba *hba, struct ufshpb_lu *hpb)
spin_lock_init(&hpb->rgn_state_lock);
spin_lock_init(&hpb->rsp_list_lock);
+ spin_lock_init(&hpb->param_lock);
INIT_LIST_HEAD(&hpb->lru_info.lh_lru_rgn);
INIT_LIST_HEAD(&hpb->lh_act_srgn);
diff --git a/drivers/scsi/ufs/ufshpb.h b/drivers/scsi/ufs/ufshpb.h
index 6df317dfe034..a79e07398970 100644
--- a/drivers/scsi/ufs/ufshpb.h
+++ b/drivers/scsi/ufs/ufshpb.h
@@ -237,7 +237,9 @@ struct ufshpb_lu {
struct ufshpb_req *pre_req;
int num_inflight_pre_req;
int throttle_pre_req;
- int num_inflight_map_req;
+ int num_inflight_map_req; /* hold param_lock */
+ spinlock_t param_lock;
+
struct list_head lh_pre_req_free;
int cur_read_id;
int pre_req_min_tr_len;
--
2.17.1
In HPB2.0, if pre_req_min_tr_len < transfer_len < pre_req_max_tr_len
the driver is expected to send a HPB-WRITE-BUFFER companion to HPB-READ.
The upper bound should fit into a single byte, regardless of
bMAX_ DATA_SIZE_FOR_HPB_SINGLE_CMD which being an attribute (u32) can
be significantly larger.
To further illustrate the issue let us consider the following scenario:
- SCSI_DEFAULT_MAX_SECTORS is 1024 limiting the IO chunks to 512KB
- The OEM changes scsi_host_template .max_sectors to be 2048, which
allows a 1M requests: transfer_len = 256
- pre_req_max_tr_len = HPB_MULTI_CHUNK_HIGH = 256
- ufshpb_is_supported_chunk returns true (256 <= 256)
- WARN_ON_ONCE(256 > 256) doesn't warn
- ufshpb_set_hpb_read_to_upiu cast transfer_len to u8: transfer_len = 0
- the command is failing with illegal request
Fixes: 41d8a9333cc9 (scsi: ufs: ufshpb: Add HPB 2.0 support)
Signed-off-by: Avri Altman <[email protected]>
---
drivers/scsi/ufs/ufshpb.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshpb.h b/drivers/scsi/ufs/ufshpb.h
index c74a6c35a446..6df317dfe034 100644
--- a/drivers/scsi/ufs/ufshpb.h
+++ b/drivers/scsi/ufs/ufshpb.h
@@ -32,7 +32,7 @@
/* hpb support chunk size */
#define HPB_LEGACY_CHUNK_HIGH 1
#define HPB_MULTI_CHUNK_LOW 7
-#define HPB_MULTI_CHUNK_HIGH 256
+#define HPB_MULTI_CHUNK_HIGH 255
/* hpb vender defined opcode */
#define UFSHPB_READ 0xF8
--
2.17.1
In host control mode, eviction is perceived as an extreme measure.
There are several conditions that both the entering and exiting regions
should meet, so that eviction will take place.
The common case however, is that those conditions are rarely met, so it
is normal that the act of eviction fails. Therefore, Do not report an
error in host control mode if eviction fails.
Fixes: 6c59cb501b86 (scsi: ufs: ufshpb: Make eviction depend on region's reads)
Signed-off-by: Avri Altman <[email protected]>
---
drivers/scsi/ufs/ufshpb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
index cd48367f94cc..aafb55136c7e 100644
--- a/drivers/scsi/ufs/ufshpb.c
+++ b/drivers/scsi/ufs/ufshpb.c
@@ -1385,7 +1385,8 @@ static int ufshpb_add_region(struct ufshpb_lu *hpb, struct ufshpb_region *rgn)
victim_rgn = ufshpb_victim_lru_info(hpb);
if (!victim_rgn) {
dev_warn(&hpb->sdev_ufs_lu->sdev_dev,
- "cannot get victim region error\n");
+ "cannot get victim region %s\n",
+ hpb->is_hcm ? "" : "error");
ret = -ENOMEM;
goto out;
}
--
2.17.1
Avri,
> This patch series include several hpb fixes, most of them host mode.
> Please consider this patch series for kernel v5.15.
Applied to 5.15/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
On Sun, 8 Aug 2021 12:00:20 +0300, Avri Altman wrote:
> This patch series include several hpb fixes, most of them host mode.
> Please consider this patch series for kernel v5.15.
>
> Thanks,
> Avri
>
> Avri Altman (4):
> scsi: ufshpb: re-wind the read timeout on every read
> scsi: ufshpb: Use a correct max multi chunk
> scsi: ufshpb: Verify that num_inflight_map_req is non-negative
> scsi: ufshpb: Do not report victim error in HCM
>
> [...]
Applied to 5.15/scsi-queue, thanks!
[1/4] scsi: ufshpb: re-wind the read timeout on every read
https://git.kernel.org/mkp/scsi/c/283e61c5a9be
[2/4] scsi: ufshpb: Use a correct max multi chunk
https://git.kernel.org/mkp/scsi/c/07106f86ae13
[3/4] scsi: ufshpb: Verify that num_inflight_map_req is non-negative
https://git.kernel.org/mkp/scsi/c/22aede9f48b6
[4/4] scsi: ufshpb: Do not report victim error in HCM
https://git.kernel.org/mkp/scsi/c/10163cee1f06
--
Martin K. Petersen Oracle Linux Engineering