fs: Improve filesystem freezing handling

vfs_check_frozen() tests are racy since the filesystem can be frozen just after
the test is performed. Thus in write paths we can end up marking some pages or
inodes dirty even though the file system is already frozen. This creates
problems with flusher thread hanging on frozen filesystem.

Another problem is that exclusion between ->page_mkwrite() and filesystem
freezing has been handled by setting page dirty and then verifying s_frozen.
This guaranteed that either the freezing code sees the faulted page, writes it,
and writeprotects it again or we see s_frozen set and bail out of page fault.
This works to protect from page being marked writeable while filesystem
freezing is running but has an unpleasant artefact of leaving dirty (although
unmodified and writeprotected) pages on frozen filesystem resulting in similar
problems with flusher thread as the first problem.

This patch aims at providing exclusion between write paths and filesystem
freezing. We implement a writer-freeze read-write semaphore in the superblock.
Actually, there are three such semaphores because of lock ranking reasons - one
for page fault handlers (->page_mkwrite), one for all other writers, and one of
internal filesystem purposes (used e.g. to track running transactions).  Write
paths which should block freezing (e.g. directory operations, ->aio_write(),
->page_mkwrite) hold reader side of the semaphore. Code freezing the filesystem
takes the writer side.

Only that we don't really want to bounce cachelines of the semaphores between
CPUs for each write happening. So we implement the reader side of the semaphore
as a per-cpu counter and the writer side is implemented using s_writers.frozen
superblock field.

[AV: microoptimize sb_start_write(); we want it fast in normal case]

BugLink: https://bugs.launchpad.net/bugs/897421
Tested-by: Kamal Mostafa <kamal@canonical.com>
Tested-by: Peter M. Petrakis <peter.petrakis@canonical.com>
Tested-by: Dann Frazier <dann.frazier@canonical.com>
Tested-by: Massimo Morana <massimo.morana@canonical.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

2 files changed, 373 insertions, 28 deletions

diff --git a/fs/super.c b/fs/super.c
index c743fb3be4b8..0f64ecb7b1bf 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -33,12 +33,19 @@
 #include <linux/rculist_bl.h>
 #include <linux/cleancache.h>
 #include <linux/fsnotify.h>
+#include <linux/lockdep.h>
 #include "internal.h"
 
 
 LIST_HEAD(super_blocks);
 DEFINE_SPINLOCK(sb_lock);
 
+static char *sb_writers_name[SB_FREEZE_LEVELS] = {
+        "sb_writers",
+        "sb_pagefaults",
+        "sb_internal",
+};
+
 /*
  * One thing we have to be careful of with a per-sb shrinker is that we don't
  * drop the last active reference to the superblock from within the shrinker.
@@ -102,6 +109,35 @@ static int prune_super(struct shrinker *shrink, struct shrink_control *sc)
         return total_objects;
 }
 
+static int init_sb_writers(struct super_block *s, struct file_system_type *type)
+{
+        int err;
+        int i;
+
+        for (i = 0; i < SB_FREEZE_LEVELS; i++) {
+                err = percpu_counter_init(&s->s_writers.counter[i], 0);
+                if (err < 0)
+                        goto err_out;
+                lockdep_init_map(&s->s_writers.lock_map[i], sb_writers_name[i],
+                                 &type->s_writers_key[i], 0);
+        }
+        init_waitqueue_head(&s->s_writers.wait);
+        init_waitqueue_head(&s->s_writers.wait_unfrozen);
+        return 0;
+err_out:
+        while (--i >= 0)
+                percpu_counter_destroy(&s->s_writers.counter[i]);
+        return err;
+}
+
+static void destroy_sb_writers(struct super_block *s)
+{
+        int i;
+
+        for (i = 0; i < SB_FREEZE_LEVELS; i++)
+                percpu_counter_destroy(&s->s_writers.counter[i]);
+}
+
 /**
  *      alloc_super     -       create new superblock
  *      @type:  filesystem type superblock should belong to
@@ -117,18 +153,19 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags)
 
         if (s) {
                 if (security_sb_alloc(s)) {
+                        /*
+                         * We cannot call security_sb_free() without
+                         * security_sb_alloc() succeeding. So bail out manually
+                         */
                         kfree(s);
                         s = NULL;
                         goto out;
                 }
 #ifdef CONFIG_SMP
                 s->s_files = alloc_percpu(struct list_head);
