All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yury Norov <yury.norov@gmail.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org,
	Alexander Lobakin <aleksander.lobakin@intel.com>,
	Eric Dumazet <edumazet@google.com>,
	netdev@vger.kernel.org, Alexander Potapenko <glider@google.com>,
	nex.sw.ncis.osdt.itp.upstreaming@intel.com,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [Intel-wired-lan] [PATCH net-next v6 13/21] bitmap: make bitmap_{get, set}_value8() use bitmap_{read, write}()
Date: Thu, 30 May 2024 10:11:11 -0700	[thread overview]
Message-ID: <ZlizL6d1_ePq-eKs@yury-ThinkPad> (raw)
In-Reply-To: <5a18f5ac-4e9a-4baf-b720-98eac7b6792f@arm.com>

On Wed, May 29, 2024 at 04:12:25PM +0100, Robin Murphy wrote:
> Hi Alexander,
> 
> On 27/03/2024 3:23 pm, Alexander Lobakin wrote:
> > Now that we have generic bitmap_read() and bitmap_write(), which are
> > inline and try to take care of non-bound-crossing and aligned cases
> > to keep them optimized, collapse bitmap_{get,set}_value8() into
> > simple wrappers around the former ones.
> > bloat-o-meter shows no difference in vmlinux and -2 bytes for
> > gpio-pca953x.ko, which says the optimization didn't suffer due to
> > that change. The converted helpers have the value width embedded
> > and always compile-time constant and that helps a lot.
> 
> This change appears to have introduced a build failure for me on arm64
> (with GCC 9.4.0 from Ubuntu 20.04.02) - reverting b44759705f7d makes
> these errors go away again:
> 
> In file included from drivers/gpio/gpio-pca953x.c:12:
> drivers/gpio/gpio-pca953x.c: In function ‘pca953x_probe’:
> ./include/linux/bitmap.h:799:17: error: array subscript [1, 1024] is outside array bounds of ‘long unsigned int[1]’ [-Werror=array-bounds]
>   799 |  map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits);
>       |                 ^~
> In file included from ./include/linux/atomic.h:5,
>                  from drivers/gpio/gpio-pca953x.c:11:
> drivers/gpio/gpio-pca953x.c:1015:17: note: while referencing ‘val’
>  1015 |  DECLARE_BITMAP(val, MAX_LINE);
>       |                 ^~~
> ./include/linux/types.h:11:16: note: in definition of macro ‘DECLARE_BITMAP’
>    11 |  unsigned long name[BITS_TO_LONGS(bits)]
>       |                ^~~~
> In file included from drivers/gpio/gpio-pca953x.c:12:
> ./include/linux/bitmap.h:800:17: error: array subscript [1, 1024] is outside array bounds of ‘long unsigned int[1]’ [-Werror=array-bounds]
>   800 |  map[index + 1] |= (value >> space);
>       |  ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
> In file included from ./include/linux/atomic.h:5,
>                  from drivers/gpio/gpio-pca953x.c:11:
> drivers/gpio/gpio-pca953x.c:1015:17: note: while referencing ‘val’
>  1015 |  DECLARE_BITMAP(val, MAX_LINE);
>       |                 ^~~
> ./include/linux/types.h:11:16: note: in definition of macro ‘DECLARE_BITMAP’
>    11 |  unsigned long name[BITS_TO_LONGS(bits)]
>       |                ^~~~
> 
> I've not dug further since I don't have any interest in the pca953x
> driver - it just happened to be enabled in my config, so for now I've
> turned it off. However I couldn't obviously see any other reports of
> this, so here it is.

It's a compiler false-positive. The straightforward fix is to disable the warning
For gcc9+, and it's in Andrew Morton's tree alrady. but there's some discussion 
ongoing on how it should be mitigated properlu:

https://lore.kernel.org/all/0ab2702f-8245-4f02-beb7-dcc7d79d5416@app.fastmail.com/T/

Thanks,
YUry

WARNING: multiple messages have this Message-ID (diff)
From: Yury Norov <yury.norov@gmail.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Alexander Lobakin <aleksander.lobakin@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Alexander Potapenko <glider@google.com>,
	nex.sw.ncis.osdt.itp.upstreaming@intel.com,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v6 13/21] bitmap: make bitmap_{get,set}_value8() use bitmap_{read,write}()
Date: Thu, 30 May 2024 10:11:11 -0700	[thread overview]
Message-ID: <ZlizL6d1_ePq-eKs@yury-ThinkPad> (raw)
In-Reply-To: <5a18f5ac-4e9a-4baf-b720-98eac7b6792f@arm.com>

