Re: [PATCH v2 00/42] Clarify module API boundaries

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2026-01-23 at 13:52 -0500, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> The first thirteen patches in this series refactor the lockd code
> base to clearly separate its public API from internal implementation
> details. The remainder are presented for context, but demonstrate
> the intended purpose of the clean-up in the first thirteen.
> 
> The lockd subsystem currently exposes internal implementation headers
> through include/linux/lockd/, creating implicit API contracts that
> complicate maintenance. External consumers such as NFSD and the NFS
> client have developed dependencies on internal structures like struct
> nlm_host, and wire protocol constants leak into high-level module
> interfaces.
> 
> These patches work to establish clean architectural boundaries. The
> public API in include/linux/lockd/ is reduced to bind.h and nlm.h,
> which define the contract between lockd and its consumers. Private
> implementation details including XDR definitions, share management,
> and host structures are relocated to fs/lockd/ where they belong.
> Layering violations are corrected: the NFS client now uses accessor
> helpers instead of dereferencing internal structures, and nlm_fopen()
> returns errno values instead of wire protocol codes.
> 
> These changes enable subsequent work to modernize the NLMv4 XDR
> layer using xdrgen without risk of breaking external consumers.
> This work appears in the remaining patches in this series, which
> are presented here only to provide context for the API adjustments.
> No need to review those closely just yet.
> 
> The series is based on the public nfsd-testing branch.
> 
> ---
> 
> Changes since v1:
> - Refine the pre-requisite header adjustments
> - Reduce stack consumption by moving large structures to wrappers
> - Additional extensive clean up
> 
> Chuck Lever (42):
>   lockd: Simplify cast_status() in svcproc.c
>   lockd: Introduce nlm__int__deadlock
>   lockd: Have nlm_fopen() return errno values
>   lockd: Relocate nlmsvc_unlock API declarations
>   NFS: Use nlmclnt_rpc_clnt() helper to retrieve nlm_host's rpc_clnt
>   lockd: Move xdr4.h from include/linux/lockd/ to fs/lockd/
>   lockd: Move share.h from include/linux/lockd/ to fs/lockd/
>   lockd: Relocate include/linux/lockd/lockd.h
>   lockd: Remove lockd/debug.h
>   lockd: Move xdr.h from include/linux/lockd/ to fs/lockd/
>   lockd: Make linux/lockd/nlm.h an internal header
>   lockd: Move nlm4svc_set_file_lock_range()
>   lockd: Relocate svc_version definitions to XDR layer
>   Documentation: Add the RPC language description of NLM version 4
>   lockd: Use xdrgen XDR functions for the NLMv4 NULL procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 TEST procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 LOCK procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 CANCEL procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 UNLOCK procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 GRANTED procedure
>   lockd: Refactor nlm4svc_callback()
>   lockd: Use xdrgen XDR functions for the NLMv4 TEST_MSG procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 LOCK_MSG procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 CANCEL_MSG procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 UNLOCK_MSG procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 GRANTED_MSG procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 TEST_RES procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 LOCK_RES procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 CANCEL_RES procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 UNLOCK_RES procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 GRANTED_RES procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 SM_NOTIFY procedure
>   lockd: Convert server-side undefined procedures to xdrgen
>   lockd: Hoist file_lock init out of nlm4svc_decode_shareargs()
>   lockd: Prepare share helpers for xdrgen conversion
>   lockd: Use xdrgen XDR functions for the NLMv4 SHARE procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 UNSHARE procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 NM_LOCK procedure
>   lockd: Use xdrgen XDR functions for the NLMv4 FREE_ALL procedure
>   lockd: Add LOCKD_SHARE_SVID constant for DOS sharing mode
>   lockd: Remove C macros that are no longer used
>   lockd: Remove dead code from fs/lockd/xdr4.c
> 
>  Documentation/sunrpc/xdr/nlm4.x     |  211 ++++
>  fs/lockd/Makefile                   |   30 +-
>  fs/lockd/clnt4xdr.c                 |    5 +-
>  fs/lockd/clntlock.c                 |    2 +-
>  fs/lockd/clntproc.c                 |    2 +-
>  fs/lockd/clntxdr.c                  |    3 +-
>  fs/lockd/host.c                     |    2 +-
>  {include/linux => fs}/lockd/lockd.h |   99 +-
>  fs/lockd/mon.c                      |    2 +-
>  {include/linux => fs}/lockd/nlm.h   |    8 +-
>  fs/lockd/nlm4xdr_gen.c              |  724 +++++++++++
>  fs/lockd/nlm4xdr_gen.h              |   32 +
>  {include/linux => fs}/lockd/share.h |   19 +-
>  fs/lockd/svc.c                      |   50 +-
>  fs/lockd/svc4proc.c                 | 1783 ++++++++++++++++++---------
>  fs/lockd/svclock.c                  |   12 +-
>  fs/lockd/svcproc.c                  |   99 +-
>  fs/lockd/svcshare.c                 |   40 +-
>  fs/lockd/svcsubs.c                  |   32 +-
>  fs/lockd/trace.h                    |    3 +-
>  fs/lockd/xdr.c                      |    6 +-
>  {include/linux => fs}/lockd/xdr.h   |   15 +-
>  fs/lockd/xdr4.c                     |  347 ------
>  fs/nfs/sysfs.c                      |   10 +-
>  fs/nfsd/lockd.c                     |   51 +-
>  fs/nfsd/nfsctl.c                    |    2 +-
>  include/linux/lockd/bind.h          |   23 +-
>  include/linux/lockd/debug.h         |   40 -
>  include/linux/lockd/xdr4.h          |   43 -
>  include/linux/sunrpc/xdrgen/nlm4.h  |  233 ++++
>  30 files changed, 2750 insertions(+), 1178 deletions(-)
>  create mode 100644 Documentation/sunrpc/xdr/nlm4.x
>  rename {include/linux => fs}/lockd/lockd.h (84%)
>  rename {include/linux => fs}/lockd/nlm.h (91%)
>  create mode 100644 fs/lockd/nlm4xdr_gen.c
>  create mode 100644 fs/lockd/nlm4xdr_gen.h
>  rename {include/linux => fs}/lockd/share.h (58%)
>  rename {include/linux => fs}/lockd/xdr.h (91%)
>  delete mode 100644 fs/lockd/xdr4.c
>  delete mode 100644 include/linux/lockd/debug.h
>  delete mode 100644 include/linux/lockd/xdr4.h
>  create mode 100644 include/linux/sunrpc/xdrgen/nlm4.h

I went through the series and it looks good overall. I definitely like
moving away from hand-rolled XDR handling. You can add this to the
series:

Reviewed-by: Jeff Layton <jlayton@kernel.org>

There is still the matter of the race vs. the shutdown file, but that's
a preexisting problem. I presume you intend to let this sit in -next
for a while? It's a big change so it'd be good to have a nice long test
cycle with it.