The patch below does not apply to the 5.15-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.15.y git checkout FETCH_HEAD git cherry-pick -x 368d3cb406cdd074d1df2ad9ec06d1bfcb664882 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to 'stable@vger.kernel.org' --in-reply-to '2023052821-wired-primate-24c3@gregkh' --subject-prefix 'PATCH 5.15.y' HEAD^..
Possible dependencies:
368d3cb406cd ("page_pool: fix inconsistency for page_pool_ring_[un]lock()") 542bcea4be86 ("net: page_pool: use in_softirq() instead") 590032a4d213 ("page_pool: Add recycle stats to page_pool_put_page_bulk") 6b95e3388b1e ("page_pool: Add function to batch and return stats") ad6fa1e1ab1b ("page_pool: Add recycle stats") 8610037e8106 ("page_pool: Add allocation stats") 64693ec7774e ("page_pool: Store the XDP mem id")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 368d3cb406cdd074d1df2ad9ec06d1bfcb664882 Mon Sep 17 00:00:00 2001 From: Yunsheng Lin linyunsheng@huawei.com Date: Mon, 22 May 2023 11:17:14 +0800 Subject: [PATCH] page_pool: fix inconsistency for page_pool_ring_[un]lock()
page_pool_ring_[un]lock() use in_softirq() to decide which spin lock variant to use, and when they are called in the context with in_softirq() being false, spin_lock_bh() is called in page_pool_ring_lock() while spin_unlock() is called in page_pool_ring_unlock(), because spin_lock_bh() has disabled the softirq in page_pool_ring_lock(), which causes inconsistency for spin lock pair calling.
This patch fixes it by returning in_softirq state from page_pool_producer_lock(), and use it to decide which spin lock variant to use in page_pool_producer_unlock().
As pool->ring has both producer and consumer lock, so rename it to page_pool_producer_[un]lock() to reflect the actual usage. Also move them to page_pool.c as they are only used there, and remove the 'inline' as the compiler may have better idea to do inlining or not.
Fixes: 7886244736a4 ("net: page_pool: Add bulk support for ptr_ring") Signed-off-by: Yunsheng Lin linyunsheng@huawei.com Acked-by: Jesper Dangaard Brouer brouer@redhat.com Acked-by: Ilias Apalodimas ilias.apalodimas@linaro.org Link: https://lore.kernel.org/r/20230522031714.5089-1-linyunsheng@huawei.com Signed-off-by: Jakub Kicinski kuba@kernel.org
diff --git a/include/net/page_pool.h b/include/net/page_pool.h index c8ec2f34722b..126f9e294389 100644 --- a/include/net/page_pool.h +++ b/include/net/page_pool.h @@ -399,22 +399,4 @@ static inline void page_pool_nid_changed(struct page_pool *pool, int new_nid) page_pool_update_nid(pool, new_nid); }
-static inline void page_pool_ring_lock(struct page_pool *pool) - __acquires(&pool->ring.producer_lock) -{ - if (in_softirq()) - spin_lock(&pool->ring.producer_lock); - else - spin_lock_bh(&pool->ring.producer_lock); -} - -static inline void page_pool_ring_unlock(struct page_pool *pool) - __releases(&pool->ring.producer_lock) -{ - if (in_softirq()) - spin_unlock(&pool->ring.producer_lock); - else - spin_unlock_bh(&pool->ring.producer_lock); -} - #endif /* _NET_PAGE_POOL_H */ diff --git a/net/core/page_pool.c b/net/core/page_pool.c index e212e9d7edcb..a3e12a61d456 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -134,6 +134,29 @@ EXPORT_SYMBOL(page_pool_ethtool_stats_get); #define recycle_stat_add(pool, __stat, val) #endif
+static bool page_pool_producer_lock(struct page_pool *pool) + __acquires(&pool->ring.producer_lock) +{ + bool in_softirq = in_softirq(); + + if (in_softirq) + spin_lock(&pool->ring.producer_lock); + else + spin_lock_bh(&pool->ring.producer_lock); + + return in_softirq; +} + +static void page_pool_producer_unlock(struct page_pool *pool, + bool in_softirq) + __releases(&pool->ring.producer_lock) +{ + if (in_softirq) + spin_unlock(&pool->ring.producer_lock); + else + spin_unlock_bh(&pool->ring.producer_lock); +} + static int page_pool_init(struct page_pool *pool, const struct page_pool_params *params) { @@ -617,6 +640,7 @@ void page_pool_put_page_bulk(struct page_pool *pool, void **data, int count) { int i, bulk_len = 0; + bool in_softirq;
for (i = 0; i < count; i++) { struct page *page = virt_to_head_page(data[i]); @@ -635,7 +659,7 @@ void page_pool_put_page_bulk(struct page_pool *pool, void **data, return;
/* Bulk producer into ptr_ring page_pool cache */ - page_pool_ring_lock(pool); + in_softirq = page_pool_producer_lock(pool); for (i = 0; i < bulk_len; i++) { if (__ptr_ring_produce(&pool->ring, data[i])) { /* ring full */ @@ -644,7 +668,7 @@ void page_pool_put_page_bulk(struct page_pool *pool, void **data, } } recycle_stat_add(pool, ring, i); - page_pool_ring_unlock(pool); + page_pool_producer_unlock(pool, in_softirq);
/* Hopefully all pages was return into ptr_ring */ if (likely(i == bulk_len))
On Sun, 28 May 2023 17:55:22 +0100 gregkh@linuxfoundation.org wrote:
The patch below does not apply to the 5.15-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
Do you track the missing backports? We fumbled the Fixes tag on this one, turns out it's not needed further back than it applies. In such cases is it useful to let you know or just silently ignore the failure notification?
On Mon, May 29, 2023 at 07:00:34PM -0700, Jakub Kicinski wrote:
On Sun, 28 May 2023 17:55:22 +0100 gregkh@linuxfoundation.org wrote:
The patch below does not apply to the 5.15-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
Do you track the missing backports?
Nope, once I email this off, it goes off of my radar. But I know Sasha sometimes tries to fix them up, as do others as it's an easy TODO list for people who want to help out.
We fumbled the Fixes tag on this one, turns out it's not needed further back than it applies. In such cases is it useful to let you know or just silently ignore the failure notification?
It's good to say "this isn't needed" so that others don't spend time on it, so thanks!
greg k-h
linux-stable-mirror@lists.linaro.org