Re: [PATCH] mm, page_alloc: cleanup usemap_size() when SPARSEMEM is not set

[Michal Hocko <mhocko@suse.com>]

On Thu 24-01-19 14:13:41, Wei Yang wrote:
> On Wed, Jan 23, 2019 at 10:55:03AM +0100, Michal Hocko wrote:
> >On Tue 22-01-19 15:56:28, Wei Yang wrote:
> >> 
> >> I think the answer is yes.
> >> 
> >>   * it reduce the code from 6 lines to 3 lines, 50% off
> >>   * by reducing calculation back and forth, it would be easier for
> >>     audience to catch what it tries to do
> >
> >To be honest, I really do not see this sufficient to justify touching
> >the code unless the resulting _generated_ code is better/more efficient.
> 
> Tried objdump to compare two version.
> 
>                Base       Patched      Reduced
> Code Size(B)   48         39           18.7%
> Instructions   12         10           16.6%

How have you compiled the code? (compiler version, any specific configs).
Because I do not see any difference.

CONFIG_CC_OPTIMIZE_FOR_SIZE:
   text    data     bss     dec     hex filename
  47087    2085      72   49244    c05c mm/page_alloc.o
  47087    2085      72   49244    c05c mm/page_alloc.o.prev

CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE:
   text    data     bss     dec     hex filename
  55046    2085      72   57203    df73 mm/page_alloc.o
  55046    2085      72   57203    df73 mm/page_alloc.o.prev

And that would actually match my expectations because I am pretty sure
the compiler can figure out what to do with those operations even
without any help.

Really, is this really worth touching and spending a non-trivial time to
discuss? I do not see the benefit.
-- 
Michal Hocko
SUSE Labs