-                if (!s->s_files) {
-                        security_sb_free(s);
-                        kfree(s);
-                        s = NULL;
-                        goto out;
-                } else {
+                if (!s->s_files)
+                        goto err_out;
+                else {
                         int i;
 
                         for_each_possible_cpu(i)
@@ -137,6 +174,8 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags)
 #else
                 INIT_LIST_HEAD(&s->s_files);
 #endif
+                if (init_sb_writers(s, type))
+                        goto err_out;
                 s->s_flags = flags;
                 s->s_bdi = &default_backing_dev_info;
                 INIT_HLIST_NODE(&s->s_instances);
@@ -190,6 +229,16 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags)
         }
 out:
         return s;
+err_out:
+        security_sb_free(s);
+#ifdef CONFIG_SMP
+        if (s->s_files)
+                free_percpu(s->s_files);
+#endif
+        destroy_sb_writers(s);
+        kfree(s);
+        s = NULL;
+        goto out;
 }
 
 /**
@@ -203,6 +252,7 @@ static inline void destroy_super(struct super_block *s)
 #ifdef CONFIG_SMP
         free_percpu(s->s_files);
 #endif
+        destroy_sb_writers(s);
         security_sb_free(s);
         WARN_ON(!list_empty(&s->s_mounts));
         kfree(s->s_subtype);
@@ -651,10 +701,11 @@ struct super_block *get_super_thawed(struct block_device *bdev)
 {
         while (1) {
                 struct super_block *s = get_super(bdev);
-                if (!s || s->s_frozen == SB_UNFROZEN)
+                if (!s || s->s_writers.frozen == SB_UNFROZEN)
                         return s;
                 up_read(&s->s_umount);
-                vfs_check_frozen(s, SB_FREEZE_WRITE);
+                wait_event(s->s_writers.wait_unfrozen,
+                           s->s_writers.frozen == SB_UNFROZEN);
                 put_super(s);
         }
 }
@@ -732,7 +783,7 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
         int retval;
         int remount_ro;
 
-        if (sb->s_frozen != SB_UNFROZEN)
+        if (sb->s_writers.frozen != SB_UNFROZEN)
                 return -EBUSY;
 
 #ifdef CONFIG_BLOCK
@@ -1163,6 +1214,120 @@ out:
         return ERR_PTR(error);
 }
 
+/*
+ * This is an internal function, please use sb_end_{write,pagefault,intwrite}
+ * instead.
+ */
+void __sb_end_write(struct super_block *sb, int level)
+{
+        percpu_counter_dec(&sb->s_writers.counter[level-1]);
+        /*
+         * Make sure s_writers are updated before we wake up waiters in
+         * freeze_super().
+         */
+        smp_mb();
+        if (waitqueue_active(&sb->s_writers.wait))
+                wake_up(&sb->s_writers.wait);
+        rwsem_release(&sb->s_writers.lock_map[level-1], 1, _RET_IP_);
+}
+EXPORT_SYMBOL(__sb_end_write);
+
+#ifdef CONFIG_LOCKDEP
+/*
+ * We want lockdep to tell us about possible deadlocks with freezing but
+ * it's it bit tricky to properly instrument it. Getting a freeze protection
+ * works as getting a read lock but there are subtle problems. XFS for example
+ * gets freeze protection on internal level twice in some cases, which is OK
+ * only because we already hold a freeze protection also on higher level. Due
+ * to these cases we have to tell lockdep we are doing trylock when we
+ * already hold a freeze protection for a higher freeze level.
+ */
+static void acquire_freeze_lock(struct super_block *sb, int level, bool trylock,
+                                unsigned long ip)
+{
+        int i;
+