[PATCH 3/7] netpoll: Move netpoll_cleanup implementation to netconsole

Breno Leitao posted 7 patches 1 month ago
[PATCH 3/7] netpoll: Move netpoll_cleanup implementation to netconsole
Posted by Breno Leitao 1 month ago
Shift the definition of netpoll_cleanup() from netpoll core to the
netconsole driver, updating all relevant file references. This change
centralizes cleanup logic alongside netconsole target management,

Given netpoll_cleanup() is only called by netconsole, keep it there.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/netconsole.c | 10 ++++++++++
 include/linux/netpoll.h  |  1 -
 net/core/netpoll.c       | 10 ----------
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 30731711571be..90e359b87469a 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -260,6 +260,16 @@ static struct netconsole_target *alloc_and_init(void)
 	return nt;
 }
 
+static void netpoll_cleanup(struct netpoll *np)
+{
+	rtnl_lock();
+	if (!np->dev)
+		goto out;
+	do_netpoll_cleanup(np);
+out:
+	rtnl_unlock();
+}
+
 /* Clean up every target in the cleanup_list and move the clean targets back to
  * the main target_list.
  */
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 481ec474fa6b9..65bfade025f09 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -72,7 +72,6 @@ int netpoll_send_udp(struct netpoll *np, const char *msg, int len);
 int __netpoll_setup(struct netpoll *np, struct net_device *ndev);
 int netpoll_setup(struct netpoll *np);
 void __netpoll_free(struct netpoll *np);
-void netpoll_cleanup(struct netpoll *np);
 void do_netpoll_cleanup(struct netpoll *np);
 netdev_tx_t netpoll_send_skb(struct netpoll *np, struct sk_buff *skb);
 struct sk_buff *find_skb(struct netpoll *np, int len, int reserve);
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index b4634e91568e8..9e12a667a5f0a 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -703,13 +703,3 @@ void do_netpoll_cleanup(struct netpoll *np)
 }
 EXPORT_SYMBOL(do_netpoll_cleanup);
 
-void netpoll_cleanup(struct netpoll *np)
-{
-	rtnl_lock();
-	if (!np->dev)
-		goto out;
-	do_netpoll_cleanup(np);
-out:
-	rtnl_unlock();
-}
-EXPORT_SYMBOL(netpoll_cleanup);

-- 
2.47.3
Re: [PATCH 3/7] netpoll: Move netpoll_cleanup implementation to netconsole
Posted by Willem de Bruijn 1 month ago
Breno Leitao wrote:
> Shift the definition of netpoll_cleanup() from netpoll core to the
> netconsole driver, updating all relevant file references. This change
> centralizes cleanup logic alongside netconsole target management,
> 
> Given netpoll_cleanup() is only called by netconsole, keep it there.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

What's the rationale for making this a separate patch, as the
previous patch also moves the other netconsole specific code from
netpoll.c to netconsole.c?

And/or consider updating prefix from netpoll_.. to netconsole_..

Just two considerations. Not blockers.

Reviewed-by: Willem de Bruijn <willemb@google.com>
Re: [PATCH 3/7] netpoll: Move netpoll_cleanup implementation to netconsole
Posted by Breno Leitao 4 weeks, 1 day ago
On Tue, Sep 02, 2025 at 06:49:26PM -0400, Willem de Bruijn wrote:
> Breno Leitao wrote:
> > Shift the definition of netpoll_cleanup() from netpoll core to the
> > netconsole driver, updating all relevant file references. This change
> > centralizes cleanup logic alongside netconsole target management,
> > 
> > Given netpoll_cleanup() is only called by netconsole, keep it there.
> > 
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> What's the rationale for making this a separate patch, as the
> previous patch also moves the other netconsole specific code from
> netpoll.c to netconsole.c?

I just tried to isolate the changes in small patches as possible.
previous functions needed to go all together, given it was they were in
a chain.

this one netpoll_cleanup() is more independent, so, I decided to
separate it, making the patches smaller individually.

> And/or consider updating prefix from netpoll_.. to netconsole_..

Good point, and I agree with the feedback.

In cases like this, should I rename the function while moving, or,
adding an additional patch to rename them?

Thanks for the review,
--breno
Re: [PATCH 3/7] netpoll: Move netpoll_cleanup implementation to netconsole
Posted by Willem de Bruijn 4 weeks, 1 day ago
Breno Leitao wrote:
> On Tue, Sep 02, 2025 at 06:49:26PM -0400, Willem de Bruijn wrote:
> > Breno Leitao wrote:
> > > Shift the definition of netpoll_cleanup() from netpoll core to the
> > > netconsole driver, updating all relevant file references. This change
> > > centralizes cleanup logic alongside netconsole target management,
> > > 
> > > Given netpoll_cleanup() is only called by netconsole, keep it there.
> > > 
> > > Signed-off-by: Breno Leitao <leitao@debian.org>
> > 
> > What's the rationale for making this a separate patch, as the
> > previous patch also moves the other netconsole specific code from
> > netpoll.c to netconsole.c?
> 
> I just tried to isolate the changes in small patches as possible.
> previous functions needed to go all together, given it was they were in
> a chain.
> 
> this one netpoll_cleanup() is more independent, so, I decided to
> separate it, making the patches smaller individually.

Sounds good. Thanks for clarifying.
 
> > And/or consider updating prefix from netpoll_.. to netconsole_..
> 
> Good point, and I agree with the feedback.
> 
> In cases like this, should I rename the function while moving, or,
> adding an additional patch to rename them?

In general, it helps review when move patches are entirely NOOPs.

In this specific case, if this is the only case that gets renamed
*and* it is such a small patch *and* it is a revision of an earlier
patch that has already been verified to be a NOOP, then by exception
it's also fine to squash.

If there are more renames, it might be best to bundle them in a single
(otherwise NOOP, again) patch at the end of the series?