GFS2: Skip taking journal recovery locks for spectators
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 GFS2: Skip taking journal recovery locks for spectators
Hi,
Before this patch, spectator mounts would try to acquire the dlm's control lock and mounted lock as part of its normal recovery sequence. It's not necessary because spectators don't ever do journal recovery. And if they acquire those locks (at all) it will prevent another first-mounter from acquiring the lock in EX mode, which means it also cannot do journal recovery because it doesn't think it's the first node to mount the file system.
This patch checks if the mounter is a spectator, and if so, avoids grabbing the control lock and mounted lock. This allows a secondary mounter who is really the first rw mounter, to do journal recovery:
since the spectator doesn't acquire those locks, it can grab them in EX mode, and therefore consider itself to be the first mounter.
--- fs/gfs2/lock_dlm.c | 2 ++ 1 file changed, 2 insertions(+)
fs/gfs2/lock_dlm.c +2 -0
@@ -807,6 +807,8 @@ static int control_mount(struct gfs2_sbd *sdp)msleep_interruptible (500 ); +if (sdp->sd_args.ar_spectator) + goto locks_done;/* * Acquire control_lock in EX and mounted_lock in either EX or PR. * control_lock lvb keeps track of any pending journal recoveries.
Andreas Gruenbacher
maintainerPATCH 1/1 GFS2: Skip taking journal recovery locks for spectators
Bob,
Quoted 10 lines · Bob Peterson
Hi,
Before this patch, spectator mounts would try to acquire the dlm's control lock and mounted lock as part of its normal recovery sequence. It's not necessary because spectators don't ever do journal recovery. And if they acquire those locks (at all) it will prevent another first-mounter from acquiring the lock in EX mode, which means it also cannot do journal recovery because it doesn't think it's the first node to mount the file system.
Quoted 5 lines
This patch checks if the mounter is a spectator, and if so, avoids grabbing the control lock and mounted lock. This allows a secondary mounter who is really the first rw mounter, to do journal recovery:
since the spectator doesn't acquire those locks, it can grab them in EX mode, and therefore consider itself to be the first mounter.
if I understand things correctly, spectator mounts were so far holding the mounted_lock in PR mode which did prevent other nodes from starting recovery. With this change, spectators can now end up reading from the filesystem during recovery. That doesn't seem safe, especially since recovery won't take the usual glocks, and so there would be no synchronization with spectators anymore. I'm wondering if spectators could pause and relinquish the mounted_lock when another node tries to acquire it in EX mode, and resume after re-acquiring the lock. Or something similar.
Thanks,
Andreas
PATCH 1/1 GFS2: Skip taking journal recovery locks for spectators
----- Original Message -----
Quoted 29 lines
Bob,
Quoted 10 lines · Bob Peterson
Hi,
Before this patch, spectator mounts would try to acquire the dlm's control lock and mounted lock as part of its normal recovery sequence. It's not necessary because spectators don't ever do journal recovery. And if they acquire those locks (at all) it will prevent another first-mounter from acquiring the lock in EX mode, which means it also cannot do journal recovery because it doesn't think it's the first node to mount the file system.
Quoted 5 lines
This patch checks if the mounter is a spectator, and if so, avoids grabbing the control lock and mounted lock. This allows a secondary mounter who is really the first rw mounter, to do journal recovery:
since the spectator doesn't acquire those locks, it can grab them in EX mode, and therefore consider itself to be the first mounter.if I understand things correctly, spectator mounts were so far holding the
mounted_lockin PR mode which did prevent other nodes from starting recovery. With this change, spectators can now end up reading from the filesystem during recovery. That doesn't seem safe, especially since recovery won't take the usual glocks, and so there would be no synchronization with spectators anymore. I'm wondering if spectators could pause and relinquish themounted_lockwhen another node tries to acquire it in EX mode, and resume after re-acquiring the lock. Or something similar.
Quoted 3 lines
Thanks,
Andreas
Hi Andreas,
With the patch, I suppose _new_ spectators can now mount the file system before recovery and see an inconsistent view. However, what's the difference between that and an already-mounted spectator, who can already see an unrecovered file system before the journal is replayed? Iow, if it's already mounted as spectator, but before the rw mounting node has done its recovery? Or even been fenced for that matter. It turns out, there is none.
I did a little experiment using only stock kernels:
1. Node 1 mounts rw
2. Node 2 mounts -o spectator
3. Node 1 writes to the file system
4. Node 1 gets fenced while it's writing
5. Node 2 does ls -lR on the file system before the rw node reboots
Guess what happens? Boom! It sees an inconsistent view of the file system. This is true, in theory, with all kernels today.
[43241.051707] GFS2: fsid=dm-12.s: fatal: invalid metadata block [43241.051707] GFS2: fsid=dm-12.s: bh = 360123 (magic number) [43241.051707] GFS2: fsid=dm-12.s: function = gfs2_meta_indirect_buffer, file = fs/gfs2/meta_io.c, line = 428 [43241.072890] GFS2: fsid=dm-12.s: about to withdraw this file system [43241.079112] GFS2: fsid=dm-12.s: withdrawn
So there's a larger issue here. We should prevent spectators from seeing an inconsistent view of the file system, regardless of the status of the mounted lock.
The concept of cluster quorum was supposed to prevent inconsistent views because there's always someone around to replay the journal. But when we introduced spectator mounts, we nullified that, unintentionally.
In the old pre-spectator way of thinking, when a cluster loses quorum, the dlm locking is temporarily halted until quorum is regained, thus preventing inconsistent views until quorum is regained and the journal is replayed. This can even be done by a read-only mounter, and before the node rejoins the cluster.
With spectators, the there might not be anyone capable of replaying the journal, regardless of quorum. You could have a 16 node cluster with 15 spectators, all of whom instantly see an inconsistent view of the file system metadata as soon as the writer disappears from the cluster.
Bottom line: My patch makes things a little better because now at least the rw node can do recovery and replay the journal, whereas before it couldn't. It's true that we used to prevent other nodes from mounting -o spectator at that point, but our existing nodes who mount as spectator already have the same (worse) problem.
We have a larger problem that is not addressed by my patch, which might be difficult to resolve.
Bob Peterson
PATCH 1/1 GFS2: Skip taking journal recovery locks for spectators
Hi,
Quoted 50 lines · Bob Peterson
----- Original Message -----
Quoted 28 lines
Bob,
Quoted 10 lines · Bob Peterson
Hi,
Before this patch, spectator mounts would try to acquire the dlm's control lock and mounted lock as part of its normal recovery sequence. It's not necessary because spectators don't ever do journal recovery. And if they acquire those locks (at all) it will prevent another first-mounter from acquiring the lock in EX mode, which means it also cannot do journal recovery because it doesn't think it's the first node to mount the file system.
Quoted 5 lines
This patch checks if the mounter is a spectator, and if so, avoids grabbing the control lock and mounted lock. This allows a secondary mounter who is really the first rw mounter, to do journal recovery:
since the spectator doesn't acquire those locks, it can grab them in EX mode, and therefore consider itself to be the first mounter.if I understand things correctly, spectator mounts were so far holding the
mounted_lockin PR mode which did prevent other nodes from starting recovery. With this change, spectators can now end up reading from the filesystem during recovery. That doesn't seem safe, especially since recovery won't take the usual glocks, and so there would be no synchronization with spectators anymore. I'm wondering if spectators could pause and relinquish themounted_lockwhen another node tries to acquire it in EX mode, and resume after re-acquiring the lock. Or something similar.Quoted 3 lines
Thanks,
AndreasHi Andreas,
With the patch, I suppose _new_ spectators can now mount the file system before recovery and see an inconsistent view. However, what's the difference between that and an already-mounted spectator, who can already see an unrecovered file system before the journal is replayed? Iow, if it's already mounted as spectator, but before the rw mounting node has done its recovery? Or even been fenced for that matter. It turns out, there is none.
I did a little experiment using only stock kernels:
1. Node 1 mounts rw
2. Node 2 mounts -o spectator
3. Node 1 writes to the file system
4. Node 1 gets fenced while it's writing
5. Node 2 does ls -lR on the file system before the rw node reboots
Quoted 8 lines
Guess what happens? Boom! It sees an inconsistent view of the file system. This is true, in theory, with all kernels today.
[43241.051707] GFS2: fsid=dm-12.s: fatal: invalid metadata block [43241.051707] GFS2: fsid=dm-12.s: bh = 360123 (magic number) [43241.051707] GFS2: fsid=dm-12.s: function = gfs2_meta_indirect_buffer, file = fs/gfs2/meta_io.c, line = 428 [43241.072890] GFS2: fsid=dm-12.s: about to withdraw this file system [43241.079112] GFS2: fsid=dm-12.s: withdrawn
That is clearly wrong. There is no reason that a spectator that has mounted the fs prior to some failure event should ever fail in this way. They should be blocked by the DLM locks until recovery has completed.
Spectators that try to mount a filesystem which needs recovery should either be blocked until recovery has taken place, or those mounts should fail if there is no way to perform recovery (e.g. if there are no non-spectator mounts)
This is something that we should make sure is included in testing to avoid regressions in this area. It sounds though as if we may need to take a wider look at this to make sure that all the corner cases are covered,
Steve.