staging: rtl8723au: Remove unnecessary OOM message
Subsystems (2)
PATCH 1/1 staging: rtl8723au: Remove unnecessary OOM message
This patch reduces the kernel size by removing error messages that duplicate the normal OOM message.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)
@@
identifier f,print,l;
expression e;
constant char[] c;
@@
e = \(kzalloc\|kmalloc\|devm_kzalloc\|devm_kmalloc\)(...); if (e = NULL) {
<+...
- print(...,c,...);
... when any
(
goto l;
|
return ...;
)
...+> }
--- drivers/staging/rtl8723au/hal/rtl8723a_cmd.c | 8 ++------ drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c | 8 ++------ drivers/staging/rtl8723au/hal/usb_ops_linux.c | 6 +----- drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c | 5 ----- 4 files changed, 5 insertions(+), 22 deletions(-)
drivers/staging/rtl8723au/hal/rtl8723a_cmd.c +2 -6
@@ -462,10 +462,8 @@ static void SetFwRsvdPagePkt(struct rtw_adapter *padapter, bool bDLFinished)DBG_8723A ("%s\n" , __func__); ReservedPagePacket =kzalloc (1000 ,GFP_KERNEL ); -if (ReservedPagePacket =NULL ) { -DBG_8723A ("%s: alloc ReservedPagePacket fail!\n" , __func__); +if (ReservedPagePacket =NULL )return ; - } pHalData =GET_HAL_DATA (padapter); pxmitpriv = &padapter->xmitpriv;
@@ -669,10 +667,8 @@ static void SetFwRsvdPagePkt_BTCoex(struct rtw_adapter *padapter)DBG_8723A ("+%s\n" , __func__); ReservedPagePacket =kzalloc (1024 ,GFP_KERNEL ); -if (ReservedPagePacket =NULL ) { -DBG_8723A ("%s: alloc ReservedPagePacket fail!\n" , __func__); +if (ReservedPagePacket =NULL )return ; - } pHalData =GET_HAL_DATA (padapter); pxmitpriv = &padapter->xmitpriv;
drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c +2 -6
@@ -401,10 +401,8 @@ hal_ReadEFuse_WiFi(struct rtw_adapter *padapter, } efuseTbl =kmalloc (EFUSE_MAP_LEN_8723A ,GFP_KERNEL ); -if (efuseTbl =NULL ) { -DBG_8723A ("%s: alloc efuseTbl fail!\n" , __func__); +if (efuseTbl =NULL )return ; - }/* 0xff will be efuse default value instead of 0x00. */ memset (efuseTbl,0xFF ,EFUSE_MAP_LEN_8723A );
@@ -494,10 +492,8 @@ hal_ReadEFuse_BT(struct rtw_adapter *padapter, } efuseTbl =kmalloc (EFUSE_BT_MAP_LEN ,GFP_KERNEL ); -if (efuseTbl =NULL ) { -DBG_8723A ("%s: efuseTbl malloc fail!\n" , __func__); +if (efuseTbl =NULL )return ; - }/* 0xff will be efuse default value instead of 0x00. */ memset (efuseTbl,0xFF ,EFUSE_BT_MAP_LEN );
drivers/staging/rtl8723au/hal/usb_ops_linux.c +1 -5
@@ -256,12 +256,8 @@ static void usb_read_interrupt_complete(struct urb *purb) c2w =kmalloc (sizeof (struct evt_work ),GFP_ATOMIC ); -if (!c2w) { -printk (KERN_WARNING "%s: unable to " -"allocate work buffer\n" , - __func__); +if (!c2w) goto urb_submit; - } c2w->adapter = padapter;INIT_WORK (&c2w->work, rtw_evt_work);
drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c +0 -5
@@ -1327,8 +1327,6 @@ static int rtw_cfg80211_set_probe_req_wpsp2pie(struct rtw_adapter *padapter, pmlmepriv->wps_probe_req_ie =kmemdup (wps_ie, wps_ie[1 ],GFP_KERNEL );if (pmlmepriv->wps_probe_req_ie =NULL ) { -DBG_8723A ("%s()-%d: kmalloc() ERROR!\n" , - __func__, __LINE__);return -EINVAL ; } pmlmepriv->wps_probe_req_ie_len = wps_ie[1 ];
@@ -2619,8 +2617,6 @@ static int rtw_cfg80211_add_monitor_if(struct rtw_adapter *padapter, char *name,/* wdev */ mon_wdev =kzalloc (sizeof (struct wireless_dev ),GFP_KERNEL );if (!mon_wdev) { -DBG_8723A ("%s(%s): allocate mon_wdev fail\n" , __func__, - padapter->pnetdev->name); ret = -ENOMEM ; goto out; }
@@ -3275,7 +3271,6 @@ int rtw_wdev_alloc(struct rtw_adapter *padapter, struct device *dev)/* wdev */ wdev =kzalloc (sizeof (struct wireless_dev ),GFP_KERNEL );if (!wdev) { -DBG_8723A ("Couldn't allocate wireless device\n" ); ret = -ENOMEM ; goto free_wiphy; }
Signature · 1 line
1.9.1
Jes Sorensen
NAKQuoted 5 lines · Quentin Lambert
This patch reduces the kernel size by removing error messages that duplicate the normal OOM message.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)
This patch removes useful warnings about what allocation failed. The messages removed are NOT duplicate!
NACK
Jes
Quoted 131 lines
@@
identifier f,print,l;
expression e;
constant char[] c;
@@e = \(kzalloc\|kmalloc\|devm_kzalloc\|devm_kmalloc\)(...); if (e = NULL) {
<+...
-print(...,c,...);
... when any
(
goto l;
|
return ...;
)
...+> }Signed-off-by Quentin Lambert <[email protected]>--- drivers/staging/rtl8723au/hal/rtl8723a_cmd.c | 8 ++------ drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c | 8 ++------ drivers/staging/rtl8723au/hal/usb_ops_linux.c | 6 +----- drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c | 5 ----- 4 files changed, 5 insertions(+), 22 deletions(-)drivers/staging/rtl8723au/hal/rtl8723a_cmd.c +2
-6@@ -462,10 +462,8 @@ static void SetFwRsvdPagePkt(struct rtw_adapter *padapter, bool bDLFinished)DBG_8723A ("%s\n" , __func__); ReservedPagePacket =kzalloc (1000 ,GFP_KERNEL ); -if (ReservedPagePacket =NULL ) { -DBG_8723A ("%s: alloc ReservedPagePacket fail!\n" , __func__); +if (ReservedPagePacket =NULL )return ; - } pHalData =GET_HAL_DATA (padapter); pxmitpriv = &padapter->xmitpriv;@@ -669,10 +667,8 @@ static void SetFwRsvdPagePkt_BTCoex(struct rtw_adapter *padapter)DBG_8723A ("+%s\n" , __func__); ReservedPagePacket =kzalloc (1024 ,GFP_KERNEL ); -if (ReservedPagePacket =NULL ) { -DBG_8723A ("%s: alloc ReservedPagePacket fail!\n" , __func__); +if (ReservedPagePacket =NULL )return ; - } pHalData =GET_HAL_DATA (padapter); pxmitpriv = &padapter->xmitpriv;drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c +2
-6@@ -401,10 +401,8 @@ hal_ReadEFuse_WiFi(struct rtw_adapter *padapter, } efuseTbl =kmalloc (EFUSE_MAP_LEN_8723A ,GFP_KERNEL ); -if (efuseTbl =NULL ) { -DBG_8723A ("%s: alloc efuseTbl fail!\n" , __func__); +if (efuseTbl =NULL )return ; - }/* 0xff will be efuse default value instead of 0x00. */ memset (efuseTbl,0xFF ,EFUSE_MAP_LEN_8723A );@@ -494,10 +492,8 @@ hal_ReadEFuse_BT(struct rtw_adapter *padapter, } efuseTbl =kmalloc (EFUSE_BT_MAP_LEN ,GFP_KERNEL ); -if (efuseTbl =NULL ) { -DBG_8723A ("%s: efuseTbl malloc fail!\n" , __func__); +if (efuseTbl =NULL )return ; - }/* 0xff will be efuse default value instead of 0x00. */ memset (efuseTbl,0xFF ,EFUSE_BT_MAP_LEN );drivers/staging/rtl8723au/hal/usb_ops_linux.c +1
-5@@ -256,12 +256,8 @@ static void usb_read_interrupt_complete(struct urb *purb) c2w =kmalloc (sizeof (struct evt_work ),GFP_ATOMIC ); -if (!c2w) { -printk (KERN_WARNING "%s: unable to " -"allocate work buffer\n" , - __func__); +if (!c2w) goto urb_submit; - } c2w->adapter = padapter;INIT_WORK (&c2w->work, rtw_evt_work);drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c +0
-5@@ -1327,8 +1327,6 @@ static int rtw_cfg80211_set_probe_req_wpsp2pie(struct rtw_adapter *padapter, pmlmepriv->wps_probe_req_ie =kmemdup (wps_ie, wps_ie[1 ],GFP_KERNEL );if (pmlmepriv->wps_probe_req_ie =NULL ) { -DBG_8723A ("%s()-%d: kmalloc() ERROR!\n" , - __func__, __LINE__);return -EINVAL ; } pmlmepriv->wps_probe_req_ie_len = wps_ie[1 ];@@ -2619,8 +2617,6 @@ static int rtw_cfg80211_add_monitor_if(struct rtw_adapter *padapter, char *name,/* wdev */ mon_wdev =kzalloc (sizeof (struct wireless_dev ),GFP_KERNEL );if (!mon_wdev) { -DBG_8723A ("%s(%s): allocate mon_wdev fail\n" , __func__, - padapter->pnetdev->name); ret = -ENOMEM ; goto out; }@@ -3275,7 +3271,6 @@ int rtw_wdev_alloc(struct rtw_adapter *padapter, struct device *dev)/* wdev */ wdev =kzalloc (sizeof (struct wireless_dev ),GFP_KERNEL );if (!wdev) { -DBG_8723A ("Couldn't allocate wireless device\n" ); ret = -ENOMEM ; goto free_wiphy; }
Quoted 9 lines · Jes Sorensen
Quoted 5 lines · Quentin Lambert
This patch reduces the kernel size by removing error messages that duplicate the normal OOM message.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)This patch removes useful warnings about what allocation failed. The messages removed are NOT duplicate!
Is it really the case that the information can't be reconstructed from the information generated by kmalloc on failure? To my understanding there is a stack trace, and from scanning through the changes I see only one change per function, so perhaps the stack trace already makes it clear where the problem occurred?
julia
Quoted 141 lines
NACK
Jes
Quoted 124 lines
@@
identifier f,print,l;
expression e;
constant char[] c;
@@e = \(kzalloc\|kmalloc\|devm_kzalloc\|devm_kmalloc\)(...); if (e = NULL) {
<+...
-print(...,c,...);
... when any
(
goto l;
|
return ...;
)
...+> }Signed-off-by Quentin Lambert <[email protected]>--- drivers/staging/rtl8723au/hal/rtl8723a_cmd.c | 8 ++------ drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c | 8 ++------ drivers/staging/rtl8723au/hal/usb_ops_linux.c | 6 +----- drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c | 5 ----- 4 files changed, 5 insertions(+), 22 deletions(-)drivers/staging/rtl8723au/hal/rtl8723a_cmd.c +2
-6@@ -462,10 +462,8 @@ static void SetFwRsvdPagePkt(struct rtw_adapter *padapter, bool bDLFinished)DBG_8723A ("%s\n" , __func__); ReservedPagePacket =kzalloc (1000 ,GFP_KERNEL ); -if (ReservedPagePacket =NULL ) { -DBG_8723A ("%s: alloc ReservedPagePacket fail!\n" , __func__); +if (ReservedPagePacket =NULL )return ; - } pHalData =GET_HAL_DATA (padapter); pxmitpriv = &padapter->xmitpriv;@@ -669,10 +667,8 @@ static void SetFwRsvdPagePkt_BTCoex(struct rtw_adapter *padapter)DBG_8723A ("+%s\n" , __func__); ReservedPagePacket =kzalloc (1024 ,GFP_KERNEL ); -if (ReservedPagePacket =NULL ) { -DBG_8723A ("%s: alloc ReservedPagePacket fail!\n" , __func__); +if (ReservedPagePacket =NULL )return ; - } pHalData =GET_HAL_DATA (padapter); pxmitpriv = &padapter->xmitpriv;drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c +2
-6@@ -401,10 +401,8 @@ hal_ReadEFuse_WiFi(struct rtw_adapter *padapter, } efuseTbl =kmalloc (EFUSE_MAP_LEN_8723A ,GFP_KERNEL ); -if (efuseTbl =NULL ) { -DBG_8723A ("%s: alloc efuseTbl fail!\n" , __func__); +if (efuseTbl =NULL )return ; - }/* 0xff will be efuse default value instead of 0x00. */ memset (efuseTbl,0xFF ,EFUSE_MAP_LEN_8723A );@@ -494,10 +492,8 @@ hal_ReadEFuse_BT(struct rtw_adapter *padapter, } efuseTbl =kmalloc (EFUSE_BT_MAP_LEN ,GFP_KERNEL ); -if (efuseTbl =NULL ) { -DBG_8723A ("%s: efuseTbl malloc fail!\n" , __func__); +if (efuseTbl =NULL )return ; - }/* 0xff will be efuse default value instead of 0x00. */ memset (efuseTbl,0xFF ,EFUSE_BT_MAP_LEN );drivers/staging/rtl8723au/hal/usb_ops_linux.c +1
-5@@ -256,12 +256,8 @@ static void usb_read_interrupt_complete(struct urb *purb) c2w =kmalloc (sizeof (struct evt_work ),GFP_ATOMIC ); -if (!c2w) { -printk (KERN_WARNING "%s: unable to " -"allocate work buffer\n" , - __func__); +if (!c2w) goto urb_submit; - } c2w->adapter = padapter;INIT_WORK (&c2w->work, rtw_evt_work);drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c +0
-4@@ -1327,8 +1327,6 @@ static int rtw_cfg80211_set_probe_req_wpsp2pie(struct rtw_adapter *padapter, pmlmepriv->wps_probe_req_ie =kmemdup (wps_ie, wps_ie[1 ],GFP_KERNEL );if (pmlmepriv->wps_probe_req_ie =NULL ) { -DBG_8723A ("%s()-%d: kmalloc() ERROR!\n" , - __func__, __LINE__);return -EINVAL ; } pmlmepriv->wps_probe_req_ie_len = wps_ie[1 ];@@ -2619,8 +2617,6 @@ static int rtw_cfg80211_add_monitor_if(struct rtw_adapter *padapter, char *name,/* wdev */ mon_wdev =kzalloc (sizeof (struct wireless_dev ),GFP_KERNEL );if (!mon_wdev) { -DBG_8723A ("%s(%s): allocate mon_wdev fail\n" , __func__, - padapter->pnetdev->name); ret = -ENOMEM ; goto out; }drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c int rtw_wdev_alloc(struct rtw_adapter *padapter, struct device *dev) · line 3276
Quoted 7 lines
drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c +0
-1@@ -3275,7 +3271,6 @@ int rtw_wdev_alloc(struct rtw_adapter *padapter, struct device *dev)/* wdev */ wdev =kzalloc (sizeof (struct wireless_dev ),GFP_KERNEL );if (!wdev) { -DBG_8723A ("Couldn't allocate wireless device\n" ); ret = -ENOMEM ; goto free_wiphy; }--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 9 lines · Julia Lawall
Quoted 7 lines · Jes Sorensen
Quoted 5 lines · Quentin Lambert
This patch reduces the kernel size by removing error messages that duplicate the normal OOM message.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)
Quoted 8 lines
Quoted 2 lines
This patch removes useful warnings about what allocation failed. The messages removed are NOT duplicate!
Is it really the case that the information can't be reconstructed from the information generated by kmalloc on failure? To my understanding there is a stack trace, and from scanning through the changes I see only one change per function, so perhaps the stack trace already makes it clear where the problem occurred?
It may be possible to backtrack, but this change just makes it harder.
There are tons of real issues to fix in this driver, this patch just increases the risk of patch conflicts for no real gain.
Jes
Quoted 9 lines · Jes Sorensen
Quoted 8 lines · Julia Lawall
Quoted 7 lines · Jes Sorensen
Quoted 4 lines · Quentin Lambert
This patch reduces the kernel size by removing error messages that duplicate the normal OOM message.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)This patch removes useful warnings about what allocation failed. The messages removed are NOT duplicate!
Quoted 8 lines
Quoted 5 lines
Is it really the case that the information can't be reconstructed from the information generated by kmalloc on failure? To my understanding there is a stack trace, and from scanning through the changes I see only one change per function, so perhaps the stack trace already makes it clear where the problem occurred?
It may be possible to backtrack, but this change just makes it harder. There are tons of real issues to fix in this driver, this patch just increases the risk of patch conflicts for no real gain.
Making the allocation less likely to fail for low memory systems is a gain.
The allocation failures themselves are low likelihood events. Determining which specific memory allocation failure occurred has near nil value.
Quoted 20 lines · Joe Perches
Quoted 10 lines · Jes Sorensen
Quoted 9 lines · Julia Lawall
Quoted 8 lines · Jes Sorensen
Quoted 5 lines · Quentin Lambert
This patch reduces the kernel size by removing error messages that duplicate
the normal OOM message.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)This patch removes useful warnings about what allocation failed. The messages removed are NOT duplicate!
Quoted 8 lines
Quoted 5 lines
Is it really the case that the information can't be reconstructed from the information generated by kmalloc on failure? To my understanding there is a stack trace, and from scanning through the changes I see only one change per function, so perhaps the stack trace already makes it clear where the problem occurred?
It may be possible to backtrack, but this change just makes it harder. There are tons of real issues to fix in this driver, this patch just increases the risk of patch conflicts for no real gain.
Quoted 7 lines
Making the allocation less likely to fail for low memory systems is a gain.
The allocation failures themselves are low likelihood events. Determining which specific memory allocation failure occurred has near nil value.
Joe,
That is bologna, knowing which allocation failed has a lot of value, it allows the developer to go back and look at the allocation sizes, parameters applied etc.
This is a classic case of blindly applied script 'fixes' causing more harm than good.
Jes
Quoted 24 lines · Jes Sorensen
Quoted 23 lines · Joe Perches
Quoted 10 lines · Jes Sorensen
Quoted 9 lines · Julia Lawall
Quoted 8 lines · Jes Sorensen
Quoted 5 lines · Quentin Lambert
This patch reduces the kernel size by removing error messages that duplicate
the normal OOM message.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)This patch removes useful warnings about what allocation failed. The messages removed are NOT duplicate!
Quoted 8 lines
Quoted 5 lines
Is it really the case that the information can't be reconstructed from the information generated by kmalloc on failure? To my understanding there is a stack trace, and from scanning through the changes I see only one change per function, so perhaps the stack trace already makes it clear where the problem occurred?
It may be possible to backtrack, but this change just makes it harder. There are tons of real issues to fix in this driver, this patch just increases the risk of patch conflicts for no real gain.
Making the allocation less likely to fail for low memory systems is a gain.
Quoted 6 lines
Quoted 4 lines
The allocation failures themselves are low likelihood events. Determining which specific memory allocation failure occurred has near nil value.
Joe,
Jes,
Quoted 1 line
That is bologna,
We disagree, and I rather like minced meat sausages.
Quoted 3 lines
knowing which allocation failed has a lot of value, it allows the developer to go back and look at the allocation sizes, parameters applied etc.
Likely enough of this is emitted by the generic OOM message and stack dump.
Quoted 2 lines
This is a classic case of blindly applied script 'fixes' causing more harm than good.
cheesy tomato/tomahto. Goes well with baloney.
cheers, Joe