Re: possible (ext4 related?) memory leak in kernel 2.6.26

From: Eric Sandeen
Date: Mon Oct 06 2008 - 11:31:23 EST


Theodore Tso wrote:
> On Sun, Oct 05, 2008 at 06:12:15PM +0200, Quentin Godfroy wrote:
>> For the two fs the only inode which shows up is the inode 8 (this
>> seems to be the journal. According to 'stat <8>' in debugfs it looks
>> like the journal is 134Megs long. I don't remember exactly how I
>> created the fs, but i'm sure I did not specified the journal
>> size. Does it seem reasonable for a 6,6G fs?
>
> 134 Megs sounds wrong. What does dumpe2fs -h say? I'm guessing you
> didn't calculate it quite correctly.
>
> I did some poking around myself, and noticed that a lot of in-use
> buffers hanging around from the journal inode. The following patch
> should fix that problem. I'm still doing some more testingto make
> sure there aren't any other buffer head leaks, but this is seems to
> fix the worst of the problems. Can you let me know how this works for
> you?
>
> - Ted
>
> jbd2: Fix buffer head leak when writing the commit block
>
> Also make sure the buffer heads are marked clean before submitting bh
> for writing. The previous code was marking the buffer head dirty,
> which would have forced an unneeded write (and seek) to the journal
> for no good reason.
>
> Signed-off-by: "Theodore Ts'o" <tytso@xxxxxxx>
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index e91f051..c2b04cd 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -127,8 +127,7 @@ static int journal_submit_commit_record(journal_t *journal,
>
> JBUFFER_TRACE(descriptor, "submit commit block");
> lock_buffer(bh);
> - get_bh(bh);
> - set_buffer_dirty(bh);
> + clear_buffer_dirty(bh);
> set_buffer_uptodate(bh);
> bh->b_end_io = journal_end_buffer_io_sync;
>
> @@ -157,7 +156,7 @@ static int journal_submit_commit_record(journal_t *journal,
>
> /* And try again, without the barrier */
> lock_buffer(bh);
> - set_buffer_uptodate(bh);
> + clear_buffer_uptodate(bh);
> set_buffer_dirty(bh);
> ret = submit_bh(WRITE, bh);
> }

Just so it doesn't get lost (discussed w/ Ted today) I think this 2nd
hunk flipped the wrong buffer funtion; this makes much more sense to me:

@@ -157,7 +156,7 @@ static int journal_submit_commit_record(journal_t
*journal,

/* And try again, without the barrier */
lock_buffer(bh);
set_buffer_uptodate(bh);
- set_buffer_dirty(bh);
+ clear_buffer_dirty(bh);
ret = submit_bh(WRITE, bh);
}


-Eric

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