[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHk-=whwcMLwcQZTmWgCnSn=LHpQG+EBbWevJEj5YTKMiE_-oQ@mail.gmail.com>
Date: Mon, 15 Jul 2019 13:07:19 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Thomas Hellström (VMware)
<thomas@...pmail.org>
Cc: Dave Airlie <airlied@...il.com>,
Thomas Hellstrom <thellstrom@...are.com>,
Daniel Vetter <daniel.vetter@...ll.ch>,
LKML <linux-kernel@...r.kernel.org>,
dri-devel <dri-devel@...ts.freedesktop.org>,
Jerome Glisse <jglisse@...hat.com>,
Jason Gunthorpe <jgg@...lanox.com>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: drm pull for v5.3-rc1
On Mon, Jul 15, 2019 at 12:36 PM Thomas Hellström (VMware)
<thomas@...pmail.org> wrote:
>
> - I've never had any kernel code more reviewed than this.
Hmm. It may have been reviewed, but that wasn't visible in the commits
themselves, so when I look at the pull request, I don't see that.
> - The combined callback / argument struct: It was strongly inspired by
> the struct mm_walk (mm.h), the page walk code being quite similar in
> functionality.
The mm_walk struct is indeed a bit similar, and is in fact a bit
problematic exactly because it mixes function pointers with non-const
data.
I wish it had been a 'const struct mm_walk *" that only passed in the
stuff that describes what to do on the walk itself. Or separated into
two different pointers - one for the "this is what to do for the walk"
and one for "this is the walking data".
In fact, I think tight now that is actually _almost_ the case and we
could make them const, except for "walk->vma" which is updated
dynamically as we walk. Oh well.
And for all I know, some of the walkers may be modifying their
"private" field too, since that's left to the walkers.
So yes, that one also has some problems, I agree.
Linus
Powered by blists - more mailing lists