On Fri, 07 Nov 2025, David Laight wrote:
On Thu, 6 Nov 2025 09:33:28 -0500 Chuck Lever cel@kernel.org wrote:
FYI
Ugg - that code is horrid. It seems to have got deleted since, but it is:
u32 slotsize = slot_bytes(ca); u32 num = ca->maxreqs; unsigned long avail, total_avail; unsigned int scale_factor;
spin_lock(&nfsd_drc_lock); if (nfsd_drc_max_mem > nfsd_drc_mem_used) total_avail = nfsd_drc_max_mem - nfsd_drc_mem_used; else /* We have handed out more space than we chose in * set_max_drc() to allow. That isn't really a * problem as long as that doesn't make us think we * have lots more due to integer overflow. */ total_avail = 0; avail = min((unsigned long)NFSD_MAX_MEM_PER_SESSION, total_avail); /* * Never use more than a fraction of the remaining memory, * unless it's the only way to give this client a slot. * The chosen fraction is either 1/8 or 1/number of threads, * whichever is smaller. This ensures there are adequate * slots to support multiple clients per thread. * Give the client one slot even if that would require * over-allocation--it is better than failure. */ scale_factor = max_t(unsigned int, 8, nn->nfsd_serv->sv_nrthreads);
avail = clamp_t(unsigned long, avail, slotsize, total_avail/scale_factor); num = min_t(int, num, avail / slotsize); num = max_t(int, num, 1);
Lets rework it a bit... if (nfsd_drc_max_mem > nfsd_drc_mem_used) { total_avail = nfsd_drc_max_mem - nfsd_drc_mem_used; avail = min(NFSD_MAX_MEM_PER_SESSION, total_avail); avail = clamp(avail, n + sizeof(xxx), total_avail/8) } else { total_avail = 0; avail = 0; avail = clamp(0, n + sizeof(xxx), 0); }
Neither of those clamp() are sane at all - should be clamp(val, lo, hi) with 'lo <= hi' otherwise the result is dependant on the order of the comparisons. The compiler sees the second one and rightly bleats.
In fact only gcc-9 bleats.
gcc-7 gcc-10 gcc-13 gcc-15 all seem to think it is fine.
NeilBrown