[GFS2] Fix a bug: scheduling under a spinlock

At some stage, a mutex was added to gfs2_glock_put() without
checking all its call sites. Two of them were called from
under a spinlock causing random delays at various points and
crashes.

Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 4ed13787b7ec3f02ebe220212840e02caeff2d40..32cc4005307d1b8b32d642ce8908ffc9a0e6d6ce 100644 (file)
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -158,6 +158,7 @@ int gfs2_glock_put(struct gfs2_glock *gl)
        if (kref_put(&gl->gl_ref, kill_glock)) {
                list_del_init(&gl->gl_list);
                write_unlock(&bucket->hb_lock);
+               BUG_ON(spin_is_locked(&gl->gl_spin));
                glock_free(gl);
                rv = 1;
                goto out;
@@ -353,13 +354,14 @@ int gfs2_glock_get(struct gfs2_sbd *sdp, uint64_t number,
  *
  */
 
-void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, int flags,
+void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, unsigned flags,
                      struct gfs2_holder *gh)
 {
+       flags |= GL_NEVER_RECURSE;
        INIT_LIST_HEAD(&gh->gh_list);
        gh->gh_gl = gl;
        gh->gh_ip = (unsigned long)__builtin_return_address(0);
-       gh->gh_owner = (flags & GL_NEVER_RECURSE) ? NULL : current;
+       gh->gh_owner = current;
        gh->gh_state = state;
        gh->gh_flags = flags;
        gh->gh_error = 0;
@@ -382,10 +384,10 @@ void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, int flags,
  *
  */
 
-void gfs2_holder_reinit(unsigned int state, int flags, struct gfs2_holder *gh)
+void gfs2_holder_reinit(unsigned int state, unsigned flags, struct gfs2_holder *gh)
 {
        gh->gh_state = state;
-       gh->gh_flags = flags;
+       gh->gh_flags = flags | GL_NEVER_RECURSE;
        if (gh->gh_state == LM_ST_EXCLUSIVE)
                gh->gh_flags |= GL_LOCAL_EXCL;
 
@@ -461,6 +463,8 @@ static void handle_recurse(struct gfs2_holder *gh)
        struct gfs2_holder *tmp_gh, *safe;
        int found = 0;
 
+       BUG_ON(!spin_is_locked(&gl->gl_spin));
+
        printk(KERN_INFO "recursion %016llx, %u\n", gl->gl_name.ln_number,
                gl->gl_name.ln_type);
 
@@ -502,6 +506,8 @@ static void do_unrecurse(struct gfs2_holder *gh)
        struct gfs2_holder *tmp_gh, *last_gh = NULL;
        int found = 0;
 
+       BUG_ON(!spin_is_locked(&gl->gl_spin));
+
        if (gfs2_assert_warn(sdp, gh->gh_owner))
                return;
 
@@ -676,7 +682,6 @@ static int rq_greedy(struct gfs2_holder *gh)
  * @gl: the glock
  *
  */
-
 static void run_queue(struct gfs2_glock *gl)
 {
        struct gfs2_holder *gh;
@@ -779,6 +784,7 @@ void gfs2_glmutex_unlock(struct gfs2_glock *gl)
        spin_lock(&gl->gl_spin);
        clear_bit(GLF_LOCK, &gl->gl_flags);
        run_queue(gl);
+       BUG_ON(!spin_is_locked(&gl->gl_spin));
        spin_unlock(&gl->gl_spin);
 }
 
@@ -1244,7 +1250,7 @@ static int recurse_check(struct gfs2_holder *existing, struct gfs2_holder *new,
 
        return 0;
 
- fail:
+fail:
        print_symbol(KERN_WARNING "GFS2: Existing holder from %s\n",
                     existing->gh_ip);
        print_symbol(KERN_WARNING "GFS2: New holder from %s\n", new->gh_ip);
@@ -1263,6 +1269,8 @@ static void add_to_queue(struct gfs2_holder *gh)
        struct gfs2_glock *gl = gh->gh_gl;
        struct gfs2_holder *existing;
 
+       BUG_ON(!gh->gh_owner);
+
        if (!gh->gh_owner)
                goto out;
 
@@ -1331,7 +1339,7 @@ int gfs2_glock_nq(struct gfs2_holder *gh)
        if (!(gh->gh_flags & GL_ASYNC)) {
                error = glock_wait_internal(gh);
                if (error == GLR_CANCELED) {
-                       msleep(1000);
+                       msleep(100);
                        goto restart;
                }
        }
@@ -1360,7 +1368,7 @@ int gfs2_glock_poll(struct gfs2_holder *gh)
        else if (list_empty(&gh->gh_list)) {
                if (gh->gh_error == GLR_CANCELED) {
                        spin_unlock(&gl->gl_spin);
-                       msleep(1000);
+                       msleep(100);
                        if (gfs2_glock_nq(gh))
                                return 1;
                        return 0;
@@ -1386,7 +1394,7 @@ int gfs2_glock_wait(struct gfs2_holder *gh)
 
        error = glock_wait_internal(gh);
        if (error == GLR_CANCELED) {
-               msleep(1000);
+               msleep(100);
                gh->gh_flags &= ~GL_ASYNC;
                error = gfs2_glock_nq(gh);
        }
@@ -2217,10 +2225,12 @@ static void clear_glock(struct gfs2_glock *gl)
        if (!list_empty(&gl->gl_reclaim)) {
                list_del_init(&gl->gl_reclaim);
                atomic_dec(&sdp->sd_reclaim_count);
+               spin_unlock(&sdp->sd_reclaim_lock);
                released = gfs2_glock_put(gl);
                gfs2_assert(sdp, !released);
+       } else {
+               spin_unlock(&sdp->sd_reclaim_lock);
        }
-       spin_unlock(&sdp->sd_reclaim_lock);
 
        if (gfs2_glmutex_trylock(gl)) {
                if (gl->gl_ops == &gfs2_inode_glops) {

diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
index b6646e7fed153bfc1cc684ea90eea3fa8d73dba5..ed5bc3e65397f161922246db081c8db1315c5757 100644 (file)
--- a/fs/gfs2/glock.h
+++ b/fs/gfs2/glock.h
@@ -81,10 +81,10 @@ int gfs2_glock_get(struct gfs2_sbd *sdp,
                   int create, struct gfs2_glock **glp);
 void gfs2_glock_hold(struct gfs2_glock *gl);
 int gfs2_glock_put(struct gfs2_glock *gl);
-
-void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, int flags,
+void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, unsigned flags,
                      struct gfs2_holder *gh);
-void gfs2_holder_reinit(unsigned int state, int flags, struct gfs2_holder *gh);
+void gfs2_holder_reinit(unsigned int state, unsigned flags,
+                       struct gfs2_holder *gh);
 void gfs2_holder_uninit(struct gfs2_holder *gh);
 struct gfs2_holder *gfs2_holder_get(struct gfs2_glock *gl, unsigned int state,
                                    int flags, gfp_t gfp_flags);

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 6140c2434e8500e4adb4d7290000a5132278d527..fb5a4d06e926b5e073d9eb04502e38054e899a42 100644 (file)
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -409,8 +409,8 @@ void gfs2_inode_destroy(struct gfs2_inode *ip)
 
        spin_lock(&io_gl->gl_spin);
        io_gl->gl_object = NULL;
-       gfs2_glock_put(i_gl);
        spin_unlock(&io_gl->gl_spin);
+       gfs2_glock_put(i_gl);
 
        gfs2_glock_dq_uninit(&ip->i_iopen_gh);
 

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index b0dd0b9ad79bae5cf86bb3c37c24776a9bd0573b..9e32e0faaf20ae3b1618a411fe72cf3a3a9641e1 100644 (file)
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -578,6 +578,7 @@ void gfs2_log_shutdown(struct gfs2_sbd *sdp)
        gfs2_assert_withdraw(sdp, !sdp->sd_log_num_revoke);
        gfs2_assert_withdraw(sdp, !sdp->sd_log_num_rg);
        gfs2_assert_withdraw(sdp, !sdp->sd_log_num_databuf);
+       gfs2_assert_withdraw(sdp, !sdp->sd_log_num_hdrs);
        gfs2_assert_withdraw(sdp, list_empty(&sdp->sd_ail1_list));
 
        sdp->sd_log_flush_head = sdp->sd_log_head;

diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index 74cf28e77b4724b0cf699532178780a76c8de3fd..da49973a90d111841b9b9b523725c36203dd4565 100644 (file)
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -214,6 +214,8 @@ void gfs2_ail1_start_one(struct gfs2_sbd *sdp, struct gfs2_ail *ai)
        struct buffer_head *bh;
        int retry;
 
+       BUG_ON(!spin_is_locked(&sdp->sd_log_lock));
+
        do {
                retry = 0;