Slab Defrag / Slab Targeted Reclaim and general Slab API changes

This patch gets rid constructors and destructors and replaces them with a slab operations structure that is passed into SLUB.

For consistency with other operations structures, can we make this struct kmem_cache_operations or kmem_cache_ops, please?

include/linux/slab.h extern void *__kmalloc_node_track_caller · line 276

Quoted 6 lines · [email protected] wrote:

include/linux/slab.h +6 -0

+     if (ctor || dtor) {
+             so = kzalloc(sizeof(struct slab_ops), GFP_KERNEL);
+             so->ctor = ctor;
+             so->dtor = dtor;
+     }
+     return  __kmem_cache_create(s, size, align, flags, so);

Is this a memory leak?

If constructor or destructor are specified then we will allocate a slab_ops structure and populate it with the values specified. Note that this will cause a memory leak if the slab is disposed of later. If you need disposable slabs then the new API must be used.

include/linux/slab.h extern void *__kmalloc_node_track_caller · line 273

Quoted 3 lines

include/linux/slab.h +3 -0

+     if (ctor || dtor) {
+             so = kzalloc(sizeof(struct slab_ops), GFP_KERNEL);
+             so->ctor = ctor;

mm/slub.c EXPORT_SYMBOL(kfree); · line 2175

Quoted 2 lines

mm/slub.c +0 -0

 	/* Perform the KICK callbacks to remove the objects */
 	for(p = addr; p < addr + s->objects * s->size; p += s->size)

missed a space after "for".

2. kick_object(void *)
After SLUB has established references to the remaining objects in a slab it will drop all locks and then use kick_object on each of the objects for which we obtained a reference. The existence of the objects is guaranteed by virtue of the earlier obtained reference. The callback may perform any slab operation since no locks are held at the time of call.
The callback should remove the object from the slab in some way. This may be accomplished by reclaiming the object and then running kmem_cache_free() or reallocating it and then running kmem_cache_free(). Reallocation is advantageous at this point because it will then allocate from the partial slabs with the most objects because we have just finished slab shrinking.
NOTE: This patch is for conceptual review. I'd appreciate any feedback especially on the locking approach taken here. It will be critical to resolve the locking issue for this approach to become feasable.

Signed-off-by Christoph Lameter <[email protected]>

kick_object() doesn't return an indicator of success, which might be helpful for determining whether an object was successfully removed. The later-added kick_dentry_object(), for instance, can't remove dentries where reference counts are still held.

I suppose one could check to see if the ->inuse counter decreased, too.

In either event, it would probably be helpful to abort the operation if there was a reclamation failure for an object within the slab.

This is a relatively minor optimization concern. I think this patch series is great and a significant foray into the problem of slab reclaim vs. fragmentation.

NOTE: This patch is for conceptual review. I'd appreciate any feedback especially on the locking approach taken here. It will be critical to resolve the locking issue for this approach to become feasable.

Quoted 4 lines · [email protected] writes:

NOTE: This patch is for conceptual review. I'd appreciate any feedback especially on the locking approach taken here. It will be critical to resolve the locking issue for this approach to become feasable.

Do you have any numbers on how this improves dcache reclaim under memory pressure?

Quoted 7 lines · Andi Kleen

Quoted 4 lines · [email protected] writes:

NOTE: This patch is for conceptual review. I'd appreciate any feedback especially on the locking approach taken here. It will be critical to resolve the locking issue for this approach to become feasable.

Do you have any numbers on how this improves dcache reclaim under memory pressure?

How does one measure something like that?

Targeted reclaim allows to target a single slab for reclaim. This is done by calling

kmem_cache_vacate(slab, page);

It will return 1 on success, 0 if the operation failed.

The vacate functionality is also used for slab shrinking. During the shrink operation SLUB will generate a list sorted by the number of objects in use.

We extract pages off that list that are only filled less than a quarter. These objects are then processed using kmem_cache_vacate.

In order for a slabcache to support this functionality two functions must be defined via slab_operations.

get_reference(void *)

Must obtain a reference to the object if it has not been freed yet. It is up to the slabcache to resolve the race.

SLUB guarantees that the objects is
still allocated. However, another thread may be blocked in slab_free attempting to free the same object. It may succeed as soon as get_reference() returns to the slab allocator.

get_reference() processing must recognize this situation (i.e. check refcount for zero) and fail in such a sitation (no problem since the object will be freed as soon we drop the slab lock before doing kick calls).

No slab operations may be performed in get_reference(). The slab lock for the page with the object is taken. Any slab operations may lead to a deadlock.

2. kick_object(void *)

After SLUB has established references to the remaining objects in a slab it will drop all locks and then use kick_object on each of the objects for which we obtained a reference. The existence of the objects is guaranteed by virtue of the earlier obtained reference. The callback may perform any slab operation since no locks are held at the time of call.

The callback should remove the object from the slab in some way. This may be accomplished by reclaiming the object and then running kmem_cache_free() or reallocating it and then running kmem_cache_free(). Reallocation is advantageous at this point because it will then allocate from the partial slabs with the most objects because we have just finished slab shrinking.

From the comments alone, it sounds like the page cache API should be

NOTE: This patch is for conceptual review. I'd appreciate any feedback especially on the locking approach taken here. It will be critical to resolve the locking issue for this approach to become feasable.

Signed-off-by Christoph Lameter <[email protected]>

---
include/linux/slab.h | 3
mm/slub.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++++---

2 files changed, 154 insertions(+), 8 deletions(-)

Index: slub/include/linux/slab.h
===================================================================

include/linux/slab.h +3 -0

@@ -42,6 +42,8 @@ struct slab_ops {
 	void (*ctor)(void *, struct kmem_cache *, unsigned long);
 	/* FIXME: Remove all destructors ? */
 	void (*dtor)(void *, struct kmem_cache *, unsigned long);
+	int (*get_reference)(void *);
+	void (*kick_object)(void *);
 };
 
 struct kmem_cache *__kmem_cache_create(const char *, size_t, size_t,

@@ -54,6 +56,7 @@ void kmem_cache_free(struct kmem_cache *
 unsigned int kmem_cache_size(struct kmem_cache *);
 const char *kmem_cache_name(struct kmem_cache *);
 int kmem_ptr_validate(struct kmem_cache *cachep, const void *ptr);
+int kmem_cache_vacate(struct page *);
 
 /*
 * Please use this macro to create slab caches. Simply specify the

Index: slub/mm/slub.c
===================================================================

mm/slub.c +2 -0

 	SYSFS		/* Sysfs up */
 } slab_state = DOWN;
 

@@ -211,6 +211,8 @@ static inline struct kmem_cache_node *ge
 
 struct slab_ops default_slab_ops = {
 	NULL,
+	NULL,
+	NULL,
 	NULL
 };
 

@@ -839,13 +841,10 @@ static struct page *new_slab(struct kmem
 	n = get_node(s, page_to_nid(page));
 	if (n)

mm/slub.c +11 -1

 	start = page_address(page);
 	end = start + s->objects * s->size;
 

@@ -862,7 +861,17 @@ static struct page *new_slab(struct kmem
 	set_freepointer(s, last, NULL);
 
 	page->freelist = start;
-	page->inuse = 0;
+
+	/*
+	 * pages->inuse must be visible when PageSlab(page) becomes
+	 * true for targeted reclaim
+	 */
+	smp_wmb();
+	page->flags |= 1 << PG_slab;
+	if (s->flags & (SLAB_DEBUG_FREE | SLAB_RED_ZONE | SLAB_POISON |
+			SLAB_STORE_USER | SLAB_TRACE))
+		page->flags |= 1 << PG_error;
+

out:

	if (flags & __GFP_WAIT)
		local_irq_disable();

mm/slub.c +19 -0

+ * remove the slab from the lists. At the end the slab will either
+ * be freed or have been returned to the partial lists.
+ *
+ * Return error code or number of remaining objects
+ */
+static int __kmem_cache_vacate(struct kmem_cache *s, struct page *page)
+{
+	void *p;
+	void *addr = page_address(page);
+	unsigned long map[BITS_TO_LONGS(s->objects)];
+	int leftover;
+
+	if (!page->inuse)
+		return 0;
+
+	/* Determine free objects */
+	bitmap_zero(map, s->objects);
+	for(p = page->freelist; p; p = get_freepointer(s, p))
+		set_bit((p - addr) / s->size, map);

mm/slub.c +5 -0

+
+/*
+ * Remove a page from the lists. Must be holding slab lock.
+ */
+static void remove_from_lists(struct kmem_cache *s, struct page *page)

mm/slub.c +19 -0

+int kmem_cache_vacate(struct page *page)
+{
+	struct kmem_cache *s;
+	int rc = 0;
+
+	/* Get a reference to the page. Return if its freed or being freed */
+	if (!get_page_unless_zero(page))
+		return 0;
+
+	/* Check that this is truly a slab page */
+	if (!PageSlab(page))
+		goto out;
+
+	slab_lock(page);
+
+	/*
+	 * We may now have locked a page that is in various stages of being
+	 * freed. If the PageSlab bit is off then we have already reached
+	 * the page allocator. If page->inuse is zero then we are

mm/slub.c +8 -0

 	struct page *t;
 	struct list_head *slabs_by_inuse =
 		kmalloc(sizeof(struct list_head) * s->objects, GFP_KERNEL);
 	unsigned long flags;
+	LIST_HEAD(zaplist);
 
 	if (!slabs_by_inuse)
 		return -ENOMEM;

@@ -2194,8 +2309,36 @@ int kmem_cache_shrink(struct kmem_cache
 		for (i = s->objects - 1; i >= 0; i--)
 			list_splice(slabs_by_inuse + i, n->partial.prev);
 
+		if (!s->slab_ops->get_reference || !s->slab_ops->kick_object)
+			goto out;
+
+		/* Take objects with just a few objects off the tail */
+		while (n->nr_partial > MAX_PARTIAL) {
+			page = container_of(n->partial.prev, struct page, lru);
+

mm/slub.c +0 -0

 	}
 
 	kfree(slabs_by_inuse);

Signature · 1 line

-- 

Quoted 2 lines

Must obtain a reference to the object if it has not been freed yet. It is up to the slabcache to resolve the race.

Is there also a race here between the object being searched for and the reference being taken? Would it be appropriate to call this find_get_slub_object() to mirror what find_get_page() does except instead of offset, it receives some key meaningful to the cache? It might help set a developers expectation of what the function is required to do when creating such a cache and simplify locking.

(written much later) Looking through, it's not super-clear if get_reference() is only for use by SLUB to get an exclusive reference to an object or something that is generally called to reference count it. It looks like only one reference is ever taken.

Quoted 4 lines

SLUB guarantees that the objects is
still allocated. However, another thread may be blocked in slab_free attempting to free the same object. It may succeed as soon as get_reference() returns to the slab allocator.

Fun. If it was find_get_slub_object() instead of get_reference(), the caller could block if the object is currently being freed until slab_free completed. It would take a hit because the object has to be recreated but maybe it would be an easier race to deal with?

Quoted 5 lines

get_reference() processing must recognize this situation (i.e. check refcount
for zero) and fail in such a sitation (no problem since the object will be freed as soon we drop the slab lock before doing kick calls).

Block or fail?

Quoted 6 lines

The callback should remove the object from the slab in some way. This may be accomplished by reclaiming the object and then running kmem_cache_free() or reallocating it and then running kmem_cache_free(). Reallocation is advantageous at this point because it will then allocate from the partial slabs with the most objects because we have just finished slab shrinking.

Quoted 1 line

From the comments alone, it sounds like the page cache API should be

copied in name at least like slub_object_get(), slub_object_release() and maybe even something like slub_object_get_unless_vacating() if the caller must not sleep. It seems the page cache shares similar problems to what SLUB is doing here.

Take everything I say here with a grain of salt. My understanding of SLUB is non-existent at the moment and it makes reviewing this a bit tricky.

Quoted 4 lines

Index: slub/mm/slub.c
===================================================================

mm/slub.c +0 -0

mm/slub.c static struct notifier_block slab_notifi · line 176

Quoted 6 lines

mm/slub.c +1 -1

@@ -173,7 +173,7 @@ static struct notifier_block slab_notifi
 static enum {
 	DOWN,		/* No slab functionality available */
 	PARTIAL,	/* kmem_cache_open() works but kmalloc does not */
-	UP,		/* Everything works */
+	UP,		/* Everything works but does not show up in sysfs */

Should this be here?

mm/slub.c static struct page *new_slab(struct kmem · line 847

Quoted 6 lines

mm/slub.c +1 -5

-	page->flags |= 1 << PG_slab;
-	if (s->flags & (SLAB_DEBUG_FREE | SLAB_RED_ZONE | SLAB_POISON |
-			SLAB_STORE_USER | SLAB_TRACE))
-		page->flags |= 1 << PG_error;
-
+	page->inuse = 0;

You probably have explained this a million times already and in the main SLUB patches, but why is _count not used?

Quoted 5 lines

/*
+ * Vacate all objects in the given slab. Slab must be locked.
+ *
+ * Will drop and regain and drop the slab lock.
+ * Slab must be marked PageActive() to avoid concurrent slab_free from

I just noticed this PageActive() overloading of the page->flags. Is it worth defining SlabPerCPU() as an alias or something?

Quoted 4 lines

mm/slub.c +4 -0

+	/*
+	 * Get a refcount for all used objects. If that fails then
+	 * no KICK callback can be performed.
+	 */

mm/slub.c void kfree(const void *x) · line 2168

Quoted 5 lines

mm/slub.c +5 -0

+	for(p = addr; p < addr + s->objects * s->size; p += s->size)
+		if (!test_bit((p - addr) / s->size, map))
+			if (!s->slab_ops->get_reference(p))
+				set_bit((p - addr) / s->size, map);
+

The comment and code implies that only one caller can hold a reference at a time and the count is either 1 or 0.

(later)

It looks like get_reference() is only intended for use by SLUB. I don't currently see how an implementer of a cache could determine if an object is being actively referenced or not when deciding whether to return 1 for get_reference() unless the cache interally implemented a referencing count API. That might lead to many, similar but subtly different implementations of reference counting.

mm/slub.c void kfree(const void *x) · line 2171

Quoted 4 lines

mm/slub.c +3 -0

+	/* Got all the references we need. Now we can drop the slab lock */
+	slab_unlock(page);
+

Where did we check we got all the references? It looks more like we got all the references we could.

mm/slub.c void kfree(const void *x) · line 2197

Quoted 4 lines

mm/slub.c +4 -0

+
+/*
+ * Attempt to free objects in a page. Return 1 when succesful.
+ */

when or if? Looks more like if

mm/slub.c void kfree(const void *x) · line 2226

Quoted 5 lines

mm/slub.c +5 -0

+	if (!PageSlab(page) || PageActive(page) || !page->inuse) {
+		slab_unlock(page);
+		goto out;
+	}
+

The PageActive() part is going to confuse someone eventually because they'll interpret it to mean that slab pages are on the LRU now.

mm/slub.c void kfree(const void *x) · line 2234

Quoted 8 lines

mm/slub.c +8 -0

+	/*
+	 * We are holding a lock on a slab page that is not in the
+	 * process of being allocated or freed.
+	 */
+	s = page->slab;
+	remove_from_lists(s, page);
+	SetPageActive(page);
+	rc = __kmem_cache_vacate(s, page) == 0;

The name rc is misleading here. I am reading it as reference count, but it's not a reference count at all.

mm/slub.c void kfree(const void *x) · line 2238

Quoted 4 lines

mm/slub.c +4 -0

+out:
+	put_page(page);
+	return rc;
+}

Where do partially freed slab pages get put back on the lists? Do they just remain off the lists until they are totally freed as a type of lazy free? If so, a wait_on_slab_page_free() call may be needed later.

Quoted 4 lines

+/*

 *  kmem_cache_shrink removes empty slabs from the partial lists
 *  and then sorts the partially allocated slabs by the number
 *  of items in use. The slabs with the most items in use

mm/slub.c int kmem_cache_shrink(struct kmem_cache · line 2254

Quoted 6 lines

mm/slub.c +1 -1

@@ -2137,11 +2251,12 @@ int kmem_cache_shrink(struct kmem_cache
 	int node;
 	int i;
 	struct kmem_cache_node *n;
-	struct page *page;
+	struct page *page, *page2;

page2, is a pretty bad name.

mm/slub.c int kmem_cache_shrink(struct kmem_cache · line 2328

Quoted 3 lines

mm/slub.c +3 -0

+			if (page->inuse > s->objects / 4)
+				break;
+

I get the idea of the check but it may have unexpected consequences if all of the slabs in use happen to be half full.

mm/slub.c int kmem_cache_shrink(struct kmem_cache · line 2333

Quoted 6 lines

mm/slub.c +5 -0

+			list_move(&page->lru, &zaplist);
+			n->nr_partial--;
+			SetPageActive(page);
+			slab_unlock(page);
+		}

Ok, not sure if this is a problem or not. I don't get why SetPageActive() is being called and it's because I don't understand SLUB yet.

Quoted 4 lines

		spin_unlock_irqrestore(&n->list_lock, flags);

mm/slub.c +3 -0

+
+		/* Now we can free objects in the slabs on the zaplist */
+		list_for_each_entry_safe(page, page2, &zaplist, lru) {

mm/slub.c int kmem_cache_shrink(struct kmem_cache · line 2341

Quoted 3 lines

mm/slub.c +3 -0

+			slab_lock(page);
+			__kmem_cache_vacate(s, page);
+		}

No checking the return value here which is why I think that slab pages once vacated are expected to be off the lists and totally freed. If that is the case, it might have consequences if the cache is badly behaving and never freeing objects.

I originally intended this for the 2.6.23 development cycle but since there is an aggressive push for SLUB I thought that we may want to introduce this earlier. Note that this covers new locking approaches that we may need to talk over before going any further.

This is an RFC for patches that do major changes to the way that slab allocations are handled in order to introduce some more advanced features and in order to get rid of some things that are no longer used or awkward.

A. Add Slab fragmentation

On kmem_cache_shrink SLUB will not only sort the partial slabs by object number but attempt to free objects out of partial slabs that have a low number of objects. Doing so increases the object density in the remaining partial slabs and frees up memory. Ideally kmem_cache_shrink would be able to completely defrag the partial list so that only one partial slab is left over. But it is advantageous to have slabs with a few free objects since that speeds up kfree. Also going to the extreme on this one would mean that the reclaimable slabs would have to be able to move objects in a reliable way. So we just free objects in slabs with a low population ratio and tolerate if a attempt to move an object fails.

B. Targeted Reclaim

Mainly to support antifragmentation / defragmentation methods. The slab adds a new function kmem_cache_vacate(struct page *) which can be used to request that a page be cleared of all objects. This makes it possible to reduce the size of the RECLAIMABLE fragmentation area and move slabs into the MOVABLE area enhancing the capabilities of antifragmentation significantly.

C. Introduces a slab_ops structure that allows a slab user to provide
operations on slabs.

This replaces the current constructor / destructor scheme. It is necessary in order to support additional methods needed to support targeted reclaim and slab defragmentation. A slab supporting targeted reclaim and slab defragmentation must support the following additional methods:

1. get_reference(void *)
Get a reference on a particular slab object.

2. kick_object(void *)
Kick an object off a slab. The object is either reclaimed (easiest) or a new object is alloced using kmem_cache_alloc() and then the object is moved to the new location.

D. Slab creation is no longer done using kmem_cache_create

kmem_cache_create is not a clean API since it has only 2 call backs for constructor and destructor, does not allow the specification of a slab ops structure. Parameters are confusing.

F.e. It is possible to specify alignment information in the alignment field and in addition in the flags field (SLAB_HWCACHE_ALIGN). The semantics of SLAB_HWCACHE_ALIGN are fuzzy because it only aligns object if larger than 1/2 cache line.

All of this is really not necessary since the compiler knows how to align structures and we should use this information instead of having the user specify an alignment. I would like to get rid of SLAB_HWCACHE_ALIGN and kmem_cache_create. Instead one would use the following macros (that then result in a call to __kmem_cache_create).

KMEM_CACHE(<struct-name>, flags)

The macro will determine the slab name from the struct name and use that for /sys/slab, will use the size of the struct for slab size and the alignment of the structure for alignment. This means one will be able to set slab object alignment by specifying the usual alignment options for static allocations when defining the structure.

Since the name is derived from the struct name it will much easier to find the source code for slabs listed in /sys/slab.

An additional macro is provided if the slab also supports slab operations.

KMEM_CACHE_OPS(<struct-name>, flags, slab_ops)

It is likely that this macro will be rarely used.

E. kmem_cache_create() SLAB_HWCACHE_ALIGN legacy interface

In order to avoid having to modify all slab creation calls throughout the kernel we will provide a kmem_cache_create emulation. That function is the only call that will still understand SLAB_HWCACHE_ALIGN. If that parameter is specified then it will set up the proper alignment (the slab allocators never see that flag).

If constructor or destructor are specified then we will allocate a slab_ops structure and populate it with the values specified. Note that this will cause a memory leak if the slab is disposed of later. If you need disposable slabs then the new API must be used.

F. Remove destructor support from all slab allocators?

I am only aware of two call sites left after all the changes that are scheduled to go into 2.6.22-rc1 have been merged. These are in FRV and sh arch code. The one in FRV will go away if they switch to quicklists like i386. Sh contains another use but a single user is no justification for keeping destructors around.

Quoted 2 lines

C. Introduces a slab_ops structure that allows a slab user to provide
operations on slabs.

Could you please make it const ?

Quoted 5 lines

All of this is really not necessary since the compiler knows how to align structures and we should use this information instead of having the user specify an alignment. I would like to get rid of SLAB_HWCACHE_ALIGN and kmem_cache_create. Instead one would use the following macros (that then result in a call to __kmem_cache_create).

Hum, the problem is the compiler sometimes doesnt know the target processor alignment.

Adding ____cacheline_aligned to 'struct ...' definitions might be overkill if you compile a generic kernel and happens to boot a Pentium III with it.

G. Being able to track the number of pages in a kmem_cache

If you look at fs/buffer.c, you'll notice the bh_accounting, recalc_bh_state() that might be overkill for large SMP configurations, when the real concern is to be able to limit the bh's not to exceed 10% of LOWMEM.

Adding a callback in slab_ops to track total number of pages in use by a given kmem_cache would be good.

Same thing for fs/file_table.c : nr_file logic (percpu_counter_dec()/percpu_counter_inc() for each file open/close) could be simplified if we could just count the pages in use by filp_cachep kmem_cache. The get_nr_files() thing is not worth the pain.

Quoted 4 lines

Quoted 3 lines

C. Introduces a slab_ops structure that allows a slab user to provide
operations on slabs.

Could you please make it const ?

Sure. Done.

Quoted 6 lines

Quoted 6 lines

All of this is really not necessary since the compiler knows how to align structures and we should use this information instead of having the user specify an alignment. I would like to get rid of SLAB_HWCACHE_ALIGN and kmem_cache_create. Instead one would use the following macros (that then result in a call to __kmem_cache_create).

Quoted 5 lines

Hum, the problem is the compiler sometimes doesnt know the target processor alignment.

Adding ____cacheline_aligned to 'struct ...' definitions might be overkill if you compile a generic kernel and happens to boot a Pentium III with it.

Then add ___cacheline_aligned_in_smp or specify the alignment in the various other ways that exist. Practice is that most slabs specify SLAB_HWCACHE_ALIGN. So most slabs are cache aligned today.

Quoted 10 lines

G. Being able to track the number of pages in a kmem_cache

If you look at fs/buffer.c, you'll notice the bh_accounting, recalc_bh_state() that might be overkill for large SMP configurations, when the real concern is to be able to limit the bh's not to exceed 10% of LOWMEM.

Adding a callback in slab_ops to track total number of pages in use by a given kmem_cache would be good.

Such functionality exists internal to SLUB and in the reporting tool. I can export that function if you need it.

Quoted 4 lines

Same thing for fs/file_table.c : nr_file logic (percpu_counter_dec()/percpu_counter_inc() for each file open/close) could be simplified if we could just count the pages in use by filp_cachep kmem_cache. The get_nr_files() thing is not worth the pain.

Sure. What exactly do you want? The absolute number of pages of memory that the slab is using?

kmem_cache_pages_in_use(struct kmem_cache *) ?

The call will not be too lightweight since we will have to loop over all nodes and add the counters in each per node struct for allocates slabs.

Christoph Lameter a ecrit :

Quoted 18 lines

Quoted 10 lines

G. Being able to track the number of pages in a kmem_cache

If you look at fs/buffer.c, you'll notice the bh_accounting, recalc_bh_state() that might be overkill for large SMP configurations, when the real concern is to be able to limit the bh's not to exceed 10% of LOWMEM.

Adding a callback in slab_ops to track total number of pages in use by a given kmem_cache would be good.

Such functionality exists internal to SLUB and in the reporting tool. I can export that function if you need it.

Quoted 4 lines

Same thing for fs/file_table.c : nr_file logic (percpu_counter_dec()/percpu_counter_inc() for each file open/close) could be simplified if we could just count the pages in use by filp_cachep kmem_cache. The get_nr_files() thing is not worth the pain.

Quoted 7 lines

Sure. What exactly do you want? The absolute number of pages of memory that the slab is using?

kmem_cache_pages_in_use(struct kmem_cache *) ?

The call will not be too lightweight since we will have to loop over all nodes and add the counters in each per node struct for allocates slabs.

On a typical system, number of pages for 'filp' kmem_cache tends to be stable

-bash-2.05b# grep filp /proc/slabinfo
filp 234727 374100 256 15 1 : tunables 120 60 8 : slabdata 24940 24940 135
-bash-2.05b# grep filp /proc/slabinfo
filp 234776 374100 256 15 1 : tunables 120 60 8 : slabdata 24940 24940 168
-bash-2.05b# grep filp /proc/slabinfo
filp 234728 374100 256 15 1 : tunables 120 60 8 : slabdata 24940 24940 180
-bash-2.05b# grep filp /proc/slabinfo
filp 234724 374100 256 15 1 : tunables 120 60 8 : slabdata 24940 24940 174

So revert nr_files logic to a single integer would be enough, even for NUMA

int nr_pages_used_by_filp;
int nr_pages_filp_limit;
int filp_in_danger __read_mostly;

static void callback_pages_in_use_by_filp(int inc)

{
    int in_danger;

    nr_pages_used_by_filp += inc;

    in_danger = nr_pages_used_by_filp >= nr_pages_filp_limit;
    if (in_danger != filp_in_danger)
        filp_in_danger = in_danger;
}

struct file *get_empty_filp(void)
{
...
if (filp_in_danger && !capable(CAP_SYS_ADMIN))

    goto over;

...
}

void __init files_init(unsigned long mempages)
{
...
nr_pages_filp_limit = (mempages * 10) / 100; /* 10% for filp use */
...
}

Quoted 3 lines

Then add ___cacheline_aligned_in_smp or specify the alignment in the various other ways that exist. Practice is that most slabs specify SLAB_HWCACHE_ALIGN. So most slabs are cache aligned today.

Yes but this alignement is dynamic, not at compile time.

include/asm-i386/processor.h:739:#define cache_line_size() (boot_cpu_data.x86_cache_alignment)

So adding ____cacheline_aligned to 'struct file' for example would be a regression for people with PII or PIII