On Wed, May 29, 2024 at 04:12:25PM +0100, Robin Murphy wrote:
> Hi Alexander,
> 
> On 27/03/2024 3:23 pm, Alexander Lobakin wrote:
> > Now that we have generic bitmap_read() and bitmap_write(), which are
> > inline and try to take care of non-bound-crossing and aligned cases
> > to keep them optimized, collapse bitmap_{get,set}_value8() into
> > simple wrappers around the former ones.
> > bloat-o-meter shows no difference in vmlinux and -2 bytes for
> > gpio-pca953x.ko, which says the optimization didn't suffer due to
> > that change. The converted helpers have the value width embedded
> > and always compile-time constant and that helps a lot.
> 
> This change appears to have introduced a build failure for me on arm64
> (with GCC 9.4.0 from Ubuntu 20.04.02) - reverting b44759705f7d makes
> these errors go away again:
> 
> In file included from drivers/gpio/gpio-pca953x.c:12:
> drivers/gpio/gpio-pca953x.c: In function ‘pca953x_probe’:
> ./include/linux/bitmap.h:799:17: error: array subscript [1, 1024] is outside array bounds of ‘long unsigned int[1]’ [-Werror=array-bounds]
>   799 |  map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits);
>       |                 ^~
> In file included from ./include/linux/atomic.h:5,
>                  from drivers/gpio/gpio-pca953x.c:11:
> drivers/gpio/gpio-pca953x.c:1015:17: note: while referencing ‘val’
>  1015 |  DECLARE_BITMAP(val, MAX_LINE);
>       |                 ^~~
> ./include/linux/types.h:11:16: note: in definition of macro ‘DECLARE_BITMAP’
>    11 |  unsigned long name[BITS_TO_LONGS(bits)]
>       |                ^~~~
> In file included from drivers/gpio/gpio-pca953x.c:12:
> ./include/linux/bitmap.h:800:17: error: array subscript [1, 1024] is outside array bounds of ‘long unsigned int[1]’ [-Werror=array-bounds]
>   800 |  map[index + 1] |= (value >> space);
>       |  ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
> In file included from ./include/linux/atomic.h:5,
>                  from drivers/gpio/gpio-pca953x.c:11:
> drivers/gpio/gpio-pca953x.c:1015:17: note: while referencing ‘val’
>  1015 |  DECLARE_BITMAP(val, MAX_LINE);
>       |                 ^~~
> ./include/linux/types.h:11:16: note: in definition of macro ‘DECLARE_BITMAP’
>    11 |  unsigned long name[BITS_TO_LONGS(bits)]
>       |                ^~~~
> 
> I've not dug further since I don't have any interest in the pca953x
> driver - it just happened to be enabled in my config, so for now I've
> turned it off. However I couldn't obviously see any other reports of
> this, so here it is.

It's a compiler false-positive. The straightforward fix is to disable the warning
For gcc9+, and it's in Andrew Morton's tree alrady. but there's some discussion 
ongoing on how it should be mitigated properlu:

https://lore.kernel.org/all/0ab2702f-8245-4f02-beb7-dcc7d79d5416@app.fastmail.com/T/

