Skip to content

Commit 6cb3b1c

Browse files
author
Andreas Gruenbacher
committed
gfs2: Fix additional unlikely request cancelation race
In gfs2_glock_dq(), we must drop the glock spin lock before calling ->lm_cancel, but this means that in the meantime, the operation we are trying to cancel could complete. If the operation completes unsuccessfully, another holder can end up at the head of the queue and another ->lm_lock operation can get started. In this case, we would end up canceling that second operation by accident. To prevent that, introduce a new GLF_CANCELING flag. Set that flag in gfs2_glock_dq() when trying to cancel an operation. When seeing that flag, finish_xmote() will then keep the GLF_LOCK flag set to prevent other glock operations from taking place. gfs2_glock_dq() then completes the cancelation attempt by clearing GLF_LOCK and GLF_CANCELING. In addition, add a missing GLF_DEMOTE_IN_PROGRESS check in gfs2_glock_dq() to make sure that we won't accidentally cancel a demote request. Signed-off-by: Andreas Gruenbacher <[email protected]>
1 parent a431d49 commit 6cb3b1c

File tree

3 files changed

+14
-3
lines changed

3 files changed

+14
-3
lines changed

fs/gfs2/glock.c

+11-2
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,8 @@ static void finish_xmote(struct gfs2_glock *gl, unsigned int ret)
665665
do_promote(gl);
666666
}
667667
out:
668-
clear_bit(GLF_LOCK, &gl->gl_flags);
668+
if (!test_bit(GLF_CANCELING, &gl->gl_flags))
669+
clear_bit(GLF_LOCK, &gl->gl_flags);
669670
}
670671

671672
static bool is_system_glock(struct gfs2_glock *gl)
@@ -1671,11 +1672,17 @@ void gfs2_glock_dq(struct gfs2_holder *gh)
16711672
}
16721673

16731674
if (list_is_first(&gh->gh_list, &gl->gl_holders) &&
1674-
!test_bit(HIF_HOLDER, &gh->gh_iflags)) {
1675+
!test_bit(HIF_HOLDER, &gh->gh_iflags) &&
1676+
test_bit(GLF_LOCK, &gl->gl_flags) &&
1677+
!test_bit(GLF_DEMOTE_IN_PROGRESS, &gl->gl_flags) &&
1678+
!test_bit(GLF_CANCELING, &gl->gl_flags)) {
1679+
set_bit(GLF_CANCELING, &gl->gl_flags);
16751680
spin_unlock(&gl->gl_lockref.lock);
16761681
gl->gl_name.ln_sbd->sd_lockstruct.ls_ops->lm_cancel(gl);
16771682
wait_on_bit(&gh->gh_iflags, HIF_WAIT, TASK_UNINTERRUPTIBLE);
16781683
spin_lock(&gl->gl_lockref.lock);
1684+
clear_bit(GLF_CANCELING, &gl->gl_flags);
1685+
clear_bit(GLF_LOCK, &gl->gl_flags);
16791686
if (!gfs2_holder_queued(gh))
16801687
goto out;
16811688
}
@@ -2352,6 +2359,8 @@ static const char *gflags2str(char *buf, const struct gfs2_glock *gl)
23522359
*p++ = 'E';
23532360
if (test_bit(GLF_DEFER_DELETE, gflags))
23542361
*p++ = 's';
2362+
if (test_bit(GLF_CANCELING, gflags))
2363+
*p++ = 'C';
23552364
*p = 0;
23562365
return buf;
23572366
}

fs/gfs2/incore.h

+1
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ enum {
332332
GLF_VERIFY_DELETE = 18, /* iopen glocks only */
333333
GLF_PENDING_REPLY = 19,
334334
GLF_DEFER_DELETE = 20, /* iopen glocks only */
335+
GLF_CANCELING = 21,
335336
};
336337

337338
struct gfs2_glock {

fs/gfs2/trace_gfs2.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@
6565
{(1UL << GLF_INSTANTIATE_IN_PROG), "N" }, \
6666
{(1UL << GLF_TRY_TO_EVICT), "e" }, \
6767
{(1UL << GLF_VERIFY_DELETE), "E" }, \
68-
{(1UL << GLF_DEFER_DELETE), "s" })
68+
{(1UL << GLF_DEFER_DELETE), "s" }, \
69+
{(1UL << GLF_CANCELING), "C" })
6970

7071
#ifndef NUMPTY
7172
#define NUMPTY

0 commit comments

Comments
 (0)