reiserfs: kill-the-BKL

This patch is an attempt to remove the Bkl based locking scheme from
reiserfs and is intended.

It is a bit inspired from an old attempt by Peter Zijlstra:

   http://lkml.indiana.edu/hypermail/linux/kernel/0704.2/2174.html

The bkl is heavily used in this filesystem to prevent from
concurrent write accesses on the filesystem.

Reiserfs makes a deep use of the specific properties of the Bkl:

- It can be acqquired recursively by a same task
- It is released on the schedule() calls and reacquired when schedule() returns

The two properties above are a roadmap for the reiserfs write locking so it's
very hard to simply replace it with a common mutex.

- We need a recursive-able locking unless we want to restructure several blocks
  of the code.
- We need to identify the sites where the bkl was implictly relaxed
  (schedule, wait, sync, etc...) so that we can in turn release and
  reacquire our new lock explicitly.
  Such implicit releases of the lock are often required to let other
  resources producer/consumer do their job or we can suffer unexpected
  starvations or deadlocks.

So the new lock that replaces the bkl here is a per superblock mutex with a
specific property: it can be acquired recursively by a same task, like the
bkl.

For such purpose, we integrate a lock owner and a lock depth field on the
superblock information structure.

The first axis on this patch is to turn reiserfs_write_(un)lock() function
into a wrapper to manage this mutex. Also some explicit calls to
lock_kernel() have been converted to reiserfs_write_lock() helpers.

The second axis is to find the important blocking sites (schedule...(),
wait_on_buffer(), sync_dirty_buffer(), etc...) and then apply an explicit
release of the write lock on these locations before blocking. Then we can
safely wait for those who can give us resources or those who need some.
Typically this is a fight between the current writer, the reiserfs workqueue
(aka the async commiter) and the pdflush threads.

The third axis is a consequence of the second. The write lock is usually
on top of a lock dependency chain which can include the journal lock, the
flush lock or the commit lock. So it's dangerous to release and trying to
reacquire the write lock while we still hold other locks.

This is fine with the bkl:

      T1                       T2

lock_kernel()
    mutex_lock(A)
    unlock_kernel()
    // do something
                            lock_kernel()
                                mutex_lock(A) -> already locked by T1
                                schedule() (and then unlock_kernel())
    lock_kernel()
    mutex_unlock(A)
    ....

This is not fine with a mutex:

      T1                       T2

mutex_lock(write)
    mutex_lock(A)
    mutex_unlock(write)
    // do something
                           mutex_lock(write)
                              mutex_lock(A) -> already locked by T1
                              schedule()

    mutex_lock(write) -> already locked by T2
    deadlock

The solution in this patch is to provide a helper which releases the write
lock and sleep a bit if we can't lock a mutex that depend on it. It's another
simulation of the bkl behaviour.

The last axis is to locate the fs callbacks that are called with the bkl held,
according to Documentation/filesystem/Locking.

Those are:

- reiserfs_remount
- reiserfs_fill_super
- reiserfs_put_super

Reiserfs didn't need to explicitly lock because of the context of these callbacks.
But now we must take care of that with the new locking.

After this patch, reiserfs suffers from a slight performance regression (for now).
On UP, a high volume write with dd reports an average of 27 MB/s instead
of 30 MB/s without the patch applied.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Reviewed-by: Ingo Molnar <mingo@elte.hu>
Cc: Jeff Mahoney <jeffm@suse.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Bron Gondwana <brong@fastmail.fm>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
LKML-Reference: <1239070789-13354-1-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

1 files changed, 105 insertions, 29 deletions

diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
index 90622200b39c..438c71f0bc91 100644
--- a/fs/reiserfs/journal.c
+++ b/fs/reiserfs/journal.c
@@ -429,21 +429,6 @@ static void clear_prepared_bits(struct buffer_head *bh)
         clear_buffer_journal_restore_dirty(bh);
 }
 
