Kswapd madness in 2.4 kernels

Folks,

We have some customers with some fairly beefy servers. They can get the system into an unusable state that has been reported on lkml before. Namely, kswapd starts taking 100% of a CPU, any other process that attempts to allocate memory starts to spin on memory locks, etc. The box slows way down, and is pretty much dead when this happens. Kswapd never drops below 50%. Slabinfo shows that the inode and buffer caches have grown enormously, and low memory is nearly gone. (But several Gb of high memory is available.)

This pathalogical behavior can be triggered by something as simple as:

	"cd / ; cp -r . /raidfs"

Where /raidfs and root are HW RAID arrays.

The two attached patches applied to 2.4.19 fix the problem on our test boxes.

Are these patches still considered a good idea for 2.4? Is there something better I should be using?

TIA,

James Cleverdon
IBM xSeries Linux Solutions
{jamesclv(Unix, preferred), cleverdj(Notes)} at us dot ibm dot com

From: Andrea Arcangeli <[email protected]>
Date: Fri, 24 May 2002 09:33:41 +0200
To: Andrew Morton <[email protected]>
Cc: Martin Schwidefsky <[email protected]>,
[email protected], Alan Cox <[email protected]>, Rik van Riel <[email protected]>, [email protected], Andrey Savochkin <[email protected]>
Subject: inode highmem imbalance fix [Re: Bug with shared memory.]
In-Reply-To: <[email protected]>
User-Agent: Mutt/1.3.27i
X-GnuPG-Key-URL: http://e-mind.com/~andrea/aa.gnupg.asc
X-PGP-Key-URL: http://e-mind.com/~andrea/aa.asc
Precedence: bulk
X-Mailing-List: [email protected]

Here it is, you should apply it together with vm-35 that you need too for the bh/highmem balance (or on top of 2.4.19pre8aa3). I tested it slightly on uml and it didn't broke so far, so be careful because it's not very well tested yet. On the lines of what Alexey suggested originally, if goal isn't reached, in a second pass we shrink the cache too, but only if the cache is the only reason for the "pinning" beahiour of the inode. If for example there are dirty blocks of metadata or of data belonging to the inode we wakeup_bdflush instead and we never shrink the cache in such case. If the inode itself is dirty as well we let the two passes fail so we will schedule the work for keventd. This logic should ensure we never fall into shrinking the cache for no good reason and that we free the cache only for the inodes that we actually go ahead and free. (basically only dirty pages set with SetPageDirty aren't trapped by the logic before calling the invalidate, like ramfs, but that's expected of course, those pages cannot be damaged by the non destructive invalidate anyways)

Comments?

