Skip to content

Commit

Permalink
[GFS2] Fix a bug: scheduling under a spinlock
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Steven Whitehouse committed Apr 20, 2006
1 parent fe1bded commit 190562b
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 14 deletions.
30 changes: 20 additions & 10 deletions fs/gfs2/glock.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions fs/gfs2/glock.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion fs/gfs2/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions fs/gfs2/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions fs/gfs2/meta_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 190562b

Please sign in to comment.