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

[Natanael Olaiz]

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@×××××××.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
>
>