On 10/28/22 12:02 PM, Al Viro wrote:
> 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?
I don't think you're missing anything, the case we need it for is when
max_pfn > max_low_pfn. I wonder when this got botched? Because some
of those statements date back probably about 20 years when we started
allowing highmem pages to do IO.
--
Jens Axboe
