Skip to content

Commit fbfdc9f

Browse files
neilbrownchucklever
authored andcommitted
nfsd: filecache: drop the list_lru lock during lock gc scans
Under a high NFSv3 load with lots of different files being accessed, the LRU list of garbage-collectable files can become quite long. Asking list_lru_scan_node() to scan the whole list can result in a long period during which a spinlock is held, blocking the addition of new LRU items. So ask list_lru_scan_node() to scan only a few entries at a time, and repeat until the scan is complete. If the shrinker runs between two consecutive calls of list_lru_scan_node() it could invalidate the "remaining" counter which could lead to premature freeing. So add a spinlock to avoid that. Signed-off-by: NeilBrown <neilb@suse.de> Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
1 parent 56221b4 commit fbfdc9f

File tree

2 files changed

+30
-3
lines changed

2 files changed

+30
-3
lines changed

fs/nfsd/filecache.c

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -544,19 +544,36 @@ nfsd_file_gc_cb(struct list_head *item, struct list_lru_one *lru,
544544
return nfsd_file_lru_cb(item, lru, arg);
545545
}
546546

547+
/* If the shrinker runs between calls to list_lru_walk_node() in
548+
* nfsd_file_gc(), the "remaining" count will be wrong. This could
549+
* result in premature freeing of some files. This may not matter much
550+
* but is easy to fix with this spinlock which temporarily disables
551+
* the shrinker.
552+
*/
553+
static DEFINE_SPINLOCK(nfsd_gc_lock);
547554
static void
548555
nfsd_file_gc(void)
549556
{
550557
unsigned long ret = 0;
551558
LIST_HEAD(dispose);
552559
int nid;
553560

561+
spin_lock(&nfsd_gc_lock);
554562
for_each_node_state(nid, N_NORMAL_MEMORY) {
555-
unsigned long nr = list_lru_count_node(&nfsd_file_lru, nid);
563+
unsigned long remaining = list_lru_count_node(&nfsd_file_lru, nid);
564+
565+
while (remaining > 0) {
566+
unsigned long nr = min(remaining, NFSD_FILE_GC_BATCH);
556567

557-
ret += list_lru_walk_node(&nfsd_file_lru, nid, nfsd_file_gc_cb,
558-
&dispose, &nr);
568+
remaining -= nr;
569+
ret += list_lru_walk_node(&nfsd_file_lru, nid, nfsd_file_gc_cb,
570+
&dispose, &nr);
571+
if (nr)
572+
/* walk aborted early */
573+
remaining = 0;
574+
}
559575
}
576+
spin_unlock(&nfsd_gc_lock);
560577
trace_nfsd_file_gc_removed(ret, list_lru_count(&nfsd_file_lru));
561578
nfsd_file_dispose_list_delayed(&dispose);
562579
}
@@ -581,8 +598,12 @@ nfsd_file_lru_scan(struct shrinker *s, struct shrink_control *sc)
581598
LIST_HEAD(dispose);
582599
unsigned long ret;
583600

601+
if (!spin_trylock(&nfsd_gc_lock))
602+
return SHRINK_STOP;
603+
584604
ret = list_lru_shrink_walk(&nfsd_file_lru, sc,
585605
nfsd_file_lru_cb, &dispose);
606+
spin_unlock(&nfsd_gc_lock);
586607
trace_nfsd_file_shrinker_removed(ret, list_lru_count(&nfsd_file_lru));
587608
nfsd_file_dispose_list_delayed(&dispose);
588609
return ret;

fs/nfsd/filecache.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@
33

44
#include <linux/fsnotify_backend.h>
55

6+
/*
7+
* Limit the time that the list_lru_one lock is held during
8+
* an LRU scan.
9+
*/
10+
#define NFSD_FILE_GC_BATCH (16UL)
11+
612
/*
713
* This is the fsnotify_mark container that nfsd attaches to the files that it
814
* is holding open. Note that we have a separate refcount here aside from the

0 commit comments

Comments
 (0)