Do not try lock_acquire after handle made invalid
PATCH 1/1 Do not try lock_acquire after handle made invalid
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.
--- fs/jbd/transaction.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
fs/jbd/transaction.c +2 -0
@@ -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; }
Signature · 1 line
1.5.2.5
Quoted 18 lines · On Wed, 16 Jan 2008 00:39:26 +0100 Jonas Bonn <[email protected]> wrote:
This likely fixes the oops in
__lock_acquirereported 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_handleis returning-EROFS.Signed-off-by Jonas Bonn <[email protected]>--- fs/jbd/transaction.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)fs/jbd/transaction.c +0
-0@@ -288,10 +288,12 @@ handle_t *journal_start(journal_t *journal, int nblocks)jbd_free_handle (handle);
fs/jbd/transaction.c handle_t *journal_start(journal_t *journal, int nblocks) · line 298
Quoted 10 lines
fs/jbd/transaction.c +2
-0current->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?
Quoted 28 lines · Andrew Morton
Quoted 25 lines · On Wed, 16 Jan 2008 00:39:26 +0100 Jonas Bonn <[email protected]> wrote:
This likely fixes the oops in
__lock_acquirereported 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_handleis returning-EROFS.Signed-off-by Jonas Bonn <[email protected]>--- fs/jbd/transaction.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)fs/jbd/transaction.c +1
-0@@ -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_);
Quoted 8 lines
fs/jbd/transaction.c handle_t *journal_start(journal_t *journal, int nblocks) · line 298
Quoted 3 lines
fs/jbd/transaction.c +1
-0+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
Quoted 2 lines
Except lockdep doesn't know about
journal_start(), which has ranking requirements similar to a semaphore.
Ported from lockdep jbd patch.
--- fs/jbd2/transaction.c | 11 +++++++++++ include/linux/jbd2.h | 4 ++++ 2 files changed, 15 insertions(+)
fs/jbd2/transaction.c +11 -0
@@ -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; }
include/linux/jbd2.h +4 -0
@@ -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 };
PATCH 1/4 jbd2: port jbd lockdep support to jbd2
In reply to Andrew MortonHi 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
Quoted 2 lines
Except lockdep doesn't know about
journal_start(), which has ranking requirements similar to a semaphore.
--- fs/jbd2/transaction.c | 11 +++++++++++ include/linux/jbd2.h | 4 ++++ 2 files changed, 15 insertions(+)
fs/jbd2/transaction.c +11 -0
@@ -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; }
include/linux/jbd2.h +4 -0
@@ -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 };
PATCH 2/4 JBD2: Group short-lived and reclaimable kernel allocations
In reply to Andrew MortonJBD2: Group short-lived and reclaimable kernel allocations
From: Mingming Cao <[email protected]>
Ported from JBD to JBD2
--------------------------------
From: Mel Gorman <[email protected]>
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.
---
--- fs/jbd2/journal.c | 4 ++-- fs/jbd2/revoke.c | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-)
fs/jbd2/journal.c +2 -2
@@ -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" );
fs/jbd2/revoke.c +4 -2
@@ -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 ;
PATCH 3/4 JBD2: user of the jiffies rounding code
In reply to Andrew MortonPorted from JBD changes from Arjan van de Ven <[email protected]>
-------------------------------------------
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)
---
--- fs/jbd2/transaction.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/jbd2/transaction.c +1 -1
@@ -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 );
PATCH 4/4 JBD2: sparse pointer use of zero as null
In reply to Andrew MortonPorted 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.
--- fs/jbd2/transaction.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
fs/jbd2/transaction.c +6 -6
@@ -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(sJ_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 jJ_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 ;