Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 22 Jun 2023 07:59:05 -0700
From:   Yury Norov <yury.norov@gmail.com>
To:     Lucas De Marchi <lucas.demarchi@intel.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        intel-gfx@lists.freedesktop.org,
        Kevin Brodsky <kevin.brodsky@arm.com>,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        Christian =?iso-8859-1?Q?K=F6nig?= <christian.koenig@amd.com>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Alex Deucher <alexander.deucher@amd.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        intel-xe@lists.freedesktop.org
Subject: Re: [PATCH 2/3] linux/bits.h: Add fixed-width GENMASK and BIT macros
Message-ID: <ZJRhuaebZC+y7B37@yury-ThinkPad>
References: <20230509051403.2748545-1-lucas.demarchi@intel.com>
 <20230509051403.2748545-3-lucas.demarchi@intel.com>
 <ZJOwC5LIEySpduQJ@yury-ThinkPad>
 <alzfewo3jado7ezyaibq52ep3vuxbyfism4ablchmvmioio3jb@3gyx6vaoscbf>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alzfewo3jado7ezyaibq52ep3vuxbyfism4ablchmvmioio3jb@3gyx6vaoscbf>
Precedence: bulk

+ Rasmus Villemoes <linux@rasmusvillemoes.dk>

> > -#define __GENMASK(h, l) \
> > -	(((~UL(0)) - (UL(1) << (l)) + 1) & \
> > -	 (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
> > -#define GENMASK(h, l) \
> > -	(GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
> > +#define __GENMASK(t, h, l) \
> > +	(GENMASK_INPUT_CHECK(h, l) + \
> > +	 (((t)~0ULL - ((t)(1) << (l)) + 1) & \
> > +	 ((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h)))))
> 
> yeah... forcing the use of ull and then casting to the type is simpler
> and does the job. Checked that it does not break the build if h is
> greater than the type and it works
> 
> ../include/linux/bits.h:40:20: error: right shift count >= width of type [-Werror=shift-count-overflow]
>    40 |          ((t)~0ULL >> (BITS_PER_TYPE(t) - 1 - (h)))))
>       |                    ^~
> 
> However this new version does increase the size. Using i915 module
> to test:
> 
> $ size build64/drivers/gpu/drm/i915/i915.ko*
>    text    data     bss     dec     hex filename
> 4355676  213473    7048 4576197  45d3c5 build64/drivers/gpu/drm/i915/i915.ko
> 4361052  213505    7048 4581605  45e8e5 build64/drivers/gpu/drm/i915/i915.ko.new

It sounds weird because all that should anyways boil down at compile
time...

I enabled DRM_I915 in config and ran bloat-o-meter against today's
master, and I don't see that much difference.

  $ size vmlinux vmlinux.new
     text	   data	    bss	    dec	    hex	filename
  44978613	23962202	3026948	71967763	44a2413	vmlinux
  44978653	23966298	3026948	71971899	44a343b	vmlinux.new
  $ scripts/bloat-o-meter vmlinux vmlinux.new 
  add/remove: 0/0 grow/shrink: 3/2 up/down: 28/-5 (23)
  Function                                     old     new   delta
  kvm_mmu_reset_all_pte_masks                  623     639     +16
  intel_psr_invalidate                        1112    1119      +7
  intel_drrs_activate                          624     629      +5
  intel_psr_flush                             1410    1409      -1
  clk_fractional_divider_general_approximation     207     203      -4
  Total: Before=35398799, After=35398822, chg +0.00%

Can you please check your numbers?

Interestingly, the kvm_mmu_reset_all_pte_masks() uses GENMASK_ULL(),
which should generate the same code across versions. Maybe it's just
a noise? Rasmus, can you please take a look?

Thanks,
Yury