Linux block layer
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-block@vger.kernel.org
Subject: [bug?] blk_queue_may_bounce() has the comparison max_low_pfn and max_pfn wrong way
Date: Fri, 28 Oct 2022 19:02:54 +0100	[thread overview]
Message-ID: <Y1wZTtENDq3fvs6n@ZenIV> (raw)

	We have this:

static inline bool blk_queue_may_bounce(struct request_queue *q)
{
        return IS_ENABLED(CONFIG_BOUNCE) &&
                q->limits.bounce == BLK_BOUNCE_HIGH &&
                max_low_pfn >= max_pfn;
}

static inline struct bio *blk_queue_bounce(struct bio *bio,
                struct request_queue *q)
{
        if (unlikely(blk_queue_may_bounce(q) && bio_has_data(bio)))
                return __blk_queue_bounce(bio, q);
        return bio;
}

Now, the last term in expression in blk_queue_may_bounce() is
true only on the boxen where max_pfn is no greater than max_low_pfn.

Unless I'm very confused, that's the boxen where we don't *have*
any highmem pages.

What's more, consider this:

static __init int init_emergency_pool(void)
{
        int ret;

#ifndef CONFIG_MEMORY_HOTPLUG
        if (max_pfn <= max_low_pfn)
                return 0;
#endif

        ret = mempool_init_page_pool(&page_pool, POOL_SIZE, 0);
        BUG_ON(ret);
        pr_info("pool size: %d pages\n", POOL_SIZE);

        init_bounce_bioset();
        return 0;
}

On the same boxen (assuming we've not hotplug) page_pool won't be set up
at all, so we wouldn't be able to bounce any highmem page if we ever
ran into one.

AFAICS, this condition is backwards - it should be

static inline bool blk_queue_may_bounce(struct request_queue *q)
{
        return IS_ENABLED(CONFIG_BOUNCE) &&
                q->limits.bounce == BLK_BOUNCE_HIGH &&
                max_low_pfn < max_pfn;
}

What am I missing here?

             reply	other threads:[~2022-10-28 18:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-28 18:02 Al Viro [this message]
2022-10-28 18:51 ` [bug?] blk_queue_may_bounce() has the comparison max_low_pfn and max_pfn wrong way Jens Axboe
2022-10-28 19:21   ` Al Viro
2022-10-29 16:45     ` Al Viro
2022-10-30  7:50       ` Christoph Hellwig
2022-10-30 18:47         ` Al Viro
2022-11-01 11:11           ` Christoph Hellwig
2022-10-30  7:48     ` Christoph Hellwig
2022-10-28 19:14 ` Al Viro
2022-10-30  7:45 ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y1wZTtENDq3fvs6n@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=hch@lst.de \
    --cc=linux-block@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox