Re: CVS commit: src

[maya%NetBSD.org@localhost]

On Tue, Nov 05, 2019 at 08:19:18PM +0000, Maxime Villard wrote:
> Module Name:	src
> Committed By:	maxv
> Date:		Tue Nov  5 20:19:18 UTC 2019
> 
> Modified Files:
> 	src/share/mk: bsd.sys.mk
> 	src/sys/arch/amd64/amd64: machdep.c mptramp.S
> 	src/sys/arch/amd64/conf: GENERIC Makefile.amd64
> 	src/sys/arch/x86/x86: cpu.c
> 	src/sys/conf: files
> 	src/sys/kern: files.kern
> 	src/sys/lib/libkern: libkern.h
> 	src/sys/sys: atomic.h bus_proto.h cdefs.h systm.h
> Added Files:
> 	src/sys/arch/amd64/include: csan.h
> 	src/sys/kern: subr_csan.c
> 	src/sys/sys: csan.h
> 
> Log Message:
> Add Kernel Concurrency Sanitizer (kCSan) support. This sanitizer allows us
> to detect race conditions at runtime. It is a variation of TSan that is
> easy to implement and more suited to kernel internals, albeit theoretically
> less precise than TSan's happens-before.
> 
> We do basically two things:
> 
>  - On every KCSAN_NACCESSES (=2000) memory accesses, we create a cell
>    describing the access, and delay the calling CPU (10ms).
> 
>  - On all memory accesses, we verify if the memory we're reading/writing
>    is referenced in a cell already.
> 
> The combination of the two means that, if for example cpu0 does a read that
> is selected and cpu1 does a write at the same address, kCSan will fire,
> because cpu1's write collides with cpu0's read cell.
> 
> The coverage of the instrumentation is the same as that of kASan. Also, the
> code is organized in a way similar to kASan, so it is easy to add support
> for more architectures than amd64. kCSan is compatible with KCOV.
> 
> Reviewed by Kamil.
> 

I don't understand how you can distinguish a race from this condition:

	CPU0			CPU1


				mutex_enter
				write (recorded to cell)
				mutex_exit

	read (checked against record)

Which is legitimate.


+       for (i = 0; i < ncpu; i++) {
+               __builtin_memcpy(&old, &kcsan_cpus[i].cell, sizeof(old));
+ 
+               if (old.addr + old.size <= new.addr)
+                       continue;
+               if (new.addr + new.size <= old.addr)
+                       continue;
+               if (__predict_true(!old.write && !new.write))
+                       continue;
+               if (__predict_true(kcsan_access_is_atomic(&new, &old)))  
+                       continue;
+       
+               kcsan_report(&new, cpu_number(), &old, i);
+               break;
+       }

It looks like you are checking the current CPU too?