mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Alessio Igor Bogani <abogani@texware.it>,
	Jeff Mahoney <jeffm@suse.com>,
	ReiserFS Development List <reiserfs-devel@vger.kernel.org>,
	Alexander Beregalov <a.beregalov@gmail.com>,
	Chris Mason <chris.mason@oracle.com>
Subject: [PATCH 3/3] kill-the-BKL/reiserfs: only acquire the write lock once in reiserfs_dirty_inode
Date: Tue, 14 Apr 2009 05:34:25 +0200	[thread overview]
Message-ID: <1239680065-25013-4-git-send-email-fweisbec@gmail.com> (raw)
In-Reply-To: <1239680065-25013-1-git-send-email-fweisbec@gmail.com>

Impact: fix a deadlock

reiserfs_dirty_inode() is the super_operations::dirty_inode() callback
of reiserfs. It can be called from different contexts where the write
lock can be already held.

But this function also grab the write lock (possibly recursively).
Subsequent release of the lock before sleep will actually not release
the lock if the caller of mark_inode_dirty() (which in turn calls
reiserfs_dirty_inode()) already owns the lock.

A typical case:

reiserfs_write_end() {
	acquire_write_lock()
	mark_inode_dirty() {
		reiserfs_dirty_inode() {
			reacquire_write_lock() {
				journal_begin() {
					do_journal_begin_r() {
						/*
						 * fail to release, still
						 * one depth of lock
						 */
						release_write_lock()
						reiserfs_wait_on_write_block() {
							wait_event()

The event is usually provided by something which needs the write lock but
it hasn't been released.

We use reiserfs_write_lock_once() here to ensure we only grab the
write lock in one level.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 fs/reiserfs/super.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 1ac4fcb..f6c5606 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -567,25 +567,28 @@ static void reiserfs_dirty_inode(struct inode *inode)
 	struct reiserfs_transaction_handle th;
 
 	int err = 0;
+	int lock_depth;
+
 	if (inode->i_sb->s_flags & MS_RDONLY) {
 		reiserfs_warning(inode->i_sb, "clm-6006",
 				 "writing inode %lu on readonly FS",
 				 inode->i_ino);
 		return;
 	}
-	reiserfs_write_lock(inode->i_sb);
+	lock_depth = reiserfs_write_lock_once(inode->i_sb);
 
 	/* this is really only used for atime updates, so they don't have
 	 ** to be included in O_SYNC or fsync
 	 */
 	err = journal_begin(&th, inode->i_sb, 1);
-	if (err) {
-		reiserfs_write_unlock(inode->i_sb);
-		return;
-	}
+	if (err)
+		goto out;
+
 	reiserfs_update_sd(&th, inode);
 	journal_end(&th, inode->i_sb, 1);
-	reiserfs_write_unlock(inode->i_sb);
+
+out:
+	reiserfs_write_unlock_once(inode->i_sb, lock_depth);
 }
 
 #ifdef CONFIG_REISERFS_FS_POSIX_ACL
-- 
1.6.1


  parent reply	other threads:[~2009-04-14  3:35 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-14  3:34 [PATCH 0/3] kill-the-BKL/reiserfs: reiserfs fixes Frederic Weisbecker
2009-04-14  3:34 ` [PATCH 1/3] kill-the-BKL/reiserfs: provide a tool to lock only once the write lock Frederic Weisbecker
2009-04-14  3:34 ` [PATCH 2/3] kill-the-BKL/reiserfs: lock only once in reiserfs_truncate_file Frederic Weisbecker
2009-04-14  3:34 ` Frederic Weisbecker [this message]
2009-04-14  4:51 ` [PATCH 0/3] kill-the-BKL/reiserfs: reiserfs fixes Alexander Beregalov
2009-04-14  9:01   ` [tree] latest kill-the-BKL tree, v12 Ingo Molnar
2009-04-14 10:02     ` Edward Shishkin
2009-04-14 21:32       ` Frederic Weisbecker
2009-04-14 21:27     ` Frederic Weisbecker
2009-04-15 22:58     ` Alexander Beregalov
2009-04-15 23:07       ` Ingo Molnar
2009-04-15 23:13         ` Trond Myklebust
2009-04-15 23:35         ` Frederic Weisbecker
2009-04-16  8:51           ` Ingo Molnar
2009-04-16 14:35             ` Alessio Igor Bogani
2009-04-16 16:40             ` Frederic Weisbecker
2009-04-14 10:00 ` [PATCH 0/3] kill-the-BKL/reiserfs: reiserfs fixes Ingo Molnar

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=1239680065-25013-4-git-send-email-fweisbec@gmail.com \
    --to=fweisbec@gmail.com \
    --cc=a.beregalov@gmail.com \
    --cc=abogani@texware.it \
    --cc=chris.mason@oracle.com \
    --cc=jeffm@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=reiserfs-devel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome