gfs2: Don't withdraw under a spin lock
authorAndreas Gruenbacher <agruenba@redhat.com>
Thu, 7 Jun 2018 10:56:46 +0000 (11:56 +0100)
committerBob Peterson <rpeterso@redhat.com>
Thu, 21 Jun 2018 12:39:44 +0000 (07:39 -0500)
In two places, the gfs2_io_error_bh macro is called while holding the
sd_ail_lock spin lock.  This isn't allowed because gfs2_io_error_bh
withdraws the filesystem, which can sleep because it issues a uevent.
To fix that, add a gfs2_io_error_bh_wd macro that does withdraw the
filesystem and change gfs2_io_error_bh to not withdraw the filesystem.
In those places where the new gfs2_io_error_bh is used, withdraw the
filesystem after releasing sd_ail_lock.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
Reviewed-by: Andrew Price <anprice@redhat.com>
fs/gfs2/log.c
fs/gfs2/lops.c
fs/gfs2/meta_io.c
fs/gfs2/util.c
fs/gfs2/util.h

index 0248835625f1368ba8bae3992150ab3806bd8312..a767fad0238605265b40b629c4347b7cc4c61a11 100644 (file)
@@ -92,7 +92,8 @@ static void gfs2_remove_from_ail(struct gfs2_bufdata *bd)
 
 static int gfs2_ail1_start_one(struct gfs2_sbd *sdp,
                               struct writeback_control *wbc,
-                              struct gfs2_trans *tr)
+                              struct gfs2_trans *tr,
+                              bool *withdraw)
 __releases(&sdp->sd_ail_lock)
 __acquires(&sdp->sd_ail_lock)
 {
@@ -107,8 +108,10 @@ __acquires(&sdp->sd_ail_lock)
                gfs2_assert(sdp, bd->bd_tr == tr);
 
                if (!buffer_busy(bh)) {
-                       if (!buffer_uptodate(bh))
+                       if (!buffer_uptodate(bh)) {
                                gfs2_io_error_bh(sdp, bh);
+                               *withdraw = true;
+                       }
                        list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list);
                        continue;
                }
@@ -148,6 +151,7 @@ void gfs2_ail1_flush(struct gfs2_sbd *sdp, struct writeback_control *wbc)
        struct list_head *head = &sdp->sd_ail1_list;
        struct gfs2_trans *tr;
        struct blk_plug plug;
+       bool withdraw = false;
 
        trace_gfs2_ail_flush(sdp, wbc, 1);
        blk_start_plug(&plug);
@@ -156,11 +160,13 @@ restart:
        list_for_each_entry_reverse(tr, head, tr_list) {
                if (wbc->nr_to_write <= 0)
                        break;
-               if (gfs2_ail1_start_one(sdp, wbc, tr))
+               if (gfs2_ail1_start_one(sdp, wbc, tr, &withdraw))
                        goto restart;
        }
        spin_unlock(&sdp->sd_ail_lock);
        blk_finish_plug(&plug);
+       if (withdraw)
+               gfs2_lm_withdraw(sdp, NULL);
        trace_gfs2_ail_flush(sdp, wbc, 0);
 }
 
@@ -188,7 +194,8 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp)
  *
  */
 
-static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr)
+static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
+                               bool *withdraw)
 {
        struct gfs2_bufdata *bd, *s;
        struct buffer_head *bh;
@@ -199,11 +206,12 @@ static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr)
                gfs2_assert(sdp, bd->bd_tr == tr);
                if (buffer_busy(bh))
                        continue;
-               if (!buffer_uptodate(bh))
+               if (!buffer_uptodate(bh)) {
                        gfs2_io_error_bh(sdp, bh);
+                       *withdraw = true;
+               }
                list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list);
        }
