This patchset contains cleanups and minor bug fixes in x86 PAT and CPA
related code. The bugs were mostly found by code inspection. There
should not be any functionality changes with this patchset.
Signed-off-by: Venkatesh Pallipadi <[email protected]>
Signed-off-by: Suresh Siddha <[email protected]>
--
* [email protected] <[email protected]> wrote:
> This patchset contains cleanups and minor bug fixes in x86 PAT and
> CPA related code. The bugs were mostly found by code inspection.
> There should not be any functionality changes with this patchset.
>
> Signed-off-by: Venkatesh Pallipadi <[email protected]>
> Signed-off-by: Suresh Siddha <[email protected]>
Great, this series looks really nice!
Does this solve the problems reported in the "2.6.29 git master and
PAT problems" thread and addressed via an earlier patch of yours:
Subject: [PATCH] x86, PAT: Remove page granularity tracking for vm_insert_pfn maps
i am worried about this particular patch - it looks more like a
workaround than a true realization of a bug.
Ingo
On Fri, 2009-04-10 at 04:53 -0700, Ingo Molnar wrote:
> * [email protected] <[email protected]> wrote:
>
> > This patchset contains cleanups and minor bug fixes in x86 PAT and
> > CPA related code. The bugs were mostly found by code inspection.
> > There should not be any functionality changes with this patchset.
> >
> > Signed-off-by: Venkatesh Pallipadi <[email protected]>
> > Signed-off-by: Suresh Siddha <[email protected]>
>
> Great, this series looks really nice!
>
> Does this solve the problems reported in the "2.6.29 git master and
> PAT problems" thread and addressed via an earlier patch of yours:
>
> Subject: [PATCH] x86, PAT: Remove page granularity tracking for vm_insert_pfn maps
> i am worried about this particular patch - it looks more like a
> workaround than a true realization of a bug.
>
No. This patchset does not fix that problem. "Remove page granularity
tracking" patch is still needed with this patchset.
Yes. That patch is more of a workaround or a direction change about how
we want to handle vm_insert_pfn with PAT.
When we added the page level tracking, there were no in kernel users of
vm_insert_pfn() and we did not consider the usages where same address
space/vma will be used to map different physical addresses over time
with unmap_mapping_range() and re -inserting different pfns to the same
vma. That is what X 915 driver is doing now.
You are right in saying that the patch is not handling the bug of
"freeing invalid memtype" errors. They are happening due to unbalanced
reserve/free (more free than reserve) in the code path of tracking
vm_insert_pfn pages. I couldn't really reproduce the problem where we
are getting those unbalanced frees as reported in that bug report. But,
that bug report also points to another issue. The issue of 1000s of
single page UC_MINUS or WC mappings from X driver. Even though it is not
a functionality problem, it will have major performance impact tracking
thousands of memtypes. We don't have to really track memtypes of such
small chunks and they want WC (or UC_MINUS) for all those mappings.
So, the plan is to forget about tracking of individual pages. That
automatically takes care of the unbalanced reserve/free problem.
We are adding a new API where driver can ask for a type for a big
address range, something like the entire PCI map range. And then used
the type for each individual page that they may map on demand. This way
we don't have to keep track of individual pages that driver may map and
unmap. This is still in works, I should have a patch for this soon (a
week or so).
Thanks,
Venki
On Thu, 2009-04-09 at 14:26 -0700, [email protected] wrote:
> This patchset contains cleanups and minor bug fixes in x86 PAT and CPA
> related code. The bugs were mostly found by code inspection. There
> should not be any functionality changes with this patchset.
I've been curious, what are you using to test PAT changes for
regressions? I've got some regression tests at:
http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/
Requires KMS enabled and master of libdrm, but after that you can sudo
make check, and it tests execution of several DRM paths without
requiring X. In benchmarks/ there are a few microbenchmarks of various
mapping types, which has been useful in making sure that we're ending up
with the right PTEs.
It should work on all Intel GPUs from the 830 through the GM45, though I
haven't tested pre-915 much.
--
Eric Anholt
[email protected] [email protected]
* Eric Anholt <[email protected]> wrote:
> On Thu, 2009-04-09 at 14:26 -0700, [email protected] wrote:
> > This patchset contains cleanups and minor bug fixes in x86 PAT and CPA
> > related code. The bugs were mostly found by code inspection. There
> > should not be any functionality changes with this patchset.
>
> I've been curious, what are you using to test PAT changes for
> regressions? I've got some regression tests at:
>
> http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/
>
> Requires KMS enabled and master of libdrm, but after that you can
> sudo make check, and it tests execution of several DRM paths
> without requiring X. In benchmarks/ there are a few
> microbenchmarks of various mapping types, which has been useful in
> making sure that we're ending up with the right PTEs.
Looks really nice! Regarding libdrm, is there a version cutoff from
where it is expected to work just fine? I've got this version:
libdrm-2.4.6-3.fc11.x86_64.
Ingo
On Sat, 2009-04-11 at 09:00 +0200, Ingo Molnar wrote:
> * Eric Anholt <[email protected]> wrote:
>
> > On Thu, 2009-04-09 at 14:26 -0700, [email protected] wrote:
> > > This patchset contains cleanups and minor bug fixes in x86 PAT and CPA
> > > related code. The bugs were mostly found by code inspection. There
> > > should not be any functionality changes with this patchset.
> >
> > I've been curious, what are you using to test PAT changes for
> > regressions? I've got some regression tests at:
> >
> > http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/
> >
> > Requires KMS enabled and master of libdrm, but after that you can
> > sudo make check, and it tests execution of several DRM paths
> > without requiring X. In benchmarks/ there are a few
> > microbenchmarks of various mapping types, which has been useful in
> > making sure that we're ending up with the right PTEs.
>
> Looks really nice! Regarding libdrm, is there a version cutoff from
> where it is expected to work just fine? I've got this version:
> libdrm-2.4.6-3.fc11.x86_64.
I should keep the pkgconfig check up to date, but in the worst case it
doesn't compile and you go get new libdrm. The 2.4.6 check right now
appears to be correct.
--
Eric Anholt
[email protected] [email protected]