On Wed, Aug 21, 2024 at 10:22:15AM -0400, Jeff Layton wrote: > On Tue, 2024-08-20 at 10:46 -0400, cel@xxxxxxxxxx wrote: > > From: Chuck Lever <chuck.lever@xxxxxxxxxx> > > > > Build an NFSv4 protocol snippet to support the delstid extensions. > > The new fs/nfsd/nfs4_1.x file can be added to over time as other > > parts of NFSD's XDR functions are converted to machine-generated > > code. > > > > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> > > --- > > fs/nfsd/nfs4_1.x | 164 +++++++++++++++++++++++++++++ > > fs/nfsd/nfs4xdr_gen.c | 236 ++++++++++++++++++++++++++++++++++++++++++ > > fs/nfsd/nfs4xdr_gen.h | 113 ++++++++++++++++++++ > > 3 files changed, 513 insertions(+) > > create mode 100644 fs/nfsd/nfs4_1.x > > create mode 100644 fs/nfsd/nfs4xdr_gen.c > > create mode 100644 fs/nfsd/nfs4xdr_gen.h > > > > I see the patches in your lkxdrgen branch. I gave this a try and > started rebasing my delstid work on top of it, but I hit the same > symbol conflicts I hit before once I started trying to include the > full-blown nfs4xdr_gen.h header: > > ------------------------8<--------------------------- > In file included from fs/nfsd/nfs4xdr.c:58: > fs/nfsd/nfs4xdr_gen.h:86:9: error: redeclaration of enumerator ‘FATTR4_OPEN_ARGUMENTS’ > 86 | FATTR4_OPEN_ARGUMENTS = 86 > | ^~~~~~~~~~~~~~~~~~~~~ > In file included from fs/nfsd/nfsfh.h:15, > from fs/nfsd/state.h:41, > from fs/nfsd/xdr4.h:40, > from fs/nfsd/nfs4xdr.c:51: > ./include/linux/nfs4.h:518:9: note: previous definition of ‘FATTR4_OPEN_ARGUMENTS’ with type ‘enum <anonymous>’ > 518 | FATTR4_OPEN_ARGUMENTS = 86, > | ^~~~~~~~~~~~~~~~~~~~~ > fs/nfsd/nfs4xdr_gen.h:102:9: error: redeclaration of enumerator ‘FATTR4_TIME_DELEG_ACCESS’ > 102 | FATTR4_TIME_DELEG_ACCESS = 84 > | ^~~~~~~~~~~~~~~~~~~~~~~~ > ./include/linux/nfs4.h:516:9: note: previous definition of ‘FATTR4_TIME_DELEG_ACCESS’ with type ‘enum <anonymous>’ > 516 | FATTR4_TIME_DELEG_ACCESS = 84, > | ^~~~~~~~~~~~~~~~~~~~~~~~ > fs/nfsd/nfs4xdr_gen.h:106:9: error: redeclaration of enumerator ‘FATTR4_TIME_DELEG_MODIFY’ > 106 | FATTR4_TIME_DELEG_MODIFY = 85 > | ^~~~~~~~~~~~~~~~~~~~~~~~ > ./include/linux/nfs4.h:517:9: note: previous definition of ‘FATTR4_TIME_DELEG_MODIFY’ with type ‘enum <anonymous>’ > 517 | FATTR4_TIME_DELEG_MODIFY = 85, > | ^~~~~~~~~~~~~~~~~~~~~~~~ > ------------------------8<--------------------------- > > I'm not sure of the best way to work around this, unless we want to try > to split up nfs4.h. That header is shared with the client, so I consider it immutable for our purposes here. One option would be to namespace the generated data items. Eg, name them: XG_FATTR4_TIME_DELEG_ACCESS XG_FATTR4_TIME_DELEG_MODIFY That way they don't conflict with existing definitions. > Also, as a side note: > > fs/nfsd/nfs4xdr.c: In function ‘nfsd4_encode_fattr4_open_arguments’: > fs/nfsd/nfs4xdr.c:3446:55: error: incompatible type for argument 2 of ‘xdrgen_encode_fattr4_open_arguments’ > 3446 | if (!xdrgen_encode_fattr4_open_arguments(xdr, &nfsd_open_arguments)) > > > OPEN_ARGUMENTS4 is a large structure with 5 different bitmaps in it. We > probably don't want to pass that by value. When the tool is dealing > with a struct, we should have it generate functions that take a pointer > instead (IMO). The decoders are passed structs by reference already, fwiw. I had been considering the same for the encoder functions, for efficiency. I can give it a try. -- Chuck Lever
