RFC: REQ_PRIO use

RFC: REQ_PRIO use

[Damien Le Moal]

include/linux/blk_types.h states that REQ_PRIO intended use is to boost a
request priority in cfq:

__REQ_PRIO,             /* boost priority in cfq */

REQ_PRIO is used by different file systems (ext4, btrfs, f2fs, gfs2, erofs) and
a couple of dm drivers (bcache and dm-zoned), but nobody actually acts on the
flag being set, certainly not cfq since this scheduler is now gone in 4.21.

I think this mandates some cleanups. I can see 2 options:
1) Have bfq act on this flag
2) Remove it entirely and replace its use with explicit setting of BIOs priority
values, either directly through the struct bio bi_ioprio field or indirectly
through setting the issuer task contecxt iopriority or eventually by adding a
bi_ionice field to struct bio. For instance, ext4 sets its journald task context
I/O priority, which result in all BIOs issued by that task to get an explicit
I/O priority value.

If option (2) is the preferred choice, looking at the current use of REQ_PRIO,
removing this flag does not seem difficult at all. In this case, changes by the
current users to explicitly set BIO I/O priorities can be done later with
different patches.

Comments, thoughts ?

Best regards.

-- 
Damien Le Moal
Western Digital Research

Re: RFC: REQ_PRIO use

[Christoph Hellwig]

We have two places that check the flag, the writeback tracking
code:

block/blk-wbt.c:#define REQ_HIPRIO      (REQ_SYNC | REQ_META | REQ_PRIO)
block/blk-wbt.c:        if ((rw & REQ_HIPRIO) || wb_recent_wait(rwb) || current_is_kswapd())

and bcache:

drivers/md/bcache/request.c:        !(bio->bi_opf & REQ_PRIO))
drivers/md/bcache/request.c:        !(bio->bi_opf & REQ_PRIO) &&

the original idea was that not all I/O is a priority we should not
blindly boost priority for metadata.  Of course wbt actually ignores
and it and we may just give in and just check REQ_META, except for
the fact that f2fs now actually submits REQ_PRIO requests without
REQ_META..

Re: RFC: REQ_PRIO use

[Damien Le Moal]

On 2018/11/20 18:39, Christoph Hellwig wrote:
> We have two places that check the flag, the writeback tracking
> code:
> 
> block/blk-wbt.c:#define REQ_HIPRIO      (REQ_SYNC | REQ_META | REQ_PRIO)
> block/blk-wbt.c:        if ((rw & REQ_HIPRIO) || wb_recent_wait(rwb) || current_is_kswapd())
> 
> and bcache:
> 
> drivers/md/bcache/request.c:        !(bio->bi_opf & REQ_PRIO))
> drivers/md/bcache/request.c:        !(bio->bi_opf & REQ_PRIO) &&
> 
> the original idea was that not all I/O is a priority we should not
> blindly boost priority for metadata.  Of course wbt actually ignores
> and it and we may just give in and just check REQ_META, except for
> the fact that f2fs now actually submits REQ_PRIO requests without
> REQ_META..

OK. I will dig further.

But the fact remains that with 4.21, the actual priority boost is gone with cfq
removal. bfq, mq-deadline or kyber should add support. Or should the priority
boosting be done with generic code before the BIO is passed to the scheduler ?
That would also cover for the no scheduler case ("none") and allow propagating
an effective ioprio down to the device if the device driver has that feature
enabled.

-- 
Damien Le Moal
Western Digital Research