Message-ID: <1c1439f4-b5d5-ba59-97a7-e2edeb879fa3@intel.com>
Date:   Tue, 14 Feb 2023 18:21:47 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH] wifi: mac80211: avoid u32_encode_bits() warning
Content-Language: en-US
To:     Arnd Bergmann <arnd@kernel.org>
CC:     Johannes Berg <johannes@sipsolutions.net>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, Arnd Bergmann <arnd@arndb.de>,
        Felix Fietkau <nbd@nbd.name>,
        Alexander Wetzel <alexander@wetzel-home.de>,
        =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= <toke@toke.dk>,
        Andrei Otcheretianski <andrei.otcheretianski@intel.com>,
        <linux-wireless@vger.kernel.org>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20230214132025.1532147-1-arnd@kernel.org>
From:   Alexander Lobakin <alexandr.lobakin@intel.com>
In-Reply-To: <20230214132025.1532147-1-arnd@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RGRWT0ZVc3JBYUlxdExMRG5GQ2VTV1NzLzRmMmV2WHptU1Z4Vis5ZktwbVR2?=
 =?utf-8?B?a3N0bnV3VzdTVHlQckxWbnVKZnpQdnlOS0lrN09leWwzTGp0NGVqY1ZiQVBQ?=
 =?utf-8?B?VDhxYkloQ1U1KzRZTExQVndoOFVBUkVIVWlmZk9GcGJxTGFJdjRsMExDU3hz?=
 =?utf-8?B?UEpvanMreFNkNGNvODl3MVBHNVFPVC9WUVFSbEtNek5YWHF1cnlORGMreEJM?=
 =?utf-8?B?cHRXVm1TbWlyckxCZWtVc0RlbUkwak1ZNlFLUW9zV0hKYTJ4d2hrN3lMN0RH?=
 =?utf-8?B?dlhickJNaERidFRKTFZiaWorU1h3VVhscXdoNXg0S1VHQk1ObDJuaGhldjZz?=
 =?utf-8?B?YmVoVHR3WFNBVmJSb3J4Mnk2c3FWYXVrRjN3dHhieU9PcFAyQlpLU2cybnlC?=
 =?utf-8?B?cUg2OStnZU1ocXBPaG5teU5NdmFTT3B5WGtiZ3RoRHFhSnNaOXJiWjdIMHdK?=
 =?utf-8?B?NEw2Vm5lSEVYTXpPWDlFTlJVM2pVUnlMa3hTZVB6UlVoZUYrYWRoRndJajJj?=
 =?utf-8?B?d0owT1dtQXlhZnJEUTJoZUo0OElJUGtRNlZ5OTd5L0Nac2FWWWU2OFlVOU5P?=
 =?utf-8?B?MnFPNnk1SGN2R0NpNUEzR0E0ZlNJaGFzRExiWm5uSlpPNlJsblhoREZPM2xZ?=
 =?utf-8?B?cFQwQXJENVFIVXg3YVBIMEZzajlYc2dSUStlYkUvKzFvNFFqZDN6K3VhdklG?=
 =?utf-8?B?T0NWSElSQmJ5UlVMeitUaW9oWlVScFpXWHhPN3ZnbGc1TVRzV0hndEd4Rkti?=
 =?utf-8?B?eHk0eVJJUjVuMGlaU3IwQ3dGeGM3bVBnd09TckRueHRFSWpNakk3RjhLZVBo?=
 =?utf-8?B?aFFOZmI2QTdRQ1NFbmMvaVJhRUNzeGNHSDBVZW8xeWYwYldRUEVjaTRuc3hX?=
 =?utf-8?B?NkFuL3FjUC9odnowZVRHZFp5eUc5ZDNpQlNaYWV5eU5hUkF5QXJuTGI4djBz?=
 =?utf-8?B?MCt6Y3dTWlhuWVZqL1ZrWVdIODRnNTdwbDBBTzJhWVh1M0lEa0V2WDJvc2I0?=
 =?utf-8?B?eTFRUk9FQUVCVnRVUENCVTV2Q01WaXNXaHU3bk8rb1hZQ0VPQWRGUjRKS1Iv?=
 =?utf-8?B?UEFOWWI5SjJPM3gxVVZWUDhDY2hBSk1RTFhsM0k2OWxXSXJGUnJpYkE2Sk9l?=
 =?utf-8?B?QmJ0Y3B1U2FYTFdjNHo2VXZHRVl3WjV5MEh2d1NGU0VxRENQUEJLbzJSbU5y?=
 =?utf-8?B?OHVPTDhlM2c1cHljUU5FOVp5UnNPWWx1YzBQN2xDN3JvdEd5b2tHTFZlWUlR?=
 =?utf-8?B?dXpTOUNFdllXZFpDRDdONzlrVG9Ua2xtS1NkREtmNlJ1Si9YaHJxTzcwdkZR?=
 =?utf-8?B?YjRjM05uZ1duY0JqYzdsU2RoYWRwcnhZRXVVTzBBNUdjZUthVjJhRXZCT2NO?=
 =?utf-8?B?Zkh1KzJrNG91bGdVWTFiZkVjaEMvRVFZZm4xZi9YT2pEK1ZoYlNkREV1UDEr?=
 =?utf-8?B?R3BZODNPWi9QcTVJMU1uZzhLQXhkeTRnUjR5M0JLQUhaNzVUY3dxeGpIclRF?=
 =?utf-8?B?eHB2SFFPVG5ORlRSU29UQk04bnVsTnpZK2Vpc09naWY1c2tQU0VuOEJheEwz?=
 =?utf-8?B?enFPTFNKdVJnNkVldlY5ZzNVY0JOUktLSCtXbXd4b2xEVHhDVDlxUFR3aFZ1?=
 =?utf-8?B?eTJxRjMyU2hSMXlVTExRM0dqZGRNNjZxWWpQRVdqZkJmVFpZZEpaMElYYWdu?=
 =?utf-8?B?VVFFNVF6NTlrSUtzWE1VNWR4YzA1NWRJRTBXYmxzZXFhamR5cE81ci9udERS?=
 =?utf-8?B?QVNacWVvM3NmWVU5UktiNVZPYjBCN3hxREU0Mit6aVBmT1ViVjFBVHRIVjUw?=
 =?utf-8?B?b05mVk82Q0U2Zk92NjB2Z1ZVK2FUODZXYldiTW9zRERkcjhKL3drbkNxVHNG?=
 =?utf-8?B?ZS9tMDl6N2UxalZxOXhmYUJiMUdlL3hXaWgxNExxa1ZMSnNNK3BEbHdYeWlx?=
 =?utf-8?B?RUdwMC9pUng4TjNVYnJML2pEWS9lRHUySURiWmRoMHZPUGdud245UzQ4SGp1?=
 =?utf-8?B?eFIvNGdKRXp1UFAwSU5ZbU1pTzJjb2ZSTlBoYy81eW1qZzlXalZubzFpcTJS?=
 =?utf-8?B?L0dQUldEbWdCYWJJU3UvdVB5VGswM05hcmNXSkVvZWpIMTZrd2h5TXNqamRM?=
 =?utf-8?B?ckJsWWIwMGZ6THpwV215alJtVUxZeU9zS3lzWmZsQnBtRjZxeFpFVEpzWi9Y?=
 =?utf-8?B?L0E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 98ab2dc4-7d12-4688-5f0e-08db0eb01f66
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2023 17:23:01.0486
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: zQNqfY15GwvQxpOx47Hvxez38FZi0JKmr2TNvH6yL4EdVBjqYRaOzwvNRGSTb1ef6fWaZA9ki7vJvFqcDzxIoomEMI3WZ2DPAnDWP2zAMNE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4705
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Arnd Bergmann <arnd@kernel.org>
Date: Tue, 14 Feb 2023 14:20:21 +0100

