On Thu, Aug 12, 2021 at 11:41:58AM -0600, Jens Axboe wrote:
> Indeed. Wonder if we should make that a small helper, as any clear of
> REQ_HIPRI should clear BIO_PERCPU_CACHE as well.
>
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 7e852242f4cc..d2722ecd4d9b 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -821,11 +821,8 @@ static noinline_for_stack bool submit_bio_checks(struct bio *bio)
> }
> }
>
> - if (!test_bit(QUEUE_FLAG_POLL, &q->queue_flags)) {
> - /* can't support alloc cache if we turn off polling */
> - bio_clear_flag(bio, BIO_PERCPU_CACHE);
> - bio->bi_opf &= ~REQ_HIPRI;
> - }
> + if (!test_bit(QUEUE_FLAG_POLL, &q->queue_flags))
> + bio_clear_hipri(bio);
Since BIO_PERCPU_CACHE doesn't work without REQ_HIRPI, should this check
look more like this?
if (!test_bit(QUEUE_FLAG_POLL, &q->queue_flags))
bio->bi_opf &= ~REQ_HIPRI;
if (!(bio->bi_opf & REQ_HIPRI))
bio_clear_flag(bio, BIO_PERCPU_CACHE);
I realise the only BIO_PERCPU_CACHE user in this series never sets it
without REQ_HIPRI, but it looks like a problem waiting to happen if
nothing enforces this pairing: someone could set the CACHE flag on a
QUEUE_FLAG_POLL enabled queue without setting HIPRI and get the wrong
bio_put() action.
