[gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[Natanael Olaiz]

[-- Attachment #1.1: Type: text/plain, Size: 992 bytes --]

Hi all,

After an update to the kernel 4.7.0, I ran into these problems with
the NVIDIA drivers: http://rglinuxtech.com/?p=1746 :

........../home/rgadsdon/kernel/NVIDIA-Linux-x86_64-367.18/kernel/nvidia-uvm/uvm_linux.h:557:13:
error: redefinition of ‘radix_tree_empty’ static bool
radix_tree_empty(struct radix_tree_root
*tree)................................../home/rgadsdon/kernel/NVIDIA-Linux-x86_64-367.18/kernel/nvidia-drm/nvidia-drm-gem.c:
In function ‘nvidia_drm_dumb_map_offset’:/home/rgadsdon/kernel/NVIDIA-Linux-x86_64-367.18/kernel/nvidia-drm/nvidia-drm-gem.c:411:33:
error: passing argument 1 of ‘drm_gem_object_lookup’ from incompatible
pointer type [-Werror=incompatible-pointer-types] gem =
drm_gem_object_lookup(dev, file, handle);........................


I applied the attached patch unconditionally locally, and it seems to work.
Which is correct way to apply the patch or not depending on the kernel version?


Best regards,

Natanael.

[-- Attachment #1.2: Type: text/html, Size: 2040 bytes --]

[-- Attachment #2: nvidia-drivers-367.35-radix_tree_empty-drm_gem_object_lookup.patch --]
[-- Type: text/x-patch, Size: 2474 bytes --]

diff -ur NVIDIA-Linux-x86_64-367.35-original/kernel/nvidia-drm/nvidia-drm-fb.c NVIDIA-Linux-x86_64-367.35/kernel/nvidia-drm/nvidia-drm-fb.c
--- NVIDIA-Linux-x86_64-367.35-original/kernel/nvidia-drm/nvidia-drm-fb.c	2016-07-12 06:53:45.000000000 +0200
+++ NVIDIA-Linux-x86_64-367.35/kernel/nvidia-drm/nvidia-drm-fb.c	2016-08-05 20:10:51.114008113 +0200
@@ -114,7 +114,7 @@
      * We don't support any planar format, pick up first buffer only.
      */
 
-    gem = drm_gem_object_lookup(dev, file, cmd->handles[0]);
+    gem = drm_gem_object_lookup(file, cmd->handles[0]);
 
     if (gem == NULL)
     {
diff -ur NVIDIA-Linux-x86_64-367.35-original/kernel/nvidia-drm/nvidia-drm-gem.c NVIDIA-Linux-x86_64-367.35/kernel/nvidia-drm/nvidia-drm-gem.c
--- NVIDIA-Linux-x86_64-367.35-original/kernel/nvidia-drm/nvidia-drm-gem.c	2016-07-12 06:53:45.000000000 +0200
+++ NVIDIA-Linux-x86_64-367.35/kernel/nvidia-drm/nvidia-drm-gem.c	2016-08-05 20:01:41.942998089 +0200
@@ -408,7 +408,7 @@
 
     mutex_lock(&dev->struct_mutex);
 
-    gem = drm_gem_object_lookup(dev, file, handle);
+    gem = drm_gem_object_lookup(file, handle);
 
     if (gem == NULL)
     {
diff -ur NVIDIA-Linux-x86_64-367.35-original/kernel/nvidia-uvm/uvm8_gpu.c NVIDIA-Linux-x86_64-367.35/kernel/nvidia-uvm/uvm8_gpu.c
--- NVIDIA-Linux-x86_64-367.35-original/kernel/nvidia-uvm/uvm8_gpu.c	2016-07-12 06:52:17.000000000 +0200
+++ NVIDIA-Linux-x86_64-367.35/kernel/nvidia-uvm/uvm8_gpu.c	2016-08-05 19:34:51.454968692 +0200
@@ -647,7 +647,7 @@
                    gpu->id, uvm_gpu_retained_count(gpu));
 
     // All channels should have been removed before the retained count went to 0
-    UVM_ASSERT(radix_tree_empty(&gpu->instance_ptr_table));
+    UVM_ASSERT(NV_radix_tree_empty(&gpu->instance_ptr_table));
 
     // Remove the GPU from the table.
     uvm_spin_lock_irqsave(&g_uvm_global.gpu_table_lock);
diff -ur NVIDIA-Linux-x86_64-367.35-original/kernel/nvidia-uvm/uvm_linux.h NVIDIA-Linux-x86_64-367.35/kernel/nvidia-uvm/uvm_linux.h
--- NVIDIA-Linux-x86_64-367.35-original/kernel/nvidia-uvm/uvm_linux.h	2016-07-12 06:52:17.000000000 +0200
+++ NVIDIA-Linux-x86_64-367.35/kernel/nvidia-uvm/uvm_linux.h	2016-08-05 19:34:45.541968585 +0200
@@ -554,7 +554,7 @@
     INIT_RADIX_TREE(tree, GFP_NOWAIT);
 }
 
-static bool radix_tree_empty(struct radix_tree_root *tree)
+static bool NV_radix_tree_empty(struct radix_tree_root *tree)
 {
     void *dummy;
     return radix_tree_gang_lookup(tree, &dummy, 0, 1) == 0;

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[R0b0t1]

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

On Aug 5, 2016 1:23 PM, "Natanael Olaiz" <nolaiz@gmail.com> wrote:
>
> I applied the attached patch unconditionally locally, and it seems to
work.
> Which is correct way to apply the patch or not depending on the kernel
version?
>

See ebuild, there is a patch phase. Shove it in proper directory. Will need
local overlay.

[-- Attachment #2: Type: text/html, Size: 439 bytes --]

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[NP-Hardass]

[-- Attachment #1.1: Type: text/plain, Size: 965 bytes --]

On 08/05/2016 02:34 PM, R0b0t1 wrote:
> On Aug 5, 2016 1:23 PM, "Natanael Olaiz" <nolaiz@gmail.com
> <mailto:nolaiz@gmail.com>> wrote:
>>
>> I applied the attached patch unconditionally locally, and it seems to
> work.
>> Which is correct way to apply the patch or not depending on the kernel
> version?
>>
> 
> See ebuild, there is a patch phase. Shove it in proper directory. Will
> need local overlay.
> 

if you are editing the ebuild in a local repo, put your patch in the
files/ directory.  If you want to conditionally patch for kernel 4.7,
you can test this with "kernel_is" from the linux-info eclass. In that
conditional block, epatch "${FILESDIR}/nameofyourfile.patch".

Alternatively, use https://wiki.gentoo.org/wiki//etc/portage/patches to
guide you on adding the patch to the proper place to have the ebuild
automatically patch (without need for an local repo) for you
(unconditionally wrt kernel version)

-- 
NP-Hardass


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[Natanael Olaiz]

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

I know that. But the patch should be applied *only* for versions of kernels
4.7+. So, I'm asking how is the policy for that.

I see in the linux-info eclass documentation (
https://devmanual.gentoo.org/eclass-reference/linux-info.eclass/index.html)
that I can maybe apply it with a line like this?

kernel_is -ge 4 7 && epatch "${P}....."


Best regards,
Natanael.

On 5 August 2016 at 20:34, R0b0t1 <r030t1@gmail.com> wrote:

> On Aug 5, 2016 1:23 PM, "Natanael Olaiz" <nolaiz@gmail.com> wrote:
> >
> > I applied the attached patch unconditionally locally, and it seems to
> work.
> > Which is correct way to apply the patch or not depending on the kernel
> version?
> >
>
> See ebuild, there is a patch phase. Shove it in proper directory. Will
> need local overlay.
>

[-- Attachment #2: Type: text/html, Size: 1423 bytes --]

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[Natanael Olaiz]

[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]

Yes, thank you. I did the local repository path at the moment. I was
thinking on pushing the patch upstream, but now I read on the ebuild I'm
editing (using the check kernel_is :) ) :

    if use kernel_linux && kernel_is ge 4 7; then
        ewarn "Gentoo supports kernels which are supported by NVIDIA"
        ewarn "which are limited to the following kernels:"
        ewarn "<sys-kernel/gentoo-sources-4.7"
        ewarn "<sys-kernel/vanilla-sources-4.7"
        ewarn ""
        ewarn "You are free to utilize epatch_user to provide whatever"
        ewarn "support you feel is appropriate, but will not receive"
        ewarn "support as a result of those changes."
        ewarn ""
        ewarn "Do not file a bug report about this."
        ewarn ""
    fi

So basically I will not have support. I didn't care about this warning when
I went for fixing the issues. But seeing that the patch was simple I though
on share it somehow...


Best regards,
Natanael.

On 5 August 2016 at 20:50, NP-Hardass <NP-Hardass@gentoo.org> wrote:

> On 08/05/2016 02:34 PM, R0b0t1 wrote:
> > On Aug 5, 2016 1:23 PM, "Natanael Olaiz" <nolaiz@gmail.com
> > <mailto:nolaiz@gmail.com>> wrote:
> >>
> >> I applied the attached patch unconditionally locally, and it seems to
> > work.
> >> Which is correct way to apply the patch or not depending on the kernel
> > version?
> >>
> >
> > See ebuild, there is a patch phase. Shove it in proper directory. Will
> > need local overlay.
> >
>
> if you are editing the ebuild in a local repo, put your patch in the
> files/ directory.  If you want to conditionally patch for kernel 4.7,
> you can test this with "kernel_is" from the linux-info eclass. In that
> conditional block, epatch "${FILESDIR}/nameofyourfile.patch".
>
> Alternatively, use https://wiki.gentoo.org/wiki//etc/portage/patches to
> guide you on adding the patch to the proper place to have the ebuild
> automatically patch (without need for an local repo) for you
> (unconditionally wrt kernel version)
>
> --
> NP-Hardass
>
>

[-- Attachment #2: Type: text/html, Size: 3118 bytes --]

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[Mike Gilbert]

On Fri, Aug 5, 2016 at 3:10 PM, Natanael Olaiz <nolaiz@gmail.com> wrote:
> I know that. But the patch should be applied *only* for versions of kernels
> 4.7+. So, I'm asking how is the policy for that.

If you're asking for policy: The Gentoo packaging policy is not to do
conditional patching. Instead, modify the patch so that the resulting
code works for both cases. This can generally be accomplished via
pre-processor macros.

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[David Haller]

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]

Hello,

On Fri, 05 Aug 2016, Mike Gilbert wrote:
>On Fri, Aug 5, 2016 at 3:10 PM, Natanael Olaiz <nolaiz@gmail.com> wrote:
>> I know that. But the patch should be applied *only* for versions of kernels
>> 4.7+. So, I'm asking how is the policy for that.
>
>If you're asking for policy: The Gentoo packaging policy is not to do
>conditional patching. Instead, modify the patch so that the resulting
>code works for both cases. This can generally be accomplished via
>pre-processor macros.

My patch does it like that. See
 https://archives.gentoo.org/gentoo-user/message/baa36d14d8cdbf58404267ee2ffd34ea
Just dumping the attached patch into
/etc/portage/patches/x11-drivers/nvidia-drivers-367.35/
(and making it readable for the portage user) is sufficient.

HTH,
-dnh

-- 
Every feature is a bug, unless it can be turned off.  -- Karl Heuer

[-- Attachment #2: /etc/portage/patches/x11-drivers/nvidia-drivers-367.35/nvidia-drivers-367.35-kernel-4.7.0.patch --]
[-- Type: text/x-patch, Size: 2114 bytes --]

diff -purN a/kernel/nvidia-drm/nvidia-drm-fb.c b/kernel/nvidia-drm/nvidia-drm-fb.c
--- a/kernel/nvidia-drm/nvidia-drm-fb.c	2016-07-12 06:53:45.000000000 +0200
+++ b/kernel/nvidia-drm/nvidia-drm-fb.c	2016-07-28 09:43:11.494515158 +0200
@@ -32,6 +32,8 @@
 
 #include <drm/drm_crtc_helper.h>
 
+#include <linux/version.h>
+
 static void nvidia_framebuffer_destroy(struct drm_framebuffer *fb)
 {
     struct nvidia_drm_device *nv_dev = fb->dev->dev_private;
@@ -114,7 +116,11 @@ static struct drm_framebuffer *internal_
      * We don't support any planar format, pick up first buffer only.
      */
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,7,0)
+    gem = drm_gem_object_lookup(file, cmd->handles[0]);
+#else
     gem = drm_gem_object_lookup(dev, file, cmd->handles[0]);
+#endif
 
     if (gem == NULL)
     {
diff -purN a/kernel/nvidia-drm/nvidia-drm-gem.c b/kernel/nvidia-drm/nvidia-drm-gem.c
--- a/kernel/nvidia-drm/nvidia-drm-gem.c	2016-07-12 06:53:45.000000000 +0200
+++ b/kernel/nvidia-drm/nvidia-drm-gem.c	2016-07-28 09:27:24.610637573 +0200
@@ -28,6 +28,8 @@
 #include "nvidia-drm-ioctl.h"
 #include "nvidia-drm-gem.h"
 
+#include <linux/version.h>
+
 static struct nvidia_drm_gem_object *nvidia_drm_gem_new
 (
     struct drm_file *file_priv,
@@ -408,7 +410,11 @@ int nvidia_drm_dumb_map_offset
 
     mutex_lock(&dev->struct_mutex);
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,7,0)
+    gem = drm_gem_object_lookup(file, handle);
+#else
     gem = drm_gem_object_lookup(dev, file, handle);
+#endif
 
     if (gem == NULL)
     {
diff -purN a/kernel/nvidia-uvm/uvm_linux.h b/kernel/nvidia-uvm/uvm_linux.h
--- a/kernel/nvidia-uvm/uvm_linux.h	2016-07-12 06:52:17.000000000 +0200
+++ b/kernel/nvidia-uvm/uvm_linux.h	2016-07-28 09:29:21.096322608 +0200
@@ -554,11 +554,13 @@ static void uvm_init_radix_tree_preloada
     INIT_RADIX_TREE(tree, GFP_NOWAIT);
 }
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4,7,0)
 static bool radix_tree_empty(struct radix_tree_root *tree)
 {
     void *dummy;
     return radix_tree_gang_lookup(tree, &dummy, 0, 1) == 0;
 }
+#endif
 
 
 #if !defined(NV_USLEEP_RANGE_PRESENT)

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[Natanael Olaiz]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

Thank you for your explanations! :)

Best regards,
Natanael.

On 6 August 2016 at 04:50, David Haller <gentoo@dhaller.de> wrote:

> Hello,
>
> On Fri, 05 Aug 2016, Mike Gilbert wrote:
> >On Fri, Aug 5, 2016 at 3:10 PM, Natanael Olaiz <nolaiz@gmail.com> wrote:
> >> I know that. But the patch should be applied *only* for versions of
> kernels
> >> 4.7+. So, I'm asking how is the policy for that.
> >
> >If you're asking for policy: The Gentoo packaging policy is not to do
> >conditional patching. Instead, modify the patch so that the resulting
> >code works for both cases. This can generally be accomplished via
> >pre-processor macros.
>
> My patch does it like that. See
>  https://archives.gentoo.org/gentoo-user/message/
> baa36d14d8cdbf58404267ee2ffd34ea
> Just dumping the attached patch into
> /etc/portage/patches/x11-drivers/nvidia-drivers-367.35/
> (and making it readable for the portage user) is sufficient.
>
> HTH,
> -dnh
>
> --
> Every feature is a bug, unless it can be turned off.  -- Karl Heuer

[-- Attachment #2: Type: text/html, Size: 1739 bytes --]

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[Natanael Olaiz]

[-- Attachment #1: Type: text/plain, Size: 1445 bytes --]

David,

Thank you for your patch. It was a good example to answer my question.

But about the patch itself, I see that you are commented the code for
radix_tree_empty(...). In my patch I renamed it and it only usage instead,
so I'm sure it's calling the same code. I don't know the expected
compatibility with the kernel function implementation... But without
knowing the specific code for neither the nvidia driver nor the kernel, I
think the rename is safer...


Best regards,
Natanael


On 6 August 2016 at 04:50, David Haller <gentoo@dhaller.de> wrote:

> Hello,
>
> On Fri, 05 Aug 2016, Mike Gilbert wrote:
> >On Fri, Aug 5, 2016 at 3:10 PM, Natanael Olaiz <nolaiz@gmail.com> wrote:
> >> I know that. But the patch should be applied *only* for versions of
> kernels
> >> 4.7+. So, I'm asking how is the policy for that.
> >
> >If you're asking for policy: The Gentoo packaging policy is not to do
> >conditional patching. Instead, modify the patch so that the resulting
> >code works for both cases. This can generally be accomplished via
> >pre-processor macros.
>
> My patch does it like that. See
>  https://archives.gentoo.org/gentoo-user/message/
> baa36d14d8cdbf58404267ee2ffd34ea
> Just dumping the attached patch into
> /etc/portage/patches/x11-drivers/nvidia-drivers-367.35/
> (and making it readable for the portage user) is sufficient.
>
> HTH,
> -dnh
>
> --
> Every feature is a bug, unless it can be turned off.  -- Karl Heuer

[-- Attachment #2: Type: text/html, Size: 2230 bytes --]

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[Natanael Olaiz]

[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]

s/you are commented/you commented/

:)

On 6 August 2016 at 12:25, Natanael Olaiz <nolaiz@gmail.com> wrote:

> David,
>
> Thank you for your patch. It was a good example to answer my question.
>
> But about the patch itself, I see that you are commented the code for
> radix_tree_empty(...). In my patch I renamed it and it only usage instead,
> so I'm sure it's calling the same code. I don't know the expected
> compatibility with the kernel function implementation... But without
> knowing the specific code for neither the nvidia driver nor the kernel, I
> think the rename is safer...
>
>
> Best regards,
> Natanael
>
>
> On 6 August 2016 at 04:50, David Haller <gentoo@dhaller.de> wrote:
>
>> Hello,
>>
>> On Fri, 05 Aug 2016, Mike Gilbert wrote:
>> >On Fri, Aug 5, 2016 at 3:10 PM, Natanael Olaiz <nolaiz@gmail.com> wrote:
>> >> I know that. But the patch should be applied *only* for versions of
>> kernels
>> >> 4.7+. So, I'm asking how is the policy for that.
>> >
>> >If you're asking for policy: The Gentoo packaging policy is not to do
>> >conditional patching. Instead, modify the patch so that the resulting
>> >code works for both cases. This can generally be accomplished via
>> >pre-processor macros.
>>
>> My patch does it like that. See
>>  https://archives.gentoo.org/gentoo-user/message/baa36d14d8c
>> dbf58404267ee2ffd34ea
>> Just dumping the attached patch into
>> /etc/portage/patches/x11-drivers/nvidia-drivers-367.35/
>> (and making it readable for the portage user) is sufficient.
>>
>> HTH,
>> -dnh
>>
>> --
>> Every feature is a bug, unless it can be turned off.  -- Karl Heuer
>
>
>

[-- Attachment #2: Type: text/html, Size: 2680 bytes --]

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[David Haller]

Hello,

On Sat, 06 Aug 2016, Natanael Olaiz wrote:
>Thank you for your patch. It was a good example to answer my question.
>
>But about the patch itself, I see that you are commented the code for
>radix_tree_empty(...). In my patch I renamed it and it only usage instead,
>so I'm sure it's calling the same code. I don't know the expected
>compatibility with the kernel function implementation... But without
>knowing the specific code for neither the nvidia driver nor the kernel, I
>think the rename is safer...

If you're in doubt or hit any trouble, yes, definitely change/adapt
the patch to only rename that function and use the renamed function in
the nvidia-driver as in your original patch. Keep/adapt the stuff
about kernel-versions though. That's the "safe" approach.

That "just put it in /etc/portage/patches/..." should work tough :)


I've looked quite sharp at the code, i.e. in the nvidia code

static bool radix_tree_empty(struct radix_tree_root *tree)
 {
     void *dummy;
     return radix_tree_gang_lookup(tree, &dummy, 0, 1) == 0;
 }

vs. the kernel function

static inline bool radix_tree_empty(struct radix_tree_root *root)
{
        return root->rnode == NULL;
}

*oik* I miss a check for root != NULL there ;)

Anyway, it was quite clear, that the driver calls kernel-stuff at this
point anyway, radix_tree_gang_lookup() is a kernel function. (I love
to use mc for digging for stuff like this!)

So, digging into the kernel-source I came to the conclusion that
calling the _kernel code_(!)

    radix_tree_gang_lookup(tree, &dummy, 0, 1);

actually _is_ (or SHOULD BE) equivalent to the kernel code for
the new

    radix_tree_empty(tree);

and the latter should be faster (unless the compiler optimizes the
'radix_tree_gang_lookup(root, &dummy, 0, 1)' call away).

All this applies if and only if the last two arguments of
radix_tree_gang_lookup() are 0 and 1! And yes, I did go through the
code of radix_tree_gang_lookup() step by step (repeatedly) until I was
sure enough that the code is equivalent). But I'm not a C guru. I
might have missed something even important. So, if more guys firm in C
can check this ...

As I'm a guy generally trusting the kernel guys _a lot_ and that
nvidia assumedly got it right implementing 'radix_tree_empty(tree)'
via 'radix_tree_gang_lookup(tree, &dummy, 0, 1);' and I think those
two equivalent (with 0, 1 being the 3rd and 4th parameters!), I tried
it, and it worked.

And Meino has not yet complained. AFAIR he's around long enough to
have complained by now (and circumvented problems with the patch). 
I'll ping him on the -user ML though in a parallel mail.

Recap: calling the kernel function in this way (via the nvidia function)

    radix_tree_gang_lookup(tree, &dummy, 0, 1);

is IMHO equivalent to calling the kernel function

    radix_tree_empty(tree);

and on this I based my patch on. I'm rather sure from looking at the
code. It worked in a short test. Meino has not complained yet. If you
want a "sure"/"safe" approach, go with renaming the function in the
nvidia-code, or wait a bit if Meino pipes up, he should've been
running the driver with my patch for a week by now. Or until an
official patch crops up.

Or, take precautions, some other way to boot+chroot, and keep the
"old" driver handy or to disable it, build a binary package of the
previous driver, have an older kernel ready etc. pp., you know the
drill, don't you?

-dnh

-- 
MCSE: "Microsoft Certified Stupidity enclosed"        -- A. Spengler

Re: [gentoo-dev] Patch for nvidia-drivers 367.35-r1 for kernels > 4.7.0

[Natanael Olaiz]

[-- Attachment #1: Type: text/plain, Size: 3896 bytes --]

Hi,

Thanks for the detailed explanation of your testing procedure! It is good
to know what is behind a local patched driver, just in case.. :)


Best regards,
Natanael.

On 6 August 2016 at 16:20, David Haller <gentoo@dhaller.de> wrote:

> Hello,
>
> On Sat, 06 Aug 2016, Natanael Olaiz wrote:
> >Thank you for your patch. It was a good example to answer my question.
> >
> >But about the patch itself, I see that you are commented the code for
> >radix_tree_empty(...). In my patch I renamed it and it only usage instead,
> >so I'm sure it's calling the same code. I don't know the expected
> >compatibility with the kernel function implementation... But without
> >knowing the specific code for neither the nvidia driver nor the kernel, I
> >think the rename is safer...
>
> If you're in doubt or hit any trouble, yes, definitely change/adapt
> the patch to only rename that function and use the renamed function in
> the nvidia-driver as in your original patch. Keep/adapt the stuff
> about kernel-versions though. That's the "safe" approach.
>
> That "just put it in /etc/portage/patches/..." should work tough :)
>
>
> I've looked quite sharp at the code, i.e. in the nvidia code
>
> static bool radix_tree_empty(struct radix_tree_root *tree)
>  {
>      void *dummy;
>      return radix_tree_gang_lookup(tree, &dummy, 0, 1) == 0;
>  }
>
> vs. the kernel function
>
> static inline bool radix_tree_empty(struct radix_tree_root *root)
> {
>         return root->rnode == NULL;
> }
>
> *oik* I miss a check for root != NULL there ;)
>
> Anyway, it was quite clear, that the driver calls kernel-stuff at this
> point anyway, radix_tree_gang_lookup() is a kernel function. (I love
> to use mc for digging for stuff like this!)
>
> So, digging into the kernel-source I came to the conclusion that
> calling the _kernel code_(!)
>
>     radix_tree_gang_lookup(tree, &dummy, 0, 1);
>
> actually _is_ (or SHOULD BE) equivalent to the kernel code for
> the new
>
>     radix_tree_empty(tree);
>
> and the latter should be faster (unless the compiler optimizes the
> 'radix_tree_gang_lookup(root, &dummy, 0, 1)' call away).
>
> All this applies if and only if the last two arguments of
> radix_tree_gang_lookup() are 0 and 1! And yes, I did go through the
> code of radix_tree_gang_lookup() step by step (repeatedly) until I was
> sure enough that the code is equivalent). But I'm not a C guru. I
> might have missed something even important. So, if more guys firm in C
> can check this ...
>
> As I'm a guy generally trusting the kernel guys _a lot_ and that
> nvidia assumedly got it right implementing 'radix_tree_empty(tree)'
> via 'radix_tree_gang_lookup(tree, &dummy, 0, 1);' and I think those
> two equivalent (with 0, 1 being the 3rd and 4th parameters!), I tried
> it, and it worked.
>
> And Meino has not yet complained. AFAIR he's around long enough to
> have complained by now (and circumvented problems with the patch).
> I'll ping him on the -user ML though in a parallel mail.
>
> Recap: calling the kernel function in this way (via the nvidia function)
>
>     radix_tree_gang_lookup(tree, &dummy, 0, 1);
>
> is IMHO equivalent to calling the kernel function
>
>     radix_tree_empty(tree);
>
> and on this I based my patch on. I'm rather sure from looking at the
> code. It worked in a short test. Meino has not complained yet. If you
> want a "sure"/"safe" approach, go with renaming the function in the
> nvidia-code, or wait a bit if Meino pipes up, he should've been
> running the driver with my patch for a week by now. Or until an
> official patch crops up.
>
> Or, take precautions, some other way to boot+chroot, and keep the
> "old" driver handy or to disable it, build a binary package of the
> previous driver, have an older kernel ready etc. pp., you know the
> drill, don't you?
>
> -dnh
>
> --
> MCSE: "Microsoft Certified Stupidity enclosed"        -- A. Spengler
>
>

[-- Attachment #2: Type: text/html, Size: 4864 bytes --]