diff mbox

[3/4] jbd2: Move lockdep tracking to journal_s

Message ID 1466073736-30447-4-git-send-email-jack@suse.cz
State Awaiting Upstream, archived
Headers show

Commit Message

Jan Kara June 16, 2016, 10:42 a.m. UTC 
Currently lockdep map is tracked in each journal handle. To be able to
expand lockdep support to cover also other cases where we depend on
transaction commit and where handle is not available, move lockdep map
into struct journal_s. Since this makes the lockdep map shared for all
handles, we have to use rwsem_acquire_read() for acquisitions now.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/jbd2/journal.c     |  4 ++++
 fs/jbd2/transaction.c | 11 +++--------
 include/linux/jbd2.h  | 16 ++++++++++++----
 3 files changed, 19 insertions(+), 12 deletions(-)

Comments

kernel test robot June 16, 2016, 11:42 a.m. UTC | #1 
Hi,

[auto build test WARNING on ext4/dev]
[also build test WARNING on v4.7-rc3 next-20160616]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jan-Kara/ext4-Fix-deadlock-during-page-writeback/20160616-184851
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/linux/jbd2.h:442: warning: No description found for parameter 'i_transaction'
   include/linux/jbd2.h:442: warning: No description found for parameter 'i_next_transaction'
   include/linux/jbd2.h:442: warning: No description found for parameter 'i_list'
   include/linux/jbd2.h:442: warning: No description found for parameter 'i_vfs_inode'
   include/linux/jbd2.h:442: warning: No description found for parameter 'i_flags'
   include/linux/jbd2.h:494: warning: No description found for parameter 'h_rsv_handle'
   include/linux/jbd2.h:494: warning: No description found for parameter 'h_reserved'
   include/linux/jbd2.h:494: warning: No description found for parameter 'h_type'
   include/linux/jbd2.h:494: warning: No description found for parameter 'h_line_no'
   include/linux/jbd2.h:494: warning: No description found for parameter 'h_start_jiffies'
   include/linux/jbd2.h:494: warning: No description found for parameter 'h_requested_credits'
   include/linux/jbd2.h:1046: warning: No description found for parameter 'j_chkpt_bhs[JBD2_NR_BATCH]'
   include/linux/jbd2.h:1046: warning: No description found for parameter 'j_devname[BDEVNAME_SIZE+24]'
   include/linux/jbd2.h:1046: warning: No description found for parameter 'j_average_commit_time'
   include/linux/jbd2.h:1046: warning: No description found for parameter 'j_min_batch_time'
   include/linux/jbd2.h:1046: warning: No description found for parameter 'j_max_batch_time'
   include/linux/jbd2.h:1046: warning: No description found for parameter 'j_commit_callback'
   include/linux/jbd2.h:1046: warning: No description found for parameter 'j_failed_commit'
   include/linux/jbd2.h:1046: warning: No description found for parameter 'j_chksum_driver'
   include/linux/jbd2.h:1046: warning: No description found for parameter 'j_csum_seed'
>> include/linux/jbd2.h:1046: warning: No description found for parameter 'j_trans_commit_map'
   fs/jbd2/transaction.c:424: warning: No description found for parameter 'rsv_blocks'
   fs/jbd2/transaction.c:424: warning: No description found for parameter 'gfp_mask'
   fs/jbd2/transaction.c:424: warning: No description found for parameter 'type'
   fs/jbd2/transaction.c:424: warning: No description found for parameter 'line_no'
   fs/jbd2/transaction.c:500: warning: No description found for parameter 'type'
   fs/jbd2/transaction.c:500: warning: No description found for parameter 'line_no'
   fs/jbd2/transaction.c:630: warning: No description found for parameter 'gfp_mask'

vim +/j_trans_commit_map +1046 include/linux/jbd2.h

