[Intel-gfx] [PATCH] file, i915: fix file reference for mmap_singleton()

[Linus Torvalds]

On Wed, 25 Oct 2023 at 10:36, Christian Brauner <brauner at kernel.org> 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