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