@@ -672,35 +672,87 @@ void prune_icache(int goal)
 {
 	LIST_HEAD(list);
 	struct list_head *entry, *freeable = &list;
-	int count;
+	int count, pass;
 	struct inode * inode;
 
-	spin_lock(&inode_lock);
+	count = pass = 0;
 
-	count = 0;
-	entry = inode_unused.prev;
-	while (entry != &inode_unused)
-	{
-		struct list_head *tmp = entry;
+	spin_lock(&inode_lock);
+	while (goal && pass++ < 2) {
+		entry = inode_unused.prev;
+		while (entry != &inode_unused)
+		{
+			struct list_head *tmp = entry;
 
-		entry = entry->prev;
-		inode = INODE(tmp);
-		if (inode->i_state & (I_FREEING|I_CLEAR|I_LOCK))
-			continue;
-		if (!CAN_UNUSE(inode))
-			continue;
-		if (atomic_read(&inode->i_count))
-			continue;
-		list_del(tmp);
-		list_del(&inode->i_hash);
-		INIT_LIST_HEAD(&inode->i_hash);
-		list_add(tmp, freeable);
-		inode->i_state |= I_FREEING;
-		count++;
-		if (!--goal)
-			break;
+			entry = entry->prev;
+			inode = INODE(tmp);
+			if (inode->i_state & (I_FREEING|I_CLEAR|I_LOCK))
+				continue;
+			if (atomic_read(&inode->i_count))
+				continue;
+			if (pass == 2 && !inode->i_state && !CAN_UNUSE(inode)) {
+				if (inode_has_buffers(inode))
+					/*
+					 * If the inode has dirty buffers
+					 * pending, start flushing out bdflush.ndirty
+					 * worth of data even if there's no dirty-memory
+					 * pressure. Do nothing else in this
+					 * case, until all dirty buffers are gone
+					 * we can do nothing about the inode other than
+					 * to keep flushing dirty stuff. We could also
+					 * flush only the dirty buffers in the inode
+					 * but there's no API to do it asynchronously
+					 * and this simpler approch to deal with the
+					 * dirty payload shouldn't make much difference
+					 * in practice. Also keep in mind if somebody
+					 * keeps overwriting data in a flood we'd
+					 * never manage to drop the inode anyways,
+					 * and we really shouldn't do that because
+					 * it's an heavily used one.
+					 */
+					wakeup_bdflush();
+				else if (inode->i_data.nrpages)
+					/*
+					 * If we're here it means the only reason
+					 * we cannot drop the inode is that its
+					 * due its pagecache so go ahead and trim it
+					 * hard. If it doesn't go away it means
+					 * they're dirty or dirty/pinned pages ala
+					 * ramfs.
+					 *
+					 * invalidate_inode_pages() is a non
+					 * blocking operation but we introduce
+					 * a dependency order between the
+					 * inode_lock and the pagemap_lru_lock,
+					 * the inode_lock must always be taken
+					 * first from now on.
+					 */
+					invalidate_inode_pages(inode);
+			}
+			if (!CAN_UNUSE(inode))
+				continue;
+			list_del(tmp);
+			list_del(&inode->i_hash);
+			INIT_LIST_HEAD(&inode->i_hash);
+			list_add(tmp, freeable);
+			inode->i_state |= I_FREEING;
+			count++;
+			if (!--goal)
+				break;
+		}
 	}
 	inodes_stat.nr_unused -= count;
+
+	/*
+	 * the unused list is hardly an LRU so it makes
+	 * more sense to rotate it so we don't bang
+	 * always on the same inodes in case they're
+	 * unfreeable for whatever reason.
+	 */
+	if (entry != &inode_unused) {
+		list_del(&inode_unused);
+		list_add(&inode_unused, entry);
+	}
 	spin_unlock(&inode_lock);
 
 	dispose_list(freeable);

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

----- End forwarded message -----

---------- End Forwarded Message ----------

From: Andrew Morton <[email protected]>
Date: Fri, 24 May 2002 12:32:31 -0700
Return-Path: <[email protected]>
Message-ID: <[email protected]>
X-Mailer: Mozilla 4.79 [en] (X11; U; Linux 2.4.19-pre4 i686)
X-Accept-Language: en
MIME-Version: 1.0
To: "Martin J. Bligh" <[email protected]>
Cc: Roy Sigurd Karlsbakk <[email protected]>,
[email protected]
Subject: Re: [BUG] 2.4 VM sucks. Again
References: <[email protected]> <1572079531.1022225730@[10.10.2.3]>
Content-Type: text/plain;
charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: [email protected]
Precedence: bulk
X-Mailing-List: [email protected]
Status: R
X-Status: N

Cripes no. It's pretty experimental. Andrea spotted a bug, too. Fixed version is below.

It's possible that keeping the number of buffers as low as possible will give improved performance over Andrea's approach because it leaves more ZONE_NORMAL for other things. It's also possible that it'll give worse performance because more get_block's need to be done for file overwriting.

@@ -89,13 +89,7 @@ extern void add_to_page_cache(struct pag
 extern void add_to_page_cache_locked(struct page * page, struct address_space *mapping, unsigned long index);
 extern int add_to_page_cache_unique(struct page * page, struct address_space *mapping, unsigned long index, struct page **hash);
 
-extern void ___wait_on_page(struct page *);
-
-static inline void wait_on_page(struct page * page)
-{
-	if (PageLocked(page))
-		___wait_on_page(page);
-}
+extern void wait_on_page(struct page *);
 
 extern struct page * grab_cache_page (struct address_space *, unsigned long);
 extern struct page * grab_cache_page_nowait (struct address_space *, unsigned long);

@@ -202,7 +202,6 @@ EXPORT_SYMBOL(ll_rw_block);
 EXPORT_SYMBOL(submit_bh);
 EXPORT_SYMBOL(unlock_buffer);
 EXPORT_SYMBOL(__wait_on_buffer);
-EXPORT_SYMBOL(___wait_on_page);
 EXPORT_SYMBOL(generic_direct_IO);
 EXPORT_SYMBOL(discard_bh_page);
 EXPORT_SYMBOL(block_write_full_page);

@@ -365,8 +365,13 @@ static int shrink_cache(int nr_pages, zo
 		if (unlikely(!page_count(page)))
 			continue;
 
-		if (!memclass(page_zone(page), classzone))
+		if (!memclass(page_zone(page), classzone)) {
+			if (page->buffers && !TryLockPage(page)) {
+				try_to_release_page(page, GFP_NOIO);
+				unlock_page(page);
+			}
 			continue;
+		}
 
 		/* Racy check to avoid trylocking when not worthwhile */
 		if (!page->buffers && (page_count(page) != 1 || !page->mapping))

@@ -562,6 +567,11 @@ static int shrink_caches(zone_t * classz
 	nr_pages -= kmem_cache_reap(gfp_mask);
 	if (nr_pages <= 0)
 		return 0;
+	if ((gfp_mask & __GFP_WAIT) && (shrink_buffer_cache() > 16)) {
+		nr_pages -= kmem_cache_reap(gfp_mask);
+		if (nr_pages <= 0)
+			return 0;
+	}
 
 	nr_pages = chunk_size;
 	/* try to keep the active list 2/3 of the size of the cache */

@@ -1116,6 +1116,7 @@ extern int FASTCALL(try_to_free_buffers(
 extern void refile_buffer(struct buffer_head * buf);
 extern void create_empty_buffers(struct page *, kdev_t, unsigned long);
 extern void end_buffer_io_sync(struct buffer_head *bh, int uptodate);
+extern int shrink_buffer_cache(void);
 
 /* reiserfs_writepage needs this */
 extern void set_buffer_async_io(struct buffer_head *bh) ;

-

That's in -aa kernels, is correct and is needed.

hmm. Someone seems to have renamed my nuke-buffers patch ;)

My main concern is that this was a real quickie; it does a very aggressive takedown of buffer_heads. Andrea's kernels contain a patch which takes a very different approach. See
http://www.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.4/2.4.20pre8aa2/05_vm_16_active_free_zone_bhs-1

I don't think anyone has tried that patch in isolation though...

If nuke-buffers passes testing and doesn't impact performance then fine. A more cautious approach would be to use the active_free_zone_bhs patch. If that proves inadequate then add in the "read" part of nuke-buffers. That means dropping the fs/buffer.c part.

Yes, these patches are a good idea. I'm curious why they haven't been submitted to Marcelo yet ;)

Rik

Bravely reimplemented by the knights who say "NIH".
http://www.surriel.com/		http://distro.conectiva.com/
Current spamtrap:  <a href=mailto:"[email protected]">[email protected]</a>

Status report:

Due to dependencies, I didn't try the two recommended patches alone. I ran Andrea's 2.4.20-pre10aa1 kernel on the test load for one week. Low memory was conserved and kswapd never went out of control. Presumably, 05_vm_16_active_free_zone_bhs-1 did the job for buffers, and the inode patch continued to work.

Are there any plans on getting these into 2.4.21?

James Cleverdon
IBM xSeries Linux Solutions
{jamesclv(Unix, preferred), cleverdj(Notes)} at us dot ibm dot com

yes, for stability the related-bh patch is known to be more than enough and this is a nice confirmation. I would also like to integrated some bit of andrew's nuke-buffer patch for performance reasons (to maximize the free memory utilization), not for stability. For stability teaching the VM about the problem is the right fix IMHO, good to have regardless in case for some reason the bh cannot be nucked if we can't take a lock or similar. But the bit that drops the bhs after reads may improve memory utilization when there is no memory pressure at all. The part I wouldn't merge in 2.4 from the Andrew's patch is the drop after writes, that has the potential of slowing down rewrite. I'm not saying it will slow down the rewrite performance, but there is definitely the potential. My fix instead has no way to affect read/writes w/o memory pressure compared to mainline (i.e. in a <1G machine).

the related-bhs fix should be definitely integrated. Then Andrew's patch that drops bh after reads may be an obvious further optimization but not really related to this bug anymore. I didn't experimented with it yet because it was low prio and it can only improve performance by saving some ram. And even only merging the drop bhs after read from the nuke-buffers, still might decrease performance in a read+write case in the same pagecache block, but that case probably isn't very common, rewrite instead is more likely to happen.

they have been sumitted, I think I covered this bit with Marcelo during the kernel summit, but you know there are many other important patches to merge, the google fix etc.. but we must not forget that lots of them are been just integrated in 2.4.20pre, it is normal that we discuss only the pending fix that aren't been integrated yet and we forget about the ones that are just included ;).

Andrea

I will look closely at -aa during 2.4.21-pre stage, yes.

Andrea, please bug me on that.

sure ;)

Andrea