public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] staging: wlan-ng: remove commented debug printk messages
@ 2022-10-21 19:33 Deepak R Varma
  2022-10-23 13:35 ` Bagas Sanjaya
  0 siblings, 1 reply; 8+ messages in thread
From: Deepak R Varma @ 2022-10-21 19:33 UTC (permalink / raw)
  To: outreachy, gregkh, linux-staging, linux-kernel

printk messages are added for program flow tracing and are left
commented. These commented log messages should be removed as they
are no more useful for program execution.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---

Changes in v2:
   1. Resending as v2 since I incorrectly send multiple emails for the patch
   earlier. Feedback from gregkh@linuxfoundation.org


 drivers/staging/wlan-ng/p80211netdev.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
index e04fc666d218..6bef419e8ad0 100644
--- a/drivers/staging/wlan-ng/p80211netdev.c
+++ b/drivers/staging/wlan-ng/p80211netdev.c
@@ -881,55 +881,42 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
 		wlandev->rx.mgmt++;
 		switch (fstype) {
 		case WLAN_FSTYPE_ASSOCREQ:
-			/* printk("assocreq"); */
 			wlandev->rx.assocreq++;
 			break;
 		case WLAN_FSTYPE_ASSOCRESP:
-			/* printk("assocresp"); */
 			wlandev->rx.assocresp++;
 			break;
 		case WLAN_FSTYPE_REASSOCREQ:
-			/* printk("reassocreq"); */
 			wlandev->rx.reassocreq++;
 			break;
 		case WLAN_FSTYPE_REASSOCRESP:
-			/* printk("reassocresp"); */
 			wlandev->rx.reassocresp++;
 			break;
 		case WLAN_FSTYPE_PROBEREQ:
-			/* printk("probereq"); */
 			wlandev->rx.probereq++;
 			break;
 		case WLAN_FSTYPE_PROBERESP:
-			/* printk("proberesp"); */
 			wlandev->rx.proberesp++;
 			break;
 		case WLAN_FSTYPE_BEACON:
-			/* printk("beacon"); */
 			wlandev->rx.beacon++;
 			break;
 		case WLAN_FSTYPE_ATIM:
-			/* printk("atim"); */
 			wlandev->rx.atim++;
 			break;
 		case WLAN_FSTYPE_DISASSOC:
-			/* printk("disassoc"); */
 			wlandev->rx.disassoc++;
 			break;
 		case WLAN_FSTYPE_AUTHEN:
-			/* printk("authen"); */
 			wlandev->rx.authen++;
 			break;
 		case WLAN_FSTYPE_DEAUTHEN:
-			/* printk("deauthen"); */
 			wlandev->rx.deauthen++;
 			break;
 		default:
-			/* printk("unknown"); */
 			wlandev->rx.mgmt_unknown++;
 			break;
 		}
-		/* printk("\n"); */
 		drop = 2;
 		break;

@@ -943,35 +930,27 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
 		wlandev->rx.ctl++;
 		switch (fstype) {
 		case WLAN_FSTYPE_PSPOLL:
-			/* printk("pspoll"); */
 			wlandev->rx.pspoll++;
 			break;
 		case WLAN_FSTYPE_RTS:
-			/* printk("rts"); */
 			wlandev->rx.rts++;
 			break;
 		case WLAN_FSTYPE_CTS:
-			/* printk("cts"); */
 			wlandev->rx.cts++;
 			break;
 		case WLAN_FSTYPE_ACK:
-			/* printk("ack"); */
 			wlandev->rx.ack++;
 			break;
 		case WLAN_FSTYPE_CFEND:
-			/* printk("cfend"); */
 			wlandev->rx.cfend++;
 			break;
 		case WLAN_FSTYPE_CFENDCFACK:
-			/* printk("cfendcfack"); */
 			wlandev->rx.cfendcfack++;
 			break;
 		default:
-			/* printk("unknown"); */
 			wlandev->rx.ctl_unknown++;
 			break;
 		}
-		/* printk("\n"); */
 		drop = 2;
 		break;

@@ -1007,7 +986,6 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
 			wlandev->rx.cfack_cfpoll++;
 			break;
 		default:
-			/* printk("unknown"); */
 			wlandev->rx.data_unknown++;
 			break;
 		}
--
2.30.2




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] staging: wlan-ng: remove commented debug printk messages
  2022-10-23 13:35 ` Bagas Sanjaya
@ 2022-10-21 22:40   ` Deepak R Varma
  2022-10-24  3:10     ` Bagas Sanjaya
  2022-10-23 14:13   ` Greg KH
  1 sibling, 1 reply; 8+ messages in thread
From: Deepak R Varma @ 2022-10-21 22:40 UTC (permalink / raw)
  To: Bagas Sanjaya; +Cc: outreachy, gregkh, linux-staging, linux-kernel

On Sun, Oct 23, 2022 at 08:35:47PM +0700, Bagas Sanjaya wrote:
> On Sat, Oct 22, 2022 at 01:03:42AM +0530, Deepak R Varma wrote:
> > diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
> > index e04fc666d218..6bef419e8ad0 100644
> > --- a/drivers/staging/wlan-ng/p80211netdev.c
> > +++ b/drivers/staging/wlan-ng/p80211netdev.c
> > @@ -881,55 +881,42 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
> >  		wlandev->rx.mgmt++;
> >  		switch (fstype) {
> >  		case WLAN_FSTYPE_ASSOCREQ:
> > -			/* printk("assocreq"); */
> >  			wlandev->rx.assocreq++;
> >  			break;
> >  			wlandev->rx.ctl_unknown++;
> >  			break;
> >  		}
> > -		/* printk("\n"); */
> >  		drop = 2;
> >  		break;
> >
> > @@ -1007,7 +986,6 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
> >  			wlandev->rx.cfack_cfpoll++;
> >  			break;
> >  		default:
> > -			/* printk("unknown"); */
> >  			wlandev->rx.data_unknown++;
> >  			break;
> >  		}
>
> Shouldn't these printks be guarded under CONFIG_DEBUG_KERNEL instead?

Hi Sanjaya,
Sure they can, but I think they are very basic tracing message and do not appear
to carry much of information useful the event of debugging. Do you have a
suggestion on what additional information may be added to make them more useful?

If you still think we should have then in the CONFIG_DEBUG_KERNEL guard, let me
know and I will attempt to improve these.

Thank you,
./drv

>
> --
> An old man doll... just what I always wanted! - Clara





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] staging: wlan-ng: remove commented debug printk messages
  2022-10-21 19:33 [PATCH v2] staging: wlan-ng: remove commented debug printk messages Deepak R Varma
@ 2022-10-23 13:35 ` Bagas Sanjaya
  2022-10-21 22:40   ` Deepak R Varma
  2022-10-23 14:13   ` Greg KH
  0 siblings, 2 replies; 8+ messages in thread
From: Bagas Sanjaya @ 2022-10-23 13:35 UTC (permalink / raw)
  To: Deepak R Varma; +Cc: outreachy, gregkh, linux-staging, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3114 bytes --]

