10:19 a.m.
On Sat, Jun 12, 2021, 8:59 AM Vlastimil Babka <vbabka(a)suse.cz> wrote:
Okay, I see the code is okay, but one thing to suggest:
The problem already existed in kmalloc_node before the patch And we
found it by adding BUILD_BUG_ON in kmalloc_index.
So I suggest adding the condition in kmalloc_node. How about this?
On 6/11/21 1:56 PM, Hyeonggon Yoo wrote:
> On Fri, Jun 11, 2021, 7:27 PM Vlastimil Babka <vbabka(a)suse.cz
> <mailto:vbabka@suse.cz>> wrote:
> I meant the the condition to use BUILD_BUG_ON instead of BUG_ON
would include
> !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES)
> You mean this? (This will make
kmalloc_index return -1 without BUG())
> if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION
>= 110000) &&
> size_is_constant)
> BUILD_BUG_ON_MSG(!IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES),
"unexpected
> size in kmalloc_index()");
> else
> BUG();
No,
> Or This?
> if ((IS_ENABLED(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION
>= 110000) &&
> size_is_constant && !IS_ENABLED(CONFIG_PROFILE_ALL_BRANCHES))
> BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
> else
> BUG();
> Maybe this version seems better
Yeah, meant that.
Ah, okay!
> But little bit worried :(
> The code is getting too complicated...
> How do you think?
Yeah, I expected that problems like this could occur as we're poking at
some
rare corner cases of compiler implementations here. But if that leads to
fixes
in compilers, good for everyone I'd say.
So I would try this, even if it becomes complicated.