On 4/7/25 7:34 AM, Cedric Blancher wrote: > On Wed, 22 Jan 2025 at 11:07, Cedric Blancher <cedric.blancher@xxxxxxxxx> wrote: >> >> Good morning! >> >> IMO it might be good to increase RPCSVC_MAXPAYLOAD to at least 8M, >> giving the NFSv4.1 session mechanism some headroom for negotiation. >> For over a decade the default value is 1M (1*1024*1024u), which IMO >> causes problems with anything faster than 2500baseT. > > Chuck pointed out that the new /sys/kernel/debug/ subdir could be used > to host "experimental" tunables. Indeed. That hurdle will be addressed in v6.16. The patch that adds the new debugfs directory for NFSD is now in nfsd-next. > Plan: > Add a /sys/kernel/debug/nfsd/RPCSVC_MAXPAYLOAD tunable file, > RPCSVC_MAXPAYLOAD defaults to 4M, on connection start it copies the > value of /sys/kernel/debug/nfsd/RPCSVC_MAXPAYLOAD into a private > variable to make it unchangeable during connection lifetime, because > Chuck is worried that svc_rqst::rq_pages might blow up in our face. > > Would that be a plan? Right now I'm still concerned about the bulky size of the rq_pages array. rq_vec and rq_bvec are a problem as well. maxpayload determines the size of the rq_pages array in each nfsd thread. It's not a per-connection thing. So, the size of the array is picked up when the threads are created. You'd have to set the number of threads to zero, change maxpayload, then set the number of threads to a positive integer -- at that point each of the freshly created threads would pick up the new maxpayload value (for example). Making the rq_pages array size dynamic has a few issues. It's in the middle of struct svc_rqst. We might move it to the end of that structure, or we could make rq_pages a pointer to a separate memory allocation. That would introduce some code churn. Or, we could simply allocate the maximum size (4MB worth of pages is more than 1000 array entries) for every thread, all the time. That's simple, but wasteful. I am a little concerned about introducing that overhead on every NFSD operation, even for small ones. The RPCSVC_MAXPAYLOAD macro is currently an integer constant and is used in several places for bounds-checking during request marshaling and unmarshaling. It would have to be replaced with a global variable; that variable would likely be stored in svc_serv, to go along with the rq_pages array size allocated in each thread in the serv's thread pool. -- Chuck Lever