On Sat, Oct 22, 2022 at 01:03:42AM +0530, Deepak R Varma wrote:
> diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
> index e04fc666d218..6bef419e8ad0 100644
> --- a/drivers/staging/wlan-ng/p80211netdev.c
> +++ b/drivers/staging/wlan-ng/p80211netdev.c
> @@ -881,55 +881,42 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
>  		wlandev->rx.mgmt++;
>  		switch (fstype) {
>  		case WLAN_FSTYPE_ASSOCREQ:
> -			/* printk("assocreq"); */
>  			wlandev->rx.assocreq++;
>  			break;
>  		case WLAN_FSTYPE_ASSOCRESP:
> -			/* printk("assocresp"); */
>  			wlandev->rx.assocresp++;
>  			break;
>  		case WLAN_FSTYPE_REASSOCREQ:
> -			/* printk("reassocreq"); */
>  			wlandev->rx.reassocreq++;
>  			break;
>  		case WLAN_FSTYPE_REASSOCRESP:
> -			/* printk("reassocresp"); */
>  			wlandev->rx.reassocresp++;
>  			break;
>  		case WLAN_FSTYPE_PROBEREQ:
> -			/* printk("probereq"); */
>  			wlandev->rx.probereq++;
>  			break;
>  		case WLAN_FSTYPE_PROBERESP:
> -			/* printk("proberesp"); */
>  			wlandev->rx.proberesp++;
>  			break;
>  		case WLAN_FSTYPE_BEACON:
> -			/* printk("beacon"); */
>  			wlandev->rx.beacon++;
>  			break;
>  		case WLAN_FSTYPE_ATIM:
> -			/* printk("atim"); */
>  			wlandev->rx.atim++;
>  			break;
>  		case WLAN_FSTYPE_DISASSOC:
> -			/* printk("disassoc"); */
>  			wlandev->rx.disassoc++;
>  			break;
>  		case WLAN_FSTYPE_AUTHEN:
> -			/* printk("authen"); */
>  			wlandev->rx.authen++;
>  			break;
>  		case WLAN_FSTYPE_DEAUTHEN:
> -			/* printk("deauthen"); */
>  			wlandev->rx.deauthen++;
>  			break;
>  		default:
> -			/* printk("unknown"); */
>  			wlandev->rx.mgmt_unknown++;
>  			break;
>  		}
> -		/* printk("\n"); */
>  		drop = 2;
>  		break;
> 
> @@ -943,35 +930,27 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
>  		wlandev->rx.ctl++;
>  		switch (fstype) {
>  		case WLAN_FSTYPE_PSPOLL:
> -			/* printk("pspoll"); */
>  			wlandev->rx.pspoll++;
>  			break;
>  		case WLAN_FSTYPE_RTS:
> -			/* printk("rts"); */
>  			wlandev->rx.rts++;
>  			break;
>  		case WLAN_FSTYPE_CTS:
> -			/* printk("cts"); */
>  			wlandev->rx.cts++;
>  			break;
>  		case WLAN_FSTYPE_ACK:
> -			/* printk("ack"); */
>  			wlandev->rx.ack++;
>  			break;
>  		case WLAN_FSTYPE_CFEND:
> -			/* printk("cfend"); */
>  			wlandev->rx.cfend++;
>  			break;
>  		case WLAN_FSTYPE_CFENDCFACK:
> -			/* printk("cfendcfack"); */
>  			wlandev->rx.cfendcfack++;
>  			break;
>  		default:
> -			/* printk("unknown"); */
>  			wlandev->rx.ctl_unknown++;
>  			break;
>  		}
> -		/* printk("\n"); */
>  		drop = 2;
>  		break;
> 
> @@ -1007,7 +986,6 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
>  			wlandev->rx.cfack_cfpoll++;
>  			break;
>  		default:
> -			/* printk("unknown"); */
>  			wlandev->rx.data_unknown++;
>  			break;
>  		}

Shouldn't these printks be guarded under CONFIG_DEBUG_KERNEL instead?

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] staging: wlan-ng: remove commented debug printk messages
  2022-10-23 13:35 ` Bagas Sanjaya
  2022-10-21 22:40   ` Deepak R Varma
@ 2022-10-23 14:13   ` Greg KH
  2022-10-24  3:08     ` Bagas Sanjaya
  1 sibling, 1 reply; 8+ messages in thread
From: Greg KH @ 2022-10-23 14:13 UTC (permalink / raw)
  To: Bagas Sanjaya; +Cc: Deepak R Varma, outreachy, linux-staging, linux-kernel

