Re: [PATCH v5 2/7] NFSD: Add NFSD_CMD_UNLOCK_IP netlink command

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2026-03-27 at 11:19 -0400, Chuck Lever wrote:
> 
> On Fri, Mar 27, 2026, at 8:06 AM, Jeff Layton wrote:
> > On Thu, 2026-03-26 at 13:55 -0400, Chuck Lever wrote:
> 
> > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > > index 988a79ec4a79..e1e89d52e6de 100644
> > > --- a/fs/nfsd/nfsctl.c
> > > +++ b/fs/nfsd/nfsctl.c
> 
> > > @@ -2200,6 +2200,44 @@ int nfsd_nl_pool_mode_get_doit(struct sk_buff *skb, struct genl_info *info)
> > >  	return err;
> > >  }
> > >  
> > > +/**
> > > + * nfsd_nl_unlock_ip_doit - release NLM locks held by an IP address
> > > + * @skb: reply buffer
> > > + * @info: netlink metadata and command arguments
> > > + *
> > > + * Return: 0 on success or a negative errno.
> > > + */
> > > +int nfsd_nl_unlock_ip_doit(struct sk_buff *skb, struct genl_info *info)
> > > +{
> > > +	struct sockaddr *sap;
> > > +
> > > +	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_UNLOCK_IP_ADDRESS))
> > > +		return -EINVAL;
> > > +	sap = nla_data(info->attrs[NFSD_A_UNLOCK_IP_ADDRESS]);
> > > +	switch (sap->sa_family) {
> > > +	case AF_INET:
> > > +		if (nla_len(info->attrs[NFSD_A_UNLOCK_IP_ADDRESS]) <
> > > +		    sizeof(struct sockaddr_in))
> > > +			return -EINVAL;
> > > +		break;
> > > +	case AF_INET6:
> > > +		if (nla_len(info->attrs[NFSD_A_UNLOCK_IP_ADDRESS]) <
> > > +		    sizeof(struct sockaddr_in6))
> > > +			return -EINVAL;
> > > +		break;
> > > +	default:
> > > +		return -EAFNOSUPPORT;
> > > +	}
> > > +	/*
> > > +	 * nlmsvc_unlock_all_by_ip() releases matching locks
> > > +	 * across all network namespaces because lockd operates
> > > +	 * a single global instance.
> > > +	 */
> > > +	trace_nfsd_ctl_unlock_ip(genl_info_net(info), sap,
> > > +				 svc_addr_len(sap));
> > 
> > All of the tracepoints get passed svc_addr_len(sap) for the length. Any
> > reason not to just determine the length inside the tracepoint, so you
> > don't need to calc the length unless it's enabled?
> 
> Unless I'm mistaken, the trace_nfsd_ctl_unlock_ip() call site
> expands to a static branch that skips everything, including
> argument evaluation, when the tracepoint is disabled. The
> svc_addr_len() call, being part of the argument list, is
> already behind that branch.
> 

It's a minor thing, but fewer arguments to a tracepoint is nicer, IMO.

> 
> > > +	return nlmsvc_unlock_all_by_ip(sap);
> > > +}
> > > +
> > >  /**
> > >   * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
> > >   * @net: a freshly-created network namespace
> 

-- 
Jeff Layton <jlayton@kernel.org>