-/* utility function to force a BUG if it is called without the big
-** kernel lock held.  caller is the string printed just before calling BUG()
-*/
-void reiserfs_check_lock_depth(struct super_block *sb, char *caller)
-{
-#ifdef CONFIG_SMP
-        if (current->lock_depth < 0) {
-                reiserfs_panic(sb, "journal-1", "%s called without kernel "
-                               "lock held", caller);
-        }
-#else
-        ;
-#endif
-}
-
 /* return a cnode with same dev, block number and size in table, or null if not found */
 static inline struct reiserfs_journal_cnode *get_journal_hash_dev(struct
                                                                   super_block
@@ -552,11 +537,48 @@ static inline void insert_journal_hash(struct reiserfs_journal_cnode **table,
         journal_hash(table, cn->sb, cn->blocknr) = cn;
 }
 
+/*
+ * Several mutexes depend on the write lock.
+ * However sometimes we want to relax the write lock while we hold
+ * these mutexes, according to the release/reacquire on schedule()
+ * properties of the Bkl that were used.
+ * Reiserfs performances and locking were based on this scheme.
+ * Now that the write lock is a mutex and not the bkl anymore, doing so
+ * may result in a deadlock:
+ *
+ * A acquire write_lock
+ * A acquire j_commit_mutex
+ * A release write_lock and wait for something
+ * B acquire write_lock
+ * B can't acquire j_commit_mutex and sleep
+ * A can't acquire write lock anymore
+ * deadlock
+ *
+ * What we do here is avoiding such deadlock by playing the same game
+ * than the Bkl: if we can't acquire a mutex that depends on the write lock,
+ * we release the write lock, wait a bit and then retry.
+ *
+ * The mutexes concerned by this hack are:
+ * - The commit mutex of a journal list
+ * - The flush mutex
+ * - The journal lock
+ */
+static inline void reiserfs_mutex_lock_safe(struct mutex *m,
+                               struct super_block *s)
+{
+        while (!mutex_trylock(m)) {
+                reiserfs_write_unlock(s);
+                schedule();
+                reiserfs_write_lock(s);
+        }
+}
+
 /* lock the current transaction */
 static inline void lock_journal(struct super_block *sb)
 {
         PROC_INFO_INC(sb, journal.lock_journal);
-        mutex_lock(&SB_JOURNAL(sb)->j_mutex);
+
+        reiserfs_mutex_lock_safe(&SB_JOURNAL(sb)->j_mutex, sb);
 }
 
 /* unlock the current transaction */
@@ -708,7 +730,9 @@ static void check_barrier_completion(struct super_block *s,
                 disable_barrier(s);
                 set_buffer_uptodate(bh);
                 set_buffer_dirty(bh);
+                reiserfs_write_unlock(s);
                 sync_dirty_buffer(bh);
+                reiserfs_write_lock(s);
         }
 }
 
@@ -996,8 +1020,13 @@ static int reiserfs_async_progress_wait(struct super_block *s)
 {
         DEFINE_WAIT(wait);
         struct reiserfs_journal *j = SB_JOURNAL(s);
-        if (atomic_read(&j->j_async_throttle))
+
+        if (atomic_read(&j->j_async_throttle)) {
+                reiserfs_write_unlock(s);
                 congestion_wait(BLK_RW_ASYNC, HZ / 10);
+                reiserfs_write_lock(s);
+        }
+
         return 0;
 }
 
@@ -1043,7 +1072,8 @@ static int flush_commit_list(struct super_block *s,
         }
 
         /* make sure nobody is trying to flush this one at the same time */