On Sun, Oct 23, 2022 at 08:35:47PM +0700, Bagas Sanjaya wrote:
> On Sat, Oct 22, 2022 at 01:03:42AM +0530, Deepak R Varma wrote:
> > diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
> > index e04fc666d218..6bef419e8ad0 100644
> > --- a/drivers/staging/wlan-ng/p80211netdev.c
> > +++ b/drivers/staging/wlan-ng/p80211netdev.c
> > @@ -881,55 +881,42 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
> >  		wlandev->rx.mgmt++;
> >  		switch (fstype) {
> >  		case WLAN_FSTYPE_ASSOCREQ:
> > -			/* printk("assocreq"); */
> >  			wlandev->rx.assocreq++;
> >  			break;
> >  		case WLAN_FSTYPE_ASSOCRESP:
> > -			/* printk("assocresp"); */
> >  			wlandev->rx.assocresp++;
> >  			break;
> >  		case WLAN_FSTYPE_REASSOCREQ:
> > -			/* printk("reassocreq"); */
> >  			wlandev->rx.reassocreq++;
> >  			break;
> >  		case WLAN_FSTYPE_REASSOCRESP:
> > -			/* printk("reassocresp"); */
> >  			wlandev->rx.reassocresp++;
> >  			break;
> >  		case WLAN_FSTYPE_PROBEREQ:
> > -			/* printk("probereq"); */
> >  			wlandev->rx.probereq++;
> >  			break;
> >  		case WLAN_FSTYPE_PROBERESP:
> > -			/* printk("proberesp"); */
> >  			wlandev->rx.proberesp++;
> >  			break;
> >  		case WLAN_FSTYPE_BEACON:
> > -			/* printk("beacon"); */
> >  			wlandev->rx.beacon++;
> >  			break;
> >  		case WLAN_FSTYPE_ATIM:
> > -			/* printk("atim"); */
> >  			wlandev->rx.atim++;
> >  			break;
> >  		case WLAN_FSTYPE_DISASSOC:
> > -			/* printk("disassoc"); */
> >  			wlandev->rx.disassoc++;
> >  			break;
> >  		case WLAN_FSTYPE_AUTHEN:
> > -			/* printk("authen"); */
> >  			wlandev->rx.authen++;
> >  			break;
> >  		case WLAN_FSTYPE_DEAUTHEN:
> > -			/* printk("deauthen"); */
> >  			wlandev->rx.deauthen++;
> >  			break;
> >  		default:
> > -			/* printk("unknown"); */
> >  			wlandev->rx.mgmt_unknown++;
> >  			break;
> >  		}
> > -		/* printk("\n"); */
> >  		drop = 2;
> >  		break;
> > 
> > @@ -943,35 +930,27 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
> >  		wlandev->rx.ctl++;
> >  		switch (fstype) {
> >  		case WLAN_FSTYPE_PSPOLL:
> > -			/* printk("pspoll"); */
> >  			wlandev->rx.pspoll++;
> >  			break;
> >  		case WLAN_FSTYPE_RTS:
> > -			/* printk("rts"); */
> >  			wlandev->rx.rts++;
> >  			break;
> >  		case WLAN_FSTYPE_CTS:
> > -			/* printk("cts"); */
> >  			wlandev->rx.cts++;
> >  			break;
> >  		case WLAN_FSTYPE_ACK:
> > -			/* printk("ack"); */
> >  			wlandev->rx.ack++;
> >  			break;
> >  		case WLAN_FSTYPE_CFEND:
> > -			/* printk("cfend"); */
> >  			wlandev->rx.cfend++;
> >  			break;
> >  		case WLAN_FSTYPE_CFENDCFACK:
> > -			/* printk("cfendcfack"); */
> >  			wlandev->rx.cfendcfack++;
> >  			break;
> >  		default:
> > -			/* printk("unknown"); */
> >  			wlandev->rx.ctl_unknown++;
> >  			break;
> >  		}
> > -		/* printk("\n"); */
> >  		drop = 2;
> >  		break;
> > 
> > @@ -1007,7 +986,6 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
> >  			wlandev->rx.cfack_cfpoll++;
> >  			break;
> >  		default:
> > -			/* printk("unknown"); */
> >  			wlandev->rx.data_unknown++;
> >  			break;
> >  		}
> 
> Shouldn't these printks be guarded under CONFIG_DEBUG_KERNEL instead?

No, they should just be removed as was done here.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] staging: wlan-ng: remove commented debug printk messages
  2022-10-23 14:13   ` Greg KH
@ 2022-10-24  3:08     ` Bagas Sanjaya
  2022-10-24  4:08       ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Bagas Sanjaya @ 2022-10-24  3:08 UTC (permalink / raw)
  To: Greg KH; +Cc: Deepak R Varma, outreachy, linux-staging, linux-kernel

On 10/23/22 21:13, Greg KH wrote:
>>> @@ -1007,7 +986,6 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
>>>  			wlandev->rx.cfack_cfpoll++;
>>>  			break;
>>>  		default:
>>> -			/* printk("unknown"); */
>>>  			wlandev->rx.data_unknown++;
>>>  			break;
>>>  		}
>>
>> Shouldn't these printks be guarded under CONFIG_DEBUG_KERNEL instead?
> 
> No, they should just be removed as was done here.
> 

What if in case of debugging without these printks?

-- 
An old man doll... just what I always wanted! - Clara


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] staging: wlan-ng: remove commented debug printk messages
  2022-10-21 22:40   ` Deepak R Varma
@ 2022-10-24  3:10     ` Bagas Sanjaya
  0 siblings, 0 replies; 8+ messages in thread
From: Bagas Sanjaya @ 2022-10-24  3:10 UTC (permalink / raw)
  To: Deepak R Varma; +Cc: outreachy, gregkh, linux-staging, linux-kernel

