On Fri, 24 Jan 2025, Chuck Lever wrote:
> On 1/22/25 5:10 PM, NeilBrown wrote:
> > On Thu, 23 Jan 2025, Chuck Lever wrote:
> >> On 1/21/25 10:54 PM, NeilBrown wrote:
> >>> The final freeing of nfsd files is done by per-net nfsd threads (which
> >>> call nfsd_file_net_dispose()) so it makes some sense to make more of the
> >>> freeing infrastructure to be per-net - in struct nfsd_fcache_disposal.
> >>>
> >>> This is a step towards replacing the list_lru with simple lists which
> >>> each share the per-net lock in nfsd_fcache_disposal and will require
> >>> less list walking.
> >>>
> >>> As the net is always shutdown before there is any chance that the rest
> >>> of the filecache is shut down we can removed the tests on
> >>> NFSD_FILE_CACHE_UP.
> >>>
> >>> For the filecache stats file, which assumes a global lru, we keep a
> >>> separate counter which includes all files in all netns lrus.
> >>
> >> Hey Neil -
> >>
> >> One of the issues with the current code is that the contention for
> >> the LRU lock has been effectively buried. It would be nice to have a
> >> way to expose that contention in the new code.
> >>
> >> Can this patch or this series add some lock class infrastructure to
> >> help account for the time spent contending for these dynamically
> >> allocated spin locks?
> >
> > Unfortunately I don't know anything about accounting for lock contention
> > time.
> >
> > I know about lock classes for the purpose of lockdep checking but not
> > how they relate to contention tracking.
> > Could you give me some pointers?
>
> Sticking these locks into a class is all you need to do. When lockstat
> is enabled, it automatically accumulates the statistics for all locks
> in a class, and treats that as a single lock in /proc/lock_stat.
>
Ahh thanks. So I don't need to do anything as this lock has it's own
spin_lock_init() so it already has it's own class.
However.... the init call is :
spin_lock_init(&l->lock);
so that appear in /proc/lock_stat as
&l->lock:
or maybe
&l->lock/1:
or even
&l->lock/2:
Maybe I should change the "l" to something more unique. "nfd" ??
Or I could actually define a lock_class_key and call __spin_lock_init:
__skin_lock_init(&l->lock, "nfsd_fcache_disposal->lock", &key,
false);
There is no precedent for that though.
NeilBrown
