On Sat, Jun 12, 2021, 8:59 AM Vlastimil Babka <vbabka@xxxxxxx> wrote:
On 6/11/21 1:56 PM, Hyeonggon Yoo wrote:
> On Fri, Jun 11, 2021, 7:27 PM Vlastimil Babka <vbabka@xxxxxxx
> <mailto:vbabka@xxxxxxx>> 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.
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?
