[PATCHv2 wireless-next] wifi: brcmsmac: use FAM for debug code

[PATCHv2 wireless-next] wifi: brcmsmac: use FAM for debug code

[Rosen Penev]

Debug code requires a separate allocation to duplicate a string. A FAM
allows properly sized allocation with a single kfree.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: rebase and remove debug from member.
 .../broadcom/brcm80211/brcmsmac/mac80211_if.c     | 15 +++++----------
 .../broadcom/brcm80211/brcmsmac/mac80211_if.h     |  4 +---
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
index 6255d673d2d3..7912a999f6f7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
@@ -317,9 +317,6 @@ static void brcms_free(struct brcms_info *wl)
 	/* free timers */
 	for (t = wl->timers; t; t = next) {
 		next = t->next;
-#ifdef DEBUG
-		kfree(t->name);
-#endif
 		kfree(t);
 	}
 }
@@ -1499,7 +1496,11 @@ struct brcms_timer *brcms_init_timer(struct brcms_info *wl,
 {
 	struct brcms_timer *t;

+#ifdef DEBUG
+	t = kzalloc_flex(*t, name, strlen(name) + 1, GFP_ATOMIC);
+#else
 	t = kzalloc_obj(*t, GFP_ATOMIC);
+#endif
 	if (!t)
 		return NULL;

@@ -1511,7 +1512,7 @@ struct brcms_timer *brcms_init_timer(struct brcms_info *wl,
 	wl->timers = t;

 #ifdef DEBUG
-	t->name = kstrdup(name, GFP_ATOMIC);
+	strcpy(t->name, name);
 #endif

 	return t;
@@ -1574,9 +1575,6 @@ void brcms_free_timer(struct brcms_timer *t)

 	if (wl->timers == t) {
 		wl->timers = wl->timers->next;
-#ifdef DEBUG
-		kfree(t->name);
-#endif
 		kfree(t);
 		return;

@@ -1586,9 +1584,6 @@ void brcms_free_timer(struct brcms_timer *t)
 	while (tmp) {
 		if (tmp->next == t) {
 			tmp->next = t->next;
-#ifdef DEBUG
-			kfree(t->name);
-#endif
 			kfree(t);
 			return;
 		}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.h b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.h
index eaf926a96a88..3b25a56958b3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.h
@@ -42,9 +42,7 @@ struct brcms_timer {
 	bool periodic;
 	bool set;		/* indicates if timer is active */
 	struct brcms_timer *next;	/* for freeing on unload */
-#ifdef DEBUG
-	char *name;		/* Description of the timer */
-#endif
+	char name[];		/* Description of the timer */
 };

 struct brcms_if {
--
2.53.0

Re: [PATCHv2 wireless-next] wifi: brcmsmac: use FAM for debug code

[Julian Calaby]

Hi Arend,

On Tue, Mar 10, 2026 at 8:59 AM Rosen Penev <rosenp@gmail.com> wrote:
>
> Debug code requires a separate allocation to duplicate a string. A FAM
> allows properly sized allocation with a single kfree.

Sorry Rosen for hijacking your patch here.

With these changes, does allocating and copying the string really need
to be behind a DEBUG ifdef?

The allocation, copying and freeing of the memory isn't in a hot path,
so allocating/freeing a couple more bytes shouldn't matter that much,
which only leaves the memory footprint, which appears to be less than
10 bytes.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCHv2 wireless-next] wifi: brcmsmac: use FAM for debug code

[Rosen Penev]

On Mon, Mar 9, 2026 at 6:11 PM Julian Calaby <julian.calaby@gmail.com> wrote:
>
> Hi Arend,
>
> On Tue, Mar 10, 2026 at 8:59 AM Rosen Penev <rosenp@gmail.com> wrote:
> >
> > Debug code requires a separate allocation to duplicate a string. A FAM
> > allows properly sized allocation with a single kfree.
>
> Sorry Rosen for hijacking your patch here.
>
> With these changes, does allocating and copying the string really need
> to be behind a DEBUG ifdef?
I don't know. I didn't write this code.
>
> The allocation, copying and freeing of the memory isn't in a hot path,
> so allocating/freeing a couple more bytes shouldn't matter that much,
> which only leaves the memory footprint, which appears to be less than
> 10 bytes.
>
> Thanks,
>
> --
> Julian Calaby
>
> Email: julian.calaby@gmail.com
> Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCHv2 wireless-next] wifi: brcmsmac: use FAM for debug code

[Arend van Spriel]

On 10/03/2026 02:47, Rosen Penev wrote:
> On Mon, Mar 9, 2026 at 6:11 PM Julian Calaby <julian.calaby@gmail.com> wrote:
>>
>> Hi Arend,
>>
>> On Tue, Mar 10, 2026 at 8:59 AM Rosen Penev <rosenp@gmail.com> wrote:
>>>
>>> Debug code requires a separate allocation to duplicate a string. A FAM
>>> allows properly sized allocation with a single kfree.
>>
>> Sorry Rosen for hijacking your patch here.
>>
>> With these changes, does allocating and copying the string really need
>> to be behind a DEBUG ifdef?
> I don't know. I didn't write this code.

Thanks, Rosen

I did before the concept of FAM landed in the kernel. Whether or not the 
#ifdef DEBUG is warranted is simply a choice. I prefer to have clean 
separation of functionality and the related data. If the code using the 
data is all conditional under DEBUG define then the data must be as well.

I understand that FAM and compiler support for it has its advantages, 
but this is more churn than gain. The code is functional as is and 
removing #defines for the sake of changing to a FAM seems not justified 
for the advantages which in my opinion are pretty limited in this 
particular case.

Regards,
Arend