| Message ID | 20231005194129.1882245-3-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Pass data temperature information to UFS devices | expand |
On Thu, Oct 05, 2023 at 12:40:48PM -0700, Bart Van Assche wrote: >A later patch will store the data lifetime in the bio->bi_ioprio member >before the blk-ioprio policy is applied. Make sure that this policy doesn't >clear more bits than necessary. > >Cc: Damien Le Moal <dlemoal@kernel.org> >Cc: Niklas Cassel <niklas.cassel@wdc.com> >Signed-off-by: Bart Van Assche <bvanassche@acm.org> >--- > block/blk-ioprio.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > >diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c >index 4051fada01f1..2db86f153b6d 100644 >--- a/block/blk-ioprio.c >+++ b/block/blk-ioprio.c >@@ -202,7 +202,8 @@ void blkcg_set_ioprio(struct bio *bio) > * to achieve this. > */ > if (IOPRIO_PRIO_CLASS(bio->bi_ioprio) != IOPRIO_CLASS_RT) >- bio->bi_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_RT, 4); >+ ioprio_set_class_and_level(&bio->bi_ioprio, >+ IOPRIO_PRIO_VALUE(IOPRIO_CLASS_RT, 4)); > return; > } > >@@ -213,10 +214,10 @@ void blkcg_set_ioprio(struct bio *bio) > * If the bio I/O priority equals IOPRIO_CLASS_NONE, the cgroup I/O > * priority is assigned to the bio. > */ >- prio = max_t(u16, bio->bi_ioprio, >+ prio = max_t(u16, bio->bi_ioprio & IOPRIO_CLASS_LEVEL_MASK, > IOPRIO_PRIO_VALUE(blkcg->prio_policy, 0)); All 9 bits (including CDL) are not taking part in this decision making now. Maybe you want to exclude only lifetime bits. >- if (prio > bio->bi_ioprio) >- bio->bi_ioprio = prio; >+ if (prio > (bio->bi_ioprio & IOPRIO_CLASS_LEVEL_MASK)) >+ ioprio_set_class_and_level(&bio->bi_ioprio, prio); Same as above.
On 10/5/23 23:36, Kanchan Joshi wrote: > On Thu, Oct 05, 2023 at 12:40:48PM -0700, Bart Van Assche wrote: >> A later patch will store the data lifetime in the bio->bi_ioprio member >> before the blk-ioprio policy is applied. Make sure that this policy >> doesn't >> clear more bits than necessary. >> >> Cc: Damien Le Moal <dlemoal@kernel.org> >> Cc: Niklas Cassel <niklas.cassel@wdc.com> >> Signed-off-by: Bart Van Assche <bvanassche@acm.org> >> --- >> block/blk-ioprio.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c >> index 4051fada01f1..2db86f153b6d 100644 >> --- a/block/blk-ioprio.c >> +++ b/block/blk-ioprio.c >> @@ -202,7 +202,8 @@ void blkcg_set_ioprio(struct bio *bio) >> * to achieve this. >> */ >> if (IOPRIO_PRIO_CLASS(bio->bi_ioprio) != IOPRIO_CLASS_RT) >> - bio->bi_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_RT, 4); >> + ioprio_set_class_and_level(&bio->bi_ioprio, >> + IOPRIO_PRIO_VALUE(IOPRIO_CLASS_RT, 4)); >> return; >> } >> >> @@ -213,10 +214,10 @@ void blkcg_set_ioprio(struct bio *bio) >> * If the bio I/O priority equals IOPRIO_CLASS_NONE, the cgroup I/O >> * priority is assigned to the bio. >> */ >> - prio = max_t(u16, bio->bi_ioprio, >> + prio = max_t(u16, bio->bi_ioprio & IOPRIO_CLASS_LEVEL_MASK, >> IOPRIO_PRIO_VALUE(blkcg->prio_policy, 0)); > > All 9 bits (including CDL) are not taking part in this decision making > now. Maybe you want to exclude only lifetime bits. > >> - if (prio > bio->bi_ioprio) >> - bio->bi_ioprio = prio; >> + if (prio > (bio->bi_ioprio & IOPRIO_CLASS_LEVEL_MASK)) >> + ioprio_set_class_and_level(&bio->bi_ioprio, prio); > > Same as above. Hi Kanchan, It is intentional that the CDL bits are left out from these decisions. I think the decisions made in this policy should be based on the I/O priority class and level only and not on the CDL value. Thanks, Bart.
diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c index 4051fada01f1..2db86f153b6d 100644 --- a/block/blk-ioprio.c +++ b/block/blk-ioprio.c @@ -202,7 +202,8 @@ void blkcg_set_ioprio(struct bio *bio) * to achieve this. */ if (IOPRIO_PRIO_CLASS(bio->bi_ioprio) != IOPRIO_CLASS_RT) - bio->bi_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_RT, 4); + ioprio_set_class_and_level(&bio->bi_ioprio, + IOPRIO_PRIO_VALUE(IOPRIO_CLASS_RT, 4)); return; } @@ -213,10 +214,10 @@ void blkcg_set_ioprio(struct bio *bio) * If the bio I/O priority equals IOPRIO_CLASS_NONE, the cgroup I/O * priority is assigned to the bio. */ - prio = max_t(u16, bio->bi_ioprio, + prio = max_t(u16, bio->bi_ioprio & IOPRIO_CLASS_LEVEL_MASK, IOPRIO_PRIO_VALUE(blkcg->prio_policy, 0)); - if (prio > bio->bi_ioprio) - bio->bi_ioprio = prio; + if (prio > (bio->bi_ioprio & IOPRIO_CLASS_LEVEL_MASK)) + ioprio_set_class_and_level(&bio->bi_ioprio, prio); } void blk_ioprio_exit(struct gendisk *disk)
A later patch will store the data lifetime in the bio->bi_ioprio member before the blk-ioprio policy is applied. Make sure that this policy doesn't clear more bits than necessary. Cc: Damien Le Moal <dlemoal@kernel.org> Cc: Niklas Cassel <niklas.cassel@wdc.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/blk-ioprio.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)