On 12/5/23 16:42, Damien Le Moal wrote:
On 12/5/23 14:32, Bart Van Assche wrote:
Fix the following two issues:
- Even with prio_aging_expire set to zero, I/O priorities still affect the
request order.
- Assigning I/O priorities with the ioprio cgroup policy breaks zoned
storage support in the mq-deadline scheduler.
Can you provide more details ? E.g. an example of a setup that breaks ordering ?
Regarding prio_aging_expire that is set to zero: a third party Android
developer reported that request prioritization in
mq-deadline is not compatible with MMC devices
(https://android-review.googlesource.com/c/kernel/common/+/2836235).
Regarding zoned storage and I/O priorities: we are working on modifying
Android such that foreground apps have a higher I/O priority than
background apps. As one can see in
https://android-review.googlesource.com/c/platform/system/core/+/2768906,
we plan to use the ioprio cgroup policy to implement this. We noticed
that this CL breaks zoned storage support if the mq-deadline I/O
scheduler has been selected. I think that this should be solved in the
mq-deadline I/O
scheduler and also that ignoring I/O priorities for zoned writes that
must be submitted in order is a good solution. Respecting the I/O
priority for writes would require to make significant changes in the
layer that submits the zoned writes (F2FS): it would require that that
layer is modified such that it adds writes from foreground apps to the
log before those of background apps.
-static u8 dd_rq_ioclass(struct request *rq)
+static u8 dd_rq_ioclass(struct deadline_data *dd, struct request *rq)
{
- return IOPRIO_PRIO_CLASS(req_get_ioprio(rq));
+ return dd->prio_aging_expire ? IOPRIO_PRIO_CLASS(req_get_ioprio(rq)) :
+ IOPRIO_CLASS_NONE;
I personally would prefer the simpler:
if (!dd->prio_aging_expire)
return IOPRIO_CLASS_NONE;
return IOPRIO_PRIO_CLASS(req_get_ioprio(rq));
}
-static u8 dd_bio_ioclass(struct bio *bio)
+static u8 dd_bio_ioclass(struct deadline_data *dd, struct bio *bio)
{
- return IOPRIO_PRIO_CLASS(bio->bi_ioprio);
+ return dd->prio_aging_expire ? IOPRIO_PRIO_CLASS(bio->bi_ioprio) :
+ IOPRIO_CLASS_NONE;
Same comment as above.
I will make the proposed changes.
What about the call to dd_dispatch_prio_aged_requests() in
dd_dispatch_request() ? Shouldn't that call be skipped if prio_aging_expire is 0 ?
That sounds like a useful optimization to me. I will look into this.
Thanks,
Bart.
