Message-ID: <CAHk-=wjMmSZzMJ3Xnskdg4+GGz=5p5p+GSYyFBTh0f-DgvdBWg@mail.gmail.com>
Date:   Sun, 8 May 2022 11:00:17 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Zhangfei Gao <zhangfei.gao@...mail.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        Jacob Pan <jacob.jun.pan@...ux.intel.com>,
        Dave Hansen <dave.hansen@...el.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>
Subject: Re: [GIT pull] core/urgent for v5.18-rc6

On Sun, May 8, 2022 at 5:05 AM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> A single bugfix for the PASID management code, which freed the PASID too
> early. The PASID needs to be tied to the mm lifetime, not to the address
> space lifetime.

So I have to once more complain about the -tip tree "Link:" usage.

Again, the commit has a link to the patch *submission*, which is
almost entirely useless. There's no link to the actual problem the
patch fixes.

It does have a "Fixes:" link, and it has an "explanation", but the
explanation doesn't actually make much sense to me.

The *sensible* thing for an iommu driver to do is to just do a
mmget/mmput, and then the whole "drop PASIX on last mmput() (ie in
__mmput)" makes lots of sense.

The commit claims that's not what the iommu drivers do, but it's
really hard to tell *what* it is that does a mmgrab. It just says

  "the IOMMU driver holds a reference on the mm itself, not the address space"

but then I tried to see where such references are held, and I couldn't find it.

*Must* users do mmget/mmput, and the commit even says that's the
logical thing to do. Apparently something that the iommu code relies
on does the mmgrab/mmdrop thing, but I really would have liked to know
what that is.

Yes, "mmgab" is the right thing to do if all you want is the "struct
mm_struct", and don't actually care about the page tables themselves,
and don't want to have the refcount keep page tables alive. But you'd
think the iommu code really does want the page tables.

So it would have been really nice to have an explanation for what it
was that did the mmgrab, especially since the commit itself makes it
clear that the logical thing to do seems to be just mmget/put. No?

I _really_ wish the -tip tree had more links to the actual problem
reports and explanations, rather than links to the patch submission.

One has the actual issue. The other just has the information that is
already in the commit, and the "Link:" adds almost no actual value
(yes, yes, you can see late "Acked-by" etc, but that really isn't
worth it).

I've pulled this, because I definitely believe the issue is real. I
just wish I could see _what_ the issue was.

Put another way: I can see that

    Reported-by: Zhangfei Gao <zhangfei.gao@...mail.com>

in the commit, but I don't have a clue what the actual report was, and
there really isn't enough information in the commit itself, except for
a fairly handwavy "Device drivers might, for instance, still need to
flush operations.."

I don't want to know what device drivers _might_ do. I would want to
have an actual pointer to what they do and where.

I suspect it's related to mmu_notifiers or something, but I really
would have liked a more exact "this is where things go wrong".

I also *suspect* that this is all about that _loong_ thread (picking
one email almost at random) here:

   https://lore.kernel.org/all/a139dbad-2f42-913b-677c-ef35f1eebfed@intel.com/

but if so, the confusion I see in that thread is even more reason to
have more background for what is going on.

Anyway, this is merged in my tree, but I'm having trouble following
the logic, which is why I'm writing this long email to complain about
lack of context.

               Linus

Powered by blists - more mailing lists