On Thu, 23 Jan 2025, Jeff Layton wrote: > On Thu, 2025-01-23 at 07:39 +1100, NeilBrown wrote: > > On Thu, 23 Jan 2025, Jeff Layton wrote: > > > > > > @@ -854,7 +855,9 @@ nfsd_alloc_fcache_disposal(void) > > > > if (!l) > > > > return NULL; > > > > spin_lock_init(&l->lock); > > > > - INIT_DELAYED_WORK(&l->filecache_laundrette, nfsd_file_gc_worker); > > > > + timer_setup(&l->timer, nfsd_file_gc_worker, 0); > > > > + INIT_LIST_HEAD(&l->recent); > > > > + INIT_LIST_HEAD(&l->older); > > > > INIT_LIST_HEAD(&l->recent); > > > > INIT_LIST_HEAD(&l->older); > > > > > > No need to do the list initializations twice. ^^^ > > > > Thanks. I fixed up a few other merge-errors too. > > > > > > > > It does seem like this is lightweight enough now that we can do the GC > > > in interrupt context. I'm not certain that's best for latency, but it's > > > worth experimenting. > > > > What sort of latency are you thinking of? By avoiding a scheduler > > switch into the workqueue task we should be reducing overhead. > > In the old code a timer would wake a thread which would need to be > > scheduled to do the work. In the new thread and identical time will do > > the work directly. > > > > > > Anytime we have to take *_bh locks, we block interrupts. In this case, > I think you're right that that's probably the lesser evil, but we will > be taking these locks somewhat frequently. It's certainly worth > starting here though, and only offloading to a workqueue if that proves > to be a problem. We only block soft-interrupts, not hard interrupts. So yes it could cause some latency for softirqs. If we really cared we could spin_try_lock in the timer and if that fails, don't bother but reschedule the timer for a shorter timeout. But I agree that we don't need to fix until we see a problem. Thanks, NeilBrown