Thanks,
YUry

  reply	other threads:[~2024-05-30 17:11 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-27 15:23 [Intel-wired-lan] [PATCH net-next v6 00/21] ice: add PFCP filter support Alexander Lobakin
2024-03-27 15:23 ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 01/21] lib/bitmap: add bitmap_{read, write}() Alexander Lobakin
2024-03-27 15:23   ` [PATCH net-next v6 01/21] lib/bitmap: add bitmap_{read,write}() Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 02/21] lib/test_bitmap: add tests for bitmap_{read, write}() Alexander Lobakin
2024-03-27 15:23   ` [PATCH net-next v6 02/21] lib/test_bitmap: add tests for bitmap_{read,write}() Alexander Lobakin
2024-03-27 15:47   ` [Intel-wired-lan] [PATCH net-next v6 02/21] lib/test_bitmap: add tests for bitmap_{read, write}() Andy Shevchenko
2024-03-27 15:47     ` [PATCH net-next v6 02/21] lib/test_bitmap: add tests for bitmap_{read,write}() Andy Shevchenko
2024-03-27 16:49     ` [Intel-wired-lan] [PATCH net-next v6 02/21] lib/test_bitmap: add tests for bitmap_{read, write}() Alexander Lobakin
2024-03-27 16:49       ` [PATCH net-next v6 02/21] lib/test_bitmap: add tests for bitmap_{read,write}() Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 03/21] lib/test_bitmap: use pr_info() for non-error messages Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 04/21] bitops: add missing prototype check Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 05/21] bitops: make BYTES_TO_BITS() treewide-available Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 06/21] bitops: let the compiler optimize {__, }assign_bit() Alexander Lobakin
2024-03-27 15:23   ` [PATCH net-next v6 06/21] bitops: let the compiler optimize {__,}assign_bit() Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 07/21] linkmode: convert linkmode_{test, set, clear, mod}_bit() to macros Alexander Lobakin
2024-03-27 15:23   ` [PATCH net-next v6 07/21] linkmode: convert linkmode_{test,set,clear,mod}_bit() " Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 08/21] s390/cio: rename bitmap_size() -> idset_bitmap_size() Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 09/21] fs/ntfs3: add prefix to bitmap_size() and use BITS_TO_U64() Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 10/21] btrfs: rename bitmap_set_bits() -> btrfs_bitmap_set_bits() Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 11/21] tools: move alignment-related macros to new <linux/align.h> Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 12/21] bitmap: introduce generic optimized bitmap_size() Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 13/21] bitmap: make bitmap_{get, set}_value8() use bitmap_{read, write}() Alexander Lobakin
2024-03-27 15:23   ` [PATCH net-next v6 13/21] bitmap: make bitmap_{get,set}_value8() use bitmap_{read,write}() Alexander Lobakin
2024-05-29 15:12   ` [Intel-wired-lan] [PATCH net-next v6 13/21] bitmap: make bitmap_{get, set}_value8() use bitmap_{read, write}() Robin Murphy
2024-05-29 15:12     ` [PATCH net-next v6 13/21] bitmap: make bitmap_{get,set}_value8() use bitmap_{read,write}() Robin Murphy
2024-05-30 17:11     ` Yury Norov [this message]
2024-05-30 17:11       ` Yury Norov
2024-05-30 17:50       ` [Intel-wired-lan] [PATCH net-next v6 13/21] bitmap: make bitmap_{get, set}_value8() use bitmap_{read, write}() Robin Murphy
2024-05-30 17:50         ` [PATCH net-next v6 13/21] bitmap: make bitmap_{get,set}_value8() use bitmap_{read,write}() Robin Murphy
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 14/21] lib/bitmap: add compile-time test for __assign_bit() optimization Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 15/21] ip_tunnel: use a separate struct to store tunnel params in the kernel Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-04-04 14:24   ` [Intel-wired-lan] " Dan Carpenter
2024-04-04 14:24     ` Dan Carpenter
2024-04-04 15:47     ` [Intel-wired-lan] " Alexander Lobakin
2024-04-04 15:47       ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 16/21] ip_tunnel: convert __be16 tunnel flags to bitmaps Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 17/21] net: net_test: add tests for IP tunnel flags conversion helpers Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 18/21] pfcp: add PFCP module Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 19/21] pfcp: always set pfcp metadata Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-04-03 20:59   ` [Intel-wired-lan] " Arnd Bergmann
2024-04-03 20:59     ` Arnd Bergmann
2024-04-04  9:45     ` [Intel-wired-lan] " Michal Swiatkowski
2024-04-04  9:45       ` Michal Swiatkowski
2024-04-04  9:56       ` [Intel-wired-lan] " Arnd Bergmann
2024-04-04  9:56         ` Arnd Bergmann
2024-04-04 10:12         ` [Intel-wired-lan] " Michal Swiatkowski
2024-04-04 10:12           ` Michal Swiatkowski
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 20/21] ice: refactor ICE_TC_FLWR_FIELD_ENC_OPTS Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-03-27 15:23 ` [Intel-wired-lan] [PATCH net-next v6 21/21] ice: Add support for PFCP hardware offload in switchdev Alexander Lobakin
2024-03-27 15:23   ` Alexander Lobakin
2024-04-01 10:00 ` [Intel-wired-lan] [PATCH net-next v6 00/21] ice: add PFCP filter support patchwork-bot+netdevbpf
2024-04-01 10:00   ` patchwork-bot+netdevbpf
2024-05-15 11:55 ` [Intel-wired-lan] " Harald Welte
2024-05-15 11:55   ` Harald Welte
2024-05-16 10:44   ` [Intel-wired-lan] " Michal Swiatkowski
2024-05-16 10:44     ` Michal Swiatkowski
2024-05-16 21:30     ` Harald Welte
2024-05-16 21:30       ` Harald Welte
2024-05-17 14:01       ` Michal Swiatkowski
2024-05-17 14:01         ` Michal Swiatkowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZlizL6d1_ePq-eKs@yury-ThinkPad \
    --to=yury.norov@gmail.com \
    --cc=aleksander.lobakin@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=glider@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nex.sw.ncis.osdt.itp.upstreaming@intel.com \
    --cc=pabeni@redhat.com \
    --cc=robin.murphy@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.