* [PATCH 2.6.38-rc8-wl 1/1] orinoco: Reduce lock contention by reporting static errors before locking
@ 2011-03-15 15:53 Joe Gunn
2011-03-17 0:34 ` Dave Kilroy
0 siblings, 1 reply; 4+ messages in thread
From: Joe Gunn @ 2011-03-15 15:53 UTC (permalink / raw)
To: orinoco-devel; +Cc: linux-wireless
Check for static error issues before obtaining lock to reduce lock contention
Signed-off-by: Joseph J. Gunn <armadefuego@yahoo.com>
---
diff --git a/drivers/net/wireless/orinoco/wext.c b/drivers/net/wireless/orinoco/wext.c
index e793679..c1fe0b5 100644
--- a/drivers/net/wireless/orinoco/wext.c
+++ b/drivers/net/wireless/orinoco/wext.c
@@ -161,36 +161,17 @@ static int orinoco_ioctl_setwap(struct net_device *dev,
static const u8 off_addr[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
static const u8 any_addr[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
- if (orinoco_lock(priv, &flags) != 0)
- return -EBUSY;
-
- /* Enable automatic roaming - no sanity checks are needed */
- if (memcmp(&ap_addr->sa_data, off_addr, ETH_ALEN) == 0 ||
- memcmp(&ap_addr->sa_data, any_addr, ETH_ALEN) == 0) {
- priv->bssid_fixed = 0;
- memset(priv->desired_bssid, 0, ETH_ALEN);
-
- /* "off" means keep existing connection */
- if (ap_addr->sa_data[0] == 0) {
- __orinoco_hw_set_wap(priv);
- err = 0;
- }
- goto out;
- }
-
if (priv->firmware_type == FIRMWARE_TYPE_AGERE) {
printk(KERN_WARNING "%s: Lucent/Agere firmware doesn't "
"support manual roaming\n",
dev->name);
- err = -EOPNOTSUPP;
- goto out;
+ return(-EOPNOTSUPP);
}
if (priv->iw_mode != NL80211_IFTYPE_STATION) {
printk(KERN_WARNING "%s: Manual roaming supported only in "
"managed mode\n", dev->name);
- err = -EOPNOTSUPP;
- goto out;
+ return(-EOPNOTSUPP);
}
/* Intersil firmware hangs without Desired ESSID */
@@ -198,10 +179,27 @@ static int orinoco_ioctl_setwap(struct net_device *dev,
strlen(priv->desired_essid) == 0) {
printk(KERN_WARNING "%s: Desired ESSID must be set for "
"manual roaming\n", dev->name);
- err = -EOPNOTSUPP;
+ return(-EOPNOTSUPP);
+ }
+
+ if (orinoco_lock(priv, &flags) != 0)
+ return -EBUSY;
+
+ /* Enable automatic roaming - no sanity checks are needed */
+ if (memcmp(&ap_addr->sa_data, off_addr, ETH_ALEN) == 0 ||
+ memcmp(&ap_addr->sa_data, any_addr, ETH_ALEN) == 0) {
+ priv->bssid_fixed = 0;
+ memset(priv->desired_bssid, 0, ETH_ALEN);
+
+ /* "off" means keep existing connection */
+ if (ap_addr->sa_data[0] == 0) {
+ __orinoco_hw_set_wap(priv);
+ err = 0;
+ }
goto out;
}
+
/* Finally, enable manual roaming */
priv->bssid_fixed = 1;
memcpy(priv->desired_bssid, &ap_addr->sa_data, ETH_ALEN);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2.6.38-rc8-wl 1/1] orinoco: Reduce lock contention by reporting static errors before locking
2011-03-15 15:53 [PATCH 2.6.38-rc8-wl 1/1] orinoco: Reduce lock contention by reporting static errors before locking Joe Gunn
@ 2011-03-17 0:34 ` Dave Kilroy
2011-03-17 1:02 ` [Orinoco-devel] " Pavel Roskin
0 siblings, 1 reply; 4+ messages in thread
From: Dave Kilroy @ 2011-03-17 0:34 UTC (permalink / raw)
To: Joe Gunn; +Cc: orinoco-devel, linux-wireless
On 15/03/2011 15:53, Joe Gunn wrote:
> Check for static error issues before obtaining lock to reduce lock contention
> Signed-off-by: Joseph J. Gunn<armadefuego@yahoo.com>
> ---
> diff --git a/drivers/net/wireless/orinoco/wext.c b/drivers/net/wireless/orinoco/wext.c
> index e793679..c1fe0b5 100644
> --- a/drivers/net/wireless/orinoco/wext.c
> +++ b/drivers/net/wireless/orinoco/wext.c
> @@ -161,36 +161,17 @@ static int orinoco_ioctl_setwap(struct net_device *dev,
> static const u8 off_addr[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
> static const u8 any_addr[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>
> - if (orinoco_lock(priv,&flags) != 0)
> - return -EBUSY;
> -
> - /* Enable automatic roaming - no sanity checks are needed */
> - if (memcmp(&ap_addr->sa_data, off_addr, ETH_ALEN) == 0 ||
> - memcmp(&ap_addr->sa_data, any_addr, ETH_ALEN) == 0) {
> - priv->bssid_fixed = 0;
> - memset(priv->desired_bssid, 0, ETH_ALEN);
> -
> - /* "off" means keep existing connection */
> - if (ap_addr->sa_data[0] == 0) {
> - __orinoco_hw_set_wap(priv);
> - err = 0;
> - }
> - goto out;
> - }
> -
> if (priv->firmware_type == FIRMWARE_TYPE_AGERE) {
> printk(KERN_WARNING "%s: Lucent/Agere firmware doesn't "
> "support manual roaming\n",
> dev->name);
> - err = -EOPNOTSUPP;
> - goto out;
> + return(-EOPNOTSUPP);
> }
>
> if (priv->iw_mode != NL80211_IFTYPE_STATION) {
> printk(KERN_WARNING "%s: Manual roaming supported only in "
> "managed mode\n", dev->name);
> - err = -EOPNOTSUPP;
> - goto out;
> + return(-EOPNOTSUPP);
> }
>
> /* Intersil firmware hangs without Desired ESSID */
> @@ -198,10 +179,27 @@ static int orinoco_ioctl_setwap(struct net_device *dev,
> strlen(priv->desired_essid) == 0) {
I avoided changing the locking too much when I was modifying this driver
as I wasn't clear on whether the locks needed to protect concurrent wext
calls from each other. Since all the wext calls hold the lock over the
entirety of the callback, I assumed that is what they were doing.
In this particular change, the strlen here may give you one result, but
it may have been changed by the time you take the lock. Also the test on
priv->iw_mode above may have been invalidated as well.
Dave.
> printk(KERN_WARNING "%s: Desired ESSID must be set for "
> "manual roaming\n", dev->name);
> - err = -EOPNOTSUPP;
> + return(-EOPNOTSUPP);
> + }
> +
> + if (orinoco_lock(priv,&flags) != 0)
> + return -EBUSY;
> +
> + /* Enable automatic roaming - no sanity checks are needed */
> + if (memcmp(&ap_addr->sa_data, off_addr, ETH_ALEN) == 0 ||
> + memcmp(&ap_addr->sa_data, any_addr, ETH_ALEN) == 0) {
> + priv->bssid_fixed = 0;
> + memset(priv->desired_bssid, 0, ETH_ALEN);
> +
> + /* "off" means keep existing connection */
> + if (ap_addr->sa_data[0] == 0) {
> + __orinoco_hw_set_wap(priv);
> + err = 0;
> + }
> goto out;
> }
>
> +
> /* Finally, enable manual roaming */
> priv->bssid_fixed = 1;
> memcpy(priv->desired_bssid,&ap_addr->sa_data, ETH_ALEN);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Orinoco-devel] [PATCH 2.6.38-rc8-wl 1/1] orinoco: Reduce lock contention by reporting static errors before locking
2011-03-17 0:34 ` Dave Kilroy
@ 2011-03-17 1:02 ` Pavel Roskin
2011-03-17 2:15 ` Joe Gunn
0 siblings, 1 reply; 4+ messages in thread
From: Pavel Roskin @ 2011-03-17 1:02 UTC (permalink / raw)
To: Dave Kilroy; +Cc: Joe Gunn, orinoco-devel, linux-wireless
Hello!
>> + return(-EOPNOTSUPP);
"return" is not a function to be written like that. Please use the
existing coding style used in the kernel. Also please use
checkpatch.pl to catch style violations.
--
Regards,
Pavel Roskin
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Orinoco-devel] [PATCH 2.6.38-rc8-wl 1/1] orinoco: Reduce lock contention by reporting static errors before locking
2011-03-17 1:02 ` [Orinoco-devel] " Pavel Roskin
@ 2011-03-17 2:15 ` Joe Gunn
0 siblings, 0 replies; 4+ messages in thread
From: Joe Gunn @ 2011-03-17 2:15 UTC (permalink / raw)
To: Dave Kilroy, Pavel Roskin; +Cc: orinoco-devel, linux-wireless
--- On Wed, 3/16/11, Pavel Roskin <proski@gnu.org> wrote:
> From: Pavel Roskin <proski@gnu.org>
> Subject: Re: [Orinoco-devel] [PATCH 2.6.38-rc8-wl 1/1] orinoco: Reduce lock contention by reporting static errors before locking
> To: "Dave Kilroy" <kilroyd@googlemail.com>
> Cc: "Joe Gunn" <armadefuego@yahoo.com>, orinoco-devel@lists.sourceforge.net, linux-wireless@vger.kernel.org
> Date: Wednesday, March 16, 2011, 9:02 PM
> Hello!
>
> >> +
> return(-EOPNOTSUPP);
>
Sorry. I even knew that.
> "return" is not a function to be written like that.
> Please use the existing coding style used in the
> kernel. Also please use checkpatch.pl to catch style
> violations.
Thanks. I'll put that on my list of things to run.
>
> --Regards,
> Pavel Roskin
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-03-17 2:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-15 15:53 [PATCH 2.6.38-rc8-wl 1/1] orinoco: Reduce lock contention by reporting static errors before locking Joe Gunn
2011-03-17 0:34 ` Dave Kilroy
2011-03-17 1:02 ` [Orinoco-devel] " Pavel Roskin
2011-03-17 2:15 ` Joe Gunn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).