On 10/22/22 05:40, Deepak R Varma wrote:
> On Sun, Oct 23, 2022 at 08:35:47PM +0700, Bagas Sanjaya wrote:
>> On Sat, Oct 22, 2022 at 01:03:42AM +0530, Deepak R Varma wrote:
>>> diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
>>> index e04fc666d218..6bef419e8ad0 100644
>>> --- a/drivers/staging/wlan-ng/p80211netdev.c
>>> +++ b/drivers/staging/wlan-ng/p80211netdev.c
>>> @@ -881,55 +881,42 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
>>>  		wlandev->rx.mgmt++;
>>>  		switch (fstype) {
>>>  		case WLAN_FSTYPE_ASSOCREQ:
>>> -			/* printk("assocreq"); */
>>>  			wlandev->rx.assocreq++;
>>>  			break;
>>>  			wlandev->rx.ctl_unknown++;
>>>  			break;
>>>  		}
>>> -		/* printk("\n"); */
>>>  		drop = 2;
>>>  		break;
>>>
>>> @@ -1007,7 +986,6 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
>>>  			wlandev->rx.cfack_cfpoll++;
>>>  			break;
>>>  		default:
>>> -			/* printk("unknown"); */
>>>  			wlandev->rx.data_unknown++;
>>>  			break;
>>>  		}
>>
>> Shouldn't these printks be guarded under CONFIG_DEBUG_KERNEL instead?
> 
> Hi Sanjaya,
> Sure they can, but I think they are very basic tracing message and do not appear
> to carry much of information useful the event of debugging. Do you have a
> suggestion on what additional information may be added to make them more useful?
> 
> If you still think we should have then in the CONFIG_DEBUG_KERNEL guard, let me
> know and I will attempt to improve these.
> 
> Thank you,
> ./drv
> 

Greg said we should just deleting these printks [1].

[1]: https://lore.kernel.org/lkml/Y1VL%2FwITM64U6qLi@kroah.com/

Thanks anyway.

-- 
An old man doll... just what I always wanted! - Clara


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] staging: wlan-ng: remove commented debug printk messages
  2022-10-24  3:08     ` Bagas Sanjaya
@ 2022-10-24  4:08       ` Greg KH
  2022-10-24  9:29         ` Bagas Sanjaya
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2022-10-24  4:08 UTC (permalink / raw)
  To: Bagas Sanjaya; +Cc: Deepak R Varma, outreachy, linux-staging, linux-kernel

On Mon, Oct 24, 2022 at 10:08:00AM +0700, Bagas Sanjaya wrote:
> On 10/23/22 21:13, Greg KH wrote:
> >>> @@ -1007,7 +986,6 @@ static int p80211_rx_typedrop(struct wlandevice *wlandev, u16 fc)
> >>>  			wlandev->rx.cfack_cfpoll++;
> >>>  			break;
> >>>  		default:
> >>> -			/* printk("unknown"); */
> >>>  			wlandev->rx.data_unknown++;
> >>>  			break;
> >>>  		}
> >>
> >> Shouldn't these printks be guarded under CONFIG_DEBUG_KERNEL instead?
> > 
> > No, they should just be removed as was done here.
> > 
> 
> What if in case of debugging without these printks?

I can not parse this line, sorry.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] staging: wlan-ng: remove commented debug printk messages
  2022-10-24  4:08       ` Greg KH
@ 2022-10-24  9:29         ` Bagas Sanjaya
  0 siblings, 0 replies; 8+ messages in thread
From: Bagas Sanjaya @ 2022-10-24  9:29 UTC (permalink / raw)
  To: Greg KH; +Cc: Deepak R Varma, outreachy, linux-staging, linux-kernel

On 10/24/22 11:08, Greg KH wrote:
>>>> Shouldn't these printks be guarded under CONFIG_DEBUG_KERNEL instead?
>>>
>>> No, they should just be removed as was done here.
>>>
>>
>> What if in case of debugging without these printks?
> 
> I can not parse this line, sorry.

OK.

Ah, I mean I have to see Documentation/dev-tools/kgdb.rst and
Documentation/dev-tools/gdb-kernel-debugging.rst.

-- 
An old man doll... just what I always wanted! - Clara


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-10-24  9:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-21 19:33 [PATCH v2] staging: wlan-ng: remove commented debug printk messages Deepak R Varma
2022-10-23 13:35 ` Bagas Sanjaya
2022-10-21 22:40   ` Deepak R Varma
2022-10-24  3:10     ` Bagas Sanjaya
2022-10-23 14:13   ` Greg KH
2022-10-24  3:08     ` Bagas Sanjaya
2022-10-24  4:08       ` Greg KH
2022-10-24  9:29         ` Bagas Sanjaya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox