[Devel] [PATCH RHEL7 COMMIT] fuse: pcs: protection against sync seq numbers wraparound
Konstantin Khorenko
khorenko at virtuozzo.com
Wed Jul 27 22:02:21 MSK 2022
The commit is pushed to "branch-rh7-3.10.0-1160.66.1.vz7.188.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-1160.66.1.vz7.188.4
------>
commit f183a1d37edcb9e673090c37c8a0a30f5fbea64e
Author: Alexey Kuznetsov <kuznet at virtuozzo.com>
Date: Mon Jul 18 19:46:35 2022 +0800
fuse: pcs: protection against sync seq numbers wraparound
There is a hole in sync protocol. When CS is journaling sequence
number grows even if chunk is not accessed at all due to commits
to other chunks. This means that after long idle period sequence
numbers can jump arbitrarily comparing to state cached at client.
The result is lockup, such chunk cannot be synced.
Affects: #VSTOR-55377
https://pmc.acronis.com/browse/VSTOR-55377
Signed-off-by: Alexey Kuznetsov <kuznet at acronis.com>
---
fs/fuse/kio/pcs/pcs_map.c | 95 +++++++++++++++++++++++++++++++++++++++++++++--
fs/fuse/kio/pcs/pcs_map.h | 16 ++++++++
2 files changed, 107 insertions(+), 4 deletions(-)
diff --git a/fs/fuse/kio/pcs/pcs_map.c b/fs/fuse/kio/pcs/pcs_map.c
index deb87a6a493b1..b65d5f91356a2 100644
--- a/fs/fuse/kio/pcs/pcs_map.c
+++ b/fs/fuse/kio/pcs/pcs_map.c
@@ -810,6 +810,7 @@ void transfer_sync_data(struct pcs_cs_list * new_cs_list, struct pcs_cs_list * o
for (k = 0; k < old_cs_list->nsrv; k++) {
if (old_cs_list->cs[k].info.id.val == new_cs_list->cs[i].info.id.val) {
new_cs_list->cs[i].sync = old_cs_list->cs[k].sync;
+ new_cs_list->cs[i].dirty_ts = old_cs_list->cs[i].dirty_ts;
break;
}
}
@@ -862,6 +863,7 @@ static void evaluate_dirty_status(struct pcs_map_entry * m)
rec->sync.dirty_integrity = 0;
rec->sync.dirty_epoch = 0;
rec->sync.dirty_seq = 0;
+ rec->dirty_ts = jiffies;
}
} else
rec->sync.dirty_integrity = 0;
@@ -965,6 +967,7 @@ struct pcs_cs_list* cslist_alloc( struct pcs_cs_set *css, struct pcs_cs_info *re
for (i = 0; i < cs_cnt; i++) {
cs_list->cs[i].info = rec[i];
memset(&cs_list->cs[i].sync, 0, sizeof(cs_list->cs[i].sync));
+ cs_list->cs[i].dirty_ts = jiffies;
RCU_INIT_POINTER(cs_list->cs[i].cslink.cs, NULL);
INIT_LIST_HEAD(&cs_list->cs[i].cslink.link);
cs_list->cs[i].cslink.index = i;
@@ -2561,11 +2564,36 @@ noinline void pcs_mapping_truncate(struct pcs_dentry_info *di, u64 new_size)
pcs_map_put(m);
}
+static void warp_sync_seq(struct pcs_map_entry * m, struct pcs_cs_record * rec, struct pcs_cs_sync_data * sync)
+{
+ struct cs_sync_state * srec = &rec->sync;
+
+ /* The procedure can force sync seq to _decrease_. Log this event just in case.
+ */
+ if (pcs_sync_seq_compare(sync->sync_epoch, srec->dirty_epoch) < 0 ||
+ (sync->sync_epoch == srec->dirty_epoch &&
+ pcs_sync_seq_compare(sync->sync_dirty, srec->dirty_seq) < 0) ||
+ pcs_sync_seq_compare(sync->sync_epoch, srec->sync_epoch) < 0 ||
+ (sync->sync_epoch == srec->sync_epoch &&
+ pcs_sync_seq_compare(sync->sync_current, srec->sync_seq) < 0))
+ FUSE_KTRACE(cc_from_maps(m->maps)->fc, "Warp [" NODE_FMT ",%u/%u,%u/%u,%u/%u] -> [%u/%u,%u/%u]",
+ NODE_ARGS(rec->info.id), rec->info.integrity_seq, srec->dirty_integrity,
+ srec->dirty_epoch, srec->dirty_seq, srec->sync_epoch, srec->sync_seq,
+ sync->sync_epoch, sync->sync_dirty, sync->sync_epoch, sync->sync_current);
+
+ srec->dirty_epoch = sync->sync_epoch;
+ srec->dirty_seq = sync->sync_dirty;
+ srec->sync_epoch = sync->sync_epoch;
+ srec->sync_seq = sync->sync_current;
+}
+
static int commit_cs_record(struct pcs_map_entry * m, struct pcs_cs_record * rec,
struct pcs_cs_sync_data * sync, u32 lat, int op_type)
{
int dirtying;
struct cs_sync_state * srec = &rec->sync;
+ int was_dirty = cs_is_dirty(srec);
+
if (sync->ts_net > sync->ts_io)
lat -= sync->ts_net;
else
@@ -2593,6 +2621,10 @@ static int commit_cs_record(struct pcs_map_entry * m, struct pcs_cs_record * rec
dirtying = (op_type == PCS_CS_WRITE_SYNC_RESP || op_type == PCS_CS_WRITE_RESP ||
op_type == PCS_CS_WRITE_AL_RESP ||
op_type == PCS_CS_WRITE_HOLE_RESP || op_type == PCS_CS_WRITE_ZERO_RESP);
+
+ if (sync->sync_dirty == 0)
+ dirtying = 0;
+
/* The following looks scary, could be more clear.
* The goal is to update sync seq numbers:
*
@@ -2602,16 +2634,60 @@ static int commit_cs_record(struct pcs_map_entry * m, struct pcs_cs_record * rec
* - sync_epoch/sync_seq advance sync_epoch/seq
* - sync_epoch/sync_dirty advance dirty_epoch/seq
*/
- if (dirtying && sync->sync_dirty) {
- srec->dirty_integrity = sync->integrity_seq;
+ if (dirtying) {
+ unsigned int tmo;
- if (srec->dirty_epoch == 0 ||
- pcs_sync_seq_compare(sync->sync_epoch, srec->dirty_epoch) > 0) {
+ if (pcs_sync_seq_compare(sync->sync_current, sync->sync_dirty) > 0) {
+ dirtying = 0;
+ tmo = PCS_MAX_SYNC_TIMEOUT;
+ } else {
+ srec->dirty_integrity = sync->integrity_seq;
+ tmo = PCS_MAX_DIRTY_TIMEOUT;
+ }
+
+ if (jiffies - rec->dirty_ts > tmo) {
+ warp_sync_seq(m, rec, sync);
+ rec->dirty_ts = jiffies;
+ } else if (srec->dirty_epoch == 0 ||
+ pcs_sync_seq_compare(sync->sync_epoch, srec->dirty_epoch) > 0) {
srec->dirty_epoch = sync->sync_epoch;
srec->dirty_seq = sync->sync_dirty;
+ rec->dirty_ts = jiffies;
} else if (sync->sync_epoch == srec->dirty_epoch &&
pcs_sync_seq_compare(sync->sync_dirty, srec->dirty_seq) > 0) {
srec->dirty_seq = sync->sync_dirty;
+ rec->dirty_ts = jiffies;
+ }
+ } else {
+ if (jiffies - rec->dirty_ts > PCS_MAX_SYNC_TIMEOUT) {
+ srec->sync_epoch = sync->sync_epoch;
+ srec->sync_seq = sync->sync_current;
+
+ if (was_dirty) {
+ /* This must not happen, dirty replica cannot remain dirty
+ * for >> PCS_SYNC_TIMEOUT unless the cluster is very sick.
+ */
+ FUSE_KTRACE(cc_from_maps(m->maps)->fc, MAP_FMT " replica " NODE_FMT " is dirty for %ld hz; [%u/%u,%u/%u,%u/%u] -> [%u/%u]",
+ MAP_ARGS(m), NODE_ARGS(rec->info.id), (long)(jiffies - rec->dirty_ts),
+ rec->info.integrity_seq, srec->dirty_integrity,
+ srec->dirty_epoch, srec->dirty_seq, srec->sync_epoch, srec->sync_seq,
+ sync->sync_epoch, sync->sync_current);
+
+ /* Old dirty seq might be invalid and new one in not dirtying response
+ * is undefined. But it must be set to _something_ _dirty_.
+ * We are in troubles. We have to violate requirement of obscurity
+ * of sequence numbers for client and to forge some sequence number.
+ */
+ srec->dirty_epoch = sync->sync_epoch;
+ srec->dirty_seq = sync->sync_current;
+ } else {
+ /* Normal case, replica is clean. Actually, we could do such reset to 0
+ * every time when replica becomes clean.
+ */
+ srec->dirty_epoch = 0;
+ srec->dirty_seq = 0;
+ }
+ rec->dirty_ts = jiffies;
}
}
@@ -2623,6 +2699,17 @@ static int commit_cs_record(struct pcs_map_entry * m, struct pcs_cs_record * rec
pcs_sync_seq_compare(sync->sync_current, srec->sync_seq) > 0) {
srec->sync_seq = sync->sync_current;
}
+
+ if (!was_dirty && !dirtying && cs_is_dirty(srec)) {
+ FUSE_KTRACE(cc_from_maps(m->maps)->fc, MAP_FMT " replica " NODE_FMT " is not expected to be dirty [%u/%u,%u/%u,%u/%u]",
+ MAP_ARGS(m), NODE_ARGS(rec->info.id),
+ rec->info.integrity_seq, srec->dirty_integrity,
+ srec->dirty_epoch, srec->dirty_seq, srec->sync_epoch, srec->sync_seq);
+ /* "Dirty" dirty fixup */
+ srec->dirty_epoch = 0;
+ srec->dirty_seq = 0;
+ rec->dirty_ts = jiffies;
+ }
return 0;
}
diff --git a/fs/fuse/kio/pcs/pcs_map.h b/fs/fuse/kio/pcs/pcs_map.h
index a6a4ee877bffe..3ef753c9ca843 100644
--- a/fs/fuse/kio/pcs/pcs_map.h
+++ b/fs/fuse/kio/pcs/pcs_map.h
@@ -12,6 +12,21 @@ struct pcs_int_request;
#define PCS_MAP_LIMIT 4096
#define PCS_SYNC_TIMEOUT (20 * HZ)
+/* Protection against sync seq wraparound */
+
+/* For completely synchronous writes, which we use with immediate writes
+ * (i.e. iscsi). It will hold INT_MAX/(PCS_MAX_SYNC_TIMEOUT/HZ)
+ * iops = ~50kiops, this should be enough: such writes are expensive and cannot
+ * be optimized.
+ */
+#define PCS_MAX_SYNC_TIMEOUT (12*3600*HZ)
+
+/* For normal dirtying writes the limit can be made a lot weaker, the only
+ * severe limitation is that PCS_MAX_DIRTY_TIMEOUT was much greater than
+ * PCS_SYNC_TIMEOUT. Factor 100 protects against ~1Miops.
+ */
+#define PCS_MAX_DIRTY_TIMEOUT (100*PCS_SYNC_TIMEOUT)
+
#define PCS_REPLICATION_BLACKLIST_TIMEOUT HZ
@@ -76,6 +91,7 @@ struct pcs_cs_record
{
struct pcs_cs_info info;
struct cs_sync_state sync;
+ abs_time_t dirty_ts;
struct pcs_cs_link cslink;
};
More information about the Devel
mailing list