mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Jakub Kicinski <kuba@kernel.org>,
	Mina Almasry <almasrymina@google.com>,
	Sasha Levin <sashal@kernel.org>,
	hawk@kernel.org, ilias.apalodimas@linaro.org,
	matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org
Subject: [PATCH AUTOSEL 6.17-5.15] page_pool: always add GFP_NOWARN for ATOMIC allocations
Date: Sat, 25 Oct 2025 12:01:29 -0400	[thread overview]
Message-ID: <20251025160905.3857885-458-sashal@kernel.org> (raw)
In-Reply-To: <20251025160905.3857885-1-sashal@kernel.org>

From: Jakub Kicinski <kuba@kernel.org>

[ Upstream commit f3b52167a0cb23b27414452fbc1278da2ee884fc ]

Driver authors often forget to add GFP_NOWARN for page allocation
from the datapath. This is annoying to users as OOMs are a fact
of life, and we pretty much expect network Rx to hit page allocation
failures during OOM. Make page pool add GFP_NOWARN for ATOMIC allocations
by default.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Link: https://patch.msgid.link/20250912161703.361272-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Based on my extensive research and analysis of this commit, here is my
determination:

## **BACKPORT RECOMMENDATION: YES**

## Detailed Analysis

### What This Commit Does

This commit adds the `__GFP_NOWARN` flag to GFP_ATOMIC allocations in
the page_pool subsystem by modifying the
`__page_pool_alloc_netmems_slow()` function in `net/core/page_pool.c`.
The change is minimal (6 lines) and specifically targets atomic
allocations that occur during network packet reception in NAPI context.

**Code change at net/core/page_pool.c:554-558:**
```c
+   /* Unconditionally set NOWARN if allocating from NAPI.
+    * Drivers forget to set it, and OOM reports on packet Rx are
useless.
+    */
+   if ((gfp & GFP_ATOMIC) == GFP_ATOMIC)
+       gfp |= __GFP_NOWARN;
```

This modification affects both bulk page allocations (via
`alloc_pages_bulk_node`) and high-order page allocations (via
`__page_pool_alloc_page_order`).

### Historical Context and Broader Pattern

Through extensive git history analysis, I discovered this is part of a
**systematic effort by Jakub Kicinski** to address OOM warnings in the
network stack:

1. **March 2024** (commit 6e9b01909a811): Modified `napi_alloc_skb()` to
   hardcode `GFP_ATOMIC | __GFP_NOWARN`
   - Commit message stated: *"the resulting OOM warning is the top
     networking warning in our fleet"* (Meta's production environment)
   - Rationale: *"allocation failures in atomic context will happen, and
     printing warnings in logs, effectively for a packet drop, is both
     too much and very likely non-actionable"*

2. **August 2024** (commit c89cca307b209): Added `__GFP_NOWARN` to
   skbuff ingress allocations
   - Similar rationale: *"build_skb() and frag allocations done with
     GFP_ATOMIC will fail in real life, when system is under memory
     pressure, and there's nothing we can do about that. So no point
     printing warnings."*

3. **September 2025** (this commit): Extends the same principle to
   page_pool allocations

### Existing Precedent Validates This Approach

My code research revealed:

**Helper function already uses this pattern**
(include/net/page_pool/helpers.h:92-96):
```c
static inline struct page *page_pool_dev_alloc_pages(struct page_pool
*pool)
{
    gfp_t gfp = (GFP_ATOMIC | __GFP_NOWARN);
    return page_pool_alloc_pages(pool, gfp);
}
```

**Drivers manually adding NOWARN since 2022**:
- `drivers/net/ethernet/mediatek/mtk_eth_soc.c:1916` - Added in July
  2022 (commit 23233e577ef973)
- `drivers/net/vmxnet3/vmxnet3_drv.c:1425` - Also includes manual NOWARN

This demonstrates driver authors were already aware of the need for
`__GFP_NOWARN` with page_pool allocations, validating the approach.

### Why This Should Be Backported

