Re: [Cluster-devel] [GFS2 PATCH 1/9] gfs2: Introduce concept of a pending withdraw
On 13/02/2019 15:21, Bob Peterson wrote:
File system withdraws can be delayed when inconsistencies are
discovered when we cannot withdraw immediately, for example, when
critical spin_locks are held. But delaying the withdraw can cause
gfs2 to ignore the error and keep running for a short period of time.
For example, an rgrp glock may be dequeued and demoted while there
are still buffers that haven't been properly revoked, due to io
errors writing to the journal.
This patch introduces a new concept of a delayed withdraw, which
means an inconsistency has been discovered and we need to withdraw
at the earliest possible opportunity. In these cases, we aren't
quite withdrawn yet, but we still need to not dequeue glocks and
other critical things. If we dequeue the glocks and the withdraw
results in our journal being replayed, the replay could overwrite
data that's been modified by a different node that acquired the
glock in the meantime.
Signed-off-by: Bob Peterson
This withdrawn() wrapper seems like a good plan anyway, since it helps
make the code more readable,
Steve.
---
fs/gfs2/aops.c | 4 ++--
fs/gfs2/file.c | 2 +-
fs/gfs2/glock.c | 7 +++
fs/gfs2/glops.c | 2 +-
fs/gfs2/incore.h | 1 +
fs/gfs2/log.c| 20
fs/gfs2/meta_io.c| 6 +++---
fs/gfs2/ops_fstype.c | 3 +--
fs/gfs2/quota.c | 2 +-
fs/gfs2/super.c | 6 +++---
fs/gfs2/sys.c| 2 +-
fs/gfs2/util.c | 1 +
fs/gfs2/util.h | 8
13 files changed, 34 insertions(+), 30 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 05dd78f4b2b3..0d3cde8a61cd 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -521,7 +521,7 @@ static int __gfs2_readpage(void *file, struct page *page)
error = mpage_readpage(page, gfs2_block_map);
}
- if (unlikely(test_bit(SDF_SHUTDOWN, >sd_flags)))
+ if (unlikely(withdrawn(sdp)))
return -EIO;
return error;
@@ -638,7 +638,7 @@ static int gfs2_readpages(struct file *file, struct
address_space *mapping,
gfs2_glock_dq();
out_uninit:
gfs2_holder_uninit();
- if (unlikely(test_bit(SDF_SHUTDOWN, >sd_flags)))
+ if (unlikely(withdrawn(sdp)))
ret = -EIO;
return ret;
}
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index a2dea5bc0427..4046f6ac7f13 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -1169,7 +1169,7 @@ static int gfs2_lock(struct file *file, int cmd, struct
file_lock *fl)
cmd = F_SETLK;
fl->fl_type = F_UNLCK;
}
- if (unlikely(test_bit(SDF_SHUTDOWN, >sd_flags))) {
+ if (unlikely(withdrawn(sdp))) {
if (fl->fl_type == F_UNLCK)
locks_lock_file_wait(file, fl);
return -EIO;
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index f66773c71bcd..c6d6e478f5e3 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -542,7 +542,7 @@ __acquires(>gl_lockref.lock)
unsigned int lck_flags = (unsigned int)(gh ? gh->gh_flags : 0);
int ret;
- if (unlikely(test_bit(SDF_SHUTDOWN, >sd_flags)) &&
+ if (unlikely(withdrawn(sdp)) &&
target != LM_ST_UNLOCKED)
return;
lck_flags &= (LM_FLAG_TRY | LM_FLAG_TRY_1CB | LM_FLAG_NOEXP |
@@ -579,8 +579,7 @@ __acquires(>gl_lockref.lock)
}
else if (ret) {
fs_err(sdp, "lm_lock ret %d\n", ret);
- GLOCK_BUG_ON(gl, !test_bit(SDF_SHUTDOWN,
- >sd_flags));
+ GLOCK_BUG_ON(gl, !withdrawn(sdp));
}
} else { /* lock_nolock */
finish_xmote(gl, target);
@@ -1092,7 +1091,7 @@ int gfs2_glock_nq(struct gfs2_holder *gh)
struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
int error = 0;
- if (unlikely(test_bit(SDF_SHUTDOWN, >sd_flags)))
+ if (unlikely(withdrawn(sdp)))
return -EIO;
if (test_bit(GLF_LRU, >gl_flags))
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index f15b4c57c4bd..9c86c8004ba7 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -539,7 +539,7 @@ static int freeze_go_xmote_bh(struct gfs2_glock *gl, struct
gfs2_holder *gh)
gfs2_consist(sdp);
/* Initialize some head of the log stuff */
- if (!test_bit(SDF_SHUTDOWN, >sd_flags)) {
+ if (!withdrawn(sdp)) {
sdp->sd_log_sequence = head.lh_sequence + 1;
gfs2_log_pointers_init(sdp, head.lh_blkno);
}
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index cdf07b408f54..8380d4db8be6 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -621,6 +621,7 @@ enum {
SDF_SKIP_DLM_UNLOCK = 8,
SDF_FORCE_AIL_FLUSH = 9,
SDF_AIL1_IO_ERROR = 10,
+ SDF_PENDING_WITHDRAW= 11, /* Will
[Cluster-devel] [GFS2 PATCH 1/9] gfs2: Introduce concept of a pending withdraw
File system withdraws can be delayed when inconsistencies are
discovered when we cannot withdraw immediately, for example, when
critical spin_locks are held. But delaying the withdraw can cause
gfs2 to ignore the error and keep running for a short period of time.
For example, an rgrp glock may be dequeued and demoted while there
are still buffers that haven't been properly revoked, due to io
errors writing to the journal.
This patch introduces a new concept of a delayed withdraw, which
means an inconsistency has been discovered and we need to withdraw
at the earliest possible opportunity. In these cases, we aren't
quite withdrawn yet, but we still need to not dequeue glocks and
other critical things. If we dequeue the glocks and the withdraw
results in our journal being replayed, the replay could overwrite
data that's been modified by a different node that acquired the
glock in the meantime.
Signed-off-by: Bob Peterson
---
fs/gfs2/aops.c | 4 ++--
fs/gfs2/file.c | 2 +-
fs/gfs2/glock.c | 7 +++
fs/gfs2/glops.c | 2 +-
fs/gfs2/incore.h | 1 +
fs/gfs2/log.c| 20
fs/gfs2/meta_io.c| 6 +++---
fs/gfs2/ops_fstype.c | 3 +--
fs/gfs2/quota.c | 2 +-
fs/gfs2/super.c | 6 +++---
fs/gfs2/sys.c| 2 +-
fs/gfs2/util.c | 1 +
fs/gfs2/util.h | 8
13 files changed, 34 insertions(+), 30 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 05dd78f4b2b3..0d3cde8a61cd 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -521,7 +521,7 @@ static int __gfs2_readpage(void *file, struct page *page)
error = mpage_readpage(page, gfs2_block_map);
}
- if (unlikely(test_bit(SDF_SHUTDOWN, >sd_flags)))
+ if (unlikely(withdrawn(sdp)))
return -EIO;
return error;
@@ -638,7 +638,7 @@ static int gfs2_readpages(struct file *file, struct
address_space *mapping,
gfs2_glock_dq();
out_uninit:
gfs2_holder_uninit();
- if (unlikely(test_bit(SDF_SHUTDOWN, >sd_flags)))
+ if (unlikely(withdrawn(sdp)))
ret = -EIO;
return ret;
}
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index a2dea5bc0427..4046f6ac7f13 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -1169,7 +1169,7 @@ static int gfs2_lock(struct file *file, int cmd, struct
file_lock *fl)
cmd = F_SETLK;
fl->fl_type = F_UNLCK;
}
- if (unlikely(test_bit(SDF_SHUTDOWN, >sd_flags))) {
+ if (unlikely(withdrawn(sdp))) {
if (fl->fl_type == F_UNLCK)
locks_lock_file_wait(file, fl);
return -EIO;
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index f66773c71bcd..c6d6e478f5e3 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -542,7 +542,7 @@ __acquires(>gl_lockref.lock)
unsigned int lck_flags = (unsigned int)(gh ? gh->gh_flags : 0);
int ret;
- if (unlikely(test_bit(SDF_SHUTDOWN, >sd_flags)) &&
+ if (unlikely(withdrawn(sdp)) &&
target != LM_ST_UNLOCKED)
return;
lck_flags &= (LM_FLAG_TRY | LM_FLAG_TRY_1CB | LM_FLAG_NOEXP |
@@ -579,8 +579,7 @@ __acquires(>gl_lockref.lock)
}
else if (ret) {
fs_err(sdp, "lm_lock ret %d\n", ret);
- GLOCK_BUG_ON(gl, !test_bit(SDF_SHUTDOWN,
- >sd_flags));
+ GLOCK_BUG_ON(gl, !withdrawn(sdp));
}
} else { /* lock_nolock */
finish_xmote(gl, target);
@@ -1092,7 +1091,7 @@ int gfs2_glock_nq(struct gfs2_holder *gh)
struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
int error = 0;
- if (unlikely(test_bit(SDF_SHUTDOWN, >sd_flags)))
+ if (unlikely(withdrawn(sdp)))
return -EIO;
if (test_bit(GLF_LRU, >gl_flags))
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index f15b4c57c4bd..9c86c8004ba7 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -539,7 +539,7 @@ static int freeze_go_xmote_bh(struct gfs2_glock *gl, struct
gfs2_holder *gh)
gfs2_consist(sdp);
/* Initialize some head of the log stuff */
- if (!test_bit(SDF_SHUTDOWN, >sd_flags)) {
+ if (!withdrawn(sdp)) {
sdp->sd_log_sequence = head.lh_sequence + 1;
gfs2_log_pointers_init(sdp, head.lh_blkno);
}
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index cdf07b408f54..8380d4db8be6 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -621,6 +621,7 @@ enum {
SDF_SKIP_DLM_UNLOCK = 8,
SDF_FORCE_AIL_FLUSH = 9,
SDF_AIL1_IO_ERROR = 10,
+ SDF_PENDING_WITHDRAW= 11, /* Will withdraw eventually */
};
enum gfs2_freeze_state {
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 5bfaf381921a..ec8675113b0d 100644
---
