| Message ID | 166214486962.101939.2252490595444681944.stgit@bazille.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] NFSD: Increase NFSD_MAX_OPS_PER_COMPOUND | expand |
Hi Chuck,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.0-rc3 next-20220901]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Chuck-Lever/NFSD-Increase-NFSD_MAX_OPS_PER_COMPOUND/20220903-025613
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 0b3acd1cc0222953035d18176b1e4aa06624fd6e
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220903/202209030446.TMHuJXY9-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2b18db50c1618d5efba82c205ab1d127cf210862
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Chuck-Lever/NFSD-Increase-NFSD_MAX_OPS_PER_COMPOUND/20220903-025613
git checkout 2b18db50c1618d5efba82c205ab1d127cf210862
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash fs/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
fs/nfsd/nfs4xdr.c: In function 'nfsd4_decode_compound':
fs/nfsd/nfs4xdr.c:2372:29: error: implicit declaration of function 'vcalloc'; did you mean 'kvcalloc'? [-Werror=implicit-function-declaration]
2372 | argp->ops = vcalloc(argp->opcnt, sizeof(*argp->ops));
| ^~~~~~~
| kvcalloc
>> fs/nfsd/nfs4xdr.c:2372:27: warning: assignment to 'struct nfsd4_op *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
2372 | argp->ops = vcalloc(argp->opcnt, sizeof(*argp->ops));
| ^
fs/nfsd/nfs4xdr.c: In function 'nfsd4_release_compoundargs':
fs/nfsd/nfs4xdr.c:5396:17: error: implicit declaration of function 'vfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
5396 | vfree(args->ops);
| ^~~~~
| kvfree
cc1: some warnings being treated as errors
vim +2372 fs/nfsd/nfs4xdr.c
2329
2330 static bool
2331 nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
2332 {
2333 struct nfsd4_op *op;
2334 bool cachethis = false;
2335 int auth_slack= argp->rqstp->rq_auth_slack;
2336 int max_reply = auth_slack + 8; /* opcnt, status */
2337 int readcount = 0;
2338 int readbytes = 0;
2339 __be32 *p;
2340 int i;
2341
2342 if (xdr_stream_decode_u32(argp->xdr, &argp->taglen) < 0)
2343 return false;
2344 max_reply += XDR_UNIT;
2345 argp->tag = NULL;
2346 if (unlikely(argp->taglen)) {
2347 if (argp->taglen > NFSD4_MAX_TAGLEN)
2348 return false;
2349 p = xdr_inline_decode(argp->xdr, argp->taglen);
2350 if (!p)
2351 return false;
2352 argp->tag = svcxdr_savemem(argp, p, argp->taglen);
2353 if (!argp->tag)
2354 return false;
2355 max_reply += xdr_align_size(argp->taglen);
2356 }
2357
2358 if (xdr_stream_decode_u32(argp->xdr, &argp->minorversion) < 0)
2359 return false;
2360 if (xdr_stream_decode_u32(argp->xdr, &argp->opcnt) < 0)
2361 return false;
2362
2363 /*
2364 * NFS4ERR_RESOURCE is a more helpful error than GARBAGE_ARGS
2365 * here, so we return success at the xdr level so that
2366 * nfsd4_proc can handle this is an NFS-level error.
2367 */
2368 if (argp->opcnt > NFSD_MAX_OPS_PER_COMPOUND)
2369 return true;
2370
2371 if (argp->opcnt > ARRAY_SIZE(argp->iops)) {
> 2372 argp->ops = vcalloc(argp->opcnt, sizeof(*argp->ops));
2373 if (!argp->ops) {
2374 argp->ops = argp->iops;
2375 return false;
2376 }
2377 }
2378
2379 if (argp->minorversion > NFSD_SUPPORTED_MINOR_VERSION)
2380 argp->opcnt = 0;
2381
2382 for (i = 0; i < argp->opcnt; i++) {
2383 op = &argp->ops[i];
2384 op->replay = NULL;
2385
2386 if (xdr_stream_decode_u32(argp->xdr, &op->opnum) < 0)
2387 return false;
2388 if (nfsd4_opnum_in_range(argp, op)) {
2389 op->status = nfsd4_dec_ops[op->opnum](argp, &op->u);
2390 if (op->status != nfs_ok)
2391 trace_nfsd_compound_decode_err(argp->rqstp,
2392 argp->opcnt, i,
2393 op->opnum,
2394 op->status);
2395 } else {
2396 op->opnum = OP_ILLEGAL;
2397 op->status = nfserr_op_illegal;
2398 }
2399 op->opdesc = OPDESC(op);
2400 /*
2401 * We'll try to cache the result in the DRC if any one
2402 * op in the compound wants to be cached:
2403 */
2404 cachethis |= nfsd4_cache_this_op(op);
2405
2406 if (op->opnum == OP_READ || op->opnum == OP_READ_PLUS) {
2407 readcount++;
2408 readbytes += nfsd4_max_reply(argp->rqstp, op);
2409 } else
2410 max_reply += nfsd4_max_reply(argp->rqstp, op);
2411 /*
2412 * OP_LOCK and OP_LOCKT may return a conflicting lock.
2413 * (Special case because it will just skip encoding this
2414 * if it runs out of xdr buffer space, and it is the only
2415 * operation that behaves this way.)
2416 */
2417 if (op->opnum == OP_LOCK || op->opnum == OP_LOCKT)
2418 max_reply += NFS4_OPAQUE_LIMIT;
2419
2420 if (op->status) {
2421 argp->opcnt = i+1;
2422 break;
2423 }
2424 }
2425 /* Sessions make the DRC unnecessary: */
2426 if (argp->minorversion)
2427 cachethis = false;
2428 svc_reserve(argp->rqstp, max_reply + readbytes);
2429 argp->rqstp->rq_cachetype = cachethis ? RC_REPLBUFF : RC_NOCACHE;
2430
2431 if (readcount > 1 || max_reply > PAGE_SIZE - auth_slack)
2432 __clear_bit(RQ_SPLICE_OK, &argp->rqstp->rq_flags);
2433
2434 return true;
2435 }
2436
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index 1e9690a061ec..a04b9b29678f 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -2369,10 +2369,9 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp) return true; if (argp->opcnt > ARRAY_SIZE(argp->iops)) { - argp->ops = kzalloc(argp->opcnt * sizeof(*argp->ops), GFP_KERNEL); + argp->ops = vcalloc(argp->opcnt, sizeof(*argp->ops)); if (!argp->ops) { argp->ops = argp->iops; - dprintk("nfsd: couldn't allocate room for COMPOUND\n"); return false; } } @@ -5394,7 +5393,7 @@ void nfsd4_release_compoundargs(struct svc_rqst *rqstp) struct nfsd4_compoundargs *args = rqstp->rq_argp; if (args->ops != args->iops) { - kfree(args->ops); + vfree(args->ops); args->ops = args->iops; } while (args->to_free) { diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index ae596dbf8667..5d28beb290fe 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -175,7 +175,7 @@ static inline struct nfs4_delegation *delegstateid(struct nfs4_stid *s) /* Maximum number of slots per session. 160 is useful for long haul TCP */ #define NFSD_MAX_SLOTS_PER_SESSION 160 /* Maximum number of operations per session compound */ -#define NFSD_MAX_OPS_PER_COMPOUND 16 +#define NFSD_MAX_OPS_PER_COMPOUND 50 /* Maximum session per slot cache size */ #define NFSD_SLOT_CACHE_SIZE 2048 /* Maximum number of NFSD_SLOT_CACHE_SIZE slots per session */
When attempting an NFSv4 mount, a Solaris NFSv4 client builds a single large COMPOUND that chains a series of LOOKUPs to get to the pseudo filesystem root directory that is to be mounted. The Linux NFS server's current maximum of 16 operations per NFSv4 COMPOUND is not large enough to ensure that this works for paths that are more than a few components deep. Since NFSD_MAX_OPS_PER_COMPOUND is mostly a sanity check, and most NFSv4 COMPOUNDS are between 3 and 6 operations (thus they do not trigger any re-allocation of the operation array on the server), increasing this maximum should result in little to no impact. The ops array can get large now, so allocate it via vmalloc() to help ensure memory fragmentation won't cause an allocation failure. Link: https://bugzilla.kernel.org/show_bug.cgi?id=216383 Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- fs/nfsd/nfs4xdr.c | 5 ++--- fs/nfsd/state.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-)