* Re: [PATCH] Do not try lock_acquire after handle made invalid
2008-01-15 23:59 ` Andrew Morton
@ 2008-01-16 23:59 ` Mingming Cao
2008-01-17 19:47 ` [PATCH 1/4]jbd2: port jbd lockdep support to jbd2 Mingming Cao
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Mingming Cao @ 2008-01-16 23:59 UTC (permalink / raw)
To: Andrew Morton; +Cc: Jonas Bonn, sct, linux-ext4, Peter Zijlstra
On Tue, 2008-01-15 at 15:59 -0800, Andrew Morton wrote:
> On Wed, 16 Jan 2008 00:39:26 +0100
> Jonas Bonn <jonas.bonn@gmail.com> wrote:
>
> > This likely fixes the oops in __lock_acquire reported as:
> >
> > http://www.kerneloops.org/raw.php?rawid=2753&msgid=
> > http://www.kerneloops.org/raw.php?rawid=2749&msgid=
> >
> > In these reported oopses, start_this_handle is returning -EROFS.
> >
> > Signed-off-by: Jonas Bonn <jonas.bonn@gmail.com>
> > ---
> > fs/jbd/transaction.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/jbd/transaction.c b/fs/jbd/transaction.c
> > index 08ff6c7..038ed74 100644
> > --- a/fs/jbd/transaction.c
> > +++ b/fs/jbd/transaction.c
> > @@ -288,10 +288,12 @@ handle_t *journal_start(journal_t *journal, int nblocks)
> > jbd_free_handle(handle);
> > current->journal_info = NULL;
> > handle = ERR_PTR(err);
> > + goto out;
> > }
> >
> > lock_acquire(&handle->h_lockdep_map, 0, 0, 0, 2, _THIS_IP_);
> >
> > +out:
> > return handle;
> > }
>
> Yeah, thanks.
>
> It looks like we forgot to port this lockdep support into ext4. This is
> bad. What else got lost?
Here is the ext4/jbd2 port of the original lockdep patch with the above
fix. Please let me know if I missed anything.
It's likely there are other jbd/ext3 changes missed out in ext4/jbd2. I
will take a look at the git tree and do a cleanup.
Thanks for pointing this out.
---------------------------------------------------------
jbd2: lockdep support for jbd2
> Except lockdep doesn't know about journal_start(), which has ranking
> requirements similar to a semaphore.
Ported from lockdep jbd patch.
Signed-off-by: Mingming Cao <cmm@us.ibm.com>
---
fs/jbd2/transaction.c | 11 +++++++++++
include/linux/jbd2.h | 4 ++++
2 files changed, 15 insertions(+)
Index: linux-2.6.24-rc7/fs/jbd2/transaction.c
===================================================================
--- linux-2.6.24-rc7.orig/fs/jbd2/transaction.c 2008-01-16 15:30:24.000000000 -0800
+++ linux-2.6.24-rc7/fs/jbd2/transaction.c 2008-01-16 15:41:14.000000000 -0800
@@ -241,6 +241,8 @@ out:
return ret;
}
+static struct lock_class_key jbd2_handle_key;
+
/* Allocate a new handle. This should probably be in a slab... */
static handle_t *new_handle(int nblocks)
{
@@ -251,6 +253,9 @@ static handle_t *new_handle(int nblocks)
handle->h_buffer_credits = nblocks;
handle->h_ref = 1;
+ lockdep_init_map(&handle->h_lockdep_map, "jbd2_handle",
+ &jbd2_handle_key, 0);
+
return handle;
}
@@ -293,7 +298,11 @@ handle_t *jbd2_journal_start(journal_t *
jbd2_free_handle(handle);
current->journal_info = NULL;
handle = ERR_PTR(err);
+ goto out;
}
+
+ lock_acquire(&handle->h_lockdep_map, 0, 0, 0, 2, _THIS_IP_);
+out:
return handle;
}
@@ -1419,6 +1428,8 @@ int jbd2_journal_stop(handle_t *handle)
spin_unlock(&journal->j_state_lock);
}
+ lock_release(&handle->h_lockdep_map, 1, _THIS_IP_);
+
jbd2_free_handle(handle);
return err;
}
Index: linux-2.6.24-rc7/include/linux/jbd2.h
===================================================================
--- linux-2.6.24-rc7.orig/include/linux/jbd2.h 2008-01-16 15:29:03.000000000 -0800
+++ linux-2.6.24-rc7/include/linux/jbd2.h 2008-01-16 15:29:54.000000000 -0800
@@ -418,6 +418,10 @@ struct handle_s
unsigned int h_sync: 1; /* sync-on-close */
unsigned int h_jdata: 1; /* force data journaling */
unsigned int h_aborted: 1; /* fatal error on handle */
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+ struct lockdep_map h_lockdep_map;
+#endif
};
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/4]jbd2: port jbd lockdep support to jbd2
2008-01-15 23:59 ` Andrew Morton
2008-01-16 23:59 ` Mingming Cao
@ 2008-01-17 19:47 ` Mingming Cao
2008-01-17 19:48 ` [PATCH 2/4]JBD2: Group short-lived and reclaimable kernel allocations Mingming Cao
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Mingming Cao @ 2008-01-17 19:47 UTC (permalink / raw)
To: Andrew Morton, tytso; +Cc: Jonas Bonn, linux-kernel, linux-ext4
Hi Andrew, Ted,
I walked through the linus's git tree history and found 4 patches should
port from ext3/jbd to ext4/jbd2, since the day ext4 was forked
(2006.10.11) to today. I have already queued the ported patches in ext4
patch queue and verified they seems fine. Here is the first one.
jbd2: port jbd lockdep support to jbd2
> Except lockdep doesn't know about journal_start(), which has ranking
> requirements similar to a semaphore.
Signed-off-by: Mingming Cao <cmm@us.ibm.com>
---
fs/jbd2/transaction.c | 11 +++++++++++
include/linux/jbd2.h | 4 ++++
2 files changed, 15 insertions(+)
Index: linux-2.6.24-rc7/fs/jbd2/transaction.c
===================================================================
--- linux-2.6.24-rc7.orig/fs/jbd2/transaction.c 2008-01-16 15:30:24.000000000 -0800
+++ linux-2.6.24-rc7/fs/jbd2/transaction.c 2008-01-16 15:41:14.000000000 -0800
@@ -241,6 +241,8 @@ out:
return ret;
}
+static struct lock_class_key jbd2_handle_key;
+
/* Allocate a new handle. This should probably be in a slab... */
static handle_t *new_handle(int nblocks)
{
@@ -251,6 +253,9 @@ static handle_t *new_handle(int nblocks)
handle->h_buffer_credits = nblocks;
handle->h_ref = 1;
+ lockdep_init_map(&handle->h_lockdep_map, "jbd2_handle",
+ &jbd2_handle_key, 0);
+
return handle;
}
@@ -293,7 +298,11 @@ handle_t *jbd2_journal_start(journal_t *
jbd2_free_handle(handle);
current->journal_info = NULL;
handle = ERR_PTR(err);
+ goto out;
}
+
+ lock_acquire(&handle->h_lockdep_map, 0, 0, 0, 2, _THIS_IP_);
+out:
return handle;
}
@@ -1419,6 +1428,8 @@ int jbd2_journal_stop(handle_t *handle)
spin_unlock(&journal->j_state_lock);
}
+ lock_release(&handle->h_lockdep_map, 1, _THIS_IP_);
+
jbd2_free_handle(handle);
return err;
}
Index: linux-2.6.24-rc7/include/linux/jbd2.h
===================================================================
--- linux-2.6.24-rc7.orig/include/linux/jbd2.h 2008-01-16 15:29:03.000000000 -0800
+++ linux-2.6.24-rc7/include/linux/jbd2.h 2008-01-16 15:29:54.000000000 -0800
@@ -418,6 +418,10 @@ struct handle_s
unsigned int h_sync: 1; /* sync-on-close */
unsigned int h_jdata: 1; /* force data journaling */
unsigned int h_aborted: 1; /* fatal error on handle */
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+ struct lockdep_map h_lockdep_map;
+#endif
};
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 2/4]JBD2: Group short-lived and reclaimable kernel allocations
2008-01-15 23:59 ` Andrew Morton
2008-01-16 23:59 ` Mingming Cao
2008-01-17 19:47 ` [PATCH 1/4]jbd2: port jbd lockdep support to jbd2 Mingming Cao
@ 2008-01-17 19:48 ` Mingming Cao
2008-01-17 19:48 ` [PATCH 3/4]JBD2: user of the jiffies rounding code Mingming Cao
2008-01-17 19:48 ` [PATCH 4/4]JBD2: sparse pointer use of zero as null Mingming Cao
4 siblings, 0 replies; 7+ messages in thread
From: Mingming Cao @ 2008-01-17 19:48 UTC (permalink / raw)
To: Andrew Morton; +Cc: Mel Gorman, linux-kernel, linux-ext4
JBD2: Group short-lived and reclaimable kernel allocations
From: Mingming Cao <cmm@us.ibm.com>
Ported from JBD to JBD2
--------------------------------
From: Mel Gorman <mel@csn.ul.ie>
Date: Tue, 16 Oct 2007 08:25:52 +0000 (-0700)
Subject: Group short-lived and reclaimable kernel allocations
X-Git-Tag: v2.6.24-rc1~1137
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=commitdiff_plain;h=e12ba74d8ff3e2f73a583500d7095e406df4d093
Group short-lived and reclaimable kernel allocations
This patch marks a number of allocations that are either short-lived such as
network buffers or are reclaimable such as inode allocations. When something
like updatedb is called, long-lived and unmovable kernel allocations tend to
be spread throughout the address space which increases fragmentation.
This patch groups these allocations together as much as possible by adding a
new MIGRATE_TYPE. The MIGRATE_RECLAIMABLE type is for allocations that can be
reclaimed on demand, but not moved. i.e. they can be migrated by deleting
them and re-reading the information from elsewhere.
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Mingming Cao <cmm@us.ibm.com>
---
---
fs/jbd2/journal.c | 4 ++--
fs/jbd2/revoke.c | 6 ++++--
2 files changed, 6 insertions(+), 4 deletions(-)
Index: linux-2.6.24-rc7/fs/jbd2/journal.c
===================================================================
--- linux-2.6.24-rc7.orig/fs/jbd2/journal.c 2008-01-16 15:02:40.000000000 -0800
+++ linux-2.6.24-rc7/fs/jbd2/journal.c 2008-01-16 17:40:24.000000000 -0800
@@ -1975,7 +1975,7 @@ static int journal_init_jbd2_journal_hea
jbd2_journal_head_cache = kmem_cache_create("jbd2_journal_head",
sizeof(struct journal_head),
0, /* offset */
- 0, /* flags */
+ SLAB_TEMPORARY, /* flags */
NULL); /* ctor */
retval = 0;
if (jbd2_journal_head_cache == 0) {
@@ -2271,7 +2271,7 @@ static int __init journal_init_handle_ca
jbd2_handle_cache = kmem_cache_create("jbd2_journal_handle",
sizeof(handle_t),
0, /* offset */
- 0, /* flags */
+ SLAB_TEMPORARY, /* flags */
NULL); /* ctor */
if (jbd2_handle_cache == NULL) {
printk(KERN_EMERG "JBD: failed to create handle cache\n");
Index: linux-2.6.24-rc7/fs/jbd2/revoke.c
===================================================================
--- linux-2.6.24-rc7.orig/fs/jbd2/revoke.c 2008-01-06 13:45:38.000000000 -0800
+++ linux-2.6.24-rc7/fs/jbd2/revoke.c 2008-01-16 17:40:24.000000000 -0800
@@ -171,13 +171,15 @@ int __init jbd2_journal_init_revoke_cach
{
jbd2_revoke_record_cache = kmem_cache_create("jbd2_revoke_record",
sizeof(struct jbd2_revoke_record_s),
- 0, SLAB_HWCACHE_ALIGN, NULL);
+ 0,
+ SLAB_HWCACHE_ALIGN|SLAB_TEMPORARY,
+ NULL);
if (jbd2_revoke_record_cache == 0)
return -ENOMEM;
jbd2_revoke_table_cache = kmem_cache_create("jbd2_revoke_table",
sizeof(struct jbd2_revoke_table_s),
- 0, 0, NULL);
+ 0, SLAB_TEMPORARY, NULL);
if (jbd2_revoke_table_cache == 0) {
kmem_cache_destroy(jbd2_revoke_record_cache);
jbd2_revoke_record_cache = NULL;
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 3/4]JBD2: user of the jiffies rounding code
2008-01-15 23:59 ` Andrew Morton
` (2 preceding siblings ...)
2008-01-17 19:48 ` [PATCH 2/4]JBD2: Group short-lived and reclaimable kernel allocations Mingming Cao
@ 2008-01-17 19:48 ` Mingming Cao
2008-01-17 19:48 ` [PATCH 4/4]JBD2: sparse pointer use of zero as null Mingming Cao
4 siblings, 0 replies; 7+ messages in thread
From: Mingming Cao @ 2008-01-17 19:48 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, linux-ext4, Arjan van de Ven
Ported from JBD changes from Arjan van de Ven <arjan@linux.intel.com>
-------------------------------------------
Date: Sun, 10 Dec 2006 10:21:26 +0000 (-0800)
Subject: [PATCH] user of the jiffies rounding code: JBD
X-Git-Tag: v2.6.20-rc1~15^2~43
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=commitdiff_plain;h=44d306e1508fef6fa7a6eb15a1aba86ef68389a6
[PATCH] user of the jiffies rounding code: JBD
This patch introduces a user: of the round_jiffies() function; the "5 second"
ext3/jbd wakeup.
While "every 5 seconds" doesn't sound as a problem, there can be many of these
(and these timers do add up over all the kernel). The "5 second" wakeup isn't
really timing sensitive; in addition even with rounding it'll still happen
every 5 seconds (with the exception of the very first time, which is likely to
be rounded up to somewhere closer to 6 seconds)
Signed-off-by: Mingming Cao <cmm@us.ibm.com>
---
---
fs/jbd2/transaction.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.24-rc7/fs/jbd2/transaction.c
===================================================================
--- linux-2.6.24-rc7.orig/fs/jbd2/transaction.c 2008-01-16 15:41:14.000000000 -0800
+++ linux-2.6.24-rc7/fs/jbd2/transaction.c 2008-01-16 17:49:48.000000000 -0800
@@ -54,7 +54,7 @@ jbd2_get_transaction(journal_t *journal,
spin_lock_init(&transaction->t_handle_lock);
/* Set up the commit timer for the new transaction. */
- journal->j_commit_timer.expires = transaction->t_expires;
+ journal->j_commit_timer.expires = round_jiffies(transaction->t_expires);
add_timer(&journal->j_commit_timer);
J_ASSERT(journal->j_running_transaction == NULL);
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 4/4]JBD2: sparse pointer use of zero as null
2008-01-15 23:59 ` Andrew Morton
` (3 preceding siblings ...)
2008-01-17 19:48 ` [PATCH 3/4]JBD2: user of the jiffies rounding code Mingming Cao
@ 2008-01-17 19:48 ` Mingming Cao
4 siblings, 0 replies; 7+ messages in thread
From: Mingming Cao @ 2008-01-17 19:48 UTC (permalink / raw)
To: linux-ext4; +Cc: linux-kernel, Andrew Morton
Ported from upstream jbd changes to jbd2
sparse pointer use of zero as null
Get rid of sparse related warnings from places that use integer as NULL
pointer.
Signed-off-by: Mingming Cao <cmm@us.ibm.com>
---
fs/jbd2/transaction.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Index: linux-2.6.24-rc7/fs/jbd2/transaction.c
===================================================================
--- linux-2.6.24-rc7.orig/fs/jbd2/transaction.c 2008-01-16 17:49:48.000000000 -0800
+++ linux-2.6.24-rc7/fs/jbd2/transaction.c 2008-01-16 18:06:33.000000000 -0800
@@ -1182,7 +1182,7 @@ int jbd2_journal_dirty_metadata(handle_t
}
/* That test should have eliminated the following case: */
- J_ASSERT_JH(jh, jh->b_frozen_data == 0);
+ J_ASSERT_JH(jh, jh->b_frozen_data == NULL);
JBUFFER_TRACE(jh, "file as BJ_Metadata");
spin_lock(&journal->j_list_lock);
@@ -1532,7 +1532,7 @@ void __jbd2_journal_temp_unlink_buffer(s
J_ASSERT_JH(jh, jh->b_jlist < BJ_Types);
if (jh->b_jlist != BJ_None)
- J_ASSERT_JH(jh, transaction != 0);
+ J_ASSERT_JH(jh, transaction != NULL);
switch (jh->b_jlist) {
case BJ_None:
@@ -1601,11 +1601,11 @@ __journal_try_to_free_buffer(journal_t *
if (buffer_locked(bh) || buffer_dirty(bh))
goto out;
- if (jh->b_next_transaction != 0)
+ if (jh->b_next_transaction != NULL)
goto out;
spin_lock(&journal->j_list_lock);
- if (jh->b_transaction != 0 && jh->b_cp_transaction == 0) {
+ if (jh->b_transaction != NULL && jh->b_cp_transaction == NULL) {
if (jh->b_jlist == BJ_SyncData || jh->b_jlist == BJ_Locked) {
/* A written-back ordered data buffer */
JBUFFER_TRACE(jh, "release data");
@@ -1613,7 +1613,7 @@ __journal_try_to_free_buffer(journal_t *
jbd2_journal_remove_journal_head(bh);
__brelse(bh);
}
- } else if (jh->b_cp_transaction != 0 && jh->b_transaction == 0) {
+ } else if (jh->b_cp_transaction != NULL && jh->b_transaction == NULL) {
/* written-back checkpointed metadata buffer */
if (jh->b_jlist == BJ_None) {
JBUFFER_TRACE(jh, "remove from checkpoint list");
@@ -1973,7 +1973,7 @@ void __jbd2_journal_file_buffer(struct j
J_ASSERT_JH(jh, jh->b_jlist < BJ_Types);
J_ASSERT_JH(jh, jh->b_transaction == transaction ||
- jh->b_transaction == 0);
+ jh->b_transaction == NULL);
if (jh->b_transaction && jh->b_jlist == jlist)
return;
^ permalink raw reply [flat|nested] 7+ messages in thread