diff --git a/kmod/Makefile b/kmod/Makefile index 63d814d1..999b3a1e 100644 --- a/kmod/Makefile +++ b/kmod/Makefile @@ -5,13 +5,6 @@ ifeq ($(SK_KSRC),) SK_KSRC := $(shell echo /lib/modules/`uname -r`/build) endif -# fail if sparse fails if we find it -ifeq ($(shell sparse && echo found),found) -SP = -else -SP = @: -endif - SCOUTFS_GIT_DESCRIBE ?= \ $(shell git describe --all --abbrev=6 --long 2>/dev/null || \ echo no-git) @@ -36,9 +29,7 @@ TARFILE = scoutfs-kmod-$(RPM_VERSION).tar all: module module: - $(MAKE) $(SCOUTFS_ARGS) - $(SP) $(MAKE) C=2 CF="-D__CHECK_ENDIAN__" $(SCOUTFS_ARGS) - + $(MAKE) CHECK=$(CURDIR)/src/sparse-filtered.sh C=1 CF="-D__CHECK_ENDIAN__" $(SCOUTFS_ARGS) modules_install: $(MAKE) $(SCOUTFS_ARGS) modules_install diff --git a/kmod/src/item.c b/kmod/src/item.c index 44357e6d..c137fa0c 100644 --- a/kmod/src/item.c +++ b/kmod/src/item.c @@ -2401,6 +2401,12 @@ out: * The caller has successfully committed all the dirty btree blocks that * contained the currently dirty items. Clear all the dirty items and * pages. + * + * This strange lock/trylock loop comes from sparse issuing spurious + * mismatched context warnings if we do anything (like unlock and relax) + * in the else branch of the failed trylock. We're jumping through + * hoops to not use the else but still drop and reacquire the dirty_lock + * if the trylock fails. */ int scoutfs_item_write_done(struct super_block *sb) { @@ -2409,40 +2415,30 @@ int scoutfs_item_write_done(struct super_block *sb) struct cached_item *tmp; struct cached_page *pg; -retry: spin_lock(&cinf->dirty_lock); - - while ((pg = list_first_entry_or_null(&cinf->dirty_list, - struct cached_page, - dirty_head))) { - - if (!write_trylock(&pg->rwlock)) { + while ((pg = list_first_entry_or_null(&cinf->dirty_list, struct cached_page, dirty_head))) { + if (write_trylock(&pg->rwlock)) { spin_unlock(&cinf->dirty_lock); - cpu_relax(); - goto retry; - } + list_for_each_entry_safe(item, tmp, &pg->dirty_list, + dirty_head) { + clear_item_dirty(sb, cinf, pg, item); + if (item->delta) + scoutfs_inc_counter(sb, item_delta_written); + + /* free deletion items */ + if (item->deletion || item->delta) + erase_item(pg, item); + else + item->persistent = 1; + } + + write_unlock(&pg->rwlock); + spin_lock(&cinf->dirty_lock); + } spin_unlock(&cinf->dirty_lock); - - list_for_each_entry_safe(item, tmp, &pg->dirty_list, - dirty_head) { - clear_item_dirty(sb, cinf, pg, item); - - if (item->delta) - scoutfs_inc_counter(sb, item_delta_written); - - /* free deletion items */ - if (item->deletion || item->delta) - erase_item(pg, item); - else - item->persistent = 1; - } - - write_unlock(&pg->rwlock); - spin_lock(&cinf->dirty_lock); - } - + } while (pg); spin_unlock(&cinf->dirty_lock); return 0; diff --git a/kmod/src/lock.c b/kmod/src/lock.c index ce77bb51..f27fb1e1 100644 --- a/kmod/src/lock.c +++ b/kmod/src/lock.c @@ -168,7 +168,6 @@ static int lock_invalidate(struct super_block *sb, struct scoutfs_lock *lock, enum scoutfs_lock_mode prev, enum scoutfs_lock_mode mode) { struct scoutfs_lock_coverage *cov; - struct scoutfs_lock_coverage *tmp; u64 ino, last; int ret = 0; @@ -192,19 +191,22 @@ static int lock_invalidate(struct super_block *sb, struct scoutfs_lock *lock, /* have to invalidate if we're not in the only usable case */ if (!(prev == SCOUTFS_LOCK_WRITE && mode == SCOUTFS_LOCK_READ)) { -retry: - /* remove cov items to tell users that their cache is stale */ + /* + * Remove cov items to tell users that their cache is + * stale. The unlock pattern comes from avoiding bad + * sparse warnings when taking else in a failed trylock. + */ spin_lock(&lock->cov_list_lock); - list_for_each_entry_safe(cov, tmp, &lock->cov_list, head) { - if (!spin_trylock(&cov->cov_lock)) { - spin_unlock(&lock->cov_list_lock); - cpu_relax(); - goto retry; + while ((cov = list_first_entry_or_null(&lock->cov_list, + struct scoutfs_lock_coverage, head))) { + if (spin_trylock(&cov->cov_lock)) { + list_del_init(&cov->head); + cov->lock = NULL; + spin_unlock(&cov->cov_lock); + scoutfs_inc_counter(sb, lock_invalidate_coverage); } - list_del_init(&cov->head); - cov->lock = NULL; - spin_unlock(&cov->cov_lock); - scoutfs_inc_counter(sb, lock_invalidate_coverage); + spin_unlock(&lock->cov_list_lock); + spin_lock(&lock->cov_list_lock); } spin_unlock(&lock->cov_list_lock); diff --git a/kmod/src/sparse-filtered.sh b/kmod/src/sparse-filtered.sh new file mode 100755 index 00000000..aacfb3bc --- /dev/null +++ b/kmod/src/sparse-filtered.sh @@ -0,0 +1,45 @@ +#!/bin/bash + +# +# Unfortunately, kernels can ship which contain sparse errors that are +# unrelated to us. +# +# The exit status of this filtering wrapper will indicate an error if +# sparse wasn't found or if there were any unfiltered output lines. It +# can hide error exit status from sparse or grep if they don't produce +# output that makes it past the filters. +# + +# must have sparse. Fail with error message, mask success path. +which sparse > /dev/null || exit 1 + +# initial unmatchable, additional added as RE+="|..." +RE="$^" + +# +# Darn. sparse has multi-line error messages, and I'd rather not bother +# with multi-line filters. So we'll just drop this context. +# +# command-line: note: in included file (through include/linux/netlink.h, include/linux/ethtool.h, include/linux/netdevice.h, include/net/sock.h, /root/scoutfs/kmod/src/kernelcompat.h, builtin): +# fprintf(stderr, "%s: note: in included file%s:\n", +# +RE+="|: note: in included file" + +# 3.10.0-1160.119.1.el7.x86_64.debug +# include/linux/posix_acl.h:138:9: warning: incorrect type in assignment (different address spaces) +# include/linux/posix_acl.h:138:9: expected struct posix_acl * +# include/linux/posix_acl.h:138:9: got struct posix_acl [noderef] * +RE+="|include/linux/posix_acl.h:" + +# 3.10.0-1160.119.1.el7.x86_64.debug +#include/uapi/linux/perf_event.h:146:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0) +RE+="|include/uapi/linux/perf_event.h:" + +# 4.18.0-513.24.1.el8_9.x86_64+debug' +#./include/linux/skbuff.h:824:1: warning: directive in macro's argument list +RE+="|include/linux/skbuff.h:" + +sparse "$@" |& \ + grep -E -v "($RE)" |& \ + awk '{ print $0 } END { exit NR > 0 }' +exit $? diff --git a/utils/sparse.sh b/utils/sparse.sh index 94590e44..0b93324f 100755 --- a/utils/sparse.sh +++ b/utils/sparse.sh @@ -1,7 +1,7 @@ #!/bin/bash -# can we find sparse? If not, we're done. -which sparse > /dev/null 2>&1 || exit 0 +# must have sparse. Fail with error message, mask success path. +which sparse > /dev/null || exit 1 # # one of the problems with using sparse in userspace is that it picks up