-
 }
 
 /**
@@ -218,10 +226,11 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp)
        struct gfs2_trans *tr, *s;
        int oldest_tr = 1;
        int ret;
+       bool withdraw = false;
 
        spin_lock(&sdp->sd_ail_lock);
        list_for_each_entry_safe_reverse(tr, s, &sdp->sd_ail1_list, tr_list) {
-               gfs2_ail1_empty_one(sdp, tr);
+               gfs2_ail1_empty_one(sdp, tr, &withdraw);
                if (list_empty(&tr->tr_ail1_list) && oldest_tr)
                        list_move(&tr->tr_list, &sdp->sd_ail2_list);
                else
@@ -230,6 +239,9 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp)
        ret = list_empty(&sdp->sd_ail1_list);
        spin_unlock(&sdp->sd_ail_lock);
 
+       if (withdraw)
+               gfs2_lm_withdraw(sdp, "fatal: I/O error(s)\n");
+
        return ret;
 }
 
index 4d6567990baf2dd639146870254d05e4716caa6c..f2567f958d002caa48d4c6133b40e5fe13234368 100644 (file)
@@ -49,7 +49,7 @@ void gfs2_pin(struct gfs2_sbd *sdp, struct buffer_head *bh)
        if (test_set_buffer_pinned(bh))
                gfs2_assert_withdraw(sdp, 0);
        if (!buffer_uptodate(bh))
-               gfs2_io_error_bh(sdp, bh);
+               gfs2_io_error_bh_wd(sdp, bh);
        bd = bh->b_private;
        /* If this buffer is in the AIL and it has already been written
         * to in-place disk block, remove it from the AIL.
index 52de1036d9f92fbd4b544dfb5495d71159fa3ca3..be9c0bf697fe6ec4d8d0659900d3de9555ba0a4a 100644 (file)
@@ -293,7 +293,7 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags,
        if (unlikely(!buffer_uptodate(bh))) {
                struct gfs2_trans *tr = current->journal_info;
                if (tr && test_bit(TR_TOUCHED, &tr->tr_flags))
-                       gfs2_io_error_bh(sdp, bh);
+                       gfs2_io_error_bh_wd(sdp, bh);
                brelse(bh);
                *bhp = NULL;
                return -EIO;
@@ -320,7 +320,7 @@ int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh)
        if (!buffer_uptodate(bh)) {
                struct gfs2_trans *tr = current->journal_info;
                if (tr && test_bit(TR_TOUCHED, &tr->tr_flags))
-                       gfs2_io_error_bh(sdp, bh);
+                       gfs2_io_error_bh_wd(sdp, bh);
                return -EIO;
        }
        if (unlikely(test_bit(SDF_SHUTDOWN, &sdp->sd_flags)))
index 763d659db91b4cf8de24c67311b16479bbe1dbb2..59c811de0dc7a36e58ede891c3dde0735d911cab 100644 (file)
@@ -46,14 +46,16 @@ int gfs2_lm_withdraw(struct gfs2_sbd *sdp, const char *fmt, ...)
            test_and_set_bit(SDF_SHUTDOWN, &sdp->sd_flags))
                return 0;
 
-       va_start(args, fmt);
+       if (fmt) {
+               va_start(args, fmt);
 
-       vaf.fmt = fmt;
-       vaf.va = &args;
+               vaf.fmt = fmt;
+               vaf.va = &args;
 
-       fs_err(sdp, "%pV", &vaf);
+               fs_err(sdp, "%pV", &vaf);
 
-       va_end(args);
+               va_end(args);
+       }
 
        if (sdp->sd_args.ar_errors == GFS2_ERRORS_WITHDRAW) {
                fs_err(sdp, "about to withdraw this file system\n");
@@ -246,21 +248,21 @@ int gfs2_io_error_i(struct gfs2_sbd *sdp, const char *function, char *file,
 }
 
 /**
- * gfs2_io_error_bh_i - Flag a buffer I/O error and withdraw
- * Returns: -1 if this call withdrew the machine,
- *          0 if it was already withdrawn
+ * gfs2_io_error_bh_i - Flag a buffer I/O error
+ * @withdraw: withdraw the filesystem
  */
 
-int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
-                      const char *function, char *file, unsigned int line)
+void gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
+                       const char *function, char *file, unsigned int line,
+                       bool withdraw)
 {
-       int rv;
-       rv = gfs2_lm_withdraw(sdp,
-                             "fatal: I/O error\n"
-                             "  block = %llu\n"
-                             "  function = %s, file = %s, line = %u\n",
-                             (unsigned long long)bh->b_blocknr,
-                             function, file, line);
-       return rv;
+       fs_err(sdp,
+              "fatal: I/O error\n"
+              "  block = %llu\n"
+              "  function = %s, file = %s, line = %u\n",
+              (unsigned long long)bh->b_blocknr,
+              function, file, line);
+       if (withdraw)
+               gfs2_lm_withdraw(sdp, NULL);
 }
 
index 3926f95a6eb7070484131bb01117249f2f94094a..96ac4aba47387eb146a8af242168d8180442bcf1 100644 (file)
@@ -136,11 +136,15 @@ int gfs2_io_error_i(struct gfs2_sbd *sdp, const char *function,
 gfs2_io_error_i((sdp), __func__, __FILE__, __LINE__);
 
 
-int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
-                      const char *function, char *file, unsigned int line);
+void gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
+                       const char *function, char *file, unsigned int line,
+                       bool withdraw);
+
+#define gfs2_io_error_bh_wd(sdp, bh) \
+gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__, true);
 
 #define gfs2_io_error_bh(sdp, bh) \
-gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__);
+gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__, false);
 
 
 extern struct kmem_cache *gfs2_glock_cachep;