On Wed, 20 Nov 2024, Chuck Lever wrote: > On Wed, Nov 20, 2024 at 09:27:51AM +1100, NeilBrown wrote: > > On Wed, 20 Nov 2024, Chuck Lever wrote: > > > On Tue, Nov 19, 2024 at 11:41:31AM +1100, NeilBrown wrote: > > > > If a client ever uses the highest available slot for a given session, > > > > attempt to allocate another slot so there is room for the client to use > > > > more slots if wanted. GFP_NOWAIT is used so if there is not plenty of > > > > free memory, failure is expected - which is what we want. It also > > > > allows the allocation while holding a spinlock. > > > > > > > > We would expect to stablise with one more slot available than the client > > > > actually uses. > > > > > > Which begs the question "why have a 2048 slot maximum session slot > > > table size?" 1025 might work too. But is there a need for any > > > maximum at all, or is this just a sanity check? > > > > Linux NFS presumably isn't the only client, and it might change in the > > future. Maybe there is no need for a maximum. It was mostly as a > > sanity check. > > > > It wouldn't take much to convince me to remove the limit. > > What's the worse that might happen if there is no cap? Can this be > used as a DoS vector? It depends on how much you trust the clients that you have decided to trust. Probably we want the option of a "public" NFS server (read only probably) so we cannot assume much trust in the implementation of the client. Certainly a client could only ever use the highest slot number available - though the RFC prefers lowest - and that could push allocating through the roof. We could defend against that in more subtle ways, but a hard upper limit is easy. > > If a maximum should be necessary, its value should be clearly > labeled as "not an architectural limit -- for sanity checking only". That is certainly sensible. > > > > > > Now that we grow the slot table on demand we can start with a smaller > > > > allocation. Define NFSD_MAX_INITIAL_SLOTS and allocate at most that > > > > many when session is created. > > > > > > Maybe NFSD_DEFAULT_INITIAL_SLOTS is more descriptive? > > > > I don't think "DEFAULT" is the right word. The client requests a number > > of slots. That is the "Default". The server can impose a limit - a > > maximum. > > Maybe we don't need a limit here either? > > I see. Well I don't think there needs to be a "maximum" number of > initial slots. NFSD can try to allocate the number the client > requested as best it can, until it hits our sane maximum above. Given that we have a shrinker to discard them if they ever become a problem, that makes sense. > > I think sessions should have a minimum number of slots to guarantee > forward progress (or IOW prevent a deadlock). I would say that > number should be larger than 1 -- perhaps 2 or even 4. I think one is enough to ensure forward progress. Otherwise the RFC would have something to say about this. > > The problem with a small initial slot count is that means the > session has a slow start heuristic. That might or might not be > desirable here. The question of how quickly to increase slot count can be relevant at any time, not just at session creation time. If there is a bust of activity after a quite time during which the shrinker discarded a lot of slots - how quickly should we rebuild? My current approach is effectively one new slot per requests round-trip. So there might be 1 request in flight. Then 2. Then 3. etc. We could aim for exponential rather than linear growth. Maybe when the highest slot is used, add 20% of the current number of slots - rounded up. So 1,2,3,4,5,6,8,10,12,15,18,22,26,31,37,44,52,62,74,88,105,126, ?? Thanks, NeilBrown
