Re: [PATCH 4/4] nfsd: filecache: change garbage collection to a timer.

[Jeff Layton <jlayton@kernel.org>]

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.
-- 
Jeff Layton <jlayton@kernel.org>