On Sat, Jun 12, 2021, 8:59 AM Vlastimil Babka <vbabka@suse.cz> wrote:

> On 6/11/21 1:56 PM, Hyeonggon Yoo wrote:
> > On Fri, Jun 11, 2021, 7:27 PM Vlastimil Babka <vbabka@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.
>

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?

>