lkml.org 
[lkml]   [2019]   [Feb]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 05/32] locking/lockdep: Prepare valid_state() to handle plain masks
On Tue, Feb 12, 2019 at 09:45:52AM -0800, Linus Torvalds wrote:
> On Tue, Feb 12, 2019 at 9:14 AM Frederic Weisbecker <frederic@kernel.org> wrote:
> >
> > +
> > + while (vectors) {
> > + long fs = __ffs64(vectors) + 1;
> > +
> > + vectors >>= fs;
>
> This is wrong.
>
> If "vectors" only has the high hit set, you end up with "fs" having
> the value "64".
>
> And then "vectors >>= fs" is undefined and won't actually do anything
> at all on x86.

Oh! ok didn't know that...

>
> In general, avoid "ffs()", and the stupid pattern of "__ffs(x)+1".
>
> Bit numbering starts at 0. "ffs()" is wrong. And you never *ever* just
> add one to a bit number in order to shift by one more bit, exactly
> because of overflow issues.
>
> So it may look inefficient, but the correct thing to do is
>
> long bit = __ffs64(vectors);
> vectors >>= fs;
> vectors >>= 1;
>
> because that actually works.

I see, perhaps I should use for_each_set_bit() that should take care about those
details?

Thanks.

\
 
 \ /
  Last update: 2019-02-13 16:17    [W:0.084 / U:3.336 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site