On Tue, Mar 29, 2022 at 7:40 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> I think alloc_fdtable() does everything correctly.
So I agree that alloc_fdtable() _works_, but it's certainly a bit opaque.
The reason it works is that
nr *= (1024 / sizeof(struct file *));
and in practice, since a "sizeof(struct file *)" is either 4 or 8,
that will multiply 'nr' by either 256 or 128.
End result: 'nr' will most definitely always be a multiple of
BITS_PER_LONG, at least until we end up with 128-bit machines, in
which case the multiplier will be only 64.
So I'm inclined to add an explicit ALIGN() there too. I just verified
that at least clang notices that "hey, after multiplying by 128 and
the subsequent ALIGN() is a no-op".
Sadly, gcc seems to be too stupid to realize that. I'm surprised.
But it seems to be because gcc has terminally confused itself and
actually combined the shifts in roundup_pow_of_two() with the
multiply, and as a result has lost sight of the fact that we just
shifted by 7.
So that's a bit sad, but the extra two instructions gcc inserts are
pretty harmless in the end, and the clarification is worth it. And
maybe some day gcc will figure it out.
Linus