**1. Fixes Real User-Visible Issue**
- OOM warnings during network Rx are non-actionable and create log spam
- Confirmed as "top networking warning" at large-scale deployments
  (Meta)
- OOM during memory pressure is expected behavior, not an error
  condition
- Warnings provide no value but clutter logs and may trigger false
  alarms

**2. Minimal Risk**
- Only 6 lines of code added to a single function
- Only suppresses warning messages, doesn't change allocation behavior
- Allocation failures are still detected and properly handled by drivers
- Network stack provides proper statistics via qstats (rx-alloc-fail
  counter)
- No change to actual page allocation logic or error handling paths

**3. No Regressions Found**
- No subsequent commits fixing or reverting this change
- No Fixes: tags referencing this commit
- Commit has been in mainline since September 2025 with no reported
  issues
- Subsequent commit (a1b501a8c6a87) is unrelated (pool size clamping)

**4. Makes Behavior Consistent**
- Aligns with existing helper function behavior
- Removes burden from driver authors who often forget this flag
- Prevents inconsistency where some drivers add NOWARN and others don't
- Follows established pattern from napi_alloc_skb() and skbuff
  allocations

**5. Meets Stable Kernel Criteria**
- ✅ Fixes a real bug that bothers people (log spam annoys users and
  operators)
- ✅ Obviously correct (trivial change, well-understood semantics)
- ✅ Small and self-contained (6 lines, single file)
- ✅ No regression risk (only suppresses warnings)
- ✅ No API changes (internal implementation detail)

### Technical Correctness

**GFP_ATOMIC context** (from include/linux/gfp_types.h:316-318):
> "GFP_ATOMIC users can not sleep and need the allocation to succeed. A
lower watermark is applied to allow access to 'atomic reserves'."

**__GFP_NOWARN semantics** (from include/linux/gfp_types.h:274):
> "__GFP_NOWARN suppresses allocation failure reports."

The change is semantically correct: When page_pool allocates pages in
atomic context (NAPI), allocation failures are expected during OOM and
warnings serve no purpose. The allocation failure is still detected and
handled - only the noisy warning is suppressed.

### Why Not Explicitly Tagged for Stable

While the commit lacks a `Cc: stable` tag, this doesn't necessarily mean
it shouldn't be backported. Examining similar commits:
- Neither the napi_alloc_skb nor skbuff NOWARN commits had explicit
  stable tags
- Quality-of-life improvements often aren't tagged by authors but are
  valuable for stable trees
- Stable backport decisions should be based on technical merit and user
  benefit

### Potential Concerns Addressed

**"Does this hide real bugs?"**
No - drivers still detect and handle allocation failures. The difference
is only in logging.

**"Could this affect debugging?"**
Minimal impact - allocation failures are tracked via qstats. OOM
warnings during memory pressure are expected, not diagnostic.

**"Is this actually a bug fix?"**
Yes - it fixes the "bug" of annoying, non-actionable log spam that
affects production deployments at scale.

### Conclusion

This commit should be backported because it:
1. Solves a real, annoying user-visible problem (confirmed at production
   scale)
2. Has essentially zero risk (only suppresses warnings)
3. Is part of a well-vetted, systematic improvement
4. Makes the page_pool behavior consistent with established patterns
5. Reduces operational burden and log noise

While this is more of a quality-of-life improvement than a critical
functional fix, the combination of high user benefit, zero risk, and
strong technical justification makes it a good candidate for stable
backporting.

