[lkml]   [2017]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove the restore_c_regs_and_iret label)
On Thu, Sep 7, 2017 at 11:25 AM, Vishwanath Pai <> wrote:
> On 09/07/2017 01:51 PM, Linus Torvalds wrote:
>> But honestly, that math is odd in other ways too (is that "r-1"
>> _supposed_ to underflow to -1 for large 'user' counts?), so somebody
>> needs to look at that logic.
> Sorry about the build failure, we have already queued up a fix for this:

Note: that patch has *exactly* the issue I was talking about above.

Doing that

if (user > 0xFFFFFFFFULL)
return 0;

is different from the old code, which used to result in a zero in the
divide, and then

r = (r - 1) << 4;

would cause it to return a large value.

So the patch in question doesn't just fix the build error, it
completely changes the semantics of the function too.

I *think* the new behavior is likely what you want, but these kinds of
things should be _described_.

Also, even with the patch, we have garbage:

0xFFFFFFFFULL / (u32)user

why is that sub-expression pointlessly doing a 64-bit divide with a
32-bit number? The compiler is hopefully smart enough to point things
out, but that "ULL" really is _wrong_ there, and could cause a stupid
compiler to still do a 64-bit divide (although hopefully the simpler
version that is 64/32).

So please clarify both the correct behavior _and_ the actual typing of
the divide, ok?


 \ /
  Last update: 2017-09-07 20:43    [W:0.038 / U:8.268 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site