Re: [PATCH v10] mm/page_alloc: boost watermarks on atomic allocation failure

Re: [PATCH v10] mm/page_alloc: boost watermarks on atomic allocation failure

[Jing Wu]

From: Qiliang Yuan <realwujing@gmail.com>

Hi Vlastimil,

Thanks for the review, and understood on taking the Ack back - the
v10 changes were substantial, and I won't send v11 just to drop the
tag. I'm folding it into an actual update instead. Let me answer your
three points directly.

1) The Ack

Yours is dropped. I've kept SeongJae's for now since he hasn't said
anything to the contrary, but I'll understand if he'd rather it not
carry over either, given v11 also fixes a debounce race in the atomic
path that nobody, including me, had caught in nine prior rounds:
last_boost_jiffies was checked and updated outside zone->lock, so
concurrent CPUs (e.g. a multi-queue NIC spreading GFP_ATOMIC
allocations across several softirqs) could all pass the
once-per-second check for the same zone before either updated the
timestamp.

2) Real-world benefit and side effects

I don't have a clean before/after deployment number for this exact
patch yet, and I want to be upfront about that. What I do have: a
production host running a downstream 4.19 kernel logged 144 order-0
GFP_ATOMIC failures over a 4h15m window, every single one through the
same NIC driver RX softirq path (bnxt_rx_pages -> net_rx_action ->
__alloc_pages_slowpath), across several unrelated network-facing
services on the box. That's evidence the failure mode is real and
ongoing - it is not evidence that this patch fixes it, and I don't
want to conflate the two.

On side effects: the mechanism is bounded by construction, not just by
intent. Each zone accepts at most one boost per second (the debounce
timer), and watermark_boost is clamped to _watermark[WMARK_HIGH] / 10
per zone, independent of how many zones get boosted in a single
slowpath call. Worst case this adds one extra kswapd wakeup per zone
per second, and the ceiling on any single zone is 10% of its own high
watermark - it can't run away or starve unrelated allocations past
that. I've put this bound into the commit message itself so it isn't
only visible in this thread.

I know a bound isn't a measurement. If it would actually help, I can
try to build a synthetic reproducer (fault-injected GFP_ATOMIC
pressure under load) and come back with before/after numbers instead
of leaving this as theory - let me know if that's the kind of
evidence that would move this forward for you.

3) netdev

Cc'd this time, along with Matthew Wilcox, who asked for the same
thing on v1 and was never followed up on. Sorry that took ten
versions.

Thanks for staying on this thread.

Qiliang