All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Arcangeli <andrea@suse.de>
To: Marcelo Tosatti <marcelo@conectiva.com.br>
Cc: Hugh Dickins <hugh@veritas.com>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@transmeta.com>
Subject: vm fixes for 2.4.19rc1
Date: Thu, 27 Jun 2002 16:14:13 -0400	[thread overview]
Message-ID: <20020627201413.GD1457@inspiron.ols.wavesec.org> (raw)

some fix for 2.4.19rc1 (btw, the lru_cache_del() in the LRU path is
needed in 2.5 too and it's also more efficient than the
page_cache_release, see ptrace freeing the anon pages with put_page(),
it will not pass through page_cache_release and it will trigger the
PageLRU check that __free_pages_ok isn't capable to handle in 2.5, I
will make a full vm update for 2.5 [in small pieces based on post-Andrew
split of the monolithic patch] in the next days anyways):

diff -urNp 2.4.19rc1/mm/page_alloc.c 2.4.19rc1aa1/mm/page_alloc.c
--- 2.4.19rc1/mm/page_alloc.c	Tue Jun 25 23:56:23 2002
+++ 2.4.19rc1aa1/mm/page_alloc.c	Wed Jun 26 00:55:35 2002
@@ -82,8 +82,11 @@ static void __free_pages_ok (struct page
 	/* Yes, think what happens when other parts of the kernel take 
 	 * a reference to a page in order to pin it for io. -ben
 	 */
-	if (PageLRU(page))
+	if (PageLRU(page)) {
+		if (unlikely(in_interrupt()))
+			BUG();
 		lru_cache_del(page);
+	}

this adds a debugging check just in case.

@@ -91,6 +94,8 @@ static void __free_pages_ok (struct page
 		BUG();
 	if (!VALID_PAGE(page))
 		BUG();
+	if (PageSwapCache(page))
+		BUG();

This resurrects a valid debugging check dropped in rc1.

@@ -280,10 +285,12 @@ static struct page * balance_classzone(z
 						BUG();
 					if (!VALID_PAGE(page))
 						BUG();
+					if (PageSwapCache(page))
+						BUG();

same as above. a page with page->mapping set cannot be freed, if it
happens it's a bug that we want to trap.

 					if (PageLocked(page))
 						BUG();
 					if (PageLRU(page))
-						BUG();
+						lru_cache_del(page);


This fixes a bug I found two days ago, it could explain the nvidia
oopses in __free_pages_ok. Not too bad, no risk of mm corruption or fs
corruption, just a bug could trigger in some unlikely case (never seen
it happening myself). (btw, the in_interrupt() check isn't needed above
because it's executed by the caller anyways)

diff -urNp 2.4.19rc1/mm/swap_state.c 2.4.19rc1aa1/mm/swap_state.c
--- 2.4.19rc1/mm/swap_state.c	Tue Jan 22 12:55:27 2002
+++ 2.4.19rc1aa1/mm/swap_state.c	Wed Jun 26 00:48:13 2002
@@ -95,11 +95,8 @@ int add_to_swap_cache(struct page *page,
  */
 void __delete_from_swap_cache(struct page *page)
 {
-	if (!PageLocked(page))
-		BUG();
 	if (!PageSwapCache(page))
 		BUG();
-	ClearPageDirty(page);
 	__remove_inode_page(page);
 	INC_CACHE_INFO(del_total);
 }

Here I play risky for a rc1, but it made perfect sense to me, when we
drop a page from the swapcache it doesn't make any sense to clear the
dirty bit, conceptually the only case where an anon page can be clean is
when it's part of swapcache, if it's not part of swapcache it cannot be
clean, the clearpagedirty was only to avoid oopsing in
__remove_inode_dirty but rc1 fixes that because the dirty bit can be
added without the page lock (i.e. mark_dirty_kiobuf) so the right thing
is to accept in __remove_inode_pages if somebody is dropping from the
swapcache a dirty page without oopsing. So the above patch will be now
safe on top of rc1, thanks to the fix in rc1 that avoids a false
positive bug to trigger in __remove_inode_page.

To bias the correctness of the above, we always mark the page dirty
after deleting from the swapcache, of course we have to or we risk to
lose it if the pte isn't dirty too (for example if there was a read
fault and the swapcache was clean and mapped as clean in the pte too).

@@ -114,9 +111,6 @@ void delete_from_swap_cache(struct page 
 {
 	swp_entry_t entry;
 
-	if (!PageLocked(page))
-		BUG();
-
 	block_flushpage(page, 0);
 
 	entry.val = page->index;

dropped also two pagelocked checks because there execute unconditionally
paths that checks the pagelocked bit at the lower layer.

here the full patch below (only slightly tested on my laptop so far, so
please make sure not to release this without an intermediate rc2 first :)

thanks,

diff -urNp 2.4.19rc1/mm/page_alloc.c 2.4.19rc1aa1/mm/page_alloc.c
--- 2.4.19rc1/mm/page_alloc.c	Tue Jun 25 23:56:23 2002
+++ 2.4.19rc1aa1/mm/page_alloc.c	Wed Jun 26 00:55:35 2002
@@ -82,8 +82,11 @@ static void __free_pages_ok (struct page
 	/* Yes, think what happens when other parts of the kernel take 
 	 * a reference to a page in order to pin it for io. -ben
 	 */
-	if (PageLRU(page))
+	if (PageLRU(page)) {
+		if (unlikely(in_interrupt()))
+			BUG();
 		lru_cache_del(page);
+	}
 
 	if (page->buffers)
 		BUG();
@@ -91,6 +94,8 @@ static void __free_pages_ok (struct page
 		BUG();
 	if (!VALID_PAGE(page))
 		BUG();
+	if (PageSwapCache(page))
+		BUG();
 	if (PageLocked(page))
 		BUG();
 	if (PageActive(page))
@@ -280,10 +285,12 @@ static struct page * balance_classzone(z
 						BUG();
 					if (!VALID_PAGE(page))
 						BUG();
+					if (PageSwapCache(page))
+						BUG();
 					if (PageLocked(page))
 						BUG();
 					if (PageLRU(page))
-						BUG();
+						lru_cache_del(page);
 					if (PageActive(page))
 						BUG();
 					if (PageDirty(page))
diff -urNp 2.4.19rc1/mm/swap_state.c 2.4.19rc1aa1/mm/swap_state.c
--- 2.4.19rc1/mm/swap_state.c	Tue Jan 22 12:55:27 2002
+++ 2.4.19rc1aa1/mm/swap_state.c	Wed Jun 26 00:48:13 2002
@@ -95,11 +95,8 @@ int add_to_swap_cache(struct page *page,
  */
 void __delete_from_swap_cache(struct page *page)
 {
-	if (!PageLocked(page))
-		BUG();
 	if (!PageSwapCache(page))
 		BUG();
-	ClearPageDirty(page);
 	__remove_inode_page(page);
 	INC_CACHE_INFO(del_total);
 }
@@ -114,9 +111,6 @@ void delete_from_swap_cache(struct page 
 {
 	swp_entry_t entry;
 
-	if (!PageLocked(page))
-		BUG();
-
 	block_flushpage(page, 0);
 
 	entry.val = page->index;

Andrea

             reply	other threads:[~2002-06-27 20:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-06-27 20:14 Andrea Arcangeli [this message]
2002-06-27 20:43 ` vm fixes for 2.4.19rc1 Andrea Arcangeli
2002-06-27 22:28 ` Hugh Dickins
2002-06-28 16:01   ` Andrea Arcangeli
2002-06-28  3:19 ` Austin Gonyou
2002-06-28  8:51   ` Hugh Dickins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20020627201413.GD1457@inspiron.ols.wavesec.org \
    --to=andrea@suse.de \
    --cc=hugh@veritas.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcelo@conectiva.com.br \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.