On Fri, Dec 20, 2024 at 09:09:00AM +0000, Bertrand Drouvot wrote: > Thanks for the report! I was not able able to reproduce (even with > asan-enabled) > but I think the test is wrong. Indeed the backend could fsync too during the > test > (see register_dirty_segment() and the case where the request queue is full). > > I think the test should look like the attached instead, thoughts?
Hmm. I cannot reproduce that here either. FWIW, I use these settings by default in my local builds, similarly to the CI which did not complain: ASAN_OPTIONS="print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0" UBSAN_OPTIONS="print_stacktrace=1:disable_coredump=0:abort_on_error=1:verbosity=2" CFLAGS+="-fsanitize=undefined " LDFLAGS+="-fsanitize=undefined " grassquit uses something a bit different, which don't allow me to see a problem with 027 even with fsync enabled: ASAN_OPTIONS="print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0:detect_stack_use_after_return=0" CFLAGS+="-fsanitize=address -fno-sanitize-recover=all " LDFLAGS+="-fsanitize=address -fno-sanitize-recover=all " Anyway, I was arriving at the same conclusion as you, because this is just telling us that there could be "some" sync activity. As rewritten, this would just check that the after-the-fact counter is never lower than the before-the-fact counter, which is still better than removing the query. I was thinking about doing the latter and remove the query, but I'm also OK with what you have here, keeping the query with a relaxed check. I'll go adjust that in a bit.. -- Michael
signature.asc
Description: PGP signature
