dentry/inode accounting for vm_enough_mem()
Subsystems (3)
PATCH 1/1 dentry/inode accounting for vm_enough_mem()
One of the things on the current must-fix list is:
Quoted 4 lines
- Overcommit accounting gets wrong answers
- underestimates reclaimable slab, gives bogus failures when dcache&icache are large.
More comments from Andrew:
Quoted 3 lines
If the cache slab fragmentation is bad it can be hugely wrong. A factor of ten has been observed (rarely). Factors of two happen often.
...
Quoted 8 lines
Quoted 2 lines
But, if prune_[di]cache() will only touch those which are being
counted bynr_unused, how can we be more aggressive?Well, just by assuming all slab is reclaimable is one way.
The problem with that is that to read slab accounting we need to do
get_page_state(), which is too expensive to be called for everymmap()on big SMP.
Instead of going through get_page_state(), the following code keeps track of entries as their space is allocated in the slab via {con,de}structors. It _will_ overestimate the amount of reclaimable slab but, previously, using the .nr_unused stat, this number was underestimated and caused too many good allocations to fail. This assumes that every dentry/inode allocated in the slab is reclaimable, which they probably will be if we get deperate enough anyway.
and, as for the counter type being an atomic_t:
Quoted 9 lines
Quoted 5 lines · Andrew Morton
Dave Hansen wrote:
Anatomic_tmight be a good idea, but I'm a bit worried that 24 bits might not be enough. At 160 bytes/dentry, that's 2.5GB of dentries before the counter overflows. I would imagine that we'll run out of plenty of other things before we get to _that_ many dentries.The 24-bit thing is only on sparc32. I don't think 2G of dentries is possible on sparc32 anyway.
The attached patch is against 2.5.69-mm7.
Signature · 2 lines
Dave Hansen [email protected]
fs/dcache.c +11 -1
@@ -1529,6 +1529,16 @@ out:return ino; } +void d_ctor (void * objp,struct kmem_cache_s *cachep,unsigned long dflags) +{ +atomic_inc (&dentry_stat.nr_alloced); +} + +void d_dtor (void * objp,struct kmem_cache_s *cachep,unsigned long dflags) +{ +atomic_dec (&dentry_stat.nr_alloced); +} +static void __init dcache_init (unsigned long mempages) {struct hlist_head *d;
@@ -1548,7 +1558,7 @@ static void __init dcache_init(unsignedsizeof (struct dentry ),0 ,SLAB_HWCACHE_ALIGN , - NULL, NULL); + d_ctor, d_dtor);if (!dentry_cache)panic ("Cannot create dentry cache" );
fs/inode.c +7 -0
@@ -197,6 +197,13 @@ static void init_once(void * foo, kmem_cif ((flags & (SLAB_CTOR_VERIFY |SLAB_CTOR_CONSTRUCTOR )) ==SLAB_CTOR_CONSTRUCTOR )inode_init_once (inode); + +atomic_inc (&inodes_stat.nr_alloced); +} + +void inode_dtor (void * objp,struct kmem_cache_s *cachep,unsigned long dflags) +{ +atomic_dec (&inodes_stat.nr_alloced); }/*
include/linux/dcache.h +1 -0
@@ -37,6 +37,7 @@ struct qstr {struct dentry_stat_t {int nr_dentry;int nr_unused; +atomic_t nr_alloced;int age_limit;/* age in seconds */ int want_pages;/* pages requested by system */ int dummy[2 ];
include/linux/fs.h +1 -0
@@ -58,6 +58,7 @@ extern struct files_stat_struct files_ststruct inodes_stat_t {int nr_inodes;int nr_unused; +atomic_t nr_alloced;int dummy[5 ]; };extern struct inodes_stat_t inodes_stat;
mm/mmap.c +14 -9
@@ -82,16 +82,21 @@ int vm_enough_memory(long pages) free += nr_swap_pages;/* -* The code below doesn't account for free space in the -* inode and dentry slab cache, slab cache fragmentation, -* inodes and dentries which will become freeable under -* VM load, etc. Lets just hope all these (complex) -* factors balance out... +* The code below will overestimate the amount of +* reclaimable slab. Previously, using the .nr_unused +* stat, this number was too low and caused too many +* good allocations to fail. This assumes that every +* dentry/inode allocated in the slab is reclaimable, +* which they probably will be if we get deperate +* enough. +* - Dave Hansen <[email protected]> */ - free += (dentry_stat.nr_unused *sizeof (struct dentry )) >> -PAGE_SHIFT ; - free += (inodes_stat.nr_unused *sizeof (struct inode )) >> -PAGE_SHIFT ; + free += (atomic_read (&dentry_stat.nr_alloced) * +sizeof (struct dentry )) >> +PAGE_SHIFT ; + free += (atomic_read (&inodes_stat.nr_alloced) * +sizeof (struct inode )) >> +PAGE_SHIFT ;/* * Leave the last 3% for root
Quoted 8 lines · Dave Hansen
struct
dentry_stat_t{
intnr_dentry;
intnr_unused;
+atomic_tnr_alloced;
intage_limit; /* age in seconds */
intwant_pages; /* pages requested by system */
int dummy[2];
We're not at liberty to do this because /proc/sys/fs/dentry-state and inode-state are implemented assuming that these structs are an array of integers. It'll screw up if the architecture's "int" and "atomic_t" representations are different.
Probably you can just make this an integer and add a spinlock for it, or not place it in dentry_stat.
Seems otherwise OK though. Thanks.
Quoted 5 lines · Dave Hansen
+void
inode_dtor(void * objp, struct kmem_cache_s *cachep, unsigned long dflags)
+{
+atomic_dec(&inodes_stat.nr_alloced);
}
This isn't called anywhere.
inodes are a problem, because their sizes differ, and because each filesystem uses its own slab cache for them.
I'm wondering if it would not be better to modify slab to do this. For all the inode caches and the dentry cache and the mb cache and that new shrinkable cache which just got added to XFS, we can pass a new flag into kmem_cache_create() which tells slab.c to account for this cache.
So then, in slab.c, it's just a matter of incrementing or decrementing a global counter each time slab allocates or frees a page on behalf of a thus-tagged cache. And simply read that counter in vm_enough_memory().