-        mutex_lock(&jl->j_commit_mutex);
+        reiserfs_mutex_lock_safe(&jl->j_commit_mutex, s);
+
         if (!journal_list_still_alive(s, trans_id)) {
                 mutex_unlock(&jl->j_commit_mutex);
                 goto put_jl;
@@ -1061,12 +1091,17 @@ static int flush_commit_list(struct super_block *s,
 
         if (!list_empty(&jl->j_bh_list)) {
                 int ret;
-                unlock_kernel();
+
+                /*
+                 * We might sleep in numerous places inside
+                 * write_ordered_buffers. Relax the write lock.
+                 */
+                reiserfs_write_unlock(s);
                 ret = write_ordered_buffers(&journal->j_dirty_buffers_lock,
                                             journal, jl, &jl->j_bh_list);
                 if (ret < 0 && retval == 0)
                         retval = ret;
-                lock_kernel();
+                reiserfs_write_lock(s);
         }
         BUG_ON(!list_empty(&jl->j_bh_list));
         /*
@@ -1114,12 +1149,19 @@ static int flush_commit_list(struct super_block *s,
                 bn = SB_ONDISK_JOURNAL_1st_BLOCK(s) +
                     (jl->j_start + i) % SB_ONDISK_JOURNAL_SIZE(s);
                 tbh = journal_find_get_block(s, bn);
+
+                reiserfs_write_unlock(s);
                 wait_on_buffer(tbh);
+                reiserfs_write_lock(s);
                 // since we're using ll_rw_blk above, it might have skipped over
                 // a locked buffer.  Double check here
                 //
-                if (buffer_dirty(tbh))  /* redundant, sync_dirty_buffer() checks */
+                /* redundant, sync_dirty_buffer() checks */
+                if (buffer_dirty(tbh)) {
+                        reiserfs_write_unlock(s);
                         sync_dirty_buffer(tbh);
+                        reiserfs_write_lock(s);
+                }
                 if (unlikely(!buffer_uptodate(tbh))) {
 #ifdef CONFIG_REISERFS_CHECK
                         reiserfs_warning(s, "journal-601",
@@ -1143,10 +1185,15 @@ static int flush_commit_list(struct super_block *s,
                         if (buffer_dirty(jl->j_commit_bh))
                                 BUG();
                         mark_buffer_dirty(jl->j_commit_bh) ;
+                        reiserfs_write_unlock(s);
                         sync_dirty_buffer(jl->j_commit_bh) ;
+                        reiserfs_write_lock(s);
                 }
-        } else
+        } else {
+                reiserfs_write_unlock(s);
                 wait_on_buffer(jl->j_commit_bh);
+                reiserfs_write_lock(s);
+        }
 
         check_barrier_completion(s, jl->j_commit_bh);
 
@@ -1286,7 +1333,9 @@ static int _update_journal_header_block(struct super_block *sb,
 
         if (trans_id >= journal->j_last_flush_trans_id) {
                 if (buffer_locked((journal->j_header_bh))) {
+                        reiserfs_write_unlock(sb);
                         wait_on_buffer((journal->j_header_bh));
+                        reiserfs_write_lock(sb);
                         if (unlikely(!buffer_uptodate(journal->j_header_bh))) {
 #ifdef CONFIG_REISERFS_CHECK
                                 reiserfs_warning(sb, "journal-699",
@@ -1312,12 +1361,16 @@ static int _update_journal_header_block(struct super_block *sb,
                                 disable_barrier(sb);
                                 goto sync;
                         }
+                        reiserfs_write_unlock(sb);
                         wait_on_buffer(journal->j_header_bh);
+                        reiserfs_write_lock(sb);
                         check_barrier_completion(sb, journal->j_header_bh);
                 } else {
                       sync:
                         set_buffer_dirty(journal->j_header_bh);
+                        reiserfs_write_unlock(sb);
                         sync_dirty_buffer(journal->j_header_bh);
+                        reiserfs_write_lock(sb);
                 }
                 if (!buffer_uptodate(journal->j_header_bh)) {
                         reiserfs_warning(sb, "journal-837",
@@ -1409,7 +1462,7 @@ static int flush_journal_list(struct super_block *s,
 
         /* if flushall == 0, the lock is already held */
         if (flushall) {
-                mutex_lock(&journal->j_flush_mutex);
+                reiserfs_mutex_lock_safe(&journal->j_flush_mutex, s);
         } else if (mutex_trylock(&journal->j_flush_mutex)) {
                 BUG();
         }
@@ -1553,7 +1606,11 @@ static int flush_journal_list(struct super_block *s,
                                         reiserfs_panic(s, "journal-1011",
                                                        "cn->bh is NULL");
                                 }
+
+                                reiserfs_write_unlock(s);
                                 wait_on_buffer(cn->bh);
+                                reiserfs_write_lock(s);
+
                                 if (!cn->bh) {
                                         reiserfs_panic(s, "journal-1012",
                                                        "cn->bh is NULL");
@@ -1973,11 +2030,19 @@ static int do_journal_release(struct reiserfs_transaction_handle *th,
         reiserfs_mounted_fs_count--;
         /* wait for all commits to finish */
         cancel_delayed_work(&SB_JOURNAL(sb)->j_work);
+
+        /*
+         * We must release the write lock here because
+         * the workqueue job (flush_async_commit) needs this lock
+         */
+        reiserfs_write_unlock(sb);
         flush_workqueue(commit_wq);
+
         if (!reiserfs_mounted_fs_count) {
                 destroy_workqueue(commit_wq);
                 commit_wq = NULL;
         }
+        reiserfs_write_lock(sb);
 
         free_journal_ram(sb);
 
@@ -2243,7 +2308,11 @@ static int journal_read_transaction(struct super_block *sb,
         /* read in the log blocks, memcpy to the corresponding real block */
         ll_rw_block(READ, get_desc_trans_len(desc), log_blocks);
         for (i = 0; i < get_desc_trans_len(desc); i++) {
+
+                reiserfs_write_unlock(sb);
                 wait_on_buffer(log_blocks[i]);
+                reiserfs_write_lock(sb);
+
                 if (!buffer_uptodate(log_blocks[i])) {
                         reiserfs_warning(sb, "journal-1212",
                                          "REPLAY FAILURE fsck required! "
@@ -2964,8 +3033,11 @@ static void queue_log_writer(struct super_block *s)
         init_waitqueue_entry(&wait, current);
         add_wait_queue(&journal->j_join_wait, &wait);
         set_current_state(TASK_UNINTERRUPTIBLE);
-        if (test_bit(J_WRITERS_QUEUED, &journal->j_state))
+        if (test_bit(J_WRITERS_QUEUED, &journal->j_state)) {
+                reiserfs_write_unlock(s);
                 schedule();
+                reiserfs_write_lock(s);
+        }
         __set_current_state(TASK_RUNNING);
         remove_wait_queue(&journal->j_join_wait, &wait);
 }
@@ -2982,7 +3054,9 @@ static void let_transaction_grow(struct super_block *sb, unsigned int trans_id)
         struct reiserfs_journal *journal = SB_JOURNAL(sb);
         unsigned long bcount = journal->j_bcount;
         while (1) {
+                reiserfs_write_unlock(sb);
                 schedule_timeout_uninterruptible(1);
+                reiserfs_write_lock(sb);
                 journal->j_current_jl->j_state |= LIST_COMMIT_PENDING;
                 while ((atomic_read(&journal->j_wcount) > 0 ||
                         atomic_read(&journal->j_jlock)) &&
@@ -3033,7 +3107,9 @@ static int do_journal_begin_r(struct reiserfs_transaction_handle *th,
 
         if (test_bit(J_WRITERS_BLOCKED, &journal->j_state)) {
                 unlock_journal(sb);
+                reiserfs_write_unlock(sb);
                 reiserfs_wait_on_write_block(sb);
+                reiserfs_write_lock(sb);
                 PROC_INFO_INC(sb, journal.journal_relock_writers);
                 goto relock;
         }
@@ -3506,14 +3582,14 @@ static void flush_async_commits(struct work_struct *work)
         struct reiserfs_journal_list *jl;
         struct list_head *entry;
 
-        lock_kernel();
+        reiserfs_write_lock(sb);
         if (!list_empty(&journal->j_journal_list)) {
                 /* last entry is the youngest, commit it and you get everything */
                 entry = journal->j_journal_list.prev;
                 jl = JOURNAL_LIST_ENTRY(entry);
                 flush_commit_list(sb, jl, 1);
         }
-        unlock_kernel();
+        reiserfs_write_unlock(sb);
 }
 
 /*
@@ -4041,7 +4117,7 @@ static int do_journal_end(struct reiserfs_transaction_handle *th,
          * the new transaction is fully setup, and we've already flushed the
          * ordered bh list
          */
-        mutex_lock(&jl->j_commit_mutex);
+        reiserfs_mutex_lock_safe(&jl->j_commit_mutex, sb);
 
         /* save the transaction id in case we need to commit it later */
         commit_trans_id = jl->j_trans_id;
@@ -4203,10 +4279,10 @@ static int do_journal_end(struct reiserfs_transaction_handle *th,
          * is lost.
          */
         if (!list_empty(&jl->j_tail_bh_list)) {
-                unlock_kernel();
+                reiserfs_write_unlock(sb);
                 write_ordered_buffers(&journal->j_dirty_buffers_lock,
                                       journal, jl, &jl->j_tail_bh_list);
-                lock_kernel();
+                reiserfs_write_lock(sb);
         }
         BUG_ON(!list_empty(&jl->j_tail_bh_list));
         mutex_unlock(&jl->j_commit_mutex);