On Fri, 29 Nov 2019 21:07:33 +0100 Daniel Vetter <daniel@xxxxxxxx> wrote: > On Fri, Nov 29, 2019 at 02:40:34PM +0000, Steven Price wrote: > > On 29/11/2019 14:33, Boris Brezillon wrote: > > > On Fri, 29 Nov 2019 14:24:48 +0000 > > > Steven Price <steven.price@xxxxxxx> wrote: > > > > > >> On 29/11/2019 13:59, Boris Brezillon wrote: > > >>> If 2 threads change the MADVISE property of the same BO in parallel we > > >>> might end up with an shmem->madv value that's inconsistent with the > > >>> presence of the BO in the shrinker list. > > >> > > >> I'm a bit worried from the point of view of user space sanity that you > > >> observed this - but clearly the kernel should be robust! > > > > > > It's not something I observed, just found the race by inspecting the > > > code, and I thought it was worth fixing it. > > > > Good! ;) Your cover letter referring to a "debug session" made me think > > you'd actually hit all these. > > Time for some igt to make sure this is actually correct? That's not something I can easily trigger without instrumenting the code: I need 2 threads doing MADVISE with 2 different values, and there has to be a context switch between the drm_gem_shmem_madvise() call and the mutex_lock(shrinker_lock) one. And last but not least, I'll need a way to report such inconsistencies to userspace (trace?).