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.
>
> > 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;
> > }
>
>
--
Jeff Layton <jlayton@xxxxxxxxxx>
