Re: [PATCH 5/5] wifi: mwifiex: use vmalloc_array() to simplify code

[Dan Carpenter <dan.carpenter@linaro.org>]

On Tue, Aug 12, 2025 at 10:13:43PM +0800, Qianfeng Rong wrote:
> 
> 在 2025/8/12 21:48, Dan Carpenter 写道:
> > On Tue, Aug 12, 2025 at 09:32:18PM +0800, Qianfeng Rong wrote:
> > > Remove array_size() calls and replace vmalloc() with vmalloc_array() to
> > > simplify the code.
> > > 
> > > Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
> > > ---
> > >   drivers/net/wireless/marvell/mwifiex/cfg80211.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > index 3498743d5ec0..fb4183ff02a9 100644
> > > --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > @@ -4673,8 +4673,8 @@ int mwifiex_init_channel_scan_gap(struct mwifiex_adapter *adapter)
> > >   	 * additional active scan request for hidden SSIDs on passive channels.
> > >   	 */
> > >   	adapter->num_in_chan_stats = 2 * (n_channels_bg + n_channels_a);
> > > -	adapter->chan_stats = vmalloc(array_size(sizeof(*adapter->chan_stats),
> > > -						 adapter->num_in_chan_stats));
> > > +	adapter->chan_stats = vmalloc_array(adapter->num_in_chan_stats,
> > > +					    sizeof(*adapter->chan_stats));
> > n_channels_bg is 14
> > n_channels_a is either 0 or 31 depending on if we're using BAND_A.
> > sizeof(*adapter->chan_stats) is 10.
> > 
> > So we're either allocating 280 or 900 bytes, which is quite small.  We
> > should just use kmalloc_array() instead of vmalloc_array().
> Switching to kmalloc_array() requires changing vfree() to kfree(),
> but I couldn't locate the memory release code.

It's not hard to locate with a bit of looking.  ;)

>  This modification
> likely requires deep familiarity with the codebase.  Also, for
> variable-sized allocations, kvmalloc_array() is preferable, but
> requires kvfree() for proper memory release.

Don't use kvmalloc_array().  The rules are:
1) small amounts of memory: kmalloc()
2) possibly large ammounts of memory up to 2GB: kvmalloc()
3) definitely large ammounts of memory or larger than 2GB: vmalloc()

There are also places where the memory needs to be contiguous and for
those situations kvmalloc() and vmalloc() can't be used.

Here we're allocating less than a PAGE so the appropriate thing is
kmalloc_array().

I'm looking at this some more and these statistics are updated in
mwifiex_update_chan_statistics() and not necessarily all of them
are updated.  But they're used in mwifiex_cfg80211_dump_survey()
when potentially the memory is still uninitialized.

I really think we should zero this memory, so lets allocate it
with kcalloc().

regards,
dan carpenter