[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXqHbhkXPiZVpqcS@smile.fi.intel.com>
Date: Thu, 29 Jan 2026 00:02:22 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Minu Jin <s9430939@...er.com>
Cc: gregkh@...uxfoundation.org, hansg@...nel.org, dan.carpenter@...aro.org,
trohan2000@...il.com, andy@...nel.org,
linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
straube.linux@...il.com
Subject: Re: [PATCH v3 2/4] staging: rtl8723bs: replace rtw_zmalloc with
kzalloc
On Wed, Jan 28, 2026 at 11:09:52PM +0900, Minu Jin wrote:
> replaces the wrapper function rtw_zmalloc with kzalloc.
>
> I reviewed all the call sites to determine the appropriate GFP flags:
> - GFP_KERNEL: Used for initialization and configuration paths.
> init/probe, config/setup
>
> - GFP_ATOMIC: Used for critical real-time paths.
> including interrupts, timer handler, region where spinlocks are held
Below I suggest more changes, but probably they needs to be done as separate
patches. It depends on Greg's preferences.
The change itself LGTM.
...
> - ph2c = rtw_zmalloc(sizeof(struct cmd_obj));
> + ph2c = kzalloc(sizeof(struct cmd_obj), GFP_KERNEL);
Since you are touching this line it might be worth to check and convert to use
sizeof(*) instead (but it needs to be done with a carefulness.
> if (!ph2c) {
> res = _FAIL;
> goto exit;
> }
>
...
> - pcmdpriv->cmd_allocated_buf = rtw_zmalloc(MAX_CMDSZ + CMDBUFF_ALIGN_SZ);
> + pcmdpriv->cmd_allocated_buf = kzalloc(MAX_CMDSZ + CMDBUFF_ALIGN_SZ, GFP_ATOMIC);
>
You can drop this blank line at the same time, we usually don't split
allocations and checks.
> if (!pcmdpriv->cmd_allocated_buf)
> return -ENOMEM;
>
> pcmdpriv->cmd_buf = PTR_ALIGN(pcmdpriv->cmd_allocated_buf, CMDBUFF_ALIGN_SZ);
>
> - pcmdpriv->rsp_allocated_buf = rtw_zmalloc(MAX_RSPSZ + 4);
> + pcmdpriv->rsp_allocated_buf = kzalloc(MAX_RSPSZ + 4, GFP_ATOMIC);
>
Ditto.
> if (!pcmdpriv->rsp_allocated_buf) {
> kfree(pcmdpriv->cmd_allocated_buf);
...
> - passoc_req = rtw_zmalloc(psta->assoc_req_len);
> + passoc_req = kzalloc(psta->assoc_req_len, GFP_ATOMIC);
> if (passoc_req) {
> assoc_req_len = psta->assoc_req_len;
> memcpy(passoc_req, psta->passoc_req, assoc_req_len);
This might be a candidate to kmemdup(), but double check it (it looks like
_len:s maybe different.
...
> - spt_band = rtw_zmalloc(sizeof(struct ieee80211_supported_band) +
> + spt_band = kzalloc(sizeof(struct ieee80211_supported_band) +
> sizeof(struct ieee80211_channel) * n_channels +
> - sizeof(struct ieee80211_rate) * n_bitrates);
> + sizeof(struct ieee80211_rate) * n_bitrates, GFP_KERNEL);
This needs a bit of array_size(), struct_size() conversion.
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists