On Tue, 15 Aug 2023, Chuck Lever wrote: > On Wed, Aug 02, 2023 at 05:34:42PM +1000, NeilBrown wrote: > > Rather than searching a list of threads to find an idle one, having a > > list of idle threads allows an idle thread to be found immediately. > > > > This adds some spin_lock calls which is not ideal, but as the hold-time > > is tiny it is still faster than searching a list. > > Keep in mind that b1691bc03d4e ("sunrpc: convert to lockless lookup > of queued server threads") did the opposite because that very > spin_lock was highly contended. I am skeptical of the above claim > without lock_stat data... but that's sort of moot as this is a > temporary situation, as you point out next. The old code did a lot more writes in the spin-locked region than this code - so more hold-time. But as you say - we would need data rather than speculation if this were to be more than an interim state. > > > > A future patch will > > remove them using llist.h. This involves some subtlety and so is left > > to a separate patch. > > Since I haven't seen that patch yet, I'm reserving judgement about > whether and how these two changes might be merged. I'll try to send the remainder of the series today. > > > > This removes the need for the RQ_BUSY flag. The rqst is "busy" > > precisely when it is not on the "idle" list. > > I've been having some trouble with this one. The server system > deadlocks hard as soon as the NFS server starts. I tracked it down > this morning: this patch never initialized the sp_idle_threads > list_head. Whoops. Looks like I didn't test this particular intermediate state. > > I will apply this patch (with one-line fix) and the patch that > removes SP_CONGESTED once I hear from the client folks on the > "integrate backchannel" patch. Thanks, NeilBrown