01b5adce Darrick J. Wong 2012-05-27  1030  	struct crypto_shash *j_chksum_driver;
4fd5ea43 Darrick J. Wong 2012-05-27  1031  
4fd5ea43 Darrick J. Wong 2012-05-27  1032  	/* Precomputed journal UUID checksum for seeding other checksums */
4fd5ea43 Darrick J. Wong 2012-05-27  1033  	__u32 j_csum_seed;
ef50ddbd Jan Kara        2016-06-16  1034  
ef50ddbd Jan Kara        2016-06-16  1035  #ifdef CONFIG_DEBUG_LOCK_ALLOC
ef50ddbd Jan Kara        2016-06-16  1036  	/*
ef50ddbd Jan Kara        2016-06-16  1037  	 * Lockdep entity to track transaction commit dependencies. Handles
ef50ddbd Jan Kara        2016-06-16  1038  	 * hold this "lock" for read, when we wait for commit, we acquire the
ef50ddbd Jan Kara        2016-06-16  1039  	 * "lock" for writing. This matches the properties of jbd2 journalling
ef50ddbd Jan Kara        2016-06-16  1040  	 * where the running transaction has to wait for all handles to be
ef50ddbd Jan Kara        2016-06-16  1041  	 * dropped to commit that transaction and also acquiring a handle may
ef50ddbd Jan Kara        2016-06-16  1042  	 * require transaction commit to finish.
ef50ddbd Jan Kara        2016-06-16  1043  	 */
ef50ddbd Jan Kara        2016-06-16  1044  	struct lockdep_map	j_trans_commit_map;
ef50ddbd Jan Kara        2016-06-16  1045  #endif
470decc6 Dave Kleikamp   2006-10-11 @1046  };
470decc6 Dave Kleikamp   2006-10-11  1047  
56316a0d Darrick J. Wong 2015-10-17  1048  /* journal feature predicate functions */
56316a0d Darrick J. Wong 2015-10-17  1049  #define JBD2_FEATURE_COMPAT_FUNCS(name, flagname) \
56316a0d Darrick J. Wong 2015-10-17  1050  static inline bool jbd2_has_feature_##name(journal_t *j) \
56316a0d Darrick J. Wong 2015-10-17  1051  { \
56316a0d Darrick J. Wong 2015-10-17  1052  	return ((j)->j_format_version >= 2 && \
56316a0d Darrick J. Wong 2015-10-17  1053  		((j)->j_superblock->s_feature_compat & \
56316a0d Darrick J. Wong 2015-10-17  1054  		 cpu_to_be32(JBD2_FEATURE_COMPAT_##flagname)) != 0); \

:::::: The code at line 1046 was first introduced by commit
:::::: 470decc613ab2048b619a01028072d932d9086ee [PATCH] jbd2: initial copy of files from jbd

:::::: TO: Dave Kleikamp <shaggy@austin.ibm.com>
:::::: CC: Linus Torvalds <torvalds@g5.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Theodore Ts'o June 30, 2016, 3:40 p.m. UTC | #2 
On Thu, Jun 16, 2016 at 12:42:15PM +0200, Jan Kara wrote:
> Currently lockdep map is tracked in each journal handle. To be able to
> expand lockdep support to cover also other cases where we depend on
> transaction commit and where handle is not available, move lockdep map
> into struct journal_s. Since this makes the lockdep map shared for all
> handles, we have to use rwsem_acquire_read() for acquisitions now.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Thanks, applied.  I added documentation for j_trans_commit_map to keep
the kbuild checker happy.

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index b31852f76f46..208e4058040b 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1091,6 +1091,7 @@  static void jbd2_stats_proc_exit(journal_t *journal)
 
 static journal_t * journal_init_common (void)
 {
+	static struct lock_class_key jbd2_trans_commit_key;
 	journal_t *journal;
 	int err;
 
@@ -1126,6 +1127,9 @@  static journal_t * journal_init_common (void)
 
 	spin_lock_init(&journal->j_history_lock);
 
+	lockdep_init_map(&journal->j_trans_commit_map, "jbd2_handle",
+			 &jbd2_trans_commit_key, 0);
+
 	return journal;
 }
 
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 41249538c047..c0065040c5be 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -382,13 +382,11 @@  repeat:
 	read_unlock(&journal->j_state_lock);
 	current->journal_info = handle;
 
-	lock_map_acquire(&handle->h_lockdep_map);
+	rwsem_acquire_read(&journal->j_trans_commit_map, 0, 0, _THIS_IP_);
 	jbd2_journal_free_transaction(new_transaction);
 	return 0;
 }
 
-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)
 {
@@ -398,9 +396,6 @@  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;
 }
 
@@ -672,7 +667,7 @@  int jbd2__journal_restart(handle_t *handle, int nblocks, gfp_t gfp_mask)
 	if (need_to_start)
 		jbd2_log_start_commit(journal, tid);
 
-	lock_map_release(&handle->h_lockdep_map);
+	rwsem_release(&journal->j_trans_commit_map, 1, _THIS_IP_);
 	handle->h_buffer_credits = nblocks;
 	ret = start_this_handle(journal, handle, gfp_mask);
 	return ret;
@@ -1750,7 +1745,7 @@  int jbd2_journal_stop(handle_t *handle)
 			wake_up(&journal->j_wait_transaction_locked);
 	}
 
-	lock_map_release(&handle->h_lockdep_map);
+	rwsem_release(&journal->j_trans_commit_map, 1, _THIS_IP_);
 
 	if (wait_for_commit)
 		err = jbd2_log_wait_commit(journal, tid);
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index efb232c5f668..f6232b28eadc 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -491,10 +491,6 @@  struct jbd2_journal_handle
 
 	unsigned long		h_start_jiffies;
 	unsigned int		h_requested_credits;
-
-#ifdef CONFIG_DEBUG_LOCK_ALLOC
-	struct lockdep_map	h_lockdep_map;
-#endif
 };
 
 
@@ -1035,6 +1031,18 @@  struct journal_s
 
 	/* Precomputed journal UUID checksum for seeding other checksums */
 	__u32 j_csum_seed;
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	/*
+	 * Lockdep entity to track transaction commit dependencies. Handles
+	 * hold this "lock" for read, when we wait for commit, we acquire the
+	 * "lock" for writing. This matches the properties of jbd2 journalling
+	 * where the running transaction has to wait for all handles to be
+	 * dropped to commit that transaction and also acquiring a handle may
+	 * require transaction commit to finish.
+	 */
+	struct lockdep_map	j_trans_commit_map;
+#endif
 };
 
 /* journal feature predicate functions */