All of lore.kernel.org
 help / color / mirror / Atom feed
From: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
To: sct@redhat.com, akpm@linux-foundation.org
Cc: linux-ext4@vger.kernel.org
Subject: [PATCH] JBD: positively dispose the unmapped data buffers in journal_commit_transaction
Date: Mon, 23 Jun 2008 20:00:51 +0900	[thread overview]
Message-ID: <485F8263.8000103@jp.fujitsu.com> (raw)

Hi.

I found the possibility that the system has the page which has
buffers (are marked 'BH_Freed') without mapping. As a result,
the resource management software etc. might not operate correctly
because there is a possibility that the page which cannot be
estimated exists.

I made a patch to positively release the pages that had the buffers
which were marked 'BH_Freed'.

Please confirm it.

This patch is for 2.6.26-rc6.
---
On ordered mode, before journal_commit_transaction does with
t_locked_list, all data buffers which are requested to dispose
by journal_unmap_buffer aren't disposed by JBD. Such all data
buffers are marked 'BH_Freed'. So, also pages corresponding
to them aren't released by JBD. The pages have data buffers
without mapping. Due to this fact, we cannot estimate free
memory correctly when there are many pages which have data
buffers without mapping. Therefore the resource management
software cannot work correctly.
But it is not memory leakage because the pages can be released
when the system has really few available memory.

BTW,
all metadata buffers on such situation are disposed at
'JBD: commit phase 7' by JBD and pages corresponding to them
can be released on the same time.

Therefore, by applying the same statements as ones for disposing
metadata buffers (marked 'BH_Freed'), JBD in
journal_commit_transaction can release the pages which has data
buffers without mapping.
As a result, the page which cannot be estimated is lost.

Signed-off-by: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
---
 fs/jbd/commit.c |   45 +++++++++++++++++++++++++++++++++++----------
 1 files changed, 35 insertions(+), 10 deletions(-)
--- linux-2.6.26-rc6.org/fs/jbd/commit.c	2008-06-13 06:22:24.000000000 +0900
+++ linux-2.6.26-rc6/fs/jbd/commit.c	2008-06-19 14:44:29.000000000 +0900
@@ -42,11 +42,11 @@ static void journal_end_buffer_io_sync(s
  * by the VM, but their apparent absence upsets the VM accounting, and it makes
  * the numbers in /proc/meminfo look odd.
  *
- * So here, we have a buffer which has just come off the forget list.  Look to
- * see if we can strip all buffers from the backing page.
+ * So here, we have a buffer which has just come off the list.  Confirm if we
+ * can strip all buffers from the backing page.
  *
- * Called under lock_journal(), and possibly under journal_datalist_lock.  The
- * caller provided us with a ref against the buffer, and we drop that here.
+ * Called under journal->j_list_lock.  The caller provided us with a ref
+ * against the buffer, and we drop that here.
  */
 static void release_buffer_page(struct buffer_head *bh)
 {
@@ -231,7 +231,16 @@ write_out_data:
 			if (locked)
 				unlock_buffer(bh);
 			BUFFER_TRACE(bh, "already cleaned up");
-			put_bh(bh);
+			if (buffer_freed(bh)) {
+				/* This bh has been marked 'BH_Feed'. */
+				clear_buffer_freed(bh);
+				/* Release the page if all other buffers
+				 * that belong to the page that has this bh
+				 * can be freed.
+				 */
+				release_buffer_page(bh);
+			} else
+				put_bh(bh);	
 			continue;
 		}
 		if (locked && test_clear_buffer_dirty(bh)) {
@@ -258,10 +267,17 @@ write_out_data:
 			if (locked)
 				unlock_buffer(bh);
 			journal_remove_journal_head(bh);
-			/* Once for our safety reference, once for
-			 * journal_remove_journal_head() */
-			put_bh(bh);
-			put_bh(bh);
+			put_bh(bh); /* for journal_remove_journal_head() */
+			if (buffer_freed(bh)) {
+				/* This bh has been marked 'BH_Feed'. */
+				clear_buffer_freed(bh);
+				/* Release the page if all other buffers
+				 * that belong to the page that has this bh
+				 * can be freed.
+				 */
+				release_buffer_page(bh);
+			} else
+				put_bh(bh);
 		}

 		if (need_resched() || spin_needbreak(&journal->j_list_lock)) {
@@ -443,7 +459,16 @@ void journal_commit_transaction(journal_
 		} else {
 			jbd_unlock_bh_state(bh);
 		}
-		put_bh(bh);
+		if (buffer_freed(bh)) {
+			/* This bh has been marked 'BH_Feed'. */
+			clear_buffer_freed(bh);
+			/* Release the page if all other buffers
+			 * that belong to the page that has this bh
+			 * can be freed.
+			 */
+			release_buffer_page(bh);
+		} else
+			put_bh(bh);
 		cond_resched_lock(&journal->j_list_lock);
 	}
 	spin_unlock(&journal->j_list_lock);


             reply	other threads:[~2008-06-23 11:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-23 11:00 Toshiyuki Okajima [this message]
2008-06-24  2:27 ` [PATCH] JBD: positively dispose the unmapped data buffers in journal_commit_transaction Andrew Morton
2008-06-24  8:01   ` Toshiyuki Okajima
2008-06-25  7:33     ` (take 2)[PATCH] " Toshiyuki Okajima
2008-06-25 20:39       ` Jan Kara
2008-06-26  6:16         ` Toshiyuki Okajima
2008-06-26 17:18           ` Jan Kara
2008-06-30  1:53             ` Toshiyuki Okajima

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=485F8263.8000103@jp.fujitsu.com \
    --to=toshi.okajima@jp.fujitsu.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=sct@redhat.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.