> From: Arnd Bergmann <arnd@arndb.de>
> 
> gcc-9 triggers a false-postive warning in ieee80211_mlo_multicast_tx()
> for u32_encode_bits(ffs(links) - 1, ...), since ffs() can return zero
> on an empty bitmask, and the negative argument to u32_encode_bits()
> is then out of range:
> 
> In file included from include/linux/ieee80211.h:21,
>                  from include/net/cfg80211.h:23,
>                  from net/mac80211/tx.c:23:
> In function 'u32_encode_bits',
>     inlined from 'ieee80211_mlo_multicast_tx' at net/mac80211/tx.c:4437:17,
>     inlined from 'ieee80211_subif_start_xmit' at net/mac80211/tx.c:4485:3:
> include/linux/bitfield.h:177:3: error: call to '__field_overflow' declared with attribute error: value doesn't fit into mask
>   177 |   __field_overflow();     \
>       |   ^~~~~~~~~~~~~~~~~~
> include/linux/bitfield.h:197:2: note: in expansion of macro '____MAKE_OP'
>   197 |  ____MAKE_OP(u##size,u##size,,)
>       |  ^~~~~~~~~~~
> include/linux/bitfield.h:200:1: note: in expansion of macro '__MAKE_OP'
>   200 | __MAKE_OP(32)
>       | ^~~~~~~~~
> 
> Newer compiler versions do not cause problems with the zero argument
> because they do not consider this a __builtin_constant_p().
> It's also harmless since the hweight16() check already guarantees
> that this cannot be 0.
> 
> Replace the ffs() with an equivalent find_first_bit() check that
> matches the later for_each_set_bit() style and avoids the warning.
> 
> Fixes: 963d0e8d08d9 ("wifi: mac80211: optionally implement MLO multicast TX")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  net/mac80211/tx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index defe97a31724..118648af979c 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -4434,7 +4434,7 @@ static void ieee80211_mlo_multicast_tx(struct net_device *dev,
>  	u32 ctrl_flags = IEEE80211_TX_CTRL_MCAST_MLO_FIRST_TX;
>  
>  	if (hweight16(links) == 1) {
> -		ctrl_flags |= u32_encode_bits(ffs(links) - 1,
> +		ctrl_flags |= u32_encode_bits(find_first_bit(&links, 16) - 1,

Uff, IIRC find_first_bit() matches __ffs() calling convention, not ffs()
one. They're off-by-one from each other, which means you need to drop
this `- 1`.
As this branch happens only when hweight is 1 => @links has a bit set,
it's safe to just use __ffs() here directly, but up to you.
find_first_bit() is fine, too, since it will be optimized out to __ffs()
at the end in this case.

>  					      IEEE80211_TX_CTRL_MLO_LINK);
>  
>  		__ieee80211_subif_start_xmit(skb, sdata->dev, 0, ctrl_flags,
Thanks,
Olek