On 1/16/25 9:46 AM, Jeff Layton wrote:
On Thu, 2025-01-16 at 09:27 -0500, Chuck Lever wrote:
On 1/15/25 6:24 PM, Olga Kornievskaia wrote:
When a particular listener is being removed we need to make sure
that we delete the entry from the list of permanent sockets
(sv_permsocks) as well as remove it from the listener transports
(sp_xprts). When adding back the leftover transports not being
removed we need to clear XPT_BUSY flag so that it can be used.
Fixes: 16a471177496 ("NFSD: add listener-{set,get} netlink command")
Signed-off-by: Olga Kornievskaia <okorniev@xxxxxxxxxx>
---
fs/nfsd/nfsctl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 95ea4393305b..3deedd511e83 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1988,7 +1988,7 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
/* Close the remaining sockets on the permsocks list */
while (!list_empty(&permsocks)) {
xprt = list_first_entry(&permsocks, struct svc_xprt, xpt_list);
- list_move(&xprt->xpt_list, &serv->sv_permsocks);
+ list_del_init(&xprt->xpt_list);
/*
* Newly-created sockets are born with the BUSY bit set. Clear
@@ -2000,6 +2000,7 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
set_bit(XPT_CLOSE, &xprt->xpt_flags);
spin_unlock_bh(&serv->sv_lock);
+ svc_xprt_dequeue_entry(xprt);
The kdoc comment in front of llist_del_entry() says:
+ * The caller must ensure that nothing can race in and change the
+ * list while this is running.
In this caller, I don't see how such a race is prevented.
This caller holds the nfsd_mutex, and prior to this, it does:
/* For now, no removing old sockets while server is running */
if (serv->sv_nrthreads && !list_empty(&permsocks)) {
list_splice_init(&permsocks, &serv->sv_permsocks);
spin_unlock_bh(&serv->sv_lock);
err = -EBUSY;
goto out_unlock_mtx;
}
No nfsd threads are running and none can be started, so nothing is
processing the queue at this time. Some comments explaining that would
be a welcome addition here.
So this would also block incoming accepts on another (active) socket?
Yeah, that's not obvious.
svc_xprt_close(xprt);
spin_lock_bh(&serv->sv_lock);
}
@@ -2031,6 +2032,7 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
xprt = svc_find_listener(serv, xcl_name, net, sa);
if (xprt) {
+ clear_bit(XPT_BUSY, &xprt->xpt_flags);
svc_xprt_put(xprt);
continue;
}
--
Chuck Lever
