From 9b4ac643120884c8c7c9eedb53144b72f4465d1f Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Fri, 10 Sep 2021 15:00:27 -0700 Subject: [PATCH 01/26] Consistently stop fencing as server stops As the server comes up it needs to fence any previous servers before it assumes exclusive access to the device. If fencing fails it can leave fence requests behind. The error path for these very early failures didn't shut down fencing so we'd have lingering fence requests span the life cycle of server startup and shutdown. The next time the server starts up in this mount it can try to create the fence request again, get an error because a lingering one already exists, and immediately shut down. The result is that fencing errors that hit that initial attempt during server startup can become persistent fencing errors for the lifetime of that mount, preventing it from every successfully starting the server. Moving the fence stop call to hit all exiting error paths consistently clean up fence requests and avoid this problem. The next server instance will get a chance to process the fence request again. It might well hit the same error, but at least it gets a chance. Signed-off-by: Zach Brown --- kmod/src/server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kmod/src/server.c b/kmod/src/server.c index d5374f4a..8624e5e1 100644 --- a/kmod/src/server.c +++ b/kmod/src/server.c @@ -3942,11 +3942,11 @@ shutdown: /* wait for extra queues by requests, won't find waiters */ flush_work(&server->commit_work); - scoutfs_fence_stop(sb); scoutfs_lock_server_destroy(sb); scoutfs_omap_server_shutdown(sb); out: + scoutfs_fence_stop(sb); scoutfs_net_free_conn(sb, conn); /* let quorum know that we've shutdown */ From d255dd3b32e34baef39d3055a81a90a88eb2cc40 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Tue, 14 Sep 2021 10:20:34 -0700 Subject: [PATCH 02/26] Fix SCOUTFs typo in totl name nr define Signed-off-by: Zach Brown --- kmod/src/ioctl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kmod/src/ioctl.h b/kmod/src/ioctl.h index 8b4decf0..f0f0037d 100644 --- a/kmod/src/ioctl.h +++ b/kmod/src/ioctl.h @@ -490,7 +490,7 @@ struct scoutfs_ioctl_resize_devices { #define SCOUTFS_IOC_RESIZE_DEVICES \ _IOR(SCOUTFS_IOCTL_MAGIC, 14, struct scoutfs_ioctl_resize_devices) -#define SCOUTFs_IOCTL_XATTR_TOTAL_NAME_NR 3 +#define SCOUTFS_IOCTL_XATTR_TOTAL_NAME_NR 3 /* * Copy global totals of .totl. xattr value payloads to the user. This @@ -521,7 +521,7 @@ struct scoutfs_ioctl_resize_devices { * for a single struct entry. */ struct scoutfs_ioctl_read_xattr_totals { - __u64 pos_name[SCOUTFs_IOCTL_XATTR_TOTAL_NAME_NR]; + __u64 pos_name[SCOUTFS_IOCTL_XATTR_TOTAL_NAME_NR]; __u64 totals_ptr; __u64 totals_bytes; }; @@ -533,7 +533,7 @@ struct scoutfs_ioctl_read_xattr_totals { * the total. */ struct scoutfs_ioctl_xattr_total { - __u64 name[SCOUTFs_IOCTL_XATTR_TOTAL_NAME_NR]; + __u64 name[SCOUTFS_IOCTL_XATTR_TOTAL_NAME_NR]; __u64 total; __u64 count; }; From 0c95388f3bb4be9a03e03f3767658d08f223270f Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 15 Sep 2021 13:51:27 -0700 Subject: [PATCH 03/26] Set TCP_USER_TIMEOUT in addition to keepalives TCP keepalive probes only work when the connection is idle. They're not sent when there's unacked send data being retramnsmitted. If the server fails while we're retransmitting we don't break the connection and try to elect and connect to a new server until the very long default conneciton timeouts or the server comes back and the stale connection is aborted. We can set TCP_USER_TIMEOUT to break an unresponsive connection when there's written data. It changes the behavior of the keepalive probes so we rework them a bit to clearly apply our timeout consistently between the two mechanisms. Signed-off-by: Zach Brown --- kmod/src/net.c | 46 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/kmod/src/net.c b/kmod/src/net.c index 8368f49b..49ea837e 100644 --- a/kmod/src/net.c +++ b/kmod/src/net.c @@ -880,13 +880,31 @@ static void destroy_conn(struct scoutfs_net_connection *conn) } /* - * Have a pretty aggressive keepalive timeout of around 10 seconds. The - * TCP keepalives are being processed out of task context so they should - * be responsive even when mounts are under load. + * By default, TCP would maintain a connection to an unresponsive peer + * for a very long time indeed. We can't do that because quorum + * members will only participate in an election when they don't have a + * healthy connection to a server. We use the KEEPALIVE* and + * TCP_USER_TIMEOUT options to ensure that we'll break an unresponsive + * connection and return to the quorum and client connection paths to + * try and establish a new connection to an active server. + * + * The TCP_KEEP* and TCP_USER_TIMEOUT option interaction is subtle. + * TCP_USER_TIMEOUT only applies if there is unacked written data in the + * send queue. It doesn't work if the connection is idle. Adding + * keepalice probes with user_timeout set changes how the keepalive + * timeout is calculated. CNT no longer matters. Each time + * additional probes (not the first) are sent the user timeout is + * checked against the last time data was received. If none of the + * keepalives are responded to then eventually the user timeout applies. + * + * Given all this, we start with the overall unresponsive timeout. Then + * we set the probes to start sending towards the end of the timeout. + * We give it a few tries for a successful response before the timeout + * elapses during the probe timer processing after the unsuccessful + * probes. */ -#define KEEPCNT 3 -#define KEEPIDLE 7 -#define KEEPINTVL 1 +#define UNRESPONSIVE_TIMEOUT_SECS 10 +#define UNRESPONSIVE_PROBES 3 static int sock_opts_and_names(struct scoutfs_net_connection *conn, struct socket *sock) { @@ -895,7 +913,7 @@ static int sock_opts_and_names(struct scoutfs_net_connection *conn, int optval; int ret; - /* but use a keepalive timeout instead of send timeout */ + /* we use a keepalive timeout instead of send timeout */ tv.tv_sec = 0; tv.tv_usec = 0; ret = kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, @@ -903,24 +921,32 @@ static int sock_opts_and_names(struct scoutfs_net_connection *conn, if (ret) goto out; - optval = KEEPCNT; + /* not checked when user_timeout != 0, but for clarity */ + optval = UNRESPONSIVE_PROBES; ret = kernel_setsockopt(sock, SOL_TCP, TCP_KEEPCNT, (char *)&optval, sizeof(optval)); if (ret) goto out; - optval = KEEPIDLE; + BUILD_BUG_ON(UNRESPONSIVE_PROBES >= UNRESPONSIVE_TIMEOUT_SECS); + optval = UNRESPONSIVE_TIMEOUT_SECS - (UNRESPONSIVE_PROBES); ret = kernel_setsockopt(sock, SOL_TCP, TCP_KEEPIDLE, (char *)&optval, sizeof(optval)); if (ret) goto out; - optval = KEEPINTVL; + optval = 1; ret = kernel_setsockopt(sock, SOL_TCP, TCP_KEEPINTVL, (char *)&optval, sizeof(optval)); if (ret) goto out; + optval = UNRESPONSIVE_TIMEOUT_SECS * MSEC_PER_SEC; + ret = kernel_setsockopt(sock, SOL_TCP, TCP_USER_TIMEOUT, + (char *)&optval, sizeof(optval)); + if (ret) + goto out; + optval = 1; ret = kernel_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, (char *)&optval, sizeof(optval)); From 4e00f9501419167fd63906095e5d07d066b34a17 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 15 Sep 2021 14:03:26 -0700 Subject: [PATCH 04/26] run-tests builds our targets with -j The test harness might as well use all cpus when building. It's reasonably safe to assume both that the test systems are otherwise idle and that the build is likely to succeed. Signed-off-by: Zach Brown --- tests/run-tests.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/run-tests.sh b/tests/run-tests.sh index d60865c1..76e1eee2 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -254,17 +254,20 @@ test -e "$T_RESULTS" || mkdir -p "$T_RESULTS" test -d "$T_RESULTS" || \ die "$T_RESULTS dir is not a directory" +# might as well build our stuff with all cpus, assuming idle system +MAKE_ARGS="-j $(getconf _NPROCESSORS_ONLN)" + # build kernel module msg "building kmod/ dir $T_KMOD" cmd cd "$T_KMOD" -cmd make +cmd make $MAKE_ARGS cmd sync cmd cd - # build utils msg "building utils/ dir $T_UTILS" cmd cd "$T_UTILS" -cmd make +cmd make $MAKE_ARGS cmd sync cmd cd - @@ -281,7 +284,7 @@ fi # building our test binaries msg "building test binaries" -cmd make +cmd make $MAKE_ARGS # set any options implied by others test -n "$T_MKFS" && T_UNMOUNT=1 From a0690070ae8222bb2719718886689c381536dd18 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 20 Sep 2021 10:26:38 -0700 Subject: [PATCH 05/26] Don't null terminate our note strings The code that shows the note sections as files uses the section size to define the size of the notes payload. We don't need to null terminate the strings to define their lengths. Doing so puts a null in the notes file which isn't appreciated by many readers. Signed-off-by: Zach Brown --- kmod/src/super.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kmod/src/super.c b/kmod/src/super.c index b6c1ea88..1fb8c209 100644 --- a/kmod/src/super.c +++ b/kmod/src/super.c @@ -714,11 +714,11 @@ static int __init scoutfs_module_init(void) */ __asm__ __volatile__ ( ".section .note.git_describe,\"a\"\n" - ".string \""SCOUTFS_GIT_DESCRIBE"\\n\"\n" + ".ascii \""SCOUTFS_GIT_DESCRIBE"\\n\"\n" ".previous\n"); __asm__ __volatile__ ( ".section .note.scoutfs_interop_version,\"a\"\n" - ".string \""SCOUTFS_INTEROP_VERSION_STR"\\n\"\n" + ".ascii \""SCOUTFS_INTEROP_VERSION_STR"\\n\"\n" ".previous\n"); scoutfs_init_counters(); From 024426df28e749ac4e1a94f216b5e52adab32c57 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 27 Sep 2021 14:34:12 -0700 Subject: [PATCH 06/26] Add a file for userspace quorum config helpers Signed-off-by: Zach Brown --- utils/src/quorum.c | 10 ++++++++++ utils/src/quorum.h | 8 ++++++++ 2 files changed, 18 insertions(+) create mode 100644 utils/src/quorum.c create mode 100644 utils/src/quorum.h diff --git a/utils/src/quorum.c b/utils/src/quorum.c new file mode 100644 index 00000000..092c4f37 --- /dev/null +++ b/utils/src/quorum.c @@ -0,0 +1,10 @@ +#include "sparse.h" +#include "util.h" +#include "format.h" + +#include "quorum.h" + +bool quorum_slot_present(struct scoutfs_super_block *super, int i) +{ + return super->qconf.slots[i].addr.v4.family == cpu_to_le16(SCOUTFS_AF_IPV4); +} diff --git a/utils/src/quorum.h b/utils/src/quorum.h new file mode 100644 index 00000000..1297dce1 --- /dev/null +++ b/utils/src/quorum.h @@ -0,0 +1,8 @@ +#ifndef _QUORUM_H_ +#define _QUORUM_H_ + +#include + +bool quorum_slot_present(struct scoutfs_super_block *super, int i); + +#endif From 1cdcf41ac72607ac72cefbb754aca7287e20b377 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 27 Sep 2021 14:35:16 -0700 Subject: [PATCH 07/26] Move more block read/write functions to util We're adding another command that does block IO so move some block reading and writing functions out of mkfs. We also grow a few function variants and call the write_sync variant from mkfs instead of having it manually sync. Signed-off-by: Zach Brown --- utils/src/mkfs.c | 50 +++--------------------- utils/src/util.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++ utils/src/util.h | 8 ++++ 3 files changed, 112 insertions(+), 44 deletions(-) diff --git a/utils/src/mkfs.c b/utils/src/mkfs.c index 6d2213e6..69999c16 100644 --- a/utils/src/mkfs.c +++ b/utils/src/mkfs.c @@ -32,30 +32,6 @@ #include "leaf_item_hash.h" #include "blkid.h" -/* - * Update the block header fields and write out the block. - */ -static int write_block(int fd, u32 magic, __le64 fsid, u64 seq, u64 blkno, - int shift, struct scoutfs_block_header *hdr) -{ - size_t size = 1ULL << shift; - ssize_t ret; - - hdr->magic = cpu_to_le32(magic); - hdr->fsid = fsid; - hdr->blkno = cpu_to_le64(blkno); - hdr->seq = cpu_to_le64(seq); - hdr->crc = cpu_to_le32(crc_block(hdr, size)); - - ret = pwrite(fd, hdr, size, blkno << shift); - if (ret != size) { - fprintf(stderr, "write to blkno %llu returned %zd: %s (%d)\n", - blkno, ret, strerror(errno), errno); - return -errno; - } - - return 0; -} /* * Return the order of the length of a free extent, which we define as @@ -356,33 +332,19 @@ static int do_mkfs(struct mkfs_args *args) } /* write the super block to data dev and meta dev*/ - ret = write_block(data_fd, SCOUTFS_BLOCK_MAGIC_SUPER, fsid, 1, - SCOUTFS_SUPER_BLKNO, SCOUTFS_BLOCK_SM_SHIFT, - &super->hdr); + ret = write_block_sync(data_fd, SCOUTFS_BLOCK_MAGIC_SUPER, fsid, 1, + SCOUTFS_SUPER_BLKNO, SCOUTFS_BLOCK_SM_SHIFT, + &super->hdr); if (ret) goto out; - if (fsync(data_fd)) { - ret = -errno; - fprintf(stderr, "failed to fsync '%s': %s (%d)\n", - args->data_device, strerror(errno), errno); - goto out; - } - super->flags |= cpu_to_le64(SCOUTFS_FLAG_IS_META_BDEV); - ret = write_block(meta_fd, SCOUTFS_BLOCK_MAGIC_SUPER, fsid, - 1, SCOUTFS_SUPER_BLKNO, SCOUTFS_BLOCK_SM_SHIFT, - &super->hdr); + ret = write_block_sync(meta_fd, SCOUTFS_BLOCK_MAGIC_SUPER, fsid, + 1, SCOUTFS_SUPER_BLKNO, SCOUTFS_BLOCK_SM_SHIFT, + &super->hdr); if (ret) goto out; - if (fsync(meta_fd)) { - ret = -errno; - fprintf(stderr, "failed to fsync '%s': %s (%d)\n", - args->meta_device, strerror(errno), errno); - goto out; - } - uuid_unparse(super->uuid, uuid_str); printf("Created scoutfs filesystem:\n" diff --git a/utils/src/util.c b/utils/src/util.c index 9c3aa6ef..53a45ed2 100644 --- a/utils/src/util.c +++ b/utils/src/util.c @@ -10,6 +10,8 @@ #include #include "util.h" +#include "format.h" +#include "crc.h" #define ENV_PATH "SCOUTFS_MOUNT_PATH" @@ -98,3 +100,99 @@ int read_block(int fd, u64 blkno, int shift, void **ret_val) return 0; } } + +int read_block_crc(int fd, u64 blkno, int shift, void **ret_val) +{ + struct scoutfs_block_header *hdr; + size_t size = 1ULL << shift; + int ret; + u32 crc; + + ret = read_block(fd, blkno, shift, ret_val); + if (ret == 0) { + hdr = *ret_val; + crc = crc_block(hdr, size); + if (crc != le32_to_cpu(hdr->crc)) { + fprintf(stderr, "crc of read blkno %llu failed, stored %08x != calculated %08x\n", + blkno, le32_to_cpu(hdr->crc), crc); + free(*ret_val); + *ret_val = NULL; + ret = -EIO; + } + } + + return ret; +} + +int read_block_verify(int fd, u32 magic, u64 fsid, u64 blkno, int shift, void **ret_val) +{ + struct scoutfs_block_header *hdr = NULL; + int ret; + + ret = read_block_crc(fd, blkno, shift, ret_val); + if (ret == 0) { + hdr = *ret_val; + ret = -EIO; + if (le32_to_cpu(hdr->magic) != magic) + fprintf(stderr, "read blkno %llu has bad magic %08x != expected %08x\n", + blkno, le32_to_cpu(hdr->magic), magic); + else if (fsid != 0 && le32_to_cpu(hdr->fsid) != fsid) + fprintf(stderr, "read blkno %llu has bad fsid %016llx != expected %016llx\n", + blkno, le64_to_cpu(hdr->fsid), fsid); + else if (le32_to_cpu(hdr->blkno) != blkno) + fprintf(stderr, "read blkno %llu has bad blkno %llu != expected %llu\n", + blkno, le64_to_cpu(hdr->blkno), blkno); + else + ret = 0; + + if (ret < 0) { + free(*ret_val); + *ret_val = NULL; + } + } + + + return ret; +} + +/* + * Update the block header fields and write out the block. + */ +int write_block(int fd, u32 magic, __le64 fsid, u64 seq, u64 blkno, + int shift, struct scoutfs_block_header *hdr) +{ + size_t size = 1ULL << shift; + ssize_t ret; + + hdr->magic = cpu_to_le32(magic); + hdr->fsid = fsid; + hdr->blkno = cpu_to_le64(blkno); + hdr->seq = cpu_to_le64(seq); + hdr->crc = cpu_to_le32(crc_block(hdr, size)); + + ret = pwrite(fd, hdr, size, blkno << shift); + if (ret != size) { + fprintf(stderr, "write to blkno %llu returned %zd: %s (%d)\n", + blkno, ret, strerror(errno), errno); + return -errno; + } + + return 0; +} + +int write_block_sync(int fd, u32 magic, __le64 fsid, u64 seq, u64 blkno, + int shift, struct scoutfs_block_header *hdr) +{ + int ret = write_block(fd, magic, fsid, seq, blkno, shift, hdr); + if (ret != 0) + return ret; + + if (fsync(fd)) { + ret = -errno; + fprintf(stderr, "fsync after write to blkno %llu failed: %s (%d)\n", + blkno, strerror(errno), errno); + return ret; + } + + return 0; +} diff --git a/utils/src/util.h b/utils/src/util.h index b504a5c0..3b05e1f1 100644 --- a/utils/src/util.h +++ b/utils/src/util.h @@ -113,6 +113,14 @@ static inline int memcmp_lens(const void *a, int a_len, int get_path(char *path, int flags); int read_block(int fd, u64 blkno, int shift, void **ret_val); +int read_block_crc(int fd, u64 blkno, int shift, void **ret_val); +int read_block_verify(int fd, u32 magic, u64 fsid, u64 blkno, int shift, void **ret_val); + +struct scoutfs_block_header; +int write_block(int fd, u32 magic, __le64 fsid, u64 seq, u64 blkno, + int shift, struct scoutfs_block_header *hdr); +int write_block_sync(int fd, u32 magic, __le64 fsid, u64 seq, u64 blkno, + int shift, struct scoutfs_block_header *hdr); #define __stringify_1(x) #x #define __stringify(x) __stringify_1(x) From ac2587017e1d5cf939a1589395007a9e37496cc8 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Tue, 28 Sep 2021 13:59:55 -0700 Subject: [PATCH 08/26] Add write_nr to quorum blocks Add a write_nr field to the quorum block header which is incremented with every write. Each event also gets a write_nr field that is set to the incremented value from the header. This gives us a history of the order of event updates that isn't sensitive to misconfigured time. Signed-off-by: Zach Brown --- kmod/src/format.h | 2 ++ kmod/src/quorum.c | 2 ++ utils/src/print.c | 6 ++++-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/kmod/src/format.h b/kmod/src/format.h index cb26ed15..b9275eda 100644 --- a/kmod/src/format.h +++ b/kmod/src/format.h @@ -733,7 +733,9 @@ enum { struct scoutfs_quorum_block { struct scoutfs_block_header hdr; + __le64 write_nr; struct scoutfs_quorum_block_event { + __le64 write_nr; __le64 rid; __le64 term; struct scoutfs_timespec ts; diff --git a/kmod/src/quorum.c b/kmod/src/quorum.c index fbaa31e4..34922d78 100644 --- a/kmod/src/quorum.c +++ b/kmod/src/quorum.c @@ -448,8 +448,10 @@ static void set_quorum_block_event(struct super_block *sb, struct scoutfs_quorum return; getnstimeofday64(&ts); + le64_add_cpu(&blk->write_nr, 1); ev = &blk->events[event]; + ev->write_nr = blk->write_nr; ev->rid = cpu_to_le64(sbi->rid); ev->term = cpu_to_le64(term); ev->ts.sec = cpu_to_le64(ts.tv_sec); diff --git a/utils/src/print.c b/utils/src/print.c index b8717048..bb566730 100644 --- a/utils/src/print.c +++ b/utils/src/print.c @@ -898,13 +898,15 @@ static int print_quorum_blocks(int fd, struct scoutfs_super_block *super) printf("quorum blkno %llu (slot %llu)\n", blkno, blkno - SCOUTFS_QUORUM_BLKNO); print_block_header(&blk->hdr, SCOUTFS_BLOCK_SM_SIZE); + printf(" write_nr %llu\n", le64_to_cpu(blk->write_nr)); for (e = 0; e < array_size(event_names); e++) { ev = &blk->events[e]; - printf(" %12s: rid %016llx term %llu ts %llu.%08u\n", + printf(" %12s: rid %016llx term %llu write_nr %llu ts %llu.%08u\n", event_names[e], le64_to_cpu(ev->rid), le64_to_cpu(ev->term), - le64_to_cpu(ev->ts.sec), le32_to_cpu(ev->ts.nsec)); + le64_to_cpu(ev->write_nr), le64_to_cpu(ev->ts.sec), + le32_to_cpu(ev->ts.nsec)); } } From 366f615c9fa98ee60760f615c045218a69f37dae Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 27 Sep 2021 14:39:39 -0700 Subject: [PATCH 09/26] Add support for our format version We had previously started on a relatively simple notion of an interoperability version which wasn't quite right. This fleshes out support for a more functional format version. The super blocks have a single version that defines behaviour of the running system. The code supports a range of versions and we add some initial interfaces for updating the version while the system is offline. All of this together should let us safely change the underlying format over time. Signed-off-by: Zach Brown --- kmod/src/client.c | 17 +- kmod/src/format.h | 15 +- kmod/src/quorum.c | 2 +- kmod/src/server.c | 16 +- kmod/src/super.c | 38 +++- kmod/src/super.h | 1 + kmod/src/sysfs.c | 11 ++ utils/man/scoutfs.5 | 81 ++++++++- utils/man/scoutfs.8 | 37 +++- utils/src/change_format_version.c | 287 ++++++++++++++++++++++++++++++ utils/src/cmd.c | 4 + utils/src/mkfs.c | 22 ++- utils/src/print.c | 4 +- utils/src/util.c | 13 +- 14 files changed, 507 insertions(+), 41 deletions(-) create mode 100644 utils/src/change_format_version.c diff --git a/kmod/src/client.c b/kmod/src/client.c index 4ddc54fb..acf9412f 100644 --- a/kmod/src/client.c +++ b/kmod/src/client.c @@ -361,7 +361,8 @@ static int client_greeting(struct super_block *sb, void *resp, unsigned int resp_len, int error, void *data) { - struct client_info *client = SCOUTFS_SB(sb)->client_info; + struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); + struct client_info *client = sbi->client_info; struct scoutfs_super_block *super = &SCOUTFS_SB(sb)->super; struct scoutfs_net_greeting *gr = resp; bool new_server; @@ -378,17 +379,15 @@ static int client_greeting(struct super_block *sb, } if (gr->fsid != super->hdr.fsid) { - scoutfs_warn(sb, "server sent fsid 0x%llx, client has 0x%llx", - le64_to_cpu(gr->fsid), - le64_to_cpu(super->hdr.fsid)); + scoutfs_warn(sb, "server greeting response fsid 0x%llx did not match client fsid 0x%llx", + le64_to_cpu(gr->fsid), le64_to_cpu(super->hdr.fsid)); ret = -EINVAL; goto out; } - if (gr->version != super->version) { - scoutfs_warn(sb, "server sent format 0x%llx, client has 0x%llx", - le64_to_cpu(gr->version), - le64_to_cpu(super->version)); + if (le64_to_cpu(gr->fmt_vers) != sbi->fmt_vers) { + scoutfs_warn(sb, "server greeting response format version %llu did not match client format version %llu", + le64_to_cpu(gr->fmt_vers), sbi->fmt_vers); ret = -EINVAL; goto out; } @@ -514,7 +513,7 @@ static void scoutfs_client_connect_worker(struct work_struct *work) /* send a greeting to verify endpoints of each connection */ greet.fsid = super->hdr.fsid; - greet.version = super->version; + greet.fmt_vers = cpu_to_le64(sbi->fmt_vers); greet.server_term = cpu_to_le64(client->server_term); greet.rid = cpu_to_le64(sbi->rid); greet.flags = 0; diff --git a/kmod/src/format.h b/kmod/src/format.h index b9275eda..3f20db33 100644 --- a/kmod/src/format.h +++ b/kmod/src/format.h @@ -1,8 +1,15 @@ #ifndef _SCOUTFS_FORMAT_H_ #define _SCOUTFS_FORMAT_H_ -#define SCOUTFS_INTEROP_VERSION 0ULL -#define SCOUTFS_INTEROP_VERSION_STR __stringify(0) +/* + * The format version defines the format of structures on devices, + * structures that are communicated over the wire, and the protocol + * behind the structures. + */ +#define SCOUTFS_FORMAT_VERSION_MIN 0 +#define SCOUTFS_FORMAT_VERSION_MIN_STR __stringify(SCOUTFS_FORMAT_VERSION_MIN) +#define SCOUTFS_FORMAT_VERSION_MAX 0 +#define SCOUTFS_FORMAT_VERSION_MAX_STR __stringify(SCOUTFS_FORMAT_VERSION_MAX) /* statfs(2) f_type */ #define SCOUTFS_SUPER_MAGIC 0x554f4353 /* "SCOU" */ @@ -783,7 +790,7 @@ struct scoutfs_volume_options { struct scoutfs_super_block { struct scoutfs_block_header hdr; __le64 id; - __le64 version; + __le64 fmt_vers; __le64 flags; __u8 uuid[SCOUTFS_UUID_BYTES]; __le64 seq; @@ -928,7 +935,7 @@ enum scoutfs_dentry_type { */ struct scoutfs_net_greeting { __le64 fsid; - __le64 version; + __le64 fmt_vers; __le64 server_term; __le64 rid; __le64 flags; diff --git a/kmod/src/quorum.c b/kmod/src/quorum.c index 34922d78..f8b547d2 100644 --- a/kmod/src/quorum.c +++ b/kmod/src/quorum.c @@ -828,7 +828,7 @@ static void scoutfs_quorum_worker(struct work_struct *work) qst.term); } - /* informational event that we're shutting down, nothing relies on it */ + /* record that this slot no longer has an active quorum */ update_quorum_block(sb, SCOUTFS_QUORUM_EVENT_END, qst.term, true); out: if (ret < 0) { diff --git a/kmod/src/server.c b/kmod/src/server.c index 8624e5e1..2ca229a6 100644 --- a/kmod/src/server.c +++ b/kmod/src/server.c @@ -3199,7 +3199,8 @@ static int server_greeting(struct super_block *sb, struct scoutfs_net_connection *conn, u8 cmd, u64 id, void *arg, u16 arg_len) { - struct scoutfs_super_block *super = &SCOUTFS_SB(sb)->super; + struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); + struct scoutfs_super_block *super = &sbi->super; struct scoutfs_net_greeting *gr = arg; struct scoutfs_net_greeting greet; DECLARE_SERVER_INFO(sb, server); @@ -3215,17 +3216,16 @@ static int server_greeting(struct super_block *sb, } if (gr->fsid != super->hdr.fsid) { - scoutfs_warn(sb, "client sent fsid 0x%llx, server has 0x%llx", - le64_to_cpu(gr->fsid), + scoutfs_warn(sb, "client rid %016llx greeting fsid 0x%llx did not match server fsid 0x%llx", + le64_to_cpu(gr->rid), le64_to_cpu(gr->fsid), le64_to_cpu(super->hdr.fsid)); ret = -EINVAL; goto send_err; } - if (gr->version != super->version) { - scoutfs_warn(sb, "client sent format 0x%llx, server has 0x%llx", - le64_to_cpu(gr->version), - le64_to_cpu(super->version)); + if (le64_to_cpu(gr->fmt_vers) != sbi->fmt_vers) { + scoutfs_warn(sb, "client rid %016llx greeting format version %llu did not match server format version %llu", + le64_to_cpu(gr->rid), le64_to_cpu(gr->fmt_vers), sbi->fmt_vers); ret = -EINVAL; goto send_err; } @@ -3249,7 +3249,7 @@ send_err: err = ret; greet.fsid = super->hdr.fsid; - greet.version = super->version; + greet.fmt_vers = cpu_to_le64(sbi->fmt_vers); greet.server_term = cpu_to_le64(server->term); greet.rid = gr->rid; greet.flags = 0; diff --git a/kmod/src/super.c b/kmod/src/super.c index 1fb8c209..612c51fa 100644 --- a/kmod/src/super.c +++ b/kmod/src/super.c @@ -403,11 +403,22 @@ static int scoutfs_read_super_from_bdev(struct super_block *sb, goto out; } + if (le64_to_cpu(super->fmt_vers) < SCOUTFS_FORMAT_VERSION_MIN || + le64_to_cpu(super->fmt_vers) > SCOUTFS_FORMAT_VERSION_MAX) { + scoutfs_err(sb, "super block has format version %llu outside of supported version range %u-%u", + le64_to_cpu(super->fmt_vers), SCOUTFS_FORMAT_VERSION_MIN, + SCOUTFS_FORMAT_VERSION_MAX); + ret = -EINVAL; + goto out; + } - if (super->version != cpu_to_le64(SCOUTFS_INTEROP_VERSION)) { - scoutfs_err(sb, "super block has invalid version %llu, expected %llu", - le64_to_cpu(super->version), - SCOUTFS_INTEROP_VERSION); + /* + * fill_supers checks the fmt_vers in both supers and then decides to use it. + * From then on we verify that the supers we read have that version. + */ + if (sbi->fmt_vers != 0 && le64_to_cpu(super->fmt_vers) != sbi->fmt_vers) { + scoutfs_err(sb, "super block has format version %llu than %llu read at mount", + le64_to_cpu(super->fmt_vers), sbi->fmt_vers); ret = -EINVAL; goto out; } @@ -524,6 +535,14 @@ static int scoutfs_read_supers(struct super_block *sb) goto out; } + if (le64_to_cpu(meta_super->fmt_vers) != le64_to_cpu(data_super->fmt_vers)) { + scoutfs_err(sb, "meta device format version %llu != data device format version %llu", + le64_to_cpu(meta_super->fmt_vers), le64_to_cpu(data_super->fmt_vers)); + goto out; + } + + + sbi->fmt_vers = le64_to_cpu(meta_super->fmt_vers); sbi->super = *meta_super; out: kfree(meta_super); @@ -717,8 +736,12 @@ static int __init scoutfs_module_init(void) ".ascii \""SCOUTFS_GIT_DESCRIBE"\\n\"\n" ".previous\n"); __asm__ __volatile__ ( - ".section .note.scoutfs_interop_version,\"a\"\n" - ".ascii \""SCOUTFS_INTEROP_VERSION_STR"\\n\"\n" + ".section .note.scoutfs_format_version_min,\"a\"\n" + ".ascii \""SCOUTFS_FORMAT_VERSION_MIN_STR"\\n\"\n" + ".previous\n"); + __asm__ __volatile__ ( + ".section .note.scoutfs_format_version_max,\"a\"\n" + ".ascii \""SCOUTFS_FORMAT_VERSION_MAX_STR"\\n\"\n" ".previous\n"); scoutfs_init_counters(); @@ -752,4 +775,5 @@ module_exit(scoutfs_module_exit) MODULE_AUTHOR("Zach Brown "); MODULE_LICENSE("GPL"); MODULE_INFO(git_describe, SCOUTFS_GIT_DESCRIBE); -MODULE_INFO(scoutfs_interop_version, SCOUTFS_INTEROP_VERSION_STR); +MODULE_INFO(scoutfs_format_version_min, SCOUTFS_FORMAT_VERSION_MIN_STR); +MODULE_INFO(scoutfs_format_version_max, SCOUTFS_FORMAT_VERSION_MAX_STR); diff --git a/kmod/src/super.h b/kmod/src/super.h index 92106b63..32fba8d6 100644 --- a/kmod/src/super.h +++ b/kmod/src/super.h @@ -36,6 +36,7 @@ struct scoutfs_sb_info { /* assigned once at the start of each mount, read-only */ u64 rid; + u64 fmt_vers; struct scoutfs_super_block super; diff --git a/kmod/src/sysfs.c b/kmod/src/sysfs.c index 5f2f024c..9bf19e48 100644 --- a/kmod/src/sysfs.c +++ b/kmod/src/sysfs.c @@ -37,6 +37,16 @@ struct attr_funcs { #define ATTR_FUNCS_RO(_name) \ static struct attr_funcs _name##_attr_funcs = __ATTR_RO(_name) +static ssize_t format_version_show(struct kobject *kobj, struct attribute *attr, + char *buf) +{ + struct super_block *sb = KOBJ_TO_SB(kobj, sb_id_kobj); + struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); + + return snprintf(buf, PAGE_SIZE, "%llu\n", sbi->fmt_vers); +} +ATTR_FUNCS_RO(format_version); + static ssize_t fsid_show(struct kobject *kobj, struct attribute *attr, char *buf) { @@ -91,6 +101,7 @@ static ssize_t attr_funcs_show(struct kobject *kobj, struct attribute *attr, static struct attribute *sb_id_attrs[] = { + &format_version_attr_funcs.attr, &fsid_attr_funcs.attr, &rid_attr_funcs.attr, NULL, diff --git a/utils/man/scoutfs.5 b/utils/man/scoutfs.5 index 63962735..a9303c9e 100644 --- a/utils/man/scoutfs.5 +++ b/utils/man/scoutfs.5 @@ -198,7 +198,86 @@ with the .IB READ_XATTR_TOTALS ioctl. .RE - + +.SH FORMAT VERSION +The format version defines the layout and use of structures stored on +devices and passed over the network. The version is incremented for +every change in structures that is not backwards compatible with +previous versions. A single version implies all changes, individual +changes can't be selectively adopted. +.sp +As a new file system is created the format version is stored in both of +the super blocks written to the metadata and data devices. By default +the greatest supported version is written while an older supported +version may be specified. +.sp +During mount the kernel module verifies that the format versions stored +in both of the super blocks match and are supported. That version +defines the set of features and behavior of all the mounts using the +file system, including the network protocol that is communicated over +the wire. +.sp +Any combination of software release versions that support the current +format version of the file system can safely be used concurrently. This +allows for rolling software updates of multiple mounts using a shared +file system. +.sp +To use new incompatible features added in newer format versions the super blocks must +be updated. This can currently only be safely performed on a +completely and cleanly unmounted file system. The +.BR scoutfs (8) +.I change-format-version +command can be used with the +.I --offline +option to write a newer supported version into the super blocks. It +will fail if it sees any indication of unresolved mounts that may be +using the devices: either active quorum members working with their +quorum blocks or persistent records of mounted clients that haven't been +resolved. Like creating a new file system, there is no protection +against multiple invocations of the change command corrupting the +system. Once the version is updated older software can no longer use +the file system so this change should be performed with care. Once the +newer format version is successfully written it can be mounted and newer +features can be used. +.sp +Each layer of the system can show its supported format versions: +.RS +.TP +.B Userspace utilities +.B scoutfs --help +includes the range of supported format versions for a given release +of the userspace utilities. +.TP +.B Kernel module +.I modinfo MODULE +shows the range of supproted versions for a kernel module file in the +.I scoutfs_format_version_min +and +.I scoutfs_format_version_min +fields. +.TP +.B Inserted module +The supported version range of an inserted module can be found in +.I .note.scoutfs_format_version_min +and +.I .note.scoutfs_format_version_max +notes files in the sysfs notes directory for the inserted module, +typically +.I /sys/module/scoutfs/notes/ +.TP +.B Metadata and data devices +.I scoutfs print DEVICE +shows the +.I fmt_vers +field in the initial output of the super block on the device. +.TP +.B Mounted filesystem +The version that a mount is using is shown in the +.I format_version +file in the mount's sysfs directory, typically +.I /sys/fs/scoutfs/f.FSID.r.RID/ +.RE + .SH CORRUPTION DETECTION A .B scoutfs diff --git a/utils/man/scoutfs.8 b/utils/man/scoutfs.8 index 25ee53c7..5c36b4f8 100644 --- a/utils/man/scoutfs.8 +++ b/utils/man/scoutfs.8 @@ -14,6 +14,34 @@ option will, when the option is omitted, fall back to using the value of the environment variable. If that variable is also absent the current working directory will be used. +.TP +.BI "change-format-version [-V, --format-version VERS] [-F|--offline META-DEVICE DATA-DEVICE]" +.sp +Change the format version of an existing file system. The maxmimum +supported version is used by default. A specific version in the range +can be specified. The range of supported versions in shown in the +output of --help. +.RS 1.0i +.PD 0 +.TP +.sp +.B "-F, --offline META-DEVICE DATA-DEVICE" +Change the format version by writing directly to the metadata and data +devices. Like mkfs, this writes directly to the devices without +protection and must only be used on completely unmounted devices. The +command will fail if it sees evidence of active quorum use of the device +or of previously connected clients which haven't been reclaimed. The +only way to avoid these checks is to fully mount and cleanly unmount the +file system. +.sp +This is not an atomic operation because it writes to blocks on two +devices. Write failure can result in the versions becoming out of sync +which will prevent the system from mouting. To recover the error must +be resolved so the command can be repeated and successfully write to +the super blocks on both devices. +.RE +.PD + .TP .BI "df [-h|--human-readable] [-p|--path PATH]" .sp @@ -32,7 +60,7 @@ A path within a ScoutFS filesystem. .PD .TP -.BI "mkfs META-DEVICE DATA-DEVICE {-Q|--quorum-slot} NR,ADDR,PORT [-m|--max-meta-size SIZE] [-d|--max-data-size SIZE] [-z|--data-alloc-zone-blocks BLOCKS] [-f|--force] [-A|--allow-small-size]" +.BI "mkfs META-DEVICE DATA-DEVICE {-Q|--quorum-slot} NR,ADDR,PORT [-m|--max-meta-size SIZE] [-d|--max-data-size SIZE] [-z|--data-alloc-zone-blocks BLOCKS] [-f|--force] [-A|--allow-small-size] [-V|--format-version VERS]" .sp Initialize a new ScoutFS filesystem on the target devices. Since ScoutFS uses separate block devices for its metadata and data storage, two are required. @@ -99,6 +127,13 @@ Set the data_alloc_zone_blocks volume option, as described in .TP .B "-f, --force" Ignore presence of existing data on the data and metadata devices. +.TP +.B "-V, --format-verson" +Specify the format version to use in the newly created file system. +The range of supported versions is visible in the output of ++.BR scoutfs (8) ++.I --help +. .RE .PD diff --git a/utils/src/change_format_version.c b/utils/src/change_format_version.c new file mode 100644 index 00000000..1447302b --- /dev/null +++ b/utils/src/change_format_version.c @@ -0,0 +1,287 @@ +#define _GNU_SOURCE /* O_DIRECT */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "sparse.h" +#include "cmd.h" +#include "util.h" +#include "format.h" +#include "parse.h" +#include "crc.h" +#include "rand.h" +#include "dev.h" +#include "key.h" +#include "bitops.h" +#include "btree.h" +#include "leaf_item_hash.h" +#include "blkid.h" +#include "quorum.h" + +struct change_fmt_vers_args { + char *meta_device; + char *data_device; + u64 fmt_vers; + bool offline; +}; + +static int do_change_fmt_vers(struct change_fmt_vers_args *args) +{ + struct scoutfs_super_block *meta_super = NULL; + struct scoutfs_super_block *data_super = NULL; + struct scoutfs_quorum_block *qblk = NULL; + struct scoutfs_quorum_block_event *beg; + struct scoutfs_quorum_block_event *end; + bool wrote_meta = false; + bool in_use = false; + char uuid_str[37]; + int meta_fd = -1; + int data_fd = -1; + int ret; + int i; + + meta_fd = open(args->meta_device, O_DIRECT | O_SYNC | O_RDWR | O_EXCL); + if (meta_fd < 0) { + ret = -errno; + fprintf(stderr, "failed to open meta device '%s': %s (%d)\n", + args->meta_device, strerror(errno), errno); + goto out; + } + + data_fd = open(args->data_device, O_DIRECT | O_SYNC | O_RDWR | O_EXCL); + if (data_fd < 0) { + ret = -errno; + fprintf(stderr, "failed to open data device '%s': %s (%d)\n", + args->data_device, strerror(errno), errno); + goto out; + } + + ret = read_block_verify(meta_fd, SCOUTFS_BLOCK_MAGIC_SUPER, 0, SCOUTFS_SUPER_BLKNO, + SCOUTFS_BLOCK_SM_SHIFT, (void **)&meta_super); + if (ret) { + ret = -errno; + fprintf(stderr, "failed to read meta super block: %s (%d)\n", + strerror(errno), errno); + goto out; + } + + ret = read_block_verify(data_fd, SCOUTFS_BLOCK_MAGIC_SUPER, + le64_to_cpu(meta_super->hdr.fsid), SCOUTFS_SUPER_BLKNO, + SCOUTFS_BLOCK_SM_SHIFT, (void **)&data_super); + if (ret) { + ret = -errno; + fprintf(stderr, "failed to read data super block: %s (%d)\n", + strerror(errno), errno); + goto out; + } + + if (le64_to_cpu(meta_super->fmt_vers) == args->fmt_vers && + meta_super->fmt_vers == data_super->fmt_vers) { + printf("both metadata and data device format version are already %llu, nothing to do.\n", + args->fmt_vers); + ret = 0; + goto out; + } + + if (le64_to_cpu(meta_super->fmt_vers) < SCOUTFS_FORMAT_VERSION_MIN || + le64_to_cpu(meta_super->fmt_vers) > SCOUTFS_FORMAT_VERSION_MAX) { + fprintf(stderr, "meta super block has format version %llu outside of supported version range %u-%u", + le64_to_cpu(meta_super->fmt_vers), SCOUTFS_FORMAT_VERSION_MIN, + SCOUTFS_FORMAT_VERSION_MAX); + ret = -EINVAL; + goto out; + } + + if (le64_to_cpu(data_super->fmt_vers) < SCOUTFS_FORMAT_VERSION_MIN || + le64_to_cpu(data_super->fmt_vers) > SCOUTFS_FORMAT_VERSION_MAX) { + fprintf(stderr, "data super block has format version %llu outside of supported version range %u-%u", + le64_to_cpu(data_super->fmt_vers), SCOUTFS_FORMAT_VERSION_MIN, + SCOUTFS_FORMAT_VERSION_MAX); + ret = -EINVAL; + goto out; + } + + if (meta_super->mounted_clients.ref.blkno != 0) { + fprintf(stderr, "meta superblock mounted clients btree is not empty.\n"); + ret = -EBUSY; + in_use = true; + goto out; + } + + /* check for active quorum slots */ + for (i = 0; i < SCOUTFS_QUORUM_BLOCKS; i++) { + if (!quorum_slot_present(meta_super, i)) + continue; + ret = read_block(meta_fd, SCOUTFS_QUORUM_BLKNO + i, SCOUTFS_BLOCK_SM_SHIFT, + (void **)&qblk); + if (ret < 0) { + fprintf(stderr, "error reading quorum block for slot %u\n", i); + goto out; + } + + beg = &qblk->events[SCOUTFS_QUORUM_EVENT_BEGIN]; + end = &qblk->events[SCOUTFS_QUORUM_EVENT_END]; + + if (le64_to_cpu(beg->write_nr) > le64_to_cpu(end->write_nr)) { + fprintf(stderr, "mount in quorum slot %u could still be running.\n" + " begin event: write_nr %llu timestamp %llu.%08u\n" + " end event: write_nr %llu timestamp %llu.%08u\n", + i, le64_to_cpu(beg->write_nr), le64_to_cpu(beg->ts.sec), + le32_to_cpu(beg->ts.nsec), + le64_to_cpu(end->write_nr), le64_to_cpu(end->ts.sec), + le32_to_cpu(end->ts.nsec)); + ret = -EBUSY; + in_use = true; + goto out; + } + + free(qblk); + qblk = NULL; + } + + if (le64_to_cpu(meta_super->fmt_vers) != args->fmt_vers) { + meta_super->fmt_vers = cpu_to_le64(args->fmt_vers); + + ret = write_block(meta_fd, SCOUTFS_BLOCK_MAGIC_SUPER, meta_super->hdr.fsid, 1, + SCOUTFS_SUPER_BLKNO, SCOUTFS_BLOCK_SM_SHIFT, &meta_super->hdr); + if (ret) + goto out; + + wrote_meta = true; + } + + if (le64_to_cpu(data_super->fmt_vers) != args->fmt_vers) { + data_super->fmt_vers = cpu_to_le64(args->fmt_vers); + + ret = write_block(data_fd, SCOUTFS_BLOCK_MAGIC_SUPER, data_super->hdr.fsid, 1, + SCOUTFS_SUPER_BLKNO, SCOUTFS_BLOCK_SM_SHIFT, &data_super->hdr); + if (ret < 0 && wrote_meta) { + fprintf(stderr, "Error writing data super block after writing the meta\n" + "super block. The two super blocks may now be out of sync which\n" + "would prevent mounting. Correct the source of the write error\n" + "and retry changing the version to write both super blocks.\n"); + goto out; + } + } + + uuid_unparse(meta_super->uuid, uuid_str); + + printf("Successfully updated format version for scoutfs filesystem:\n" + " meta device path: %s\n" + " data device path: %s\n" + " fsid: %llx\n" + " uuid: %s\n" + " format version: %llu\n", + args->meta_device, + args->data_device, + le64_to_cpu(meta_super->hdr.fsid), + uuid_str, + le64_to_cpu(meta_super->fmt_vers)); + +out: + if (in_use) + fprintf(stderr, "The filesystem must be fully recovered and cleanly unmounted to change the format version\n"); + + if (qblk) + free(qblk); + if (meta_super) + free(meta_super); + if (data_super) + free(data_super); + if (meta_fd != -1) + close(meta_fd); + if (data_fd != -1) + close(data_fd); + return ret; +} + +static int parse_opt(int key, char *arg, struct argp_state *state) +{ + struct change_fmt_vers_args *args = state->input; + int ret; + + switch (key) { + case 'F': + args->offline = true; + break; + case 'V': + ret = parse_u64(arg, &args->fmt_vers); + if (ret) + return ret; + if (args->fmt_vers < SCOUTFS_FORMAT_VERSION_MIN || + args->fmt_vers > SCOUTFS_FORMAT_VERSION_MAX) + argp_error(state, "format-version %llu is outside supported range of %u-%u", + args->fmt_vers, SCOUTFS_FORMAT_VERSION_MIN, + SCOUTFS_FORMAT_VERSION_MAX); + break; + case ARGP_KEY_ARG: + if (!args->meta_device) + args->meta_device = strdup_or_error(state, arg); + else if (!args->data_device) + args->data_device = strdup_or_error(state, arg); + else + argp_error(state, "more than two device arguments given"); + break; + case ARGP_KEY_FINI: + if (!args->offline) + argp_error(state, "must specify --offline"); + if (!args->meta_device) + argp_error(state, "no metadata device argument given"); + if (!args->data_device) + argp_error(state, "no data device argument given"); + break; + default: + break; + } + + return 0; +} + +static struct argp_option options[] = { + { "offline", 'F', NULL, 0, "Write format version in offline device super blocks"}, + { "format-version", 'V', "VERS", 0, "Specify a format version within supported range ("SCOUTFS_FORMAT_VERSION_MIN_STR"-"SCOUTFS_FORMAT_VERSION_MAX_STR", default "SCOUTFS_FORMAT_VERSION_MAX_STR")"}, + { NULL } +}; + +static struct argp argp = { + options, + parse_opt, + "", + "Change format version of an existing ScoutFS filesystem" +}; + +static int change_fmt_vers_cmd(int argc, char *argv[]) +{ + struct change_fmt_vers_args change_fmt_vers_args = { + .offline = false, + .fmt_vers = SCOUTFS_FORMAT_VERSION_MAX, + }; + int ret; + + ret = argp_parse(&argp, argc, argv, 0, NULL, &change_fmt_vers_args); + if (ret) + return ret; + + return do_change_fmt_vers(&change_fmt_vers_args); +} + +static void __attribute__((constructor)) change_fmt_vers_ctor(void) +{ + cmd_register_argp("change-format-version", &argp, GROUP_CORE, change_fmt_vers_cmd); +} diff --git a/utils/src/cmd.c b/utils/src/cmd.c index ddd49710..10e10ca7 100644 --- a/utils/src/cmd.c +++ b/utils/src/cmd.c @@ -8,6 +8,7 @@ #include "cmd.h" #include "util.h" +#include "format.h" static struct argp_command { char *name; @@ -69,6 +70,9 @@ static void usage(void) fprintf(stderr, "Selected fs defaults to current working directory.\n"); fprintf(stderr, "See --help for more details.\n"); + fprintf(stderr, "\nSupported format version: %u-%u\n", + SCOUTFS_FORMAT_VERSION_MIN, SCOUTFS_FORMAT_VERSION_MAX); + fprintf(stderr, "\nCore admin:\n"); print_cmds_for_group(GROUP_CORE); fprintf(stderr, "\nAdditional Information:\n"); diff --git a/utils/src/mkfs.c b/utils/src/mkfs.c index 69999c16..cb8eb21e 100644 --- a/utils/src/mkfs.c +++ b/utils/src/mkfs.c @@ -110,6 +110,7 @@ struct mkfs_args { unsigned long long max_meta_size; unsigned long long max_data_size; u64 data_alloc_zone_blocks; + u64 fmt_vers; bool force; bool allow_small_size; int nr_slots; @@ -212,7 +213,7 @@ static int do_mkfs(struct mkfs_args *args) /* partially initialize the super so we can use it to init others */ memset(super, 0, SCOUTFS_BLOCK_SM_SIZE); - super->version = cpu_to_le64(SCOUTFS_INTEROP_VERSION); + super->fmt_vers = cpu_to_le64(args->fmt_vers); uuid_generate(super->uuid); super->next_ino = cpu_to_le64(round_up(SCOUTFS_ROOT_INO + 1, SCOUTFS_LOCK_INODE_GROUP_NR)); super->seq = cpu_to_le64(1); @@ -351,16 +352,16 @@ static int do_mkfs(struct mkfs_args *args) " meta device path: %s\n" " data device path: %s\n" " fsid: %llx\n" - " version: %llx\n" " uuid: %s\n" + " format version: %llu\n" " 64KB metadata blocks: "SIZE_FMT"\n" " 4KB data blocks: "SIZE_FMT"\n" " quorum slots: ", args->meta_device, args->data_device, le64_to_cpu(super->hdr.fsid), - le64_to_cpu(super->version), uuid_str, + le64_to_cpu(super->fmt_vers), SIZE_ARGS(le64_to_cpu(super->total_meta_blocks), SCOUTFS_BLOCK_LG_SIZE), SIZE_ARGS(le64_to_cpu(super->total_data_blocks), @@ -484,6 +485,16 @@ static int parse_opt(int key, char *arg, struct argp_state *state) case 'A': args->allow_small_size = true; break; + case 'V': + ret = parse_u64(arg, &args->fmt_vers); + if (ret) + return ret; + if (args->fmt_vers < SCOUTFS_FORMAT_VERSION_MIN || + args->fmt_vers > SCOUTFS_FORMAT_VERSION_MAX) + argp_error(state, "format-version %llu is outside supported range of %u-%u", + args->fmt_vers, SCOUTFS_FORMAT_VERSION_MIN, + SCOUTFS_FORMAT_VERSION_MAX); + break; case 'z': /* data-alloc-zone-blocks */ { ret = parse_u64(arg, &args->data_alloc_zone_blocks); @@ -527,6 +538,7 @@ static struct argp_option options[] = { { "max-meta-size", 'm', "SIZE", 0, "Use a size less than the base metadata device size (bytes or KMGTP units)"}, { "max-data-size", 'd', "SIZE", 0, "Use a size less than the base data device size (bytes or KMGTP units)"}, { "data-alloc-zone-blocks", 'z', "BLOCKS", 0, "Divide data device into block zones so each mounts writes to a zone (4KB blocks)"}, + { "format-version", 'V', "version", 0, "Specify a format version within supported range, ("SCOUTFS_FORMAT_VERSION_MIN_STR"-"SCOUTFS_FORMAT_VERSION_MAX_STR", default "SCOUTFS_FORMAT_VERSION_MAX_STR")"}, { NULL } }; @@ -539,7 +551,9 @@ static struct argp argp = { static int mkfs_cmd(int argc, char *argv[]) { - struct mkfs_args mkfs_args = {NULL,}; + struct mkfs_args mkfs_args = { + .fmt_vers = SCOUTFS_FORMAT_VERSION_MAX, + }; int ret; ret = argp_parse(&argp, argc, argv, 0, NULL, &mkfs_args); diff --git a/utils/src/print.c b/utils/src/print.c index bb566730..ba6195b8 100644 --- a/utils/src/print.c +++ b/utils/src/print.c @@ -935,8 +935,8 @@ static void print_super_block(struct scoutfs_super_block *super, u64 blkno) printf("super blkno %llu\n", blkno); print_block_header(&super->hdr, SCOUTFS_BLOCK_SM_SIZE); - printf(" version %llx uuid %s\n", - le64_to_cpu(super->version), uuid_str); + printf(" fmt_vers %llu uuid %s\n", + le64_to_cpu(super->fmt_vers), uuid_str); printf(" flags: 0x%016llx\n", le64_to_cpu(super->flags)); /* XXX these are all in a crazy order */ diff --git a/utils/src/util.c b/utils/src/util.c index 53a45ed2..0dc4a437 100644 --- a/utils/src/util.c +++ b/utils/src/util.c @@ -79,11 +79,16 @@ int read_block(int fd, u64 blkno, int shift, void **ret_val) void *buf; int ret; + buf = NULL; *ret_val = NULL; - buf = malloc(size); - if (!buf) - return -ENOMEM; + ret = posix_memalign(&buf, size, size); + if (ret != 0) { + ret = -errno; + fprintf(stderr, "%zu byte aligned buffer allocation failed: %s (%d)\n", + size, strerror(errno), errno); + return ret; + } ret = pread(fd, buf, size, blkno << shift); if (ret == -1) { @@ -136,7 +141,7 @@ int read_block_verify(int fd, u32 magic, u64 fsid, u64 blkno, int shift, void ** if (le32_to_cpu(hdr->magic) != magic) fprintf(stderr, "read blkno %llu has bad magic %08x != expected %08x\n", blkno, le32_to_cpu(hdr->magic), magic); - else if (fsid != 0 && le32_to_cpu(hdr->fsid) != fsid) + else if (fsid != 0 && le64_to_cpu(hdr->fsid) != fsid) fprintf(stderr, "read blkno %llu has bad fsid %016llx != expected %016llx\n", blkno, le64_to_cpu(hdr->fsid), fsid); else if (le32_to_cpu(hdr->blkno) != blkno) From 94e5bc1457bed51172e8960df974209ca8450a6a Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 29 Sep 2021 10:09:45 -0700 Subject: [PATCH 10/26] Remove unused scoutfs_last_ino() Signed-off-by: Zach Brown --- kmod/src/inode.c | 16 ---------------- kmod/src/inode.h | 2 -- 2 files changed, 18 deletions(-) diff --git a/kmod/src/inode.c b/kmod/src/inode.c index 0cb61a89..6fb586f2 100644 --- a/kmod/src/inode.c +++ b/kmod/src/inode.c @@ -1299,22 +1299,6 @@ static int remove_index_items(struct super_block *sb, u64 ino, return ret; } -/* - * A quick atomic sample of the last inode number that's been allocated. - */ -u64 scoutfs_last_ino(struct super_block *sb) -{ - struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); - struct scoutfs_super_block *super = &sbi->super; - u64 last; - - spin_lock(&sbi->next_ino_lock); - last = le64_to_cpu(super->next_ino); - spin_unlock(&sbi->next_ino_lock); - - return last; -} - /* * Return an allocated and unused inode number. Returns -ENOSPC if * we're out of inode. diff --git a/kmod/src/inode.h b/kmod/src/inode.h index 26de2d62..98f7db4b 100644 --- a/kmod/src/inode.h +++ b/kmod/src/inode.h @@ -127,8 +127,6 @@ int scoutfs_inode_orphan_delete(struct super_block *sb, u64 ino, struct scoutfs_ void scoutfs_inode_queue_writeback(struct inode *inode); int scoutfs_inode_walk_writeback(struct super_block *sb, bool write); -u64 scoutfs_last_ino(struct super_block *sb); - void scoutfs_inode_exit(void); int scoutfs_inode_init(void); From 95ed36f9d3b35e4dfd2e449e909e0342d44bee34 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 29 Sep 2021 10:26:04 -0700 Subject: [PATCH 11/26] Maintain inode count in super and log trees Add a count of used inodes to the super block and a change in the inode count to the log_trees struct. Client transactions track the change in inode count as they create and delete inodes. The log_trees delta is added to the count in the super as finalized log_trees are deleted. Signed-off-by: Zach Brown --- kmod/src/dir.c | 4 +++ kmod/src/forest.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++ kmod/src/forest.h | 5 ++++ kmod/src/format.h | 2 ++ kmod/src/inode.c | 3 +++ kmod/src/server.c | 4 +++ utils/src/mkfs.c | 1 + utils/src/print.c | 5 +++- 8 files changed, 85 insertions(+), 1 deletion(-) diff --git a/kmod/src/dir.c b/kmod/src/dir.c index 75420229..f380a5d8 100644 --- a/kmod/src/dir.c +++ b/kmod/src/dir.c @@ -31,6 +31,7 @@ #include "lock.h" #include "hash.h" #include "omap.h" +#include "forest.h" #include "counters.h" #include "scoutfs_trace.h" @@ -836,6 +837,7 @@ static int scoutfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, si->crtime = inode->i_mtime; inode_inc_iversion(dir); inode_inc_iversion(inode); + scoutfs_forest_inc_inode_count(sb); if (S_ISDIR(mode)) { inc_nlink(inode); @@ -1309,6 +1311,7 @@ static int scoutfs_symlink(struct inode *dir, struct dentry *dentry, si->crtime = inode->i_ctime; i_size_write(inode, name_len); inode_inc_iversion(inode); + scoutfs_forest_inc_inode_count(sb); scoutfs_update_inode_item(inode, inode_lock, &ind_locks); scoutfs_update_inode_item(dir, dir_lock, &ind_locks); @@ -1908,6 +1911,7 @@ static int scoutfs_tmpfile(struct inode *dir, struct dentry *dentry, umode_t mod ihold(inode); /* need to update inode modifications in d_tmpfile */ d_tmpfile(dentry, inode); inode_inc_iversion(inode); + scoutfs_forest_inc_inode_count(sb); scoutfs_update_inode_item(inode, inode_lock, &ind_locks); scoutfs_update_inode_item(dir, dir_lock, &ind_locks); diff --git a/kmod/src/forest.c b/kmod/src/forest.c index 6890fbd7..1b4c9c4b 100644 --- a/kmod/src/forest.c +++ b/kmod/src/forest.c @@ -66,6 +66,8 @@ struct forest_info { struct workqueue_struct *workq; struct delayed_work log_merge_dwork; + + atomic64_t inode_count_delta; }; #define DECLARE_FOREST_INFO(sb, name) \ @@ -523,6 +525,62 @@ int scoutfs_forest_srch_add(struct super_block *sb, u64 hash, u64 ino, u64 id) return ret; } +void scoutfs_forest_inc_inode_count(struct super_block *sb) +{ + DECLARE_FOREST_INFO(sb, finf); + + atomic64_inc(&finf->inode_count_delta); +} + +void scoutfs_forest_dec_inode_count(struct super_block *sb) +{ + DECLARE_FOREST_INFO(sb, finf); + + atomic64_dec(&finf->inode_count_delta); +} + +/* + * Return the total inode count from the super block and all the + * log_btrees it references. This assumes it's working with a block + * reference hierarchy that should be fully consistent. If we see + * ESTALE we've hit persistent corruption. + */ +int scoutfs_forest_inode_count(struct super_block *sb, struct scoutfs_super_block *super, + u64 *inode_count) +{ + struct scoutfs_log_trees *lt; + SCOUTFS_BTREE_ITEM_REF(iref); + struct scoutfs_key key; + int ret; + + *inode_count = le64_to_cpu(super->inode_count); + + scoutfs_key_init_log_trees(&key, 0, 0); + for (;;) { + ret = scoutfs_btree_next(sb, &super->logs_root, &key, &iref); + if (ret == 0) { + if (iref.val_len == sizeof(*lt)) { + key = *iref.key; + scoutfs_key_inc(&key); + lt = iref.val; + *inode_count += le64_to_cpu(lt->inode_count_delta); + } else { + ret = -EIO; + } + scoutfs_btree_put_iref(&iref); + } + if (ret < 0) { + if (ret == -ENOENT) + ret = 0; + else if (ret == -ESTALE) + ret = -EIO; + break; + } + } + + return ret; +} + /* * This is called from transactions as a new transaction opens and is * serialized with all writers. @@ -551,6 +609,8 @@ void scoutfs_forest_init_btrees(struct super_block *sb, WARN_ON_ONCE(finf->srch_bl); /* commiting should have put the block */ finf->srch_bl = NULL; + atomic64_set(&finf->inode_count_delta, le64_to_cpu(lt->inode_count_delta)); + trace_scoutfs_forest_init_our_log(sb, le64_to_cpu(lt->rid), le64_to_cpu(lt->nr), le64_to_cpu(lt->item_root.ref.blkno), @@ -578,6 +638,8 @@ void scoutfs_forest_get_btrees(struct super_block *sb, scoutfs_block_put(sb, finf->srch_bl); finf->srch_bl = NULL; + lt->inode_count_delta = cpu_to_le64(atomic64_read(&finf->inode_count_delta)); + trace_scoutfs_forest_prepare_commit(sb, <->item_root.ref, <->bloom_ref); } diff --git a/kmod/src/forest.h b/kmod/src/forest.h index 8084731f..30564a11 100644 --- a/kmod/src/forest.h +++ b/kmod/src/forest.h @@ -33,6 +33,11 @@ int scoutfs_forest_insert_list(struct super_block *sb, struct scoutfs_btree_item_list *lst); int scoutfs_forest_srch_add(struct super_block *sb, u64 hash, u64 ino, u64 id); +void scoutfs_forest_inc_inode_count(struct super_block *sb); +void scoutfs_forest_dec_inode_count(struct super_block *sb); +int scoutfs_forest_inode_count(struct super_block *sb, struct scoutfs_super_block *super, + u64 *inode_count); + void scoutfs_forest_init_btrees(struct super_block *sb, struct scoutfs_alloc *alloc, struct scoutfs_block_writer *wri, diff --git a/kmod/src/format.h b/kmod/src/format.h index 3f20db33..825d7327 100644 --- a/kmod/src/format.h +++ b/kmod/src/format.h @@ -472,6 +472,7 @@ struct scoutfs_log_trees { struct scoutfs_srch_file srch_file; __le64 data_alloc_zone_blocks; __le64 data_alloc_zones[SCOUTFS_DATA_ALLOC_ZONE_LE64S]; + __le64 inode_count_delta; __le64 max_item_seq; __le64 finalize_seq; __le64 rid; @@ -795,6 +796,7 @@ struct scoutfs_super_block { __u8 uuid[SCOUTFS_UUID_BYTES]; __le64 seq; __le64 next_ino; + __le64 inode_count; __le64 total_meta_blocks; /* both static and dynamic */ __le64 total_data_blocks; struct scoutfs_quorum_config qconf; diff --git a/kmod/src/inode.c b/kmod/src/inode.c index 6fb586f2..b4190dbf 100644 --- a/kmod/src/inode.c +++ b/kmod/src/inode.c @@ -34,6 +34,7 @@ #include "client.h" #include "cmp.h" #include "omap.h" +#include "forest.h" #include "btree.h" /* @@ -1578,6 +1579,8 @@ retry: goto out; ret = scoutfs_inode_orphan_delete(sb, ino, orph_lock); + if (ret == 0) + scoutfs_forest_dec_inode_count(sb); out: del_deleting_ino(inf, &del); if (release) diff --git a/kmod/src/server.c b/kmod/src/server.c index 2ca229a6..acb16cf4 100644 --- a/kmod/src/server.c +++ b/kmod/src/server.c @@ -938,6 +938,7 @@ static int finalize_and_start_log_merge(struct super_block *sb, struct scoutfs_l memset(<->item_root, 0, sizeof(lt->item_root)); memset(<->bloom_ref, 0, sizeof(lt->bloom_ref)); + lt->inode_count_delta = 0; lt->max_item_seq = 0; lt->finalize_seq = 0; le64_add_cpu(<->nr, 1); @@ -2010,6 +2011,9 @@ static int splice_log_merge_completions(struct super_block *sb, err_str = "deleting log trees item"; goto out; } + + le64_add_cpu(&super->inode_count, le64_to_cpu(lt.inode_count_delta)); + } init_log_merge_key(&key, SCOUTFS_LOG_MERGE_STATUS_ZONE, 0, 0); diff --git a/utils/src/mkfs.c b/utils/src/mkfs.c index cb8eb21e..b4d1f77e 100644 --- a/utils/src/mkfs.c +++ b/utils/src/mkfs.c @@ -216,6 +216,7 @@ static int do_mkfs(struct mkfs_args *args) super->fmt_vers = cpu_to_le64(args->fmt_vers); uuid_generate(super->uuid); super->next_ino = cpu_to_le64(round_up(SCOUTFS_ROOT_INO + 1, SCOUTFS_LOCK_INODE_GROUP_NR)); + super->inode_count = cpu_to_le64(1); super->seq = cpu_to_le64(1); super->total_meta_blocks = cpu_to_le64(last_meta + 1); super->total_data_blocks = cpu_to_le64(last_data + 1); diff --git a/utils/src/print.c b/utils/src/print.c index ba6195b8..8e5d4852 100644 --- a/utils/src/print.c +++ b/utils/src/print.c @@ -277,6 +277,7 @@ static int print_log_trees_item(struct scoutfs_key *key, u64 seq, u8 flags, void " data_avail: "ALCROOT_F"\n" " data_freed: "ALCROOT_F"\n" " srch_file: "SRF_FMT"\n" + " inode_count_delta: %lld\n" " max_item_seq: %llu\n" " finalize_seq: %llu\n" " rid: %016llx\n" @@ -294,6 +295,7 @@ static int print_log_trees_item(struct scoutfs_key *key, u64 seq, u8 flags, void ALCROOT_A(<->data_avail), ALCROOT_A(<->data_freed), SRF_A(<->srch_file), + le64_to_cpu(lt->inode_count_delta), le64_to_cpu(lt->max_item_seq), le64_to_cpu(lt->finalize_seq), le64_to_cpu(lt->rid), @@ -940,7 +942,7 @@ static void print_super_block(struct scoutfs_super_block *super, u64 blkno) printf(" flags: 0x%016llx\n", le64_to_cpu(super->flags)); /* XXX these are all in a crazy order */ - printf(" next_ino %llu seq %llu\n" + printf(" next_ino %llu inode_count %llu seq %llu\n" " total_meta_blocks %llu total_data_blocks %llu\n" " meta_alloc[0]: "ALCROOT_F"\n" " meta_alloc[1]: "ALCROOT_F"\n" @@ -956,6 +958,7 @@ static void print_super_block(struct scoutfs_super_block *super, u64 blkno) " mounted_clients: "BTR_FMT"\n" " srch_root: "BTR_FMT"\n", le64_to_cpu(super->next_ino), + le64_to_cpu(super->inode_count), le64_to_cpu(super->seq), le64_to_cpu(super->total_meta_blocks), le64_to_cpu(super->total_data_blocks), From a53d6d1a8e185cc42a756de60ad7c0631be97816 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 29 Sep 2021 14:37:54 -0700 Subject: [PATCH 12/26] Add scoutfs_alloc_foreach_super which takes super Add an alloc_foreach variant which uses the caller's super to walk the allocators rather than always reading it off the device. Signed-off-by: Zach Brown --- kmod/src/alloc.c | 63 +++++++++++++++++++++++++++++++++--------------- kmod/src/alloc.h | 2 ++ 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/kmod/src/alloc.c b/kmod/src/alloc.c index d27c4df6..7ef3ff30 100644 --- a/kmod/src/alloc.c +++ b/kmod/src/alloc.c @@ -1327,15 +1327,17 @@ bool scoutfs_alloc_test_flag(struct super_block *sb, } /* - * Call the callers callback for every persistent allocator structure - * we can find. + * Iterate over the allocator structures referenced by the caller's + * super and call the caller's callback with summaries of the blocks + * found in each structure. + * + * The caller's responsible for the stability of the referenced blocks. + * If the blocks could be stale the caller must deal with retrying when + * it sees ESTALE. */ -int scoutfs_alloc_foreach(struct super_block *sb, - scoutfs_alloc_foreach_cb_t cb, void *arg) +int scoutfs_alloc_foreach_super(struct super_block *sb, struct scoutfs_super_block *super, + scoutfs_alloc_foreach_cb_t cb, void *arg) { - struct scoutfs_block_ref stale_refs[2] = {{0,}}; - struct scoutfs_block_ref refs[2] = {{0,}}; - struct scoutfs_super_block *super = NULL; struct scoutfs_srch_compact *sc; struct scoutfs_log_merge_request *lmreq; struct scoutfs_log_merge_complete *lmcomp; @@ -1348,21 +1350,12 @@ int scoutfs_alloc_foreach(struct super_block *sb, u64 id; int ret; - super = kmalloc(sizeof(struct scoutfs_super_block), GFP_NOFS); sc = kmalloc(sizeof(struct scoutfs_srch_compact), GFP_NOFS); - if (!super || !sc) { + if (!sc) { ret = -ENOMEM; goto out; } -retry: - ret = scoutfs_read_super(sb, super); - if (ret < 0) - goto out; - - refs[0] = super->logs_root.ref; - refs[1] = super->srch_root.ref; - /* all the server allocators */ ret = cb(sb, arg, SCOUTFS_ALLOC_OWNER_SERVER, 0, true, true, le64_to_cpu(super->meta_alloc[0].total_len)) ?: @@ -1505,6 +1498,40 @@ retry: ret = 0; out: + + kfree(sc); + return ret; +} + +/* + * Read the current on-disk super and use it to walk the allocators and + * call the caller's callback. This assumes that the super it's reading + * could be stale and will retry if it encounters stale blocks. + */ +int scoutfs_alloc_foreach(struct super_block *sb, + scoutfs_alloc_foreach_cb_t cb, void *arg) +{ + struct scoutfs_super_block *super = NULL; + struct scoutfs_block_ref stale_refs[2] = {{0,}}; + struct scoutfs_block_ref refs[2] = {{0,}}; + int ret; + + super = kmalloc(sizeof(struct scoutfs_super_block), GFP_NOFS); + if (!super) { + ret = -ENOMEM; + goto out; + } + +retry: + ret = scoutfs_read_super(sb, super); + if (ret < 0) + goto out; + + refs[0] = super->logs_root.ref; + refs[1] = super->srch_root.ref; + + ret = scoutfs_alloc_foreach_super(sb, super, cb, arg); +out: if (ret == -ESTALE) { if (memcmp(&stale_refs, &refs, sizeof(refs)) == 0) { ret = -EIO; @@ -1516,11 +1543,9 @@ out: } kfree(super); - kfree(sc); return ret; } - struct foreach_cb_args { scoutfs_alloc_extent_cb_t cb; void *cb_arg; diff --git a/kmod/src/alloc.h b/kmod/src/alloc.h index 9dcbd94c..38e1c1f2 100644 --- a/kmod/src/alloc.h +++ b/kmod/src/alloc.h @@ -166,6 +166,8 @@ typedef int (*scoutfs_alloc_foreach_cb_t)(struct super_block *sb, void *arg, bool meta, bool avail, u64 blocks); int scoutfs_alloc_foreach(struct super_block *sb, scoutfs_alloc_foreach_cb_t cb, void *arg); +int scoutfs_alloc_foreach_super(struct super_block *sb, struct scoutfs_super_block *super, + scoutfs_alloc_foreach_cb_t cb, void *arg); typedef void (*scoutfs_alloc_extent_cb_t)(struct super_block *sb, void *cb_arg, struct scoutfs_extent *ext); From cf512c5fcfe563eea5aaeb1c9d94b17fd0c701d9 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 29 Sep 2021 15:09:14 -0700 Subject: [PATCH 13/26] Use inode_count field for statfs file counts Our statfs implementation had clients reading the super block and using the next free inode number to guess how many inodes there might be. We are very aggressive with giving directories private pools of inode numbers to allocate from. They're often not used at all, creating huge gaps in allocated inode numbers. The ratio of the average number of allocations per directory to the batch size given to each directory is the factor that the used inode count can be off by. Now that we have a precise count of active inodes we can use that to return accurate counts of inodes in the files fields in the statfs struct. We still don't have static inode allocation so the fields don't make a ton of sense. We fake the total and free count to give a reasonable estimate of the total files that doesn't change while the free count is calculated from the correct count of used inodes. While we're at it we add a request to get the summed fields that the server can cheaply discover in cache rather than having the client always perform read IOs. Signed-off-by: Zach Brown --- kmod/src/client.c | 8 +++++ kmod/src/client.h | 1 + kmod/src/format.h | 10 ++++++ kmod/src/server.c | 63 ++++++++++++++++++++++++++++++++++++ kmod/src/super.c | 81 ++++++++++++++++++++--------------------------- 5 files changed, 117 insertions(+), 46 deletions(-) diff --git a/kmod/src/client.c b/kmod/src/client.c index acf9412f..98f14526 100644 --- a/kmod/src/client.c +++ b/kmod/src/client.c @@ -306,6 +306,14 @@ int scoutfs_client_resize_devices(struct super_block *sb, struct scoutfs_net_res nrd, sizeof(*nrd), NULL, 0); } +int scoutfs_client_statfs(struct super_block *sb, struct scoutfs_net_statfs *nst) +{ + struct client_info *client = SCOUTFS_SB(sb)->client_info; + + return scoutfs_net_sync_request(sb, client->conn, SCOUTFS_NET_CMD_STATFS, + NULL, 0, nst, sizeof(*nst)); +} + /* * The server is asking that we trigger a commit of the current log * trees so that they can ensure an item seq discontinuity between diff --git a/kmod/src/client.h b/kmod/src/client.h index e62eccae..07dab8a9 100644 --- a/kmod/src/client.h +++ b/kmod/src/client.h @@ -34,6 +34,7 @@ int scoutfs_client_get_volopt(struct super_block *sb, struct scoutfs_volume_opti int scoutfs_client_set_volopt(struct super_block *sb, struct scoutfs_volume_options *volopt); int scoutfs_client_clear_volopt(struct super_block *sb, struct scoutfs_volume_options *volopt); int scoutfs_client_resize_devices(struct super_block *sb, struct scoutfs_net_resize_devices *nrd); +int scoutfs_client_statfs(struct super_block *sb, struct scoutfs_net_statfs *nst); int scoutfs_client_setup(struct super_block *sb); void scoutfs_client_destroy(struct super_block *sb); diff --git a/kmod/src/format.h b/kmod/src/format.h index 825d7327..9151e4a9 100644 --- a/kmod/src/format.h +++ b/kmod/src/format.h @@ -1003,6 +1003,7 @@ enum scoutfs_net_cmd { SCOUTFS_NET_CMD_SET_VOLOPT, SCOUTFS_NET_CMD_CLEAR_VOLOPT, SCOUTFS_NET_CMD_RESIZE_DEVICES, + SCOUTFS_NET_CMD_STATFS, SCOUTFS_NET_CMD_FAREWELL, SCOUTFS_NET_CMD_UNKNOWN, }; @@ -1050,6 +1051,15 @@ struct scoutfs_net_resize_devices { __le64 new_total_data_blocks; }; +struct scoutfs_net_statfs { + __u8 uuid[SCOUTFS_UUID_BYTES]; + __le64 free_meta_blocks; + __le64 total_meta_blocks; + __le64 free_data_blocks; + __le64 total_data_blocks; + __le64 inode_count; +}; + struct scoutfs_net_lock { struct scoutfs_key key; __le64 write_seq; diff --git a/kmod/src/server.c b/kmod/src/server.c index acb16cf4..af994546 100644 --- a/kmod/src/server.c +++ b/kmod/src/server.c @@ -2921,6 +2921,68 @@ out: return scoutfs_net_response(sb, conn, cmd, id, ret, NULL, 0); }; +struct statfs_free_blocks { + u64 meta; + u64 data; +}; + +static int count_free_blocks(struct super_block *sb, void *arg, int owner, + u64 id, bool meta, bool avail, u64 blocks) +{ + struct statfs_free_blocks *sfb = arg; + + if (meta) + sfb->meta += blocks; + else + sfb->data += blocks; + + return 0; +} + +/* + * We calculate the total inode count and free blocks from the current in-memory dirty + * versions of the super block and log_trees structs, so we have to lock them. + */ +static int server_statfs(struct super_block *sb, struct scoutfs_net_connection *conn, + u8 cmd, u64 id, void *arg, u16 arg_len) +{ + DECLARE_SERVER_INFO(sb, server); + struct scoutfs_super_block *super = &SCOUTFS_SB(sb)->super; + struct scoutfs_net_statfs nst = {{0,}}; + struct statfs_free_blocks sfb = {0,}; + u64 inode_count; + int ret; + + if (arg_len != 0) { + ret = -EINVAL; + goto out; + } + + mutex_lock(&server->alloc_mutex); + ret = scoutfs_alloc_foreach_super(sb, super, count_free_blocks, &sfb); + mutex_unlock(&server->alloc_mutex); + if (ret < 0) + goto out; + + mutex_lock(&server->logs_mutex); + ret = scoutfs_forest_inode_count(sb, super, &inode_count); + mutex_unlock(&server->logs_mutex); + if (ret < 0) + goto out; + + BUILD_BUG_ON(sizeof(nst.uuid) != sizeof(super->uuid)); + memcpy(nst.uuid, super->uuid, sizeof(nst.uuid)); + nst.free_meta_blocks = cpu_to_le64(sfb.meta); + nst.total_meta_blocks = super->total_meta_blocks; + nst.free_data_blocks = cpu_to_le64(sfb.data); + nst.total_data_blocks = super->total_data_blocks; + nst.inode_count = cpu_to_le64(inode_count); + + ret = 0; +out: + return scoutfs_net_response(sb, conn, cmd, id, ret, &nst, sizeof(nst)); +} + static void init_mounted_client_key(struct scoutfs_key *key, u64 rid) { *key = (struct scoutfs_key) { @@ -3558,6 +3620,7 @@ static scoutfs_net_request_t server_req_funcs[] = { [SCOUTFS_NET_CMD_SET_VOLOPT] = server_set_volopt, [SCOUTFS_NET_CMD_CLEAR_VOLOPT] = server_clear_volopt, [SCOUTFS_NET_CMD_RESIZE_DEVICES] = server_resize_devices, + [SCOUTFS_NET_CMD_STATFS] = server_statfs, [SCOUTFS_NET_CMD_FAREWELL] = server_farewell, }; diff --git a/kmod/src/super.c b/kmod/src/super.c index 612c51fa..e22a0420 100644 --- a/kmod/src/super.c +++ b/kmod/src/super.c @@ -83,35 +83,34 @@ retry: return cpu_to_le64(ret); } -struct statfs_free_blocks { - u64 meta; - u64 data; -}; - -static int count_free_blocks(struct super_block *sb, void *arg, int owner, - u64 id, bool meta, bool avail, u64 blocks) +/* the statfs file fields can be small (and signed?) :/ */ +static __statfs_word saturate_truncated_word(u64 files) { - struct statfs_free_blocks *sfb = arg; + __statfs_word word = files; - if (meta) - sfb->meta += blocks; - else - sfb->data += blocks; + if (word != files) { + word = ~0ULL; + if (word < 0) + word = (unsigned long)word >> 1; + } - return 0; + return word; } /* - * Build the free block counts by having alloc read all the persistent - * blocks which contain allocators and calling us for each of them. - * Only the super block reads aren't cached so repeatedly calling statfs - * is like repeated O_DIRECT IO. We can add a cache and stale results - * if that IO becomes a problem. + * The server gives us the current sum of free blocks and the total + * inode count that it can see across all the clients' log trees. It + * won't see allocations and inode creations or deletions that are dirty + * in client memory as it builds a transaction. * - * We fake the number of free inodes value by assuming that we can fill - * free blocks with a certain number of inodes. We then the number of - * current inodes to that free count to determine the total possible - * inodes. + * We don't have static limits on the number of files so the statfs + * fields for the total possible files and the number free isn't + * particularly helpful. What we do want to report is the number of + * inodes, so we fake a max possible number of inodes given a + * conservative estimate of the total space consumption per file and + * then find the free by subtracting our precise count of active inodes. + * This seems like the least surprising compromise where the file max + * doesn't change and the caller gets the correct count of used inodes. * * The fsid that we report is constructed from the xor of the first two * and second two little endian u32s that make up the uuid bytes. @@ -119,41 +118,33 @@ static int count_free_blocks(struct super_block *sb, void *arg, int owner, static int scoutfs_statfs(struct dentry *dentry, struct kstatfs *kst) { struct super_block *sb = dentry->d_inode->i_sb; - struct scoutfs_super_block *super = NULL; - struct statfs_free_blocks sfb = {0,}; + struct scoutfs_net_statfs nst; + u64 files; + u64 ffree; __le32 uuid[4]; int ret; scoutfs_inc_counter(sb, statfs); - super = kzalloc(sizeof(struct scoutfs_super_block), GFP_NOFS); - if (!super) { - ret = -ENOMEM; - goto out; - } - - ret = scoutfs_read_super(sb, super); + ret = scoutfs_client_statfs(sb, &nst); if (ret) goto out; - ret = scoutfs_alloc_foreach(sb, count_free_blocks, &sfb); - if (ret < 0) - goto out; - - kst->f_bfree = (sfb.meta << SCOUTFS_BLOCK_SM_LG_SHIFT) + sfb.data; + kst->f_bfree = (le64_to_cpu(nst.free_meta_blocks) << SCOUTFS_BLOCK_SM_LG_SHIFT) + + le64_to_cpu(nst.free_data_blocks); kst->f_type = SCOUTFS_SUPER_MAGIC; kst->f_bsize = SCOUTFS_BLOCK_SM_SIZE; - kst->f_blocks = (le64_to_cpu(super->total_meta_blocks) << - SCOUTFS_BLOCK_SM_LG_SHIFT) + - le64_to_cpu(super->total_data_blocks); + kst->f_blocks = (le64_to_cpu(nst.total_meta_blocks) << SCOUTFS_BLOCK_SM_LG_SHIFT) + + le64_to_cpu(nst.total_data_blocks); kst->f_bavail = kst->f_bfree; - /* arbitrarily assume ~1K / empty file */ - kst->f_ffree = sfb.meta * (SCOUTFS_BLOCK_LG_SIZE / 1024); - kst->f_files = kst->f_ffree + le64_to_cpu(super->next_ino); + files = div_u64(le64_to_cpu(nst.total_meta_blocks) << SCOUTFS_BLOCK_LG_SHIFT, 2048); + ffree = files - le64_to_cpu(nst.inode_count); + kst->f_files = saturate_truncated_word(files); + kst->f_ffree = saturate_truncated_word(ffree); - BUILD_BUG_ON(sizeof(uuid) != sizeof(super->uuid)); - memcpy(uuid, super->uuid, sizeof(uuid)); + BUILD_BUG_ON(sizeof(uuid) != sizeof(nst.uuid)); + memcpy(uuid, nst.uuid, sizeof(uuid)); kst->f_fsid.val[0] = le32_to_cpu(uuid[0]) ^ le32_to_cpu(uuid[1]); kst->f_fsid.val[1] = le32_to_cpu(uuid[2]) ^ le32_to_cpu(uuid[3]); kst->f_namelen = SCOUTFS_NAME_LEN; @@ -162,8 +153,6 @@ static int scoutfs_statfs(struct dentry *dentry, struct kstatfs *kst) /* the vfs fills f_flags */ ret = 0; out: - kfree(super); - /* * We don't take cluster locks in statfs which makes it a very * convenient place to trigger lock reclaim for debugging. We From 75f9aabe75ecc5a36e35dee031132dae215cb254 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 6 Oct 2021 11:33:31 -0700 Subject: [PATCH 14/26] Allow compacting logs down to a single page The k-way merge function at the core of the srch file entry merging had some bookkeeping math (calculating number of parents) that couldn't handle merging a single incoming entry stream, so it threw a warning and returned an error. When refusing to handle that case, it was assuming that caller was trying to merge down a single log file which doesn't make any sense. But in the case of multiple small unsorted logs we can absolutely end up with their entries stored in one sorted page. We have one sorted input page that's merging multiple log files. The merge function is also the path that writes to the output file so we absolutely need to handle this case. We more carefully calculate the number of parents, clamping it to one parent when we'd otherwise get "(roundup(1) -> 1) - 1 == 0" when calculating the number of parents from the number of inputs. We can relax the warning and error to refuse to merge nothing. The test triggers this case by putting single search entries in the log files for mounts and unmounting them to force rotation of the mount log files into mergable rotated log files. Signed-off-by: Zach Brown --- kmod/src/srch.c | 5 +++-- tests/golden/srch-basic-functionality | 1 + tests/tests/srch-basic-functionality.sh | 25 +++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/kmod/src/srch.c b/kmod/src/srch.c index b113614c..54b0711d 100644 --- a/kmod/src/srch.c +++ b/kmod/src/srch.c @@ -1481,10 +1481,11 @@ static int kway_merge(struct super_block *sb, int ind; int i; - if (WARN_ON_ONCE(nr <= 1)) + if (WARN_ON_ONCE(nr <= 0)) return -EINVAL; - nr_parents = roundup_pow_of_two(nr) - 1; + /* always at least one parent for single leaf */ + nr_parents = max_t(unsigned long, 1, roundup_pow_of_two(nr) - 1); /* root at [1] for easy sib/parent index calc, final pad for odd sib */ nr_nodes = 1 + nr_parents + nr + 1; tnodes = __vmalloc(nr_nodes * sizeof(struct tourn_node), diff --git a/tests/golden/srch-basic-functionality b/tests/golden/srch-basic-functionality index c46d1701..e135bb58 100644 --- a/tests/golden/srch-basic-functionality +++ b/tests/golden/srch-basic-functionality @@ -2,6 +2,7 @@ == update existing xattr == remove an xattr == remove xattr with files +== trigger small log merges by rotating single block with unmount == create entries in current log == delete small fraction == remove files diff --git a/tests/tests/srch-basic-functionality.sh b/tests/tests/srch-basic-functionality.sh index 55c709f5..1b982489 100644 --- a/tests/tests/srch-basic-functionality.sh +++ b/tests/tests/srch-basic-functionality.sh @@ -42,6 +42,31 @@ echo "== remove xattr with files" rm -f "$T_D0/"{create,update} diff_srch_find scoutfs.srch.test +echo "== trigger small log merges by rotating single block with unmount" +sv=$(t_server_nr) +i=1 +while [ "$i" -lt "8" ]; do + for nr in $(t_fs_nrs); do + # not checking, can go over limit by fs_nrs + ((i++)) + + if [ $nr == $sv ]; then + continue; + fi + + eval path="\$T_D${nr}/single-block-$i" + touch "$path" + setfattr -n scoutfs.srch.single-block-logs -v $i "$path" + t_umount $nr + t_mount $nr + + ((i++)) + done +done +# wait for srch compaction worker delay +sleep 10 +rm -rf "$T_D0/single-block-*" + echo "== create entries in current log" DIR="$T_D0/dir" NR=$((LOG / 4)) From b477604339ec9c8d2e3d5d2ab542872eca97047a Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 6 Oct 2021 13:42:50 -0700 Subject: [PATCH 15/26] Don't clobber srch compact errors The srch compaction worker will wait a bit before attempting another compaction as it finishes a compaction that failed. Unfortunately, it clobbered the errors it got during compaction with the result of sending the commit to the server with the error flag. If the commit is successful then it thinks there were no errors and immediately re-queues itself to try the next compaction. If the error is persistent, as it was with a bug in how we merged log files with a single page's worth of entries, then we can spin indefinitely getting and error, clobbering the error with the commit result, and immediately queueing our work to do it all over again. This fix preserves existing errors when geting the result of the commit and will correctly back off. If we get persistent merge errors at least they won't consume significant resources. We add a counter for commit for the errors so we can get some visibility if this happens. Signed-off-by: Zach Brown --- kmod/src/counters.h | 1 + kmod/src/srch.c | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/kmod/src/counters.h b/kmod/src/counters.h index 234c489d..b3e68bd4 100644 --- a/kmod/src/counters.h +++ b/kmod/src/counters.h @@ -178,6 +178,7 @@ EXPAND_COUNTER(srch_add_entry) \ EXPAND_COUNTER(srch_compact_dirty_block) \ EXPAND_COUNTER(srch_compact_entry) \ + EXPAND_COUNTER(srch_compact_error) \ EXPAND_COUNTER(srch_compact_flush) \ EXPAND_COUNTER(srch_compact_log_page) \ EXPAND_COUNTER(srch_compact_removed_entry) \ diff --git a/kmod/src/srch.c b/kmod/src/srch.c index 54b0711d..b23fc6c2 100644 --- a/kmod/src/srch.c +++ b/kmod/src/srch.c @@ -28,6 +28,7 @@ #include "btree.h" #include "spbm.h" #include "client.h" +#include "counters.h" #include "scoutfs_trace.h" /* @@ -2128,6 +2129,7 @@ static void scoutfs_srch_compact_worker(struct work_struct *work) struct scoutfs_alloc alloc; unsigned long delay; int ret; + int err; sc = kmalloc(sizeof(struct scoutfs_srch_compact), GFP_NOFS); if (sc == NULL) { @@ -2166,10 +2168,14 @@ commit: sc->meta_freed = alloc.freed; sc->flags |= ret < 0 ? SCOUTFS_SRCH_COMPACT_FLAG_ERROR : 0; - ret = scoutfs_client_srch_commit_compact(sb, sc); + err = scoutfs_client_srch_commit_compact(sb, sc); + if (err < 0 && ret == 0) + ret = err; out: /* our allocators and files should be stable */ WARN_ON_ONCE(ret == -ESTALE); + if (ret < 0) + scoutfs_inc_counter(sb, srch_compact_error); scoutfs_block_writer_forget_all(sb, &wri); if (!atomic_read(&srinf->shutdown)) { From 70ede28e39f0027677d7e53007c04674c8867366 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 20 Oct 2021 12:21:38 -0700 Subject: [PATCH 16/26] Remove unused traced_extent leavings Remove some lingering support helpers for the traced_extent struct that we haven't used in a while. Signed-off-by: Zach Brown --- kmod/src/data.h | 7 ------- kmod/src/scoutfs_trace.h | 3 --- 2 files changed, 10 deletions(-) diff --git a/kmod/src/data.h b/kmod/src/data.h index 064564f6..c056915e 100644 --- a/kmod/src/data.h +++ b/kmod/src/data.h @@ -38,13 +38,6 @@ struct scoutfs_data_wait { .err = 0, \ } -struct scoutfs_traced_extent { - u64 iblock; - u64 count; - u64 blkno; - u8 flags; -}; - extern const struct address_space_operations scoutfs_file_aops; extern const struct file_operations scoutfs_file_fops; struct scoutfs_alloc; diff --git a/kmod/src/scoutfs_trace.h b/kmod/src/scoutfs_trace.h index e1e53acd..ce9e707f 100644 --- a/kmod/src/scoutfs_trace.h +++ b/kmod/src/scoutfs_trace.h @@ -58,9 +58,6 @@ struct lock_info; __entry->pref##_map, \ __entry->pref##_flags -#define DECLARE_TRACED_EXTENT(name) \ - struct scoutfs_traced_extent name = {0} - DECLARE_EVENT_CLASS(scoutfs_ino_ret_class, TP_PROTO(struct super_block *sb, u64 ino, int ret), From 7d71b610af80aef557d57b2e4f9d7f62829555e1 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 20 Oct 2021 14:06:02 -0700 Subject: [PATCH 17/26] Add server extent motion tracking Add tracking in the alloc functions that the server uses to move extents between allocator structures on behalf of client mounts. Signed-off-by: Zach Brown --- kmod/src/alloc.c | 6 ++++++ kmod/src/scoutfs_trace.h | 30 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/kmod/src/alloc.c b/kmod/src/alloc.c index 7ef3ff30..1d64012d 100644 --- a/kmod/src/alloc.c +++ b/kmod/src/alloc.c @@ -970,6 +970,8 @@ int scoutfs_alloc_move(struct super_block *sb, struct scoutfs_alloc *alloc, moved += ext.len; scoutfs_inc_counter(sb, alloc_moved_extent); + + trace_scoutfs_alloc_move_extent(sb, &ext); } scoutfs_inc_counter(sb, alloc_move); @@ -1157,6 +1159,8 @@ int scoutfs_alloc_fill_list(struct super_block *sb, for (i = 0; i < ext.len; i++) list_block_add(lhead, lblk, ext.start + i); + + trace_scoutfs_alloc_fill_extent(sb, &ext); } out: @@ -1225,6 +1229,8 @@ int scoutfs_alloc_empty_list(struct super_block *sb, break; list_block_remove(lhead, lblk, ext.len); + + trace_scoutfs_alloc_empty_extent(sb, &ext); } scoutfs_block_put(sb, bl); diff --git a/kmod/src/scoutfs_trace.h b/kmod/src/scoutfs_trace.h index ce9e707f..635a1597 100644 --- a/kmod/src/scoutfs_trace.h +++ b/kmod/src/scoutfs_trace.h @@ -2609,6 +2609,36 @@ TRACE_EVENT(scoutfs_alloc_move, __entry->ret) ); +DECLARE_EVENT_CLASS(scoutfs_alloc_extent_class, + TP_PROTO(struct super_block *sb, struct scoutfs_extent *ext), + + TP_ARGS(sb, ext), + + TP_STRUCT__entry( + SCSB_TRACE_FIELDS + STE_FIELDS(ext) + ), + + TP_fast_assign( + SCSB_TRACE_ASSIGN(sb); + STE_ASSIGN(ext, ext); + ), + + TP_printk(SCSBF" ext "STE_FMT, SCSB_TRACE_ARGS, STE_ENTRY_ARGS(ext)) +); +DEFINE_EVENT(scoutfs_alloc_extent_class, scoutfs_alloc_move_extent, + TP_PROTO(struct super_block *sb, struct scoutfs_extent *ext), + TP_ARGS(sb, ext) +); +DEFINE_EVENT(scoutfs_alloc_extent_class, scoutfs_alloc_fill_extent, + TP_PROTO(struct super_block *sb, struct scoutfs_extent *ext), + TP_ARGS(sb, ext) +); +DEFINE_EVENT(scoutfs_alloc_extent_class, scoutfs_alloc_empty_extent, + TP_PROTO(struct super_block *sb, struct scoutfs_extent *ext), + TP_ARGS(sb, ext) +); + TRACE_EVENT(scoutfs_item_read_page, TP_PROTO(struct super_block *sb, struct scoutfs_key *key, struct scoutfs_key *pg_start, struct scoutfs_key *pg_end), From 42c4c6dd247fe025b2616eb55cf6c077555c96a3 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Thu, 21 Oct 2021 14:45:15 -0700 Subject: [PATCH 18/26] Move transaction sbi fields to trans_info The transaction code was built a million years ago and put all of its data in our core super block info. This finally moves the rest of the private transaction fields out of the core super block and into the transaction info. This makes it clear that it's private to trans.c and brings it line with the rest of the subsystems in the tree. Signed-off-by: Zach Brown --- kmod/src/super.c | 4 -- kmod/src/super.h | 11 +---- kmod/src/trans.c | 123 ++++++++++++++++++++++++++--------------------- 3 files changed, 69 insertions(+), 69 deletions(-) diff --git a/kmod/src/super.c b/kmod/src/super.c index e22a0420..63e82259 100644 --- a/kmod/src/super.c +++ b/kmod/src/super.c @@ -569,12 +569,8 @@ static int scoutfs_fill_super(struct super_block *sb, void *data, int silent) return ret; spin_lock_init(&sbi->next_ino_lock); - init_waitqueue_head(&sbi->trans_hold_wq); spin_lock_init(&sbi->data_wait_root.lock); sbi->data_wait_root.root = RB_ROOT; - spin_lock_init(&sbi->trans_write_lock); - INIT_DELAYED_WORK(&sbi->trans_write_work, scoutfs_trans_write_func); - init_waitqueue_head(&sbi->trans_write_wq); scoutfs_sysfs_init_attrs(sb, &sbi->mopts_ssa); ret = scoutfs_parse_options(sb, data, &opts); diff --git a/kmod/src/super.h b/kmod/src/super.h index 32fba8d6..4ca4b88f 100644 --- a/kmod/src/super.h +++ b/kmod/src/super.h @@ -57,20 +57,11 @@ struct scoutfs_sb_info { struct item_cache_info *item_cache_info; struct fence_info *fence_info; - wait_queue_head_t trans_hold_wq; - struct task_struct *trans_task; - /* tracks tasks waiting for data extents */ struct scoutfs_data_wait_root data_wait_root; - spinlock_t trans_write_lock; - u64 trans_write_count; + /* set as transaction opens with trans holders excluded */ u64 trans_seq; - int trans_write_ret; - struct delayed_work trans_write_work; - wait_queue_head_t trans_write_wq; - struct workqueue_struct *trans_write_workq; - bool trans_deadline_expired; struct trans_info *trans_info; struct lock_info *lock_info; diff --git a/kmod/src/trans.c b/kmod/src/trans.c index 1c17c631..f5980778 100644 --- a/kmod/src/trans.c +++ b/kmod/src/trans.c @@ -53,15 +53,24 @@ /* sync dirty data at least this often */ #define TRANS_SYNC_DELAY (HZ * 10) -/* - * XXX move the rest of the super trans_ fields here. - */ struct trans_info { + struct super_block *sb; + atomic_t holders; struct scoutfs_log_trees lt; struct scoutfs_alloc alloc; struct scoutfs_block_writer wri; + + wait_queue_head_t hold_wq; + struct task_struct *task; + spinlock_t write_lock; + u64 write_count; + int write_ret; + struct delayed_work write_work; + wait_queue_head_t write_wq; + struct workqueue_struct *write_workq; + bool deadline_expired; }; #define DECLARE_TRANS_INFO(sb, name) \ @@ -120,13 +129,12 @@ bool scoutfs_trans_has_dirty(struct super_block *sb) */ static void sub_holders_and_wake(struct super_block *sb, int val) { - struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); DECLARE_TRANS_INFO(sb, tri); atomic_sub(val, &tri->holders); smp_mb(); /* make sure sub is visible before we wake */ - if (waitqueue_active(&sbi->trans_hold_wq)) - wake_up(&sbi->trans_hold_wq); + if (waitqueue_active(&tri->hold_wq)) + wake_up(&tri->hold_wq); } /* @@ -170,20 +178,19 @@ static bool drained_holders(struct trans_info *tri) */ void scoutfs_trans_write_func(struct work_struct *work) { - struct scoutfs_sb_info *sbi = container_of(work, struct scoutfs_sb_info, - trans_write_work.work); - struct super_block *sb = sbi->sb; - DECLARE_TRANS_INFO(sb, tri); + struct trans_info *tri = container_of(work, struct trans_info, write_work.work); + struct super_block *sb = tri->sb; + struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); u64 trans_seq = sbi->trans_seq; char *s = NULL; int ret = 0; - sbi->trans_task = current; + tri->task = current; /* mark that we're writing so holders wait for us to finish and clear our bit */ atomic_add(TRANS_HOLDERS_WRITE_FUNC_BIT, &tri->holders); - wait_event(sbi->trans_hold_wq, drained_holders(tri)); + wait_event(tri->hold_wq, drained_holders(tri)); if (scoutfs_forcing_unmount(sb)) { ret = -EIO; @@ -193,7 +200,7 @@ void scoutfs_trans_write_func(struct work_struct *work) trace_scoutfs_trans_write_func(sb, scoutfs_block_writer_dirty_bytes(sb, &tri->wri), scoutfs_item_dirty_pages(sb)); - if (sbi->trans_deadline_expired) + if (tri->deadline_expired) scoutfs_inc_counter(sb, trans_commit_timer); scoutfs_inc_counter(sb, trans_commit_written); @@ -213,17 +220,17 @@ void scoutfs_trans_write_func(struct work_struct *work) s, ret); out: - spin_lock(&sbi->trans_write_lock); - sbi->trans_write_count++; - sbi->trans_write_ret = ret; + spin_lock(&tri->write_lock); + tri->write_count++; + tri->write_ret = ret; sbi->trans_seq = trans_seq; - spin_unlock(&sbi->trans_write_lock); - wake_up(&sbi->trans_write_wq); + spin_unlock(&tri->write_lock); + wake_up(&tri->write_wq); /* we're done, wake waiting holders */ sub_holders_and_wake(sb, TRANS_HOLDERS_WRITE_FUNC_BIT); - sbi->trans_task = NULL; + tri->task = NULL; scoutfs_trans_restart_sync_deadline(sb); } @@ -234,17 +241,17 @@ struct write_attempt { }; /* this is called as a wait_event() condition so it can't change task state */ -static int write_attempted(struct scoutfs_sb_info *sbi, - struct write_attempt *attempt) +static int write_attempted(struct super_block *sb, struct write_attempt *attempt) { + DECLARE_TRANS_INFO(sb, tri); int done = 1; - spin_lock(&sbi->trans_write_lock); - if (sbi->trans_write_count > attempt->count) - attempt->ret = sbi->trans_write_ret; + spin_lock(&tri->write_lock); + if (tri->write_count > attempt->count) + attempt->ret = tri->write_ret; else done = 0; - spin_unlock(&sbi->trans_write_lock); + spin_unlock(&tri->write_lock); return done; } @@ -254,10 +261,12 @@ static int write_attempted(struct scoutfs_sb_info *sbi, * We always have delayed sync work pending but the caller wants it * to execute immediately. */ -static void queue_trans_work(struct scoutfs_sb_info *sbi) +static void queue_trans_work(struct super_block *sb) { - sbi->trans_deadline_expired = false; - mod_delayed_work(sbi->trans_write_workq, &sbi->trans_write_work, 0); + DECLARE_TRANS_INFO(sb, tri); + + tri->deadline_expired = false; + mod_delayed_work(tri->write_workq, &tri->write_work, 0); } /* @@ -270,23 +279,23 @@ static void queue_trans_work(struct scoutfs_sb_info *sbi) */ int scoutfs_trans_sync(struct super_block *sb, int wait) { - struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); + DECLARE_TRANS_INFO(sb, tri); struct write_attempt attempt = { .ret = 0 }; int ret; if (!wait) { - queue_trans_work(sbi); + queue_trans_work(sb); return 0; } - spin_lock(&sbi->trans_write_lock); - attempt.count = sbi->trans_write_count; - spin_unlock(&sbi->trans_write_lock); + spin_lock(&tri->write_lock); + attempt.count = tri->write_count; + spin_unlock(&tri->write_lock); - queue_trans_work(sbi); + queue_trans_work(sb); - wait_event(sbi->trans_write_wq, write_attempted(sbi, &attempt)); + wait_event(tri->write_wq, write_attempted(sb, &attempt)); ret = attempt.ret; return ret; @@ -303,10 +312,10 @@ int scoutfs_file_fsync(struct file *file, loff_t start, loff_t end, void scoutfs_trans_restart_sync_deadline(struct super_block *sb) { - struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); + DECLARE_TRANS_INFO(sb, tri); - sbi->trans_deadline_expired = true; - mod_delayed_work(sbi->trans_write_workq, &sbi->trans_write_work, + tri->deadline_expired = true; + mod_delayed_work(tri->write_workq, &tri->write_work, TRANS_SYNC_DELAY); } @@ -455,12 +464,11 @@ static bool holders_no_writer(struct trans_info *tri) */ int scoutfs_hold_trans(struct super_block *sb, bool allocing) { - struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); DECLARE_TRANS_INFO(sb, tri); u64 seq; int ret; - if (current == sbi->trans_task) + if (current == tri->task) return 0; for (;;) { @@ -474,7 +482,7 @@ int scoutfs_hold_trans(struct super_block *sb, bool allocing) /* wait until the writer work is finished */ if (!inc_holders_unless_writer(tri)) { dec_journal_info_holders(); - wait_event(sbi->trans_hold_wq, holders_no_writer(tri)); + wait_event(tri->hold_wq, holders_no_writer(tri)); continue; } @@ -489,8 +497,8 @@ int scoutfs_hold_trans(struct super_block *sb, bool allocing) if (commit_before_hold(sb, tri)) { seq = scoutfs_trans_sample_seq(sb); release_holders(sb); - queue_trans_work(sbi); - wait_event(sbi->trans_hold_wq, scoutfs_trans_sample_seq(sb) != seq); + queue_trans_work(sb); + wait_event(tri->hold_wq, scoutfs_trans_sample_seq(sb) != seq); continue; } @@ -516,10 +524,9 @@ bool scoutfs_trans_held(void) void scoutfs_release_trans(struct super_block *sb) { - struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); DECLARE_TRANS_INFO(sb, tri); - if (current == sbi->trans_task) + if (current == tri->task) return; release_holders(sb); @@ -534,12 +541,13 @@ void scoutfs_release_trans(struct super_block *sb) */ u64 scoutfs_trans_sample_seq(struct super_block *sb) { + DECLARE_TRANS_INFO(sb, tri); struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); u64 ret; - spin_lock(&sbi->trans_write_lock); + spin_lock(&tri->write_lock); ret = sbi->trans_seq; - spin_unlock(&sbi->trans_write_lock); + spin_unlock(&tri->write_lock); return ret; } @@ -553,12 +561,17 @@ int scoutfs_setup_trans(struct super_block *sb) if (!tri) return -ENOMEM; + tri->sb = sb; atomic_set(&tri->holders, 0); scoutfs_block_writer_init(sb, &tri->wri); - sbi->trans_write_workq = alloc_workqueue("scoutfs_trans", - WQ_UNBOUND, 1); - if (!sbi->trans_write_workq) { + spin_lock_init(&tri->write_lock); + INIT_DELAYED_WORK(&tri->write_work, scoutfs_trans_write_func); + init_waitqueue_head(&tri->write_wq); + init_waitqueue_head(&tri->hold_wq); + + tri->write_workq = alloc_workqueue("scoutfs_trans", WQ_UNBOUND, 1); + if (!tri->write_workq) { kfree(tri); return -ENOMEM; } @@ -585,14 +598,14 @@ void scoutfs_shutdown_trans(struct super_block *sb) DECLARE_TRANS_INFO(sb, tri); if (tri) { - if (sbi->trans_write_workq) { + if (tri->write_workq) { /* immediately queues pending timer */ - flush_delayed_work(&sbi->trans_write_work); + flush_delayed_work(&tri->write_work); /* prevents re-arming if it has to wait */ - cancel_delayed_work_sync(&sbi->trans_write_work); - destroy_workqueue(sbi->trans_write_workq); + cancel_delayed_work_sync(&tri->write_work); + destroy_workqueue(tri->write_workq); /* trans work schedules after shutdown see null */ - sbi->trans_write_workq = NULL; + tri->write_workq = NULL; } scoutfs_block_writer_forget_all(sb, &tri->wri); From 80ee2c6d57b38e8f16e5049ac0aedbd831d1dde7 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Tue, 19 Oct 2021 14:24:51 -0700 Subject: [PATCH 19/26] Harden client transaction processing There are a few bad corner cases in the state machine that governs how client transactions are opened, modified, and committed. The worst problem is on the server side. All server request handlers need to cope with resent requests without causing bad side effects. Both get_log_trees and commit_log_trees would try to fully processes resent requests. _get_log_trees() looks safe because it works with the log_trees that was stored previously. _commit_log_trees() is not safe because it can rotate out the srch log file referenced by the sent log_trees every time it's processed. This could create extra srch entries which would delete the first instance of entries. Worse still, by injecting the same block structure into the system multiple times it ends up causing multiple frees of the blocks that make up the srch file. The client side problems are slightly different, but related. There aren't strong constraints which guarantee that we'll only send a commit request after a get request succeeds. In crazy circumstances the commit request in the write worker could come before the first get in mount succeeds. Far worse is that we can send multiple commit requests for one transaction if it changes as we get errors during multiple queued write attempts, particularly if we get errors from get_log_trees after having successfully committed. This hardens all these paths to ensure a strict sequence of get_log_trees, transaction modification, and commit_log_trees. On the server we add *_trans_seq fields to the log_trees struct so that both get_ and commit_ can see that they've already prepared a commit to send or have already committed the incoming commit, respectively. We can use the get_trans_seq field as the trans_seq of the open transaction and get rid of the entire seperate mechanism we used to have for tracking open trans seqs in the clients. We can get the same info by walking the log_trees and looking at their *_trans_seq fields. In the client we have the write worker immediately return success if mount hasn't opened the first transaction. Then we don't have the worker return to allow further modification until it has gotten success from get_log_trees. Signed-off-by: Zach Brown --- kmod/src/client.c | 15 --- kmod/src/client.h | 1 - kmod/src/format.h | 33 +++--- kmod/src/scoutfs_trace.h | 42 ------- kmod/src/server.c | 235 +++++++++++++-------------------------- kmod/src/super.c | 3 +- kmod/src/trans.c | 86 +++++++++----- utils/src/print.c | 16 --- 8 files changed, 155 insertions(+), 276 deletions(-) diff --git a/kmod/src/client.c b/kmod/src/client.c index 98f14526..38cf3ec7 100644 --- a/kmod/src/client.c +++ b/kmod/src/client.c @@ -117,21 +117,6 @@ int scoutfs_client_get_roots(struct super_block *sb, NULL, 0, roots, sizeof(*roots)); } -int scoutfs_client_advance_seq(struct super_block *sb, u64 *seq) -{ - struct client_info *client = SCOUTFS_SB(sb)->client_info; - __le64 leseq; - int ret; - - ret = scoutfs_net_sync_request(sb, client->conn, - SCOUTFS_NET_CMD_ADVANCE_SEQ, - NULL, 0, &leseq, sizeof(leseq)); - if (ret == 0) - *seq = le64_to_cpu(leseq); - - return ret; -} - int scoutfs_client_get_last_seq(struct super_block *sb, u64 *seq) { struct client_info *client = SCOUTFS_SB(sb)->client_info; diff --git a/kmod/src/client.h b/kmod/src/client.h index 07dab8a9..a360da1c 100644 --- a/kmod/src/client.h +++ b/kmod/src/client.h @@ -10,7 +10,6 @@ int scoutfs_client_commit_log_trees(struct super_block *sb, int scoutfs_client_get_roots(struct super_block *sb, struct scoutfs_net_roots *roots); u64 *scoutfs_client_bulk_alloc(struct super_block *sb); -int scoutfs_client_advance_seq(struct super_block *sb, u64 *seq); int scoutfs_client_get_last_seq(struct super_block *sb, u64 *seq); int scoutfs_client_lock_request(struct super_block *sb, struct scoutfs_net_lock *nl); diff --git a/kmod/src/format.h b/kmod/src/format.h index 9151e4a9..d94fbe82 100644 --- a/kmod/src/format.h +++ b/kmod/src/format.h @@ -207,10 +207,6 @@ struct scoutfs_key { #define sklt_rid _sk_first #define sklt_nr _sk_second -/* seqs */ -#define skts_trans_seq _sk_first -#define skts_rid _sk_second - /* mounted clients */ #define skmc_rid _sk_first @@ -461,6 +457,12 @@ struct scoutfs_srch_compact { * XXX I imagine we should rename these now that they've evolved to track * all the btrees that clients use during a transaction. It's not just * about item logs, it's about clients making changes to trees. + * + * @get_trans_seq, @commit_trans_seq: These pair of sequence numbers + * determine if a transaction is currently open for the mount that owns + * the log_trees struct. get_trans_seq is advanced by the server as the + * transaction is opened. The server sets comimt_trans_seq equal to + * get_ as the transaction is committed. */ struct scoutfs_log_trees { struct scoutfs_alloc_list_head meta_avail; @@ -473,6 +475,8 @@ struct scoutfs_log_trees { __le64 data_alloc_zone_blocks; __le64 data_alloc_zones[SCOUTFS_DATA_ALLOC_ZONE_LE64S]; __le64 inode_count_delta; + __le64 get_trans_seq; + __le64 commit_trans_seq; __le64 max_item_seq; __le64 finalize_seq; __le64 rid; @@ -586,17 +590,16 @@ struct scoutfs_log_merge_freeing { #define SCOUTFS_LOCK_ZONE 5 /* Items only stored in server btrees */ #define SCOUTFS_LOG_TREES_ZONE 6 -#define SCOUTFS_TRANS_SEQ_ZONE 7 -#define SCOUTFS_MOUNTED_CLIENT_ZONE 8 -#define SCOUTFS_SRCH_ZONE 9 -#define SCOUTFS_FREE_EXTENT_BLKNO_ZONE 10 -#define SCOUTFS_FREE_EXTENT_ORDER_ZONE 11 +#define SCOUTFS_MOUNTED_CLIENT_ZONE 7 +#define SCOUTFS_SRCH_ZONE 8 +#define SCOUTFS_FREE_EXTENT_BLKNO_ZONE 9 +#define SCOUTFS_FREE_EXTENT_ORDER_ZONE 10 /* Items only stored in log merge server btrees */ -#define SCOUTFS_LOG_MERGE_STATUS_ZONE 12 -#define SCOUTFS_LOG_MERGE_RANGE_ZONE 13 -#define SCOUTFS_LOG_MERGE_REQUEST_ZONE 14 -#define SCOUTFS_LOG_MERGE_COMPLETE_ZONE 15 -#define SCOUTFS_LOG_MERGE_FREEING_ZONE 16 +#define SCOUTFS_LOG_MERGE_STATUS_ZONE 11 +#define SCOUTFS_LOG_MERGE_RANGE_ZONE 12 +#define SCOUTFS_LOG_MERGE_REQUEST_ZONE 13 +#define SCOUTFS_LOG_MERGE_COMPLETE_ZONE 14 +#define SCOUTFS_LOG_MERGE_FREEING_ZONE 15 /* inode index zone */ #define SCOUTFS_INODE_INDEX_META_SEQ_TYPE 1 @@ -807,7 +810,6 @@ struct scoutfs_super_block { struct scoutfs_btree_root fs_root; struct scoutfs_btree_root logs_root; struct scoutfs_btree_root log_merge; - struct scoutfs_btree_root trans_seqs; struct scoutfs_btree_root mounted_clients; struct scoutfs_btree_root srch_root; struct scoutfs_volume_options volopt; @@ -990,7 +992,6 @@ enum scoutfs_net_cmd { SCOUTFS_NET_CMD_COMMIT_LOG_TREES, SCOUTFS_NET_CMD_SYNC_LOG_TREES, SCOUTFS_NET_CMD_GET_ROOTS, - SCOUTFS_NET_CMD_ADVANCE_SEQ, SCOUTFS_NET_CMD_GET_LAST_SEQ, SCOUTFS_NET_CMD_LOCK, SCOUTFS_NET_CMD_LOCK_RECOVER, diff --git a/kmod/src/scoutfs_trace.h b/kmod/src/scoutfs_trace.h index 635a1597..13ba3ae6 100644 --- a/kmod/src/scoutfs_trace.h +++ b/kmod/src/scoutfs_trace.h @@ -1980,48 +1980,6 @@ DEFINE_EVENT(scoutfs_clock_sync_class, scoutfs_recv_clock_sync, TP_ARGS(clock_sync_id) ); -TRACE_EVENT(scoutfs_trans_seq_advance, - TP_PROTO(struct super_block *sb, u64 rid, u64 trans_seq), - - TP_ARGS(sb, rid, trans_seq), - - TP_STRUCT__entry( - SCSB_TRACE_FIELDS - __field(__u64, s_rid) - __field(__u64, trans_seq) - ), - - TP_fast_assign( - SCSB_TRACE_ASSIGN(sb); - __entry->s_rid = rid; - __entry->trans_seq = trans_seq; - ), - - TP_printk(SCSBF" rid %016llx trans_seq %llu\n", - SCSB_TRACE_ARGS, __entry->s_rid, __entry->trans_seq) -); - -TRACE_EVENT(scoutfs_trans_seq_remove, - TP_PROTO(struct super_block *sb, u64 rid, u64 trans_seq), - - TP_ARGS(sb, rid, trans_seq), - - TP_STRUCT__entry( - SCSB_TRACE_FIELDS - __field(__u64, s_rid) - __field(__u64, trans_seq) - ), - - TP_fast_assign( - SCSB_TRACE_ASSIGN(sb); - __entry->s_rid = rid; - __entry->trans_seq = trans_seq; - ), - - TP_printk(SCSBF" rid %016llx trans_seq %llu", - SCSB_TRACE_ARGS, __entry->s_rid, __entry->trans_seq) -); - TRACE_EVENT(scoutfs_trans_seq_last, TP_PROTO(struct super_block *sb, u64 rid, u64 trans_seq), diff --git a/kmod/src/server.c b/kmod/src/server.c index af994546..f15d196a 100644 --- a/kmod/src/server.c +++ b/kmod/src/server.c @@ -73,9 +73,6 @@ struct server_info { struct llist_head commit_waiters; struct work_struct commit_work; - /* server tracks seq use */ - struct rw_semaphore seq_rwsem; - struct list_head clients; unsigned long nr_clients; @@ -1023,6 +1020,16 @@ static int finalize_and_start_log_merge(struct super_block *sb, struct scoutfs_l * * If the committed log trees are large enough we finalize them and make * them available to log merging. + * + * As we prepare a new transaction we get its get_trans_seq to indicate + * that it's open. The client uses this to identify its open + * transaction and we watch all the log trees to track the sequence + * numbers of transactions that clients have open. This limits the + * transaction sequence numbers that can be returned in the index of + * inodes by meta and data transaction numbers. We communicate the + * largest possible sequence number to clients via an rpc. The + * transactions are closed by setting the commit_trans_seq during commit + * or as the mount is cleaned up. */ static int server_get_log_trees(struct super_block *sb, struct scoutfs_net_connection *conn, @@ -1071,6 +1078,19 @@ static int server_get_log_trees(struct super_block *sb, lt.nr = cpu_to_le64(nr); } + /* the commit_trans_seq can never go past the open_trans_seq */ + if (le64_to_cpu(lt.get_trans_seq) < le64_to_cpu(lt.commit_trans_seq)) { + err_str = "invalid open_trans_seq and commit_trans_seq"; + ret = -EINVAL; + goto unlock; + } + + /* transaction's already open, client resent get_ after server failover */ + if (le64_to_cpu(lt.get_trans_seq) > le64_to_cpu(lt.commit_trans_seq)) { + ret = 0; + goto unlock; + } + /* drops and re-acquires the mutex and commit if it has to wait */ ret = finalize_and_start_log_merge(sb, <, rid); if (ret < 0) @@ -1151,6 +1171,9 @@ static int server_get_log_trees(struct super_block *sb, lt.data_alloc_zone_blocks = cpu_to_le64(data_zone_blocks); } + /* give the transaction a new seq (must have been ==) */ + lt.get_trans_seq = cpu_to_le64(scoutfs_server_next_seq(sb)); + /* update client's log tree's item */ scoutfs_key_init_log_trees(&key, le64_to_cpu(lt.rid), le64_to_cpu(lt.nr)); @@ -1187,9 +1210,11 @@ static int server_commit_log_trees(struct super_block *sb, const u64 rid = scoutfs_net_client_rid(conn); DECLARE_SERVER_INFO(sb, server); SCOUTFS_BTREE_ITEM_REF(iref); + struct scoutfs_log_trees *exist; struct scoutfs_log_trees lt; struct scoutfs_key key; char *err_str = NULL; + bool committed = false; int ret; if (arg_len != sizeof(struct scoutfs_log_trees)) { @@ -1214,12 +1239,26 @@ static int server_commit_log_trees(struct super_block *sb, scoutfs_key_init_log_trees(&key, le64_to_cpu(lt.rid), le64_to_cpu(lt.nr)); ret = scoutfs_btree_lookup(sb, &super->logs_root, &key, &iref); - if (ret < 0) { + if (ret < 0) err_str = "finding log trees item"; - goto unlock; - } - if (ret == 0) + if (ret == 0) { + if (iref.val_len == sizeof(struct scoutfs_log_trees)) { + exist = iref.val; + if (exist->get_trans_seq != lt.get_trans_seq) { + ret = -EIO; + err_str = "invalid log trees item get_trans_seq"; + } else { + if (exist->commit_trans_seq == lt.get_trans_seq) + committed = true; + } + } else { + ret = -EIO; + err_str = "invalid log trees item size"; + } scoutfs_btree_put_iref(&iref); + } + if (ret < 0 || committed) + goto unlock; /* try to rotate the srch log when big enough */ mutex_lock(&server->srch_mutex); @@ -1231,6 +1270,8 @@ static int server_commit_log_trees(struct super_block *sb, goto unlock; } + lt.commit_trans_seq = lt.get_trans_seq; + ret = scoutfs_btree_update(sb, &server->alloc, &server->wri, &super->logs_root, &key, <, sizeof(lt)); if (ret < 0) @@ -1357,6 +1398,9 @@ static int reclaim_open_log_tree(struct super_block *sb, u64 rid) alloc_move_empty(sb, &super->data_alloc, <.data_freed)); mutex_unlock(&server->alloc_mutex); + /* the transaction is no longer open */ + lt.commit_trans_seq = lt.get_trans_seq; + /* the mount is no longer writing to the zones */ zero_data_alloc_zone_bits(<); le64_add_cpu(<.flags, SCOUTFS_LOG_TREES_FINALIZED); @@ -1376,140 +1420,6 @@ out: return ret; } -static void init_trans_seq_key(struct scoutfs_key *key, u64 seq, u64 rid) -{ - *key = (struct scoutfs_key) { - .sk_zone = SCOUTFS_TRANS_SEQ_ZONE, - .skts_trans_seq = cpu_to_le64(seq), - .skts_rid = cpu_to_le64(rid), - }; -} - -/* - * Remove all trans_seq items owned by the client rid, the caller holds - * the seq_rwsem. - */ -static int remove_trans_seq_locked(struct super_block *sb, u64 rid) -{ - DECLARE_SERVER_INFO(sb, server); - struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); - struct scoutfs_super_block *super = &sbi->super; - SCOUTFS_BTREE_ITEM_REF(iref); - struct scoutfs_key key; - int ret = 0; - - init_trans_seq_key(&key, 0, 0); - - for (;;) { - ret = scoutfs_btree_next(sb, &super->trans_seqs, &key, &iref); - if (ret < 0) { - if (ret == -ENOENT) - ret = 0; - break; - } - - key = *iref.key; - scoutfs_btree_put_iref(&iref); - - if (le64_to_cpu(key.skts_rid) == rid) { - trace_scoutfs_trans_seq_remove(sb, rid, - le64_to_cpu(key.skts_trans_seq)); - ret = scoutfs_btree_delete(sb, &server->alloc, - &server->wri, - &super->trans_seqs, &key); - if (ret < 0) - break; - } - - scoutfs_key_inc(&key); - } - - return ret; -} - -/* - * Give the client the next sequence number for the transaction that - * they're opening. - * - * We track the sequence numbers of transactions that clients have open. - * This limits the transaction sequence numbers that can be returned in - * the index of inodes by meta and data transaction numbers. We - * communicate the largest possible sequence number to clients via an - * rpc. - * - * The transaction sequence tracking is stored in a btree so it is - * shared across servers. Final entries are removed when processing a - * client's farewell or when it's removed. We can be processent a - * resent request that was committed by a previous server before the - * reply was lost. At this point the client has no transactions open - * and may or may not have just finished one. To keep it simple we - * always remove any previous seq items, if there are any, and then - * insert a new item for the client at the next greatest seq. - */ -static int server_advance_seq(struct super_block *sb, - struct scoutfs_net_connection *conn, - u8 cmd, u64 id, void *arg, u16 arg_len) -{ - DECLARE_SERVER_INFO(sb, server); - struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); - struct scoutfs_super_block *super = &sbi->super; - u64 rid = scoutfs_net_client_rid(conn); - struct scoutfs_key key; - __le64 leseq = 0; - u64 seq; - int ret; - - if (arg_len != 0) { - ret = -EINVAL; - goto out; - } - - scoutfs_server_hold_commit(sb); - - down_write(&server->seq_rwsem); - - ret = remove_trans_seq_locked(sb, rid); - if (ret < 0) - goto unlock; - - seq = scoutfs_server_next_seq(sb); - - trace_scoutfs_trans_seq_advance(sb, rid, seq); - - init_trans_seq_key(&key, seq, rid); - ret = scoutfs_btree_insert(sb, &server->alloc, &server->wri, - &super->trans_seqs, &key, NULL, 0); - if (ret == 0) - leseq = cpu_to_le64(seq); -unlock: - up_write(&server->seq_rwsem); - ret = scoutfs_server_apply_commit(sb, ret); - -out: - return scoutfs_net_response(sb, conn, cmd, id, ret, - &leseq, sizeof(leseq)); -} - -/* - * Remove any transaction sequences owned by the client who's sent a - * farewell They must have committed any final transaction by the time - * they get here via sending their farewell message. This can be called - * multiple times as the client's farewell is retransmitted so it's OK - * to not find any entries. This is called with the server commit rwsem - * held. - */ -static int remove_trans_seq(struct super_block *sb, u64 rid) -{ - DECLARE_SERVER_INFO(sb, server); - int ret = 0; - - down_write(&server->seq_rwsem); - ret = remove_trans_seq_locked(sb, rid); - up_write(&server->seq_rwsem); - - return ret; -} - /* * Give the caller the last seq before outstanding client commits. All * seqs up to and including this are stable, new client transactions can @@ -1521,27 +1431,41 @@ static int get_stable_trans_seq(struct super_block *sb, u64 *last_seq_ret) struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); struct scoutfs_super_block *super = &sbi->super; SCOUTFS_BTREE_ITEM_REF(iref); + struct scoutfs_log_trees *lt; struct scoutfs_key key; u64 last_seq = 0; int ret; - down_read(&server->seq_rwsem); + last_seq = scoutfs_server_seq(sb) - 1; + scoutfs_key_init_log_trees(&key, 0, 0); - init_trans_seq_key(&key, 0, 0); - ret = scoutfs_btree_next(sb, &super->trans_seqs, &key, &iref); - if (ret == 0) { - last_seq = le64_to_cpu(iref.key->skts_trans_seq) - 1; - scoutfs_btree_put_iref(&iref); + mutex_lock(&server->logs_mutex); - } else if (ret == -ENOENT) { - last_seq = scoutfs_server_seq(sb) - 1; - ret = 0; + for (;; scoutfs_key_inc(&key)) { + ret = scoutfs_btree_next(sb, &super->logs_root, &key, &iref); + if (ret == 0) { + if (iref.val_len == sizeof(*lt)) { + lt = iref.val; + if ((le64_to_cpu(lt->get_trans_seq) > + le64_to_cpu(lt->commit_trans_seq)) && + le64_to_cpu(lt->get_trans_seq) <= last_seq) { + last_seq = le64_to_cpu(lt->get_trans_seq) - 1; + } + key = *iref.key; + } else { + ret = -EIO; + } + scoutfs_btree_put_iref(&iref); + } + if (ret < 0) { + if (ret == -ENOENT) { + ret = 0; + break; + } + } } - up_read(&server->seq_rwsem); - - if (ret < 0) - last_seq = 0; + mutex_unlock(&server->logs_mutex); *last_seq_ret = last_seq; return ret; @@ -3374,7 +3298,6 @@ static int reclaim_rid(struct super_block *sb, u64 rid) /* delete mounted client last, recovery looks for it */ ret = scoutfs_lock_server_farewell(sb, rid) ?: - remove_trans_seq(sb, rid) ?: reclaim_open_log_tree(sb, rid) ?: cancel_srch_compact(sb, rid) ?: cancel_log_merge(sb, rid) ?: @@ -3608,7 +3531,6 @@ static scoutfs_net_request_t server_req_funcs[] = { [SCOUTFS_NET_CMD_GET_LOG_TREES] = server_get_log_trees, [SCOUTFS_NET_CMD_COMMIT_LOG_TREES] = server_commit_log_trees, [SCOUTFS_NET_CMD_GET_ROOTS] = server_get_roots, - [SCOUTFS_NET_CMD_ADVANCE_SEQ] = server_advance_seq, [SCOUTFS_NET_CMD_GET_LAST_SEQ] = server_get_last_seq, [SCOUTFS_NET_CMD_LOCK] = server_lock, [SCOUTFS_NET_CMD_SRCH_GET_COMPACT] = server_srch_get_compact, @@ -4088,7 +4010,6 @@ int scoutfs_server_setup(struct super_block *sb) init_rwsem(&server->commit_rwsem); init_llist_head(&server->commit_waiters); INIT_WORK(&server->commit_work, scoutfs_server_commit_func); - init_rwsem(&server->seq_rwsem); INIT_LIST_HEAD(&server->clients); spin_lock_init(&server->farewell_lock); INIT_LIST_HEAD(&server->farewell_requests); diff --git a/kmod/src/super.c b/kmod/src/super.c index 63e82259..b2e821af 100644 --- a/kmod/src/super.c +++ b/kmod/src/super.c @@ -646,8 +646,7 @@ static int scoutfs_fill_super(struct super_block *sb, void *data, int silent) } /* send requests once iget progress shows we had a server */ - ret = scoutfs_trans_get_log_trees(sb) ?: - scoutfs_client_advance_seq(sb, &sbi->trans_seq); + ret = scoutfs_trans_get_log_trees(sb); if (ret) goto out; diff --git a/kmod/src/trans.c b/kmod/src/trans.c index f5980778..14e45c15 100644 --- a/kmod/src/trans.c +++ b/kmod/src/trans.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "super.h" #include "trans.h" @@ -100,6 +101,7 @@ static int commit_btrees(struct super_block *sb) */ int scoutfs_trans_get_log_trees(struct super_block *sb) { + struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); DECLARE_TRANS_INFO(sb, tri); struct scoutfs_log_trees lt; int ret = 0; @@ -112,6 +114,11 @@ int scoutfs_trans_get_log_trees(struct super_block *sb) scoutfs_forest_init_btrees(sb, &tri->alloc, &tri->wri, <); scoutfs_data_init_btrees(sb, &tri->alloc, &tri->wri, <); + + /* first set during mount from 0 to nonzero allows commits */ + spin_lock(&tri->write_lock); + sbi->trans_seq = le64_to_cpu(lt.get_trans_seq); + spin_unlock(&tri->write_lock); } return ret; } @@ -162,26 +169,22 @@ static bool drained_holders(struct trans_info *tri) * functions that would try to hold the transaction. We record the task * whose committing the transaction so that holding won't deadlock. * - * Any dirty block had to have allocated a new blkno which would have - * created dirty allocator metadata blocks. We can avoid writing - * entirely if we don't have any dirty metadata blocks. This is - * important because we don't try to serialize this work during - * unmount.. we can execute as the vfs is shutting down.. we need to - * decide that nothing is dirty without calling the vfs at all. + * Once we clear the write func bit in holders then waiting holders can + * enter the transaction and continue modifying the transaction. Once + * we start writing we consider the transaction done and won't exit, + * clearing the write func bit, until get_log_trees has opened the next + * transaction. The exception is forced unmount which is allowed to + * generate errors and throw away data. * - * We first try to sync the dirty inodes and write their dirty data blocks, - * then we write all our dirty metadata blocks, and only when those succeed - * do we write the new super that references all of these newly written blocks. - * - * If there are write errors then blocks are kept dirty in memory and will - * be written again at the next sync. + * This means that the only way fsync can return an error is if we're in + * forced unmount. */ void scoutfs_trans_write_func(struct work_struct *work) { struct trans_info *tri = container_of(work, struct trans_info, write_work.work); struct super_block *sb = tri->sb; struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); - u64 trans_seq = sbi->trans_seq; + bool retrying = false; char *s = NULL; int ret = 0; @@ -192,6 +195,12 @@ void scoutfs_trans_write_func(struct work_struct *work) wait_event(tri->hold_wq, drained_holders(tri)); + /* mount hasn't opened first transaction yet, still complete sync */ + if (sbi->trans_seq == 0) { + ret = 0; + goto out; + } + if (scoutfs_forcing_unmount(sb)) { ret = -EIO; goto out; @@ -205,25 +214,41 @@ void scoutfs_trans_write_func(struct work_struct *work) scoutfs_inc_counter(sb, trans_commit_written); - /* XXX this all needs serious work for dealing with errors */ - ret = (s = "data submit", scoutfs_inode_walk_writeback(sb, true)) ?: - (s = "item dirty", scoutfs_item_write_dirty(sb)) ?: - (s = "data prepare", scoutfs_data_prepare_commit(sb)) ?: - (s = "alloc prepare", scoutfs_alloc_prepare_commit(sb, &tri->alloc, &tri->wri)) ?: - (s = "meta write", scoutfs_block_writer_write(sb, &tri->wri)) ?: - (s = "data wait", scoutfs_inode_walk_writeback(sb, false)) ?: - (s = "commit log trees", commit_btrees(sb)) ?: scoutfs_item_write_done(sb) ?: - (s = "get log trees", scoutfs_trans_get_log_trees(sb)) ?: - (s = "advance seq", scoutfs_client_advance_seq(sb, &trans_seq)); - if (ret < 0) - scoutfs_err(sb, "critical transaction commit failure: %s, %d", - s, ret); + do { + ret = (s = "data submit", scoutfs_inode_walk_writeback(sb, true)) ?: + (s = "item dirty", scoutfs_item_write_dirty(sb)) ?: + (s = "data prepare", scoutfs_data_prepare_commit(sb)) ?: + (s = "alloc prepare", scoutfs_alloc_prepare_commit(sb, &tri->alloc, + &tri->wri)) ?: + (s = "meta write", scoutfs_block_writer_write(sb, &tri->wri)) ?: + (s = "data wait", scoutfs_inode_walk_writeback(sb, false)) ?: + (s = "commit log trees", commit_btrees(sb)) ?: + scoutfs_item_write_done(sb) ?: + (s = "get log trees", scoutfs_trans_get_log_trees(sb)); + if (ret < 0) { + if (!retrying) { + scoutfs_warn(sb, "critical transaction commit failure: %s = %d, retrying", + s, ret); + retrying = true; + } + + if (scoutfs_forcing_unmount(sb)) { + ret = -EIO; + break; + } + + msleep(2 * MSEC_PER_SEC); + + } else if (retrying) { + scoutfs_info(sb, "retried transaction commit succeeded"); + } + + } while (ret < 0); out: spin_lock(&tri->write_lock); tri->write_count++; tri->write_ret = ret; - sbi->trans_seq = trans_seq; spin_unlock(&tri->write_lock); wake_up(&tri->write_wq); @@ -464,6 +489,7 @@ static bool holders_no_writer(struct trans_info *tri) */ int scoutfs_hold_trans(struct super_block *sb, bool allocing) { + struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); DECLARE_TRANS_INFO(sb, tri); u64 seq; int ret; @@ -472,6 +498,12 @@ int scoutfs_hold_trans(struct super_block *sb, bool allocing) return 0; for (;;) { + /* shouldn't get holders until mount finishes, (not locking for cheap test) */ + if (WARN_ON_ONCE(sbi->trans_seq == 0)) { + ret = -EINVAL; + break; + } + /* if a caller already has a hold we acquire unconditionally */ if (inc_journal_info_holders()) { atomic_inc(&tri->holders); diff --git a/utils/src/print.c b/utils/src/print.c index 8e5d4852..1e5e996b 100644 --- a/utils/src/print.c +++ b/utils/src/print.c @@ -353,15 +353,6 @@ static int print_srch_root_item(struct scoutfs_key *key, u64 seq, u8 flags, void return 0; } -static int print_trans_seqs_entry(struct scoutfs_key *key, u64 seq, u8 flags, void *val, - unsigned val_len, void *arg) -{ - printf(" trans_seq %llu rid %016llx\n", - le64_to_cpu(key->skts_trans_seq), le64_to_cpu(key->skts_rid)); - - return 0; -} - static int print_mounted_client_entry(struct scoutfs_key *key, u64 seq, u8 flags, void *val, unsigned val_len, void *arg) { @@ -954,7 +945,6 @@ static void print_super_block(struct scoutfs_super_block *super, u64 blkno) " fs_root: "BTR_FMT"\n" " logs_root: "BTR_FMT"\n" " log_merge: "BTR_FMT"\n" - " trans_seqs: "BTR_FMT"\n" " mounted_clients: "BTR_FMT"\n" " srch_root: "BTR_FMT"\n", le64_to_cpu(super->next_ino), @@ -972,7 +962,6 @@ static void print_super_block(struct scoutfs_super_block *super, u64 blkno) BTR_ARG(&super->fs_root), BTR_ARG(&super->logs_root), BTR_ARG(&super->log_merge), - BTR_ARG(&super->trans_seqs), BTR_ARG(&super->mounted_clients), BTR_ARG(&super->srch_root)); @@ -1024,11 +1013,6 @@ static int print_volume(int fd) if (err && !ret) ret = err; - err = print_btree(fd, super, "trans_seqs", &super->trans_seqs, - print_trans_seqs_entry, NULL); - if (err && !ret) - ret = err; - err = print_btree(fd, super, "log_merge", &super->log_merge, print_log_merge_item, NULL); if (err && !ret) From 20ac2e35fa19b749a1e14a141edde78f43582029 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 25 Oct 2021 14:45:01 -0700 Subject: [PATCH 20/26] Remove clock_sync field from net message As we freeze the format let's remove this old experiment to try and make it easier to line up traces from different mounts. It never worked particularly well and I think it could be argued that trying to merge trace logs on different machines isn't a particularly meaningful thing to do. You care about how they interact not what they were doing at the same time with their indepdendent resources. Signed-off-by: Zach Brown --- kmod/src/format.h | 1 - kmod/src/net.c | 5 ----- kmod/src/scoutfs_trace.h | 26 -------------------------- kmod/src/super.c | 32 -------------------------------- kmod/src/super.h | 2 -- 5 files changed, 66 deletions(-) diff --git a/kmod/src/format.h b/kmod/src/format.h index d94fbe82..3cc2bb4f 100644 --- a/kmod/src/format.h +++ b/kmod/src/format.h @@ -970,7 +970,6 @@ struct scoutfs_net_greeting { * response messages. */ struct scoutfs_net_header { - __le64 clock_sync_id; __le64 seq; __le64 recv_seq; __le64 id; diff --git a/kmod/src/net.c b/kmod/src/net.c index 49ea837e..45e907ac 100644 --- a/kmod/src/net.c +++ b/kmod/src/net.c @@ -629,8 +629,6 @@ static void scoutfs_net_recv_worker(struct work_struct *work) break; } - trace_scoutfs_recv_clock_sync(nh.clock_sync_id); - data_len = le16_to_cpu(nh.data_len); scoutfs_inc_counter(sb, net_recv_messages); @@ -785,9 +783,6 @@ static void scoutfs_net_send_worker(struct work_struct *work) trace_scoutfs_net_send_message(sb, &conn->sockname, &conn->peername, &msend->nh); - msend->nh.clock_sync_id = scoutfs_clock_sync_id(); - trace_scoutfs_send_clock_sync(msend->nh.clock_sync_id); - ret = sendmsg_full(conn->sock, &msend->nh, len); spin_lock(&conn->lock); diff --git a/kmod/src/scoutfs_trace.h b/kmod/src/scoutfs_trace.h index 13ba3ae6..80db3247 100644 --- a/kmod/src/scoutfs_trace.h +++ b/kmod/src/scoutfs_trace.h @@ -1954,32 +1954,6 @@ TRACE_EVENT(scoutfs_quorum_loop, __entry->timeout_sec, __entry->timeout_nsec) ); -/* - * We can emit trace events to make it easier to synchronize the - * monotonic clocks in trace logs between nodes. By looking at the send - * and recv times of many messages flowing between nodes we can get - * surprisingly good estimates of the clock offset between them. - */ -DECLARE_EVENT_CLASS(scoutfs_clock_sync_class, - TP_PROTO(__le64 clock_sync_id), - TP_ARGS(clock_sync_id), - TP_STRUCT__entry( - __field(__u64, clock_sync_id) - ), - TP_fast_assign( - __entry->clock_sync_id = le64_to_cpu(clock_sync_id); - ), - TP_printk("clock_sync_id %016llx", __entry->clock_sync_id) -); -DEFINE_EVENT(scoutfs_clock_sync_class, scoutfs_send_clock_sync, - TP_PROTO(__le64 clock_sync_id), - TP_ARGS(clock_sync_id) -); -DEFINE_EVENT(scoutfs_clock_sync_class, scoutfs_recv_clock_sync, - TP_PROTO(__le64 clock_sync_id), - TP_ARGS(clock_sync_id) -); - TRACE_EVENT(scoutfs_trans_seq_last, TP_PROTO(struct super_block *sb, u64 rid, u64 trans_seq), diff --git a/kmod/src/super.c b/kmod/src/super.c index b2e821af..6dcb244f 100644 --- a/kmod/src/super.c +++ b/kmod/src/super.c @@ -20,7 +20,6 @@ #include #include #include -#include #include "super.h" #include "block.h" @@ -52,37 +51,6 @@ static struct dentry *scoutfs_debugfs_root; -static DEFINE_PER_CPU(u64, clock_sync_ids) = 0; - -/* - * Give the caller a unique clock sync id for a message they're about to - * send. We make the ids reasonably globally unique by using randomly - * initialized per-cpu 64bit counters. - */ -__le64 scoutfs_clock_sync_id(void) -{ - u64 rnd = 0; - u64 ret; - u64 *id; - -retry: - preempt_disable(); - id = this_cpu_ptr(&clock_sync_ids); - if (*id == 0) { - if (rnd == 0) { - preempt_enable(); - get_random_bytes(&rnd, sizeof(rnd)); - goto retry; - } - *id = rnd; - } - - ret = ++(*id); - preempt_enable(); - - return cpu_to_le64(ret); -} - /* the statfs file fields can be small (and signed?) :/ */ static __statfs_word saturate_truncated_word(u64 files) { diff --git a/kmod/src/super.h b/kmod/src/super.h index 4ca4b88f..51c11722 100644 --- a/kmod/src/super.h +++ b/kmod/src/super.h @@ -160,6 +160,4 @@ int scoutfs_write_super(struct super_block *sb, /* to keep this out of the ioctl.h public interface definition */ long scoutfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg); -__le64 scoutfs_clock_sync_id(void); - #endif From e38beee85a93e6f9f4ad4479f4b3c58455afde70 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 25 Oct 2021 15:10:55 -0700 Subject: [PATCH 21/26] Stop using inode index key type as array index The code that updates inode index items on behalf of indexed fields uses an array to track changes in the fields. Those array indexes were the raw key type values. We're about to introduce some sparse space between all the key values so that we have some room to add keys in the future at arbitrary sort positions amongst the previous keys. We don't want the inode index item updating code to keep using raw types as array indices when the type values are no longer small dense values. We introduce indirection from type values to array indices to keep the tracking array in the in-memory inode struct small. Signed-off-by: Zach Brown --- kmod/src/format.h | 1 - kmod/src/inode.c | 61 ++++++++++++++++++++++++++++++++++------------- kmod/src/inode.h | 6 +++-- 3 files changed, 48 insertions(+), 20 deletions(-) diff --git a/kmod/src/format.h b/kmod/src/format.h index 3cc2bb4f..6956a26a 100644 --- a/kmod/src/format.h +++ b/kmod/src/format.h @@ -604,7 +604,6 @@ struct scoutfs_log_merge_freeing { /* inode index zone */ #define SCOUTFS_INODE_INDEX_META_SEQ_TYPE 1 #define SCOUTFS_INODE_INDEX_DATA_SEQ_TYPE 2 -#define SCOUTFS_INODE_INDEX_NR 3 /* don't forget to update */ /* orphan zone, redundant type used for clarity */ #define SCOUTFS_ORPHAN_TYPE 1 diff --git a/kmod/src/inode.c b/kmod/src/inode.c index b4190dbf..bbc48daf 100644 --- a/kmod/src/inode.c +++ b/kmod/src/inode.c @@ -186,6 +186,37 @@ static void set_inode_ops(struct inode *inode) mapping_set_gfp_mask(inode->i_mapping, GFP_USER); } +static unsigned int item_index_arr_ind(u8 type) +{ + switch (type) { + case SCOUTFS_INODE_INDEX_META_SEQ_TYPE: return 0; break; + case SCOUTFS_INODE_INDEX_DATA_SEQ_TYPE: return 1; break; + /* should never get here, we control callers, not untrusted data */ + default: BUG(); break; + } +} + +static void set_item_major(struct scoutfs_inode_info *si, u8 type, __le64 maj) +{ + unsigned int ind = item_index_arr_ind(type); + + si->item_majors[ind] = le64_to_cpu(maj); +} + +static u64 get_item_major(struct scoutfs_inode_info *si, u8 type) +{ + unsigned int ind = item_index_arr_ind(type); + + return si->item_majors[ind]; +} + +static u64 get_item_minor(struct scoutfs_inode_info *si, u8 type) +{ + unsigned int ind = item_index_arr_ind(type); + + return si->item_minors[ind]; +} + /* * The caller has ensured that the fields in the incoming scoutfs inode * reflect both the inode item and the inode index items. This happens @@ -202,10 +233,8 @@ static void set_item_info(struct scoutfs_inode_info *si, memset(si->item_minors, 0, sizeof(si->item_minors)); si->have_item = true; - si->item_majors[SCOUTFS_INODE_INDEX_META_SEQ_TYPE] = - le64_to_cpu(sinode->meta_seq); - si->item_majors[SCOUTFS_INODE_INDEX_DATA_SEQ_TYPE] = - le64_to_cpu(sinode->data_seq); + set_item_major(si, SCOUTFS_INODE_INDEX_META_SEQ_TYPE, sinode->meta_seq); + set_item_major(si, SCOUTFS_INODE_INDEX_DATA_SEQ_TYPE, sinode->data_seq); } static void load_inode(struct inode *inode, struct scoutfs_inode *cinode) @@ -795,16 +824,14 @@ static bool will_del_index(struct scoutfs_inode_info *si, u8 type, u64 major, u32 minor) { return si && si->have_item && - (si->item_majors[type] != major || - si->item_minors[type] != minor); + (get_item_major(si, type) != major || get_item_minor(si, type) != minor); } static bool will_ins_index(struct scoutfs_inode_info *si, u8 type, u64 major, u32 minor) { return !si || !si->have_item || - (si->item_majors[type] != major || - si->item_minors[type] != minor); + (get_item_major(si, type) != major || get_item_minor(si, type) != minor); } static bool inode_has_index(umode_t mode, u8 type) @@ -912,14 +939,14 @@ static int update_index_items(struct super_block *sb, if (ret || !will_del_index(si, type, major, minor)) return ret; - trace_scoutfs_delete_index_item(sb, type, si->item_majors[type], - si->item_minors[type], ino); + trace_scoutfs_delete_index_item(sb, type, get_item_major(si, type), + get_item_minor(si, type), ino); - scoutfs_inode_init_index_key(&del, type, si->item_majors[type], - si->item_minors[type], ino); + scoutfs_inode_init_index_key(&del, type, get_item_major(si, type), + get_item_minor(si, type), ino); - del_lock = find_index_lock(lock_list, type, si->item_majors[type], - si->item_minors[type], ino); + del_lock = find_index_lock(lock_list, type, get_item_major(si, type), + get_item_minor(si, type), ino); ret = scoutfs_item_delete_force(sb, &del, del_lock); if (ret) { err = scoutfs_item_delete(sb, &ins, ins_lock); @@ -1056,8 +1083,8 @@ static int prepare_index_items(struct scoutfs_inode_info *si, } if (will_del_index(si, type, major, minor)) { - ret = add_index_lock(list, ino, type, si->item_majors[type], - si->item_minors[type]); + ret = add_index_lock(list, ino, type, get_item_major(si, type), + get_item_minor(si, type)); if (ret) return ret; } @@ -1076,7 +1103,7 @@ static u64 upd_data_seq(struct scoutfs_sb_info *sbi, if (!si || !si->have_item || set_data_seq) return sbi->trans_seq; - return si->item_majors[SCOUTFS_INODE_INDEX_DATA_SEQ_TYPE]; + return get_item_major(si, SCOUTFS_INODE_INDEX_DATA_SEQ_TYPE); } /* diff --git a/kmod/src/inode.h b/kmod/src/inode.h index 98f7db4b..9b98b966 100644 --- a/kmod/src/inode.h +++ b/kmod/src/inode.h @@ -9,6 +9,8 @@ struct scoutfs_lock; +#define SCOUTFS_INODE_NR_INDICES 2 + struct scoutfs_inode_info { /* read or initialized for each inode instance */ u64 ino; @@ -38,8 +40,8 @@ struct scoutfs_inode_info { */ struct mutex item_mutex; bool have_item; - u64 item_majors[SCOUTFS_INODE_INDEX_NR]; - u32 item_minors[SCOUTFS_INODE_INDEX_NR]; + u64 item_majors[SCOUTFS_INODE_NR_INDICES]; + u32 item_minors[SCOUTFS_INODE_NR_INDICES]; /* updated at on each new lock acquisition */ atomic64_t last_refreshed; From 9ebf43db9905e0122c7749eb1ca021a9061a427b Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 25 Oct 2021 15:16:07 -0700 Subject: [PATCH 22/26] Spread out key zone and type values Introduce some space between the current key zone and type values so that we have room to insert new keys amongst the current keys if we need to. A spacing of 4 is arbitrarily chosen as small enough to still give us intuitively small numbers while leaving enough room to grow, given how long its taken to come to the current number of keys. Signed-off-by: Zach Brown --- kmod/src/format.h | 60 +++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/kmod/src/format.h b/kmod/src/format.h index 6956a26a..bc6e5e6b 100644 --- a/kmod/src/format.h +++ b/kmod/src/format.h @@ -583,48 +583,48 @@ struct scoutfs_log_merge_freeing { /* * Keys are first sorted by major key zones. */ -#define SCOUTFS_INODE_INDEX_ZONE 1 -#define SCOUTFS_ORPHAN_ZONE 2 -#define SCOUTFS_XATTR_TOTL_ZONE 3 -#define SCOUTFS_FS_ZONE 4 -#define SCOUTFS_LOCK_ZONE 5 +#define SCOUTFS_INODE_INDEX_ZONE 4 +#define SCOUTFS_ORPHAN_ZONE 8 +#define SCOUTFS_XATTR_TOTL_ZONE 12 +#define SCOUTFS_FS_ZONE 16 +#define SCOUTFS_LOCK_ZONE 20 /* Items only stored in server btrees */ -#define SCOUTFS_LOG_TREES_ZONE 6 -#define SCOUTFS_MOUNTED_CLIENT_ZONE 7 -#define SCOUTFS_SRCH_ZONE 8 -#define SCOUTFS_FREE_EXTENT_BLKNO_ZONE 9 -#define SCOUTFS_FREE_EXTENT_ORDER_ZONE 10 +#define SCOUTFS_LOG_TREES_ZONE 24 +#define SCOUTFS_MOUNTED_CLIENT_ZONE 28 +#define SCOUTFS_SRCH_ZONE 32 +#define SCOUTFS_FREE_EXTENT_BLKNO_ZONE 36 +#define SCOUTFS_FREE_EXTENT_ORDER_ZONE 40 /* Items only stored in log merge server btrees */ -#define SCOUTFS_LOG_MERGE_STATUS_ZONE 11 -#define SCOUTFS_LOG_MERGE_RANGE_ZONE 12 -#define SCOUTFS_LOG_MERGE_REQUEST_ZONE 13 -#define SCOUTFS_LOG_MERGE_COMPLETE_ZONE 14 -#define SCOUTFS_LOG_MERGE_FREEING_ZONE 15 +#define SCOUTFS_LOG_MERGE_STATUS_ZONE 44 +#define SCOUTFS_LOG_MERGE_RANGE_ZONE 48 +#define SCOUTFS_LOG_MERGE_REQUEST_ZONE 52 +#define SCOUTFS_LOG_MERGE_COMPLETE_ZONE 56 +#define SCOUTFS_LOG_MERGE_FREEING_ZONE 60 /* inode index zone */ -#define SCOUTFS_INODE_INDEX_META_SEQ_TYPE 1 -#define SCOUTFS_INODE_INDEX_DATA_SEQ_TYPE 2 +#define SCOUTFS_INODE_INDEX_META_SEQ_TYPE 4 +#define SCOUTFS_INODE_INDEX_DATA_SEQ_TYPE 8 /* orphan zone, redundant type used for clarity */ -#define SCOUTFS_ORPHAN_TYPE 1 +#define SCOUTFS_ORPHAN_TYPE 4 /* fs zone */ -#define SCOUTFS_INODE_TYPE 1 -#define SCOUTFS_XATTR_TYPE 2 -#define SCOUTFS_DIRENT_TYPE 3 -#define SCOUTFS_READDIR_TYPE 4 -#define SCOUTFS_LINK_BACKREF_TYPE 5 -#define SCOUTFS_SYMLINK_TYPE 6 -#define SCOUTFS_DATA_EXTENT_TYPE 7 +#define SCOUTFS_INODE_TYPE 4 +#define SCOUTFS_XATTR_TYPE 8 +#define SCOUTFS_DIRENT_TYPE 12 +#define SCOUTFS_READDIR_TYPE 16 +#define SCOUTFS_LINK_BACKREF_TYPE 20 +#define SCOUTFS_SYMLINK_TYPE 24 +#define SCOUTFS_DATA_EXTENT_TYPE 28 /* lock zone, only ever found in lock ranges, never in persistent items */ -#define SCOUTFS_RENAME_TYPE 1 +#define SCOUTFS_RENAME_TYPE 4 /* srch zone, only in server btrees */ -#define SCOUTFS_SRCH_LOG_TYPE 1 -#define SCOUTFS_SRCH_BLOCKS_TYPE 2 -#define SCOUTFS_SRCH_PENDING_TYPE 3 -#define SCOUTFS_SRCH_BUSY_TYPE 4 +#define SCOUTFS_SRCH_LOG_TYPE 4 +#define SCOUTFS_SRCH_BLOCKS_TYPE 8 +#define SCOUTFS_SRCH_PENDING_TYPE 12 +#define SCOUTFS_SRCH_BUSY_TYPE 16 /* file data extents have start and len in key */ struct scoutfs_data_extent_val { From 618a7a4c4763f2e444d1c8662415e433eaad1558 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 27 Oct 2021 11:41:16 -0700 Subject: [PATCH 23/26] Remove unused lock server alloc and wri While checking in on some other code I noticed that we have lingering allocator and writer contexts over in the lock server. The lock server used to manage its own client state and recovery. We've sinced moved that into shared recov functionality in the server. The lock server no longer manipulates its own btrees and doesn't need these unused references to the server's contexts. Signed-off-by: Zach Brown --- kmod/src/lock_server.c | 9 +-------- kmod/src/lock_server.h | 4 +--- kmod/src/server.c | 2 +- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/kmod/src/lock_server.c b/kmod/src/lock_server.c index e9178962..2de5e9e7 100644 --- a/kmod/src/lock_server.c +++ b/kmod/src/lock_server.c @@ -80,9 +80,6 @@ struct lock_server_info { struct dentry *tseq_dentry; struct scoutfs_tseq_tree stats_tseq_tree; struct dentry *stats_tseq_dentry; - - struct scoutfs_alloc *alloc; - struct scoutfs_block_writer *wri; }; #define DECLARE_LOCK_SERVER_INFO(sb, name) \ @@ -815,9 +812,7 @@ static void stats_tseq_show(struct seq_file *m, struct scoutfs_tseq_entry *ent) * Setup the lock server. This is called before networking can deliver * requests. */ -int scoutfs_lock_server_setup(struct super_block *sb, - struct scoutfs_alloc *alloc, - struct scoutfs_block_writer *wri) +int scoutfs_lock_server_setup(struct super_block *sb) { struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb); struct lock_server_info *inf; @@ -831,8 +826,6 @@ int scoutfs_lock_server_setup(struct super_block *sb, inf->locks_root = RB_ROOT; scoutfs_tseq_tree_init(&inf->tseq_tree, lock_server_tseq_show); scoutfs_tseq_tree_init(&inf->stats_tseq_tree, stats_tseq_show); - inf->alloc = alloc; - inf->wri = wri; inf->tseq_dentry = scoutfs_tseq_create("server_locks", sbi->debug_root, &inf->tseq_tree); diff --git a/kmod/src/lock_server.h b/kmod/src/lock_server.h index 60ce31ce..adb76fd7 100644 --- a/kmod/src/lock_server.h +++ b/kmod/src/lock_server.h @@ -11,9 +11,7 @@ int scoutfs_lock_server_response(struct super_block *sb, u64 rid, struct scoutfs_net_lock *nl); int scoutfs_lock_server_farewell(struct super_block *sb, u64 rid); -int scoutfs_lock_server_setup(struct super_block *sb, - struct scoutfs_alloc *alloc, - struct scoutfs_block_writer *wri); +int scoutfs_lock_server_setup(struct super_block *sb); void scoutfs_lock_server_destroy(struct super_block *sb); #endif diff --git a/kmod/src/server.c b/kmod/src/server.c index f15d196a..1b338775 100644 --- a/kmod/src/server.c +++ b/kmod/src/server.c @@ -3887,7 +3887,7 @@ static void scoutfs_server_worker(struct work_struct *work) } scoutfs_server_set_seq_if_greater(sb, max_seq); - ret = scoutfs_lock_server_setup(sb, &server->alloc, &server->wri); + ret = scoutfs_lock_server_setup(sb); if (ret) { scoutfs_err(sb, "server error %d starting lock server", ret); goto shutdown; From 932a842ae3e35373ac882f1460e699ec6166ffb8 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 27 Oct 2021 14:43:50 -0700 Subject: [PATCH 24/26] Remove valid_bytes from stat _more ioctls The idea here was that we'd expand the size of the struct and valid_bytes would tell the kernel which fields were present in userspace's struct. That doesn't combine well with the ioctl convention of having the size of the type baked into the ioctl number. We'll remove this to make the world less surprising. If we expand the interface we'd add additional ioctls and types. Signed-off-by: Zach Brown --- kmod/src/ioctl.c | 14 ++------------ kmod/src/ioctl.h | 20 -------------------- tests/src/stage_tmpfile.c | 1 - utils/src/df.c | 1 - utils/src/stat.c | 2 -- 5 files changed, 2 insertions(+), 36 deletions(-) diff --git a/kmod/src/ioctl.c b/kmod/src/ioctl.c index dc3d12da..014527ec 100644 --- a/kmod/src/ioctl.c +++ b/kmod/src/ioctl.c @@ -546,11 +546,6 @@ static long scoutfs_ioc_stat_more(struct file *file, unsigned long arg) struct scoutfs_inode_info *si = SCOUTFS_I(inode); struct scoutfs_ioctl_stat_more stm; - if (get_user(stm.valid_bytes, (__u64 __user *)arg)) - return -EFAULT; - - stm.valid_bytes = min_t(u64, stm.valid_bytes, - sizeof(struct scoutfs_ioctl_stat_more)); stm.meta_seq = scoutfs_inode_meta_seq(inode); stm.data_seq = scoutfs_inode_data_seq(inode); stm.data_version = scoutfs_inode_data_version(inode); @@ -558,7 +553,7 @@ static long scoutfs_ioc_stat_more(struct file *file, unsigned long arg) stm.crtime_sec = si->crtime.tv_sec; stm.crtime_nsec = si->crtime.tv_nsec; - if (copy_to_user((void __user *)arg, &stm, stm.valid_bytes)) + if (copy_to_user((void __user *)arg, &stm, sizeof(stm))) return -EFAULT; return 0; @@ -879,9 +874,6 @@ static long scoutfs_ioc_statfs_more(struct file *file, unsigned long arg) struct scoutfs_ioctl_statfs_more sfm; int ret; - if (get_user(sfm.valid_bytes, (__u64 __user *)arg)) - return -EFAULT; - super = kzalloc(sizeof(struct scoutfs_super_block), GFP_NOFS); if (!super) return -ENOMEM; @@ -890,8 +882,6 @@ static long scoutfs_ioc_statfs_more(struct file *file, unsigned long arg) if (ret) goto out; - sfm.valid_bytes = min_t(u64, sfm.valid_bytes, - sizeof(struct scoutfs_ioctl_statfs_more)); sfm.fsid = le64_to_cpu(super->hdr.fsid); sfm.rid = sbi->rid; sfm.total_meta_blocks = le64_to_cpu(super->total_meta_blocks); @@ -902,7 +892,7 @@ static long scoutfs_ioc_statfs_more(struct file *file, unsigned long arg) if (ret) goto out; - if (copy_to_user((void __user *)arg, &sfm, sfm.valid_bytes)) + if (copy_to_user((void __user *)arg, &sfm, sizeof(sfm))) ret = -EFAULT; else ret = 0; diff --git a/kmod/src/ioctl.h b/kmod/src/ioctl.h index f0f0037d..940d67f3 100644 --- a/kmod/src/ioctl.h +++ b/kmod/src/ioctl.h @@ -215,18 +215,8 @@ struct scoutfs_ioctl_stage { /* * Give the user inode fields that are not otherwise visible. statx() * isn't always available and xattrs are relatively expensive. - * - * @valid_bytes stores the number of bytes that are valid in the - * structure. The caller sets this to the size of the struct that they - * understand. The kernel then fills and copies back the min of the - * size they and the user caller understand. The user can tell if a - * field is set if all of its bytes are within the valid_bytes that the - * kernel set on return. - * - * New fields are only added to the end of the struct. */ struct scoutfs_ioctl_stat_more { - __u64 valid_bytes; __u64 meta_seq; __u64 data_seq; __u64 data_version; @@ -356,21 +346,11 @@ struct scoutfs_ioctl_search_xattrs { /* * Give the user information about the filesystem. * - * @valid_bytes stores the number of bytes that are valid in the - * structure. The caller sets this to the size of the struct that they - * understand. The kernel then fills and copies back the min of the - * size they and the user caller understand. The user can tell if a - * field is set if all of its bytes are within the valid_bytes that the - * kernel set on return. - * * @committed_seq: All seqs up to and including this seq have been * committed. Can be compared with meta_seq and data_seq from inodes in * stat_more to discover if changes have been committed to disk. - * - * New fields are only added to the end of the struct. */ struct scoutfs_ioctl_statfs_more { - __u64 valid_bytes; __u64 fsid; __u64 rid; __u64 committed_seq; diff --git a/tests/src/stage_tmpfile.c b/tests/src/stage_tmpfile.c index 35cc403b..8ecfc042 100644 --- a/tests/src/stage_tmpfile.c +++ b/tests/src/stage_tmpfile.c @@ -113,7 +113,6 @@ int main(int argc, char **argv) } // get current data_version after fallocate's size extensions - stm.valid_bytes = sizeof(struct scoutfs_ioctl_stat_more); ret = ioctl(dest_fd, SCOUTFS_IOC_STAT_MORE, &stm); if (ret < 0) { perror("stat_more ioctl error"); diff --git a/utils/src/df.c b/utils/src/df.c index 585d658c..53e28b99 100644 --- a/utils/src/df.c +++ b/utils/src/df.c @@ -48,7 +48,6 @@ static int do_df(struct df_args *args) if (fd < 0) return fd; - sfm.valid_bytes = sizeof(struct scoutfs_ioctl_statfs_more); ret = ioctl(fd, SCOUTFS_IOC_STATFS_MORE, &sfm); if (ret < 0) { fprintf(stderr, "statfs_more returned %d: error %s (%d)\n", diff --git a/utils/src/stat.c b/utils/src/stat.c index a0679a71..6b8ae407 100644 --- a/utils/src/stat.c +++ b/utils/src/stat.c @@ -131,12 +131,10 @@ static int do_stat(struct stat_args *args) if (args->is_inode) { cmd = SCOUTFS_IOC_STAT_MORE; fields = inode_fields; - st.stm.valid_bytes = sizeof(struct scoutfs_ioctl_stat_more); pr = print_inode_field; } else { cmd = SCOUTFS_IOC_STATFS_MORE; fields = fs_fields; - st.sfm.valid_bytes = sizeof(struct scoutfs_ioctl_statfs_more); pr = print_fs_field; } From 8f60ac06c51561d1313ff11d852f0f0ae4e5dd35 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 27 Oct 2021 14:53:29 -0700 Subject: [PATCH 25/26] Clean up our ioctl numbers We had arbitrarily chosen an ioctl code 's' to match scoutfs, but of course that conflicts. This chooses an arbitrary hole in the upstream reservations from inode-numbers.rst. Then we make sure to have our _IO[WR] usage reflect the direction of the final type paramater. For most of our ioctls userspace is writing an argument parameter to perform an operation (that often has side effects). Most of our ioctls should be _IOW because userspace is writing the parameter, not _IOR (though the operation tends to read state). A few ioctls copy output back to userspace in the parameter so they're _IOWR. Signed-off-by: Zach Brown --- kmod/src/ioctl.h | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/kmod/src/ioctl.h b/kmod/src/ioctl.h index 940d67f3..24cd9a46 100644 --- a/kmod/src/ioctl.h +++ b/kmod/src/ioctl.h @@ -13,8 +13,7 @@ * This is enforced by pahole scripting in external build environments. */ -/* XXX I have no idea how these are chosen. */ -#define SCOUTFS_IOCTL_MAGIC 's' +#define SCOUTFS_IOCTL_MAGIC 0xE8 /* arbitrarily chosen hole in ioctl-number.rst */ /* * Packed scoutfs keys rarely cross the ioctl boundary so we have a @@ -88,7 +87,7 @@ enum scoutfs_ino_walk_seq_type { * Adds entries to the user's buffer for each inode that is found in the * given index between the first and last positions. */ -#define SCOUTFS_IOC_WALK_INODES _IOR(SCOUTFS_IOCTL_MAGIC, 1, \ +#define SCOUTFS_IOC_WALK_INODES _IOW(SCOUTFS_IOCTL_MAGIC, 1, \ struct scoutfs_ioctl_walk_inodes) /* @@ -167,7 +166,7 @@ struct scoutfs_ioctl_ino_path_result { }; /* Get a single path from the root to the given inode number */ -#define SCOUTFS_IOC_INO_PATH _IOR(SCOUTFS_IOCTL_MAGIC, 2, \ +#define SCOUTFS_IOC_INO_PATH _IOW(SCOUTFS_IOCTL_MAGIC, 2, \ struct scoutfs_ioctl_ino_path) /* @@ -254,7 +253,7 @@ struct scoutfs_ioctl_data_waiting { #define SCOUTFS_IOC_DATA_WAITING_FLAGS_UNKNOWN (U64_MAX << 0) -#define SCOUTFS_IOC_DATA_WAITING _IOR(SCOUTFS_IOCTL_MAGIC, 6, \ +#define SCOUTFS_IOC_DATA_WAITING _IOW(SCOUTFS_IOCTL_MAGIC, 6, \ struct scoutfs_ioctl_data_waiting) /* @@ -286,8 +285,8 @@ struct scoutfs_ioctl_listxattr_hidden { __u32 hash_pos; }; -#define SCOUTFS_IOC_LISTXATTR_HIDDEN _IOR(SCOUTFS_IOCTL_MAGIC, 8, \ - struct scoutfs_ioctl_listxattr_hidden) +#define SCOUTFS_IOC_LISTXATTR_HIDDEN _IOWR(SCOUTFS_IOCTL_MAGIC, 8, \ + struct scoutfs_ioctl_listxattr_hidden) /* * Return the inode numbers of inodes which might contain the given @@ -340,8 +339,8 @@ struct scoutfs_ioctl_search_xattrs { /* set in output_flags if returned inodes reached last_ino */ #define SCOUTFS_SEARCH_XATTRS_OFLAG_END (1ULL << 0) -#define SCOUTFS_IOC_SEARCH_XATTRS _IOR(SCOUTFS_IOCTL_MAGIC, 9, \ - struct scoutfs_ioctl_search_xattrs) +#define SCOUTFS_IOC_SEARCH_XATTRS _IOW(SCOUTFS_IOCTL_MAGIC, 9, \ + struct scoutfs_ioctl_search_xattrs) /* * Give the user information about the filesystem. @@ -377,7 +376,7 @@ struct scoutfs_ioctl_data_wait_err { __s64 err; }; -#define SCOUTFS_IOC_DATA_WAIT_ERR _IOR(SCOUTFS_IOCTL_MAGIC, 11, \ +#define SCOUTFS_IOC_DATA_WAIT_ERR _IOW(SCOUTFS_IOCTL_MAGIC, 11, \ struct scoutfs_ioctl_data_wait_err) @@ -396,7 +395,7 @@ struct scoutfs_ioctl_alloc_detail_entry { __u8 __pad[6]; }; -#define SCOUTFS_IOC_ALLOC_DETAIL _IOR(SCOUTFS_IOCTL_MAGIC, 12, \ +#define SCOUTFS_IOC_ALLOC_DETAIL _IOW(SCOUTFS_IOCTL_MAGIC, 12, \ struct scoutfs_ioctl_alloc_detail) /* @@ -459,7 +458,7 @@ struct scoutfs_ioctl_move_blocks { __u64 flags; }; -#define SCOUTFS_IOC_MOVE_BLOCKS _IOR(SCOUTFS_IOCTL_MAGIC, 13, \ +#define SCOUTFS_IOC_MOVE_BLOCKS _IOW(SCOUTFS_IOCTL_MAGIC, 13, \ struct scoutfs_ioctl_move_blocks) struct scoutfs_ioctl_resize_devices { @@ -468,7 +467,7 @@ struct scoutfs_ioctl_resize_devices { }; #define SCOUTFS_IOC_RESIZE_DEVICES \ - _IOR(SCOUTFS_IOCTL_MAGIC, 14, struct scoutfs_ioctl_resize_devices) + _IOW(SCOUTFS_IOCTL_MAGIC, 14, struct scoutfs_ioctl_resize_devices) #define SCOUTFS_IOCTL_XATTR_TOTAL_NAME_NR 3 @@ -519,6 +518,6 @@ struct scoutfs_ioctl_xattr_total { }; #define SCOUTFS_IOC_READ_XATTR_TOTALS \ - _IOR(SCOUTFS_IOCTL_MAGIC, 15, struct scoutfs_ioctl_read_xattr_totals) + _IOW(SCOUTFS_IOCTL_MAGIC, 15, struct scoutfs_ioctl_read_xattr_totals) #endif From 223ee5deef4130590098d907fbb4fd2f5b954018 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Wed, 27 Oct 2021 14:59:15 -0700 Subject: [PATCH 26/26] Declare v1 of the stable persistent format From now on if we make incompatible changes to structures or messages then we update the format version and ensure that the code can deal with all the versions in its supported range. Signed-off-by: Zach Brown --- kmod/src/format.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kmod/src/format.h b/kmod/src/format.h index bc6e5e6b..66bbf1c1 100644 --- a/kmod/src/format.h +++ b/kmod/src/format.h @@ -6,9 +6,9 @@ * structures that are communicated over the wire, and the protocol * behind the structures. */ -#define SCOUTFS_FORMAT_VERSION_MIN 0 +#define SCOUTFS_FORMAT_VERSION_MIN 1 #define SCOUTFS_FORMAT_VERSION_MIN_STR __stringify(SCOUTFS_FORMAT_VERSION_MIN) -#define SCOUTFS_FORMAT_VERSION_MAX 0 +#define SCOUTFS_FORMAT_VERSION_MAX 1 #define SCOUTFS_FORMAT_VERSION_MAX_STR __stringify(SCOUTFS_FORMAT_VERSION_MAX) /* statfs(2) f_type */