On Wed, 25 Oct 2023 at 10:36, Christian Brauner <brauner@xxxxxxxxxx> wrote: > > I did think that the loop didn't really matter for this case but since > it seemingly does paper over the weird semantics here let's drop it. > Anyway, pushed to: > > https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs.misc LGTM. We could then make the i915 mmap_singleton() do the proper loop over the whole thing. Not quite the way Jann suggested - which would not increment the file refcount properly, but instead of the current smp_store_mb(i915->gem.mmap_singleton, file); drm_dev_get(&i915->drm); return file; it could do something much more complicated like drm_dev_get(&i915->drm); if (cmpxchg(&i915->gem.mmap_singleton, NULL, file) == NULL) return file; // mmap_singleton wasn't NULL: it might be an old one in the // process of being torn down (with a zero refcount), or a new // one that was just installed that we should use instead fput(file); file = READ_ONCE(i915->gem.mmap_singleton); if (!file) goto repeat; // Is it valid? Just try again if (atomic_read(&file->f_count)) goto repeat; // We have a file pointer, but it's in the process of being torn // down but gem.mmap_singleton hasn't been cleared yet. Yield to // make progress. sched_yield(); goto repeat; which is disgusting, but would probably work. Note the "probably work". I'm handwaving: "something like the above". Presumably not even worth doing - I assume a correct client always just does a single mmap() before starting work. Linus
