Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <ZE5NR5Ml8I2/Ze0f@8bytes.org> <CAHk-=wiriLmq6OgLLF9seANqCJqjCrgUC384zcJUFtv3xJgVkQ@mail.gmail.com>
 <ZFUMja5MpRPWCRXw@8bytes.org>
In-Reply-To: <ZFUMja5MpRPWCRXw@8bytes.org>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Fri, 5 May 2023 11:03:46 -0700
Message-ID: <CAHk-=wjW4CzM9YZqwB3jU9mt7FKdctLWAbOcBQAwJ0_2eRmP=g@mail.gmail.com>
Subject: Re: [git pull] IOMMU Updates for Linux v6.4
To:     Joerg Roedel <joro@8bytes.org>
Cc:     Jason Gunthorpe <jgg@ziepe.ca>, Will Deacon <will@kernel.org>,
        linux-kernel@vger.kernel.org, iommu@lists.linux.dev
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, May 5, 2023 at 7:02=E2=80=AFAM Joerg Roedel <joro@8bytes.org> wrote=
:
>
> On Sun, Apr 30, 2023 at 01:07:45PM -0700, Linus Torvalds wrote:
> > Anyway, I'm not super-happy with how this all turned out. The example
> > merge seems to be wrong, and the Kconfig situation is confusing.
>
> You are right, looks like I missed some context when doing my example
> merge, sorry for that. I will take more care next time.

So I find the suggested merges useful even if they are wrong - it just
makes me look at what I did in an area that is clearly non-obvious.

And the part that made me unhappy about the merge was actually how
non-obvious it was, not so much the mis-merge itself.

That config option rename in particular I find to just be bad. We now
have some code that is *very* central, to the point where we have a
field for it in the 'struct mm_struct', and special callback for
fork() and exit(), and then the config option is called something
completely incomprehensible like 'IOMMU_SVA'?

That just smells to high heaven to me.

Now, I'm not saying that 'struct mm_struct' is some holy thing, and it
has a lot of other strange fields that get enabled by strange config
options. But I *am* saying that it's one of our most central data
structures in the kernel (just behind 'struct task_struct'), and when
it has something like this in it:

    #ifdef CONFIG_IOMMU_SVA
                u32 pasid;
    #endif

that just doesn't look right. "IOMMU_SVA" just isn't an obvious or
descriptive enough name to be involved in that data structure.

The old name ("CONFIG_IOMMU_SUPPORT") honestly made a ton more sense
in that context.

So I was a bit frustrated with the merge.  When there are merge
conflicts, the way I resolve code is to make the resolution make
sense. Most of the time that's completely trivial, then occasionally I
have to look at the code history to see what people were doing.

And in this case, I didn't feel like the end result made sense, if you
see what I mean. The resolution is _right_, but I'm not happy about
it.

                 Linus