**Final Answer: YES** - This commit should be backported to stable
kernel trees.

 net/core/page_pool.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 19c92aa04e549..e224d2145eed9 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -596,6 +596,12 @@ static noinline netmem_ref __page_pool_alloc_netmems_slow(struct page_pool *pool
 	netmem_ref netmem;
 	int i, nr_pages;
 
+	/* Unconditionally set NOWARN if allocating from NAPI.
+	 * Drivers forget to set it, and OOM reports on packet Rx are useless.
+	 */
+	if ((gfp & GFP_ATOMIC) == GFP_ATOMIC)
+		gfp |= __GFP_NOWARN;
+
 	/* Don't support bulk alloc for high-order pages */
 	if (unlikely(pp_order))
 		return page_to_netmem(__page_pool_alloc_page_order(pool, gfp));
-- 
2.51.0


      parent reply	other threads:[~2025-10-25 16:29 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251025160905.3857885-1-sashal@kernel.org>
2025-10-25 15:53 ` [PATCH AUTOSEL 6.17] wifi: mt76: improve phy reset on hw restart Sasha Levin
2025-10-25 15:53 ` [PATCH AUTOSEL 6.17-6.12] Bluetooth: btusb: Add new VID/PID 13d3/3633 for MT7922 Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17] extcon: axp288: Fix wakeup source leaks on device unbind Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17] wifi: mt76: mt7996: Set def_wcid pointer in mt7996_mac_sta_init_link() Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17-6.12] ASoC: mediatek: Use SND_JACK_AVOUT for HDMI/DP jacks Sasha Levin
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17-6.6] wifi: mt76: mt7996: Temporarily disable EPCS Sasha Levin
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17] scsi: ufs: core: Change MCQ interrupt enable flow Sasha Levin
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17-6.1] scsi: ufs: host: mediatek: Fix invalid access in vccqx handling Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-6.1] scsi: ufs: host: mediatek: Change reset sequence for improved stability Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-6.6] scsi: ufs: host: mediatek: Disable auto-hibern8 during power mode changes Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-6.6] char: Use list_del_init() in misc_deregister() to reinitialize list pointer Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-6.12] wifi: mt76: mt76_eeprom_override to int Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17] wifi: mt76: mt7996: disable promiscuous mode by default Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17-6.12] scsi: ufs: host: mediatek: Fix PWM mode switch issue Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17-6.6] scsi: ufs: host: mediatek: Enhance recovery on hibernation exit failure Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17-6.12] wifi: mt76: mt7996: fix memory leak on mt7996_mcu_sta_key_tlv error Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17] extcon: fsa9480: Fix wakeup source leaks on device unbind Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17] wifi: mt76: mt7925: add pci restore for hibernate Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17] scsi: ufs: host: mediatek: Fix adapt issue after PA_Init Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-6.1] wifi: mt76: mt7921: Add 160MHz beamformee capability for mt7922 device Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-6.6] scsi: ufs: core: Disable timestamp functionality if not supported Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-6.1] scsi: ufs: host: mediatek: Assign power mode userdata before FASTAUTO mode change Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17] Bluetooth: btusb: Add new VID/PID 13d3/3627 for MT7925 Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17-5.4] extcon: adc-jack: Fix wakeup source leaks on device unbind Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17] wifi: mt76: mt7996: support writing MAC TXD for AddBA Request Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17] mei: make a local copy of client uuid in connect Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17-6.1] ftrace: Fix softlockup in ftrace_module_enable Sasha Levin
2025-10-25 19:25   ` Steven Rostedt
2025-10-28 17:48     ` Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17-6.6] scsi: ufs: host: mediatek: Fix auto-hibern8 timer configuration Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17-6.12] scsi: ufs: host: mediatek: Fix unbalanced IRQ enable issue Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17-6.1] scsi: ufs: host: mediatek: Enhance recovery on resume failure Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17] wifi: mt76: use altx queue for offchannel tx on connac+ Sasha Levin
2025-10-25 16:01 ` [PATCH AUTOSEL 6.17] wifi: mt76: mt7996: Fix mt7996_reverse_frag0_hdr_trans for MLO Sasha Levin
2025-10-25 16:01 ` [PATCH AUTOSEL 6.17-6.12] scsi: ufs: host: mediatek: Correct system PM flow Sasha Levin
2025-10-25 16:01 ` Sasha Levin [this message]

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=20251025160905.3857885-458-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=almasrymina@google.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=hawk@kernel.org \
    --cc=ilias.apalodimas@linaro.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@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

all inboxes | Powered by JetHome®