* Re: [PATCH]hostap:hostap_ioctl.c Fix variable 'ret' set but not used
From: Luis R. Rodriguez @ 2010-08-03 0:15 UTC (permalink / raw)
To: Justin P. Mattock; +Cc: linux-wireless, netdev, linux-kernel, j, linville
In-Reply-To: <1280793851-1719-1-git-send-email-justinmattock@gmail.com>
On Mon, Aug 2, 2010 at 5:04 PM, Justin P. Mattock
<justinmattock@gmail.com> wrote:
> The below patch fixes a warning message generated by GCC:
> CC [M] drivers/net/wireless/hostap/hostap_ioctl.o
> drivers/net/wireless/hostap/hostap_ioctl.c: In function 'prism2_request_scan':
> drivers/net/wireless/hostap/hostap_ioctl.c:1666:6: warning: variable 'ret' set but not used
>
> Keep in mind Im not sure if this is the right fix for this, so please
> feedback is appreciated.
>
> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
>
> ---
> drivers/net/wireless/hostap/hostap_ioctl.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/hostap/hostap_ioctl.c b/drivers/net/wireless/hostap/hostap_ioctl.c
> index a85e43a..27d462f 100644
> --- a/drivers/net/wireless/hostap/hostap_ioctl.c
> +++ b/drivers/net/wireless/hostap/hostap_ioctl.c
> @@ -1663,7 +1663,6 @@ static int prism2_request_scan(struct net_device *dev)
> struct hostap_interface *iface;
> local_info_t *local;
> struct hfa384x_scan_request scan_req;
> - int ret = 0;
>
> iface = netdev_priv(dev);
> local = iface->local;
> @@ -1689,7 +1688,7 @@ static int prism2_request_scan(struct net_device *dev)
> if (local->func->set_rid(dev, HFA384X_RID_SCANREQUEST, &scan_req,
> sizeof(scan_req))) {
> printk(KERN_DEBUG "SCANREQUEST failed\n");
> - ret = -EINVAL;
> + return -EINVAL;
> }
>
> if (!local->host_roaming)
NACK please read the code instead of just patching blindly to cure a
compile issue. For example read this big block:
/* FIX:
* It seems to be enough to set roaming mode for a short moment to
* host-based and then setup scanrequest data and return the mode to
* firmware-based.
*
* Master mode would need to drop to Managed mode for a short while
* to make scanning work.. Or sweep through the different channels and
* use passive scan based on beacons. */
So.. it seems to me you likely want to flip back to
HFA384X_ROAMING_FIRMWARE *after* you've tried to issue the
HFA384X_RID_SCANREQUEST, even if it has failed. So the compile fix
should require just changing the last return statement of the function
to return ret value.
Luis
^ permalink raw reply
* virtual access point setup
From: Ryszard @ 2010-08-03 0:09 UTC (permalink / raw)
To: Ben Greear; +Cc: Florian Fainelli, Patrick McHardy, linux-wireless
hey ben,
here is a list of actions i've done just now to get the previously
mentioned stack trace:
==============================================
1. restart box
2. uname -a Linux sknwireless 2.6.32-22-generic #33-Ubuntu SMP Wed Apr
28 13:27:30 UTC 2010 i586 GNU/Linux
3.lsmod:
Module Size Used by
ipt_REDIRECT 917 3
ipt_MASQUERADE 1407 3
xt_state 1098 6
xt_tcpudp 2011 33
iptable_filter 2271 1
nf_nat_ftp 1836 0
iptable_nat 4414 1
ip_tables 9991 2 iptable_filter,iptable_nat
nf_nat 15735 4
ipt_REDIRECT,ipt_MASQUERADE,nf_nat_ftp,iptable_nat
x_tables 14299 6
ipt_REDIRECT,ipt_MASQUERADE,xt_state,xt_tcpudp,iptable_nat,ip_tables
nf_conntrack_ftp 5381 1 nf_nat_ftp
nf_conntrack_ipv4 10672 9 iptable_nat,nf_nat
nf_conntrack 61583 7
ipt_MASQUERADE,xt_state,nf_nat_ftp,iptable_nat,nf_nat,nf_conntrack_ftp,nf_conntrack_ipv4
nf_defrag_ipv4 1073 1 nf_conntrack_ipv4
arc4 1153 2
aes_i586 7268 0
ath5k 118956 0
aes_generic 26863 1 aes_i586
mac80211 206808 1 ath5k
ath 7611 1 ath5k
leds_alix2 1470 0
geode_aes 4558 0
cs5535_gpio 2571 0
cfg80211 125477 3 ath5k,mac80211,ath
geode_rng 1011 0
led_class 2864 2 ath5k,leds_alix2
pata_cs5536 3224 0
pata_amd 8766 1
via_rhine 19154 0
mii 4381 1 via_rhine
4. rmmod ath5k
5. modprobe ath5k nohwcrypt=1
6. syslog shows no stack trace
7. iw dev wlan0 interface add sta0 type station
8. ifconfig sta0
sta0 Link encap:Ethernet HWaddr 00:0b:6b:22:5b:cd
BROADCAST MULTICAST MTU:1500 Metric:1
RX packets:0 errors:0 dropped:0 overruns:0 frame:0
TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:0 (0.0 B) TX bytes:0 (0.0 B)
9. iwconfig sta0
sta0 IEEE 802.11abg ESSID:off/any
Mode:Managed Access Point: Not-Associated Tx-Power=20 dBm
Retry long limit:7 RTS thr:off Fragment thr:off
Encryption key:off
Power Management:off
10. no syslog stacktrace
11. iw dev wlan0 interface add vap0 type __ap
12. ifconfig vap0
vap0 Link encap:Ethernet HWaddr 00:0b:6b:22:5b:cd
BROADCAST MULTICAST MTU:1500 Metric:1
RX packets:0 errors:0 dropped:0 overruns:0 frame:0
TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:0 (0.0 B) TX bytes:0 (0.0 B)
13. iwconfig vap0
vap0 IEEE 802.11abg Mode:Master Tx-Power=20 dBm
Retry long limit:7 RTS thr:off Fragment thr:off
Power Management:off
14. no syslog stacktrace
15. iw dev wlan0 interface add vap1 type __ap
16. ifconfig vap1
vap1 Link encap:Ethernet HWaddr 00:0b:6b:22:5b:cd
BROADCAST MULTICAST MTU:1500 Metric:1
RX packets:0 errors:0 dropped:0 overruns:0 frame:0
TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:0 (0.0 B) TX bytes:0 (0.0 B)
17. iwconfig vap1
vap1 IEEE 802.11abg Mode:Master Tx-Power=20 dBm
Retry long limit:7 RTS thr:off Fragment thr:off
Power Management:off
18. no syslog stacktrace
19. ifconfig vap0 172.16.40.1 up
SIOCSIFFLAGS: Name not unique on network
SIOCSIFFLAGS: Name not unique on network
20. no syslog stacktrace
21 ifconfig vap1 172.16.40.1 up
SIOCSIFFLAGS: Name not unique on network
SIOCSIFFLAGS: Name not unique on network
22. no syslog stacktrace
23. macchanger vap0
Current MAC: 00:0b:6b:22:5b:cd (Wistron Neweb Corp.)
Faked MAC: 00:0b:6b:22:5b:ce (Wistron Neweb Corp.)
24. no syslog stacktrace
25. macchanger vap1
Current MAC: 00:0b:6b:22:5b:cd (Wistron Neweb Corp.)
Faked MAC: 00:0b:6b:22:5b:ce (Wistron Neweb Corp.)
26. no syslog stacktrace
27. ifconfig vap0 172.168.40.1 up
28. syslog stacktrace:
Aug 3 10:04:03 sknwireless kernel: [ 908.740408] ------------[ cut
here ]------------
Aug 3 10:04:03 sknwireless kernel: [ 908.740479] WARNING: at
/home/skn/wireless/compat-wireless-2.6.32.15/drivers/net/wireless/ath/ath5k/base.c:3221
ath5k_bss_info_changed+0x1a6/0x1b0 [ath5k]()
Aug 3 10:04:03 sknwireless kernel: [ 908.740508] Modules linked in:
ipt_REDIRECT ipt_MASQUERADE xt_state xt_tcpudp iptable_filter
nf_nat_ftp iptable_nat ip_tables nf_nat x_tables nf_conntrack_ftp
nf_conntrack_ipv4 nf_conntrack nf_defrag_ipv4 arc4 aes_i586 ath5k
aes_generic mac80211 ath leds_alix2 geode_aes cs5535_gpio cfg80211
geode_rng led_class pata_cs5536 pata_amd via_rhine mii
Aug 3 10:04:03 sknwireless kernel: [ 908.740683] Pid: 3711, comm:
ifconfig Not tainted 2.6.32-22-generic #33-Ubuntu
Aug 3 10:04:03 sknwireless kernel: [ 908.740701] Call Trace:
Aug 3 10:04:03 sknwireless kernel: [ 908.740739] [<c014c3d2>]
warn_slowpath_common+0x72/0xa0
Aug 3 10:04:03 sknwireless kernel: [ 908.740785] [<d09d6406>] ?
ath5k_bss_info_changed+0x1a6/0x1b0 [ath5k]
Aug 3 10:04:03 sknwireless kernel: [ 908.740829] [<d09d6406>] ?
ath5k_bss_info_changed+0x1a6/0x1b0 [ath5k]
Aug 3 10:04:03 sknwireless kernel: [ 908.740861] [<c014c41a>]
warn_slowpath_null+0x1a/0x20
Aug 3 10:04:03 sknwireless kernel: [ 908.740904] [<d09d6406>]
ath5k_bss_info_changed+0x1a6/0x1b0 [ath5k]
Aug 3 10:04:03 sknwireless kernel: [ 908.741012] [<d0946bda>]
ieee80211_bss_info_change_notify+0x9a/0x1b0 [mac80211]
Aug 3 10:04:03 sknwireless kernel: [ 908.741058] [<d09d6260>] ?
ath5k_bss_info_changed+0x0/0x1b0 [ath5k]
Aug 3 10:04:03 sknwireless kernel: [ 908.741134] [<d0951d58>]
ieee80211_open+0x2a8/0x7d0 [mac80211]
Aug 3 10:04:03 sknwireless kernel: [ 908.741238] [<c04c3ec2>]
dev_open+0x92/0xf0
Aug 3 10:04:03 sknwireless kernel: [ 908.741268] [<c058b866>] ?
_spin_unlock_bh+0x16/0x20
Aug 3 10:04:03 sknwireless kernel: [ 908.741296] [<c04c1a2f>] ?
dev_set_rx_mode+0x2f/0x40
Aug 3 10:04:03 sknwireless kernel: [ 908.741325] [<c04c3739>]
dev_change_flags+0x139/0x1b0
Aug 3 10:04:03 sknwireless kernel: [ 908.741360] [<c0515f0a>]
devinet_ioctl+0x54a/0x5a0
Aug 3 10:04:03 sknwireless kernel: [ 908.741389] [<c04c512f>] ?
dev_ioctl+0x2af/0x720
Aug 3 10:04:03 sknwireless kernel: [ 908.741422] [<c0517065>]
inet_ioctl+0x95/0xb0
Aug 3 10:04:03 sknwireless kernel: [ 908.741450] [<c04b237d>]
sock_ioctl+0x6d/0x270
Aug 3 10:04:03 sknwireless kernel: [ 908.741475] [<c04b2310>] ?
sock_ioctl+0x0/0x270
Aug 3 10:04:03 sknwireless kernel: [ 908.741501] [<c0216231>]
vfs_ioctl+0x21/0x90
Aug 3 10:04:03 sknwireless kernel: [ 908.741526] [<c058955c>] ?
schedule+0x44c/0x840
Aug 3 10:04:03 sknwireless kernel: [ 908.741553] [<c0216519>]
do_vfs_ioctl+0x79/0x310
Aug 3 10:04:03 sknwireless kernel: [ 908.741578] [<c0216817>]
sys_ioctl+0x67/0x80
Aug 3 10:04:03 sknwireless kernel: [ 908.741607] [<c01033ec>]
syscall_call+0x7/0xb
Aug 3 10:04:03 sknwireless kernel: [ 908.741628] ---[ end trace
c6a7c8dea11614de ]---
Aug 3 10:04:13 sknwireless kernel: [ 919.169554] vap0: no IPv6 routers present
29. ifconfig vap1 172.168.40.1 up
SIOCSIFFLAGS: Name not unique on network
SIOCSIFFLAGS: Name not unique on network
30. macchanger vap1
Current MAC: 00:0b:6b:22:5b:ce (Wistron Neweb Corp.)
Faked MAC: 00:0b:6b:22:5b:cf (Wistron Neweb Corp.)
31. ifconfig vap1 172.168.40.1 up
32. no additional stack trace in syslog
==============================================
i'm doing this remote right now and dont have access to a wireless
client to attempt to get a lease (after running hostapd of course).
is this information relevant/useful to you? is there anything else
you'd like me to do?
regs
R
^ permalink raw reply
* [PATCH]hostap:hostap_hw.c Fix variable 'fc' set but not used
From: Justin P. Mattock @ 2010-08-03 0:05 UTC (permalink / raw)
To: linux-wireless; +Cc: netdev, linux-kernel, j, linville, Justin P. Mattock
The below fixes a build wanring from GCC.
CC [M] drivers/net/wireless/hostap/hostap_plx.o
In file included from drivers/net/wireless/hostap/hostap_plx.c:264:0:
drivers/net/wireless/hostap/hostap_hw.c: In function 'prism2_tx_80211':
drivers/net/wireless/hostap/hostap_hw.c:1816:18: warning: variable 'fc' set but not used
Removing fc from this just introduces a new warning
CC [M] drivers/net/wireless/hostap/hostap_plx.o
In file included from drivers/net/wireless/hostap/hostap_plx.c:264:0:
drivers/net/wireless/hostap/hostap_hw.c: In function 'prism2_tx_80211':
drivers/net/wireless/hostap/hostap_hw.c:1839:3: warning: statement with no effect
So I went and removed the whole thing. Let me know if theres another solution.
Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
---
drivers/net/wireless/hostap/hostap_hw.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/hostap/hostap_hw.c b/drivers/net/wireless/hostap/hostap_hw.c
index e9d9d62..d734f3e 100644
--- a/drivers/net/wireless/hostap/hostap_hw.c
+++ b/drivers/net/wireless/hostap/hostap_hw.c
@@ -1813,7 +1813,7 @@ static int prism2_tx_80211(struct sk_buff *skb, struct net_device *dev)
struct hfa384x_tx_frame txdesc;
struct hostap_skb_tx_data *meta;
int hdr_len, data_len, idx, res, ret = -1;
- u16 tx_control, fc;
+ u16 tx_control;
iface = netdev_priv(dev);
local = iface->local;
@@ -1836,7 +1836,6 @@ static int prism2_tx_80211(struct sk_buff *skb, struct net_device *dev)
/* skb->data starts with txdesc->frame_control */
hdr_len = 24;
skb_copy_from_linear_data(skb, &txdesc.frame_control, hdr_len);
- fc = le16_to_cpu(txdesc.frame_control);
if (ieee80211_is_data(txdesc.frame_control) &&
ieee80211_has_a4(txdesc.frame_control) &&
skb->len >= 30) {
--
1.7.1.rc1.21.gf3bd6
^ permalink raw reply related
* [PATCH]hostap:hostap_ioctl.c Fix variable 'hostscan' set but not used
From: Justin P. Mattock @ 2010-08-03 0:04 UTC (permalink / raw)
To: linux-wireless; +Cc: netdev, linux-kernel, j, linville, Justin P. Mattock
The patch below fixes a compiler warning:
CC [M] drivers/net/wireless/hostap/hostap_ioctl.o
drivers/net/wireless/hostap/hostap_ioctl.c: In function 'prism2_translate_scan':
drivers/net/wireless/hostap/hostap_ioctl.c:1954:13: warning: variable 'hostscan' set but not used
Keep in mind I've looked and searched for a fix for this but
couldnt come up with anything.(if there is let me know and I'll have a try at it).
Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
---
drivers/net/wireless/hostap/hostap_ioctl.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/hostap/hostap_ioctl.c b/drivers/net/wireless/hostap/hostap_ioctl.c
index 27d462f..25420e3 100644
--- a/drivers/net/wireless/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/hostap/hostap_ioctl.c
@@ -1951,7 +1951,7 @@ static inline int prism2_translate_scan(local_info_t *local,
char *buffer, int buflen)
{
struct hfa384x_hostscan_result *scan;
- int entry, hostscan;
+ int entry;
char *current_ev = buffer;
char *end_buf = buffer + buflen;
struct list_head *ptr;
@@ -1964,7 +1964,6 @@ static inline int prism2_translate_scan(local_info_t *local,
bss->included = 0;
}
- hostscan = local->last_scan_type == PRISM2_HOSTSCAN;
for (entry = 0; entry < local->last_scan_results_count; entry++) {
int found = 0;
scan = &local->last_scan_results[entry];
--
1.7.1.rc1.21.gf3bd6
^ permalink raw reply related
* [PATCH]hostap:hostap_ioctl.c Fix variable 'ret' set but not used
From: Justin P. Mattock @ 2010-08-03 0:04 UTC (permalink / raw)
To: linux-wireless; +Cc: netdev, linux-kernel, j, linville, Justin P. Mattock
The below patch fixes a warning message generated by GCC:
CC [M] drivers/net/wireless/hostap/hostap_ioctl.o
drivers/net/wireless/hostap/hostap_ioctl.c: In function 'prism2_request_scan':
drivers/net/wireless/hostap/hostap_ioctl.c:1666:6: warning: variable 'ret' set but not used
Keep in mind Im not sure if this is the right fix for this, so please
feedback is appreciated.
Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
---
drivers/net/wireless/hostap/hostap_ioctl.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/hostap/hostap_ioctl.c b/drivers/net/wireless/hostap/hostap_ioctl.c
index a85e43a..27d462f 100644
--- a/drivers/net/wireless/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/hostap/hostap_ioctl.c
@@ -1663,7 +1663,6 @@ static int prism2_request_scan(struct net_device *dev)
struct hostap_interface *iface;
local_info_t *local;
struct hfa384x_scan_request scan_req;
- int ret = 0;
iface = netdev_priv(dev);
local = iface->local;
@@ -1689,7 +1688,7 @@ static int prism2_request_scan(struct net_device *dev)
if (local->func->set_rid(dev, HFA384X_RID_SCANREQUEST, &scan_req,
sizeof(scan_req))) {
printk(KERN_DEBUG "SCANREQUEST failed\n");
- ret = -EINVAL;
+ return -EINVAL;
}
if (!local->host_roaming)
--
1.7.1.rc1.21.gf3bd6
^ permalink raw reply related
* Re: wireless-regdb: Why crda db.txt disallows 40Mhz BW on Channel 12-13 for JP?
From: Luis R. Rodriguez @ 2010-08-03 0:02 UTC (permalink / raw)
To: yogeshp, Michael Green
Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org
In-Reply-To: <4C56B234.3040209@marvell.com>
On Mon, Aug 2, 2010 at 4:55 AM, yogeshp <yogeshp@marvell.com> wrote:
> Hello All,
>
> Crda db.txt has following entries for JP:
> (2402 - 2472 @ 40), (N/A, 20)
> (2457 - 2482 @ 20), (N/A, 20)
>
> Why are channel 12 and 13 for JP disallowed to operate in 40Mhz by this file?
>
> As per Annex 11J of latest 802.11n spec, for Japan, channel 1-7 are allowed in HT40+ and
> channel 5-13 are allowed in HT40- mode.
Do you mean Section 11J.1? That stuff defines a regulatory class, its
not a regulatory text though. Linux dynamically computes the HT40+ or
HT40- allowed channel map based on frequency ranges supported and the
max allowed bandwidth for its frequency ranges.
Michael, can you confirm if HT0 is allowed on the 2.4 GHz band in JP ?
Luis
^ permalink raw reply
* [Patch] ath9k: fix erased ieee80211_rx_status.mactime
From: Jan Friedrich @ 2010-08-02 21:55 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, lrodriguez
ath9k_rx_skb_preprocess nulls rxs and the mactime is never set again -
mactime is always 0. This causes problems in IBSS mode.
ieee80211_rx_bss_info uses mactime to decide if an IBSS merge is needed.
Without this patch the merge is triggered by each beacon received.
This can be recognized by the "beacon TSF higher than local TSF - IBSS
merge with BSSID" log message accompanying each beacon.
This problem was not completely fixed in commit
a6d2055b02dde1067075795274672720baadd3ca and is not a stable kernel fix.
It is solely intended for wireless-testing.
Signed-off-by: Jan Friedrich <jft@dev2day.de>
---
drivers/net/wireless/ath/ath9k/recv.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/recv.c
b/drivers/net/wireless/ath/ath9k/recv.c
index da0cfe9..ae4bd30 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1140,6 +1140,11 @@ int ath_rx_tasklet(struct ath_softc *sc, int
flush, bool hp)
if (flush)
goto requeue;
+ retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
+ rxs, &decrypt_error);
+ if (retval)
+ goto requeue;
+
rxs->mactime = (tsf & ~0xffffffffULL) | rs.rs_tstamp;
if (rs.rs_tstamp > tsf_lower &&
unlikely(rs.rs_tstamp - tsf_lower > 0x10000000))
@@ -1149,11 +1154,6 @@ int ath_rx_tasklet(struct ath_softc *sc, int
flush, bool hp)
unlikely(tsf_lower - rs.rs_tstamp > 0x10000000))
rxs->mactime += 0x100000000ULL;
- retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
- rxs, &decrypt_error);
- if (retval)
- goto requeue;
-
/* Ensure we always have an skb to requeue once we are done
* processing the current buffer's skb */
requeue_skb = ath_rxbuf_alloc(common, common->rx_bufsize, GFP_ATOMIC);
^ permalink raw reply related
* Re: [PATCH v2 03/20] mmc: support embedded data field in mmc_host
From: Ohad Ben-Cohen @ 2010-08-02 21:35 UTC (permalink / raw)
To: Vitaly Wool
Cc: linux-wireless, linux-mmc, linux-omap, Kalle Valo, Pandita Vikram,
linux, Nicolas Pitre, Tony Lindgren, Roger Quadros, San Mehat,
Chikkature Rajashekar Madhusudhan, Luciano Coelho, akpm,
linux-arm-kernel
In-Reply-To: <AANLkTikfRja-h9mz7uyRXCZNZjKdU_yB8e58XQucUzrn@mail.gmail.com>
Hi Vitaly,
On Mon, Aug 2, 2010 at 7:25 PM, Vitaly Wool <vitalywool@gmail.com> wrote:
> On Mon, Aug 2, 2010 at 5:54 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>> SPI is using these spi_board_info tables to populate the SPI device
>> trees. These tables are registered early at the board-specific init
>> code, and are later used by SPI core to populate the devices when the
>> SPI master controller is registered.
>>
>> SDIO doesn't normally needs any kind of hard coded data: most devices
>> are dynamically probed and populated.
>>
>> On rare cases like the wl1271, we have some platform-specific data we
>> need to deliver the SDIO function driver (i.e. the irq info in this
>> case). Since the device is hardwired to a specific controller, it does
>> make some sense to pass this private data from the controller's info
>> in the board files, through the host driver, and make it accessible
>> through the specific host instance that drives this controller.
>>
>> Btw, if our problem was be broader (e.g., needs to supply private data
>> for non-hardwired devices), then I agree that a more complex
>> mechanism, such as the one you suggest, would be needed. But currently
>> the problem is very simple and the solution is even simpler: just add
>> 1 private pointer to the host.
>>
>> Hope you find this reasonable,
>
> no, actually I don't. I think this is a hack that intrudes into the
> area it completely doesn't belong to.
>
> In fact, one can have 2 views on this problem: either this is a fairly
> generic problem we need to address, or this is a very specific corner
> case.
> Your solution will be treated as a hack in both cases.
>
> If we consider it a generic problem, then we need to find a generic
> solution, which is the board_info solution, for instance. FWIW, I2C
> also uses this approach now.
>
> If we consider it to be a corner case, let's just add a dummy
> platform_device like it's done in WL1251 implementation and keep the
> MMC subsystem clean.
Let's start by defining the problem exactly:
SDIO devices that are hardwired to a specific controller and have some
platform data that needs to be available to the SDIO function driver.
It doesn't get more generic than that - it's not needed with common
fully-enumerable SDIO devices (at least I'm not aware of such need),
hence a generic mechanism of maintaining a global pile of platform
data pointers, which can be queried with the device and vendor ID,
really sounds like an overkill.
But it's not specific to the wl1271 device - I prefer to support this
at the MMC level, so we wouldn't have to add an extra platform device
for every SDIO function driver that needs to cope with such issue (we
already have two drivers - wl1271_sdio.c and wl1251_sdio.c); Adding an
extra platform device for every driver is just too much dummy code
(that btw adds a well-hidden race condition between the two probe
calls), which can be nicely eliminated if we just introduce this new
per-host private data pointer.
So we have a SDIO device hardwired to a specific controller, that is
driven by a specific host controller driver instance. This patch
allows this specific host instance to carry platform data for the
device that is hardwired to it. The platform data would be available
only to SDIO function driver instances of that specific host
controller. The solution is generic enough to support all SDIO
function drivers with similar needs, and it's extremely simple.
I'm honestly trying to understand your concerns, but I'm afraid that
just saying "it's a hack" is not too informative. Can you please
explain what do you think is technically wrong with the proposed
solution ? is it not general enough for other use cases ? will it
break something ?
Thanks,
Ohad.
>
> Thanks,
> Vitaly
>
^ permalink raw reply
* Re: [Patch] ath9k: fix erased ieee80211_rx_status.mactime
From: Luis R. Rodriguez @ 2010-08-02 20:52 UTC (permalink / raw)
To: Jan Friedrich
Cc: Luis Rodriguez, linville@tuxdriver.com,
linux-wireless@vger.kernel.org
In-Reply-To: <4C572F31.3050809@dev2day.de>
On Mon, Aug 02, 2010 at 01:48:49PM -0700, Jan Friedrich wrote:
> > Think? Does the patch actually fix something or do you think it fixes
> > something? Next time please include all these details on your patch
> > commit log mesage so its easy to understand the reasoning behind
> > the patch.
>
> It fixes the problem I described. Now the mactime is set correctly and
> the IBSS merge is done only once and by the correct station. I noticed
> no negative side effect and tested it with 10+ stations in IBSS mode.
>
> Sorry for the inconvenience. Next time I will provide more information.
no problem, can you resubmit but add all these details to the commit log?
ALso, is this a stable kernel fix, that is, does this fix need to be
propagated to the stable kernels, 2.6.35, 2.6.34, 2.6.33, 2.6.32?
Luis
^ permalink raw reply
* Re: [Patch] ath9k: fix erased ieee80211_rx_status.mactime
From: Jan Friedrich @ 2010-08-02 20:48 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org
In-Reply-To: <20100802203843.GD8920@tux>
> Think? Does the patch actually fix something or do you think it fixes
> something? Next time please include all these details on your patch
> commit log mesage so its easy to understand the reasoning behind
> the patch.
It fixes the problem I described. Now the mactime is set correctly and
the IBSS merge is done only once and by the correct station. I noticed
no negative side effect and tested it with 10+ stations in IBSS mode.
Sorry for the inconvenience. Next time I will provide more information.
Greetings,
Jan
^ permalink raw reply
* Re: [PATCH 3/3] compat: define struct va_format introduced in v2.6.36
From: Luis R. Rodriguez @ 2010-08-02 20:42 UTC (permalink / raw)
To: Joe Perches
Cc: Kshitij Kulshreshtha, Luis Rodriguez, Hauke Mehrtens,
linux-wireless@vger.kernel.org
In-Reply-To: <1280779727.1752.71.camel@Joe-Laptop.home>
On Mon, Aug 02, 2010 at 01:08:47PM -0700, Joe Perches wrote:
> On Mon, 2010-08-02 at 21:54 +0200, Kshitij Kulshreshtha wrote:
> > As on 2010-08-02 21:48, Joe Perches did write:
> > > On Mon, 2010-08-02 at 21:39 +0200, Kshitij Kulshreshtha wrote:
> > >> As on 2010-08-02 20:00, Luis R. Rodriguez did write:
> > >>> On Sun, Aug 1, 2010 at 3:02 PM, Kshitij Kulshreshtha
> > >>>> +struct va_format {
> > >>>> + const char *fmt;
> > >>>> + va_list *va;
> > >>>> +};
> > >>> I'll apply this for now but what caller uses this for example?
> > > Why is this necessary?
> > I needed this to compile compat-wireless which is based on linux-next.
> > Otherwise net/wireless/core.c fails to compile, due to the use of this
> > struct va_format in the functions defined in lines 910--958. If this
> > usage is reverted, we won't need it in compat.
>
> I see your problem.
>
> I think your problem should not be a constraint
> on new development and I do not think the new use
> should be reverted.
Joe, compat.git does not go upstream into the kernel, compat.git
tries to backport as much new kernel stuff for usage on older kernels.
compat-wireless uses compat.git to help with backporting the 802.11/BT/ethernet
subsystems to older kernels.
In this case the patch Kshitij sent for compat.git is OK since the struct is
used in code only by cfg80211 which we end up updating using compat-wireless.
Thanks for the clarifications Kshitij.
Luis
^ permalink raw reply
* Re: [Patch] ath9k: fix erased ieee80211_rx_status.mactime
From: Luis R. Rodriguez @ 2010-08-02 20:38 UTC (permalink / raw)
To: Jan Friedrich
Cc: Luis Rodriguez, linville@tuxdriver.com,
linux-wireless@vger.kernel.org
In-Reply-To: <4C572C18.6070003@dev2day.de>
On Mon, Aug 02, 2010 at 01:35:36PM -0700, Jan Friedrich wrote:
> > What effect does this have? What issue does this fix? Is this a
> > fix which also needs to be propagated to the stable kernels, if so
> > why? etc.
>
> The mactime is always set to 0. I encountered the problem in IBSS mode.
> In ieee80211_rx_bss_info (net/mac80211/ibss.c) the mactime is used to
> trigger an IBSS merge. At the moment the merge is always triggered.
>
> I got the "beacon TSF higher than local TSF - IBSS merge with BSSID" log
> message with every beacon and so recognized this odd behaviour.
>
> I think this has to be fixed and at least influences IBSS mode negatively.
Think? Does the patch actually fix something or do you think it fixes
something? Next time please include all these details on your patch
commit log mesage so its easy to understand the reasoning behind
the patch.
Luis
^ permalink raw reply
* Re: [Patch] ath9k: fix erased ieee80211_rx_status.mactime
From: Jan Friedrich @ 2010-08-02 20:35 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org,
Luis Rodriguez
In-Reply-To: <20100802202137.GC8920@tux>
> What effect does this have? What issue does this fix? Is this a
> fix which also needs to be propagated to the stable kernels, if so
> why? etc.
The mactime is always set to 0. I encountered the problem in IBSS mode.
In ieee80211_rx_bss_info (net/mac80211/ibss.c) the mactime is used to
trigger an IBSS merge. At the moment the merge is always triggered.
I got the "beacon TSF higher than local TSF - IBSS merge with BSSID" log
message with every beacon and so recognized this odd behaviour.
I think this has to be fixed and at least influences IBSS mode negatively.
Greetings,
Jan
^ permalink raw reply
* [Patch] ath9k: fix erased ieee80211_rx_status.mactime
From: Jan Friedrich @ 2010-08-02 20:16 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, lrodriguez
This patch addresses the erasure of ieee80211_rx_status.mactime.
After the mactime is set in "ath_rx_tasklet" it is immediately nulled in
"ath9k_rx_skb_preprocess".
Signed-off-by: Jan Friedrich <jft@dev2day.de>
---
drivers/net/wireless/ath/ath9k/recv.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/recv.c
b/drivers/net/wireless/ath/ath9k/recv.c
index da0cfe9..ae4bd30 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1140,6 +1140,11 @@ int ath_rx_tasklet(struct ath_softc *sc, int
flush, bool hp)
if (flush)
goto requeue;
+ retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
+ rxs, &decrypt_error);
+ if (retval)
+ goto requeue;
+
rxs->mactime = (tsf & ~0xffffffffULL) | rs.rs_tstamp;
if (rs.rs_tstamp > tsf_lower &&
unlikely(rs.rs_tstamp - tsf_lower > 0x10000000))
@@ -1149,11 +1154,6 @@ int ath_rx_tasklet(struct ath_softc *sc, int
flush, bool hp)
unlikely(tsf_lower - rs.rs_tstamp > 0x10000000))
rxs->mactime += 0x100000000ULL;
- retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
- rxs, &decrypt_error);
- if (retval)
- goto requeue;
-
/* Ensure we always have an skb to requeue once we are done
* processing the current buffer's skb */
requeue_skb = ath_rxbuf_alloc(common, common->rx_bufsize, GFP_ATOMIC);
^ permalink raw reply related
* Re: [Patch] ath9k: fix erased ieee80211_rx_status.mactime
From: Luis R. Rodriguez @ 2010-08-02 20:21 UTC (permalink / raw)
To: Jan Friedrich
Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org,
Luis Rodriguez
In-Reply-To: <4C57278E.1030601@dev2day.de>
On Mon, Aug 02, 2010 at 01:16:14PM -0700, Jan Friedrich wrote:
> This patch addresses the erasure of ieee80211_rx_status.mactime.
>
> After the mactime is set in "ath_rx_tasklet" it is immediately nulled in
> "ath9k_rx_skb_preprocess".
What effect does this have? What issue does this fix? Is this a
fix which also needs to be propagated to the stable kernels, if so
why? etc.
Your patch describes what it does, it doesn't say why its a good
fix or what was affected before. Please clarify all this as best
as you can so we can help ACK/NACK/propagate the patch to stable
if it is appropriate.
Luis
> requeue_skb = ath_rxbuf_alloc(common, common->rx_bufsize, GFP_ATOMIC);
^ permalink raw reply
* Re: [PATCH 01/11] pcmcia: use pcmica_{read,write}_config_byte
From: Dominik Brodowski @ 2010-08-02 19:52 UTC (permalink / raw)
To: Komuro; +Cc: Michael Buesch, netdev, linux-pcmcia, linux-wireless,
linux-serial
In-Reply-To: <15238373.192661280750366920.komurojun-mbn@nifty.com>
Hey,
On Mon, Aug 02, 2010 at 08:59:26PM +0900, Komuro wrote:
> >--- a/drivers/net/pcmcia/xirc2ps_cs.c
> >+++ b/drivers/net/pcmcia/xirc2ps_cs.c
>
>
> >+ if (err)
> > goto config_error;
> >- reg.Action = CS_WRITE;
> >- reg.Offset = CISREG_IOBASE_1;
> >- reg.Value = (link->io.BasePort2 >> 8) & 0xff;
> >- if ((err = pcmcia_access_configuration_register(link, ®)))
> >+
> >+ err = pcmcia_write_config_byte(link, CISREG_IOBASE_1,
> >+ link->io.BasePort2 & 0xff);
>
> It should be
>
> err = pcmcia_write_config_byte(link, CISREG_IOBASE_1,
> (link->io.BasePort2 >> 8) & 0xff);
>
Fixed, thanks.
Best,
Dominik
^ permalink raw reply
* Re: [PATCH 3/3] compat: define struct va_format introduced in v2.6.36
From: Joe Perches @ 2010-08-02 20:08 UTC (permalink / raw)
To: Kshitij Kulshreshtha; +Cc: Luis R. Rodriguez, Hauke Mehrtens, linux-wireless
In-Reply-To: <4C57227A.1050802@gmail.com>
On Mon, 2010-08-02 at 21:54 +0200, Kshitij Kulshreshtha wrote:
> As on 2010-08-02 21:48, Joe Perches did write:
> > On Mon, 2010-08-02 at 21:39 +0200, Kshitij Kulshreshtha wrote:
> >> As on 2010-08-02 20:00, Luis R. Rodriguez did write:
> >>> On Sun, Aug 1, 2010 at 3:02 PM, Kshitij Kulshreshtha
> >>>> +struct va_format {
> >>>> + const char *fmt;
> >>>> + va_list *va;
> >>>> +};
> >>> I'll apply this for now but what caller uses this for example?
> > Why is this necessary?
> I needed this to compile compat-wireless which is based on linux-next.
> Otherwise net/wireless/core.c fails to compile, due to the use of this
> struct va_format in the functions defined in lines 910--958. If this
> usage is reverted, we won't need it in compat.
I see your problem.
I think your problem should not be a constraint
on new development and I do not think the new use
should be reverted.
cheers, Joe
^ permalink raw reply
* Re: [PATCH 3/3] compat: define struct va_format introduced in v2.6.36
From: Kshitij Kulshreshtha @ 2010-08-02 19:54 UTC (permalink / raw)
To: Joe Perches; +Cc: Luis R. Rodriguez, Hauke Mehrtens, linux-wireless
In-Reply-To: <1280778534.1752.68.camel@Joe-Laptop.home>
As on 2010-08-02 21:48, Joe Perches did write:
> On Mon, 2010-08-02 at 21:39 +0200, Kshitij Kulshreshtha wrote:
>> As on 2010-08-02 20:00, Luis R. Rodriguez did write:
>>> On Sun, Aug 1, 2010 at 3:02 PM, Kshitij Kulshreshtha
>>>> +struct va_format {
>>>> + const char *fmt;
>>>> + va_list *va;
>>>> +};
>>> I'll apply this for now but what caller uses this for example?
>
> Why is this necessary?
>
I needed this to compile compat-wireless which is based on linux-next.
Otherwise net/wireless/core.c fails to compile, due to the use of this
struct va_format in the functions defined in lines 910--958. If this
usage is reverted, we won't need it in compat.
--
Kshitij Kulshreshtha
Institut für Mathematik,
Universität Paderborn,
Warburger Straße 100,
33098 Paderborn.
Büro: A3.319
^ permalink raw reply
* Re: [PATCH 3/3] compat: define struct va_format introduced in v2.6.36
From: Joe Perches @ 2010-08-02 19:48 UTC (permalink / raw)
To: Kshitij Kulshreshtha; +Cc: Luis R. Rodriguez, Hauke Mehrtens, linux-wireless
In-Reply-To: <4C571F0B.3000200@gmail.com>
On Mon, 2010-08-02 at 21:39 +0200, Kshitij Kulshreshtha wrote:
> As on 2010-08-02 20:00, Luis R. Rodriguez did write:
> > On Sun, Aug 1, 2010 at 3:02 PM, Kshitij Kulshreshtha
> >> +struct va_format {
> >> + const char *fmt;
> >> + va_list *va;
> >> +};
> > I'll apply this for now but what caller uses this for example?
Why is this necessary?
^ permalink raw reply
* Re: [PATCH 3/3] compat: define struct va_format introduced in v2.6.36
From: Kshitij Kulshreshtha @ 2010-08-02 19:39 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: Hauke Mehrtens, linux-wireless
In-Reply-To: <AANLkTinfrBRm8J2G+k8bB6N-YniVkmUYOBkRpyF_tC2j@mail.gmail.com>
As on 2010-08-02 20:00, Luis R. Rodriguez did write:
> On Sun, Aug 1, 2010 at 3:02 PM, Kshitij Kulshreshtha
> <kkhere.geo@gmail.com> wrote:
>> Signed-off-by: Kshitij Kulshreshtha <kkhere.geo@gmail.com>
>> ---
>> include/linux/compat-2.6.36.h | 5 +++++
>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/compat-2.6.36.h b/include/linux/compat-2.6.36.h
>> index 0307108..b14c772 100644
>> --- a/include/linux/compat-2.6.36.h
>> +++ b/include/linux/compat-2.6.36.h
>> @@ -8,6 +8,11 @@
>> #define kparam_block_sysfs_write(a)
>> #define kparam_unblock_sysfs_write(a)
>>
>> +struct va_format {
>> + const char *fmt;
>> + va_list *va;
>> +};
>> +
>> #endif /* (LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36)) */
>>
>> #endif /* LINUX_26_36_COMPAT_H */
>
> I'll apply this for now but what caller uses this for example?
>
in linux-next.git
net/wireless/core.c:912
static int ___wiphy_printk(const char *level, const struct wiphy *wiphy,
struct va_format *vaf)
> Luis
--
Kshitij Kulshreshtha
Institut für Mathematik,
Universität Paderborn,
Warburger Straße 100,
33098 Paderborn.
Büro: A3.319
Privatanschrift:
Arnikaweg 62
33100 Paderborn.
^ permalink raw reply
* Compat-wireless release for 2010-08-02 is baked
From: Compat-wireless cronjob account @ 2010-08-02 19:03 UTC (permalink / raw)
To: linux-wireless
>From git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/compat-wireless-2.6
04898a5..b3703a3 master -> origin/master
>From git://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/compat
8444e0f..1b21e25 master -> origin/master
>From git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next
57f21f4..523343a history -> origin/history
+ 850bd51...539c54f master -> origin/master (forced update)
a2dccdb..9fe6206 stable -> origin/stable
* [new tag] next-20100802 -> next-20100802
* [new tag] v2.6.35 -> v2.6.35
compat-wireless code metrics
494063 - Total upstream lines of code being pulled
^ permalink raw reply
* Re: [PATCH 3/3] compat: define struct va_format introduced in v2.6.36
From: Luis R. Rodriguez @ 2010-08-02 18:00 UTC (permalink / raw)
To: Kshitij Kulshreshtha; +Cc: Hauke Mehrtens, linux-wireless
In-Reply-To: <1280700173-14690-4-git-send-email-kkhere.geo@gmail.com>
On Sun, Aug 1, 2010 at 3:02 PM, Kshitij Kulshreshtha
<kkhere.geo@gmail.com> wrote:
> Signed-off-by: Kshitij Kulshreshtha <kkhere.geo@gmail.com>
> ---
> include/linux/compat-2.6.36.h | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/compat-2.6.36.h b/include/linux/compat-2.6.36.h
> index 0307108..b14c772 100644
> --- a/include/linux/compat-2.6.36.h
> +++ b/include/linux/compat-2.6.36.h
> @@ -8,6 +8,11 @@
> #define kparam_block_sysfs_write(a)
> #define kparam_unblock_sysfs_write(a)
>
> +struct va_format {
> + const char *fmt;
> + va_list *va;
> +};
> +
> #endif /* (LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36)) */
>
> #endif /* LINUX_26_36_COMPAT_H */
I'll apply this for now but what caller uses this for example?
Luis
^ permalink raw reply
* Re: [PATCH v5] cfg80211: Add nl80211 antenna configuration
From: Luis R. Rodriguez @ 2010-08-02 17:45 UTC (permalink / raw)
To: Felix Fietkau; +Cc: Bruno Randolf, johannes, linville, linux-wireless
In-Reply-To: <4C56A969.6060406@openwrt.org>
On Mon, Aug 2, 2010 at 4:18 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2010-08-02 12:51 PM, Luis R. Rodriguez wrote:
>> On Mon, Aug 2, 2010 at 3:47 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>> 802.11n hardware => need to bring down the interface before changing
>>> the antenna settings
>>
>> The current patch does not deal with the 802.11n case you mentioned.
> And why should it? The series does not add antenna control support to
> any specific 802.11n driver.
Because it documents support for 802.11n. You should not expect
someone writing their ops for an 802.11n driver to fill in the 802.11n
gaps for you unless you mark your API as being only for legacy.
Luis
^ permalink raw reply
* Re: [PATCH v2 00/20] native support for wl1271 on ZOOM
From: Ohad Ben-Cohen @ 2010-08-02 16:40 UTC (permalink / raw)
To: Vitaly Wool, Luciano Coelho
Cc: Tony Lindgren, Kalle Valo, Pandita Vikram, linux@arm.linux.org.uk,
Quadros Roger (Nokia-MS/Helsinki), linux-wireless@vger.kernel.org,
Nicolas Pitre, linux-mmc@vger.kernel.org, ext John W. Linville,
San Mehat, Chikkature Rajashekar Madhusudhan,
akpm@linux-foundation.org, linux-omap@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <AANLkTikPSp0_S8kSFmeZJ0qLR3DHKBN=iW_As5891a6v@mail.gmail.com>
Hi Vitaly,
On Mon, Aug 2, 2010 at 7:19 PM, Vitaly Wool <vitalywool@gmail.com> wrote:
> On Mon, Aug 2, 2010 at 5:59 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>>
>> Check out patch no. 9:
>
> sorry for the confusion. However, I guess if the platform doesn't
> supply this value, we'll just consider it 0? Without any warning,
> right?
>
> The problem is, we can't really distinguish between this field not set
> and value zero, and that means something is wrong with the design. The
> easiest way is probably changing the clock ids to start with 1.
I don't think it's a huge deal as adding new devices on board files is
anyway something that is done carefully and not so often. Btw, we have
this kind of mechanism in other locations of the kernel as well and it
doesn't seem to be problematic (e.g. spi->chip_select, spi->mode,
spi->irq, ...).
Having said that I don't mind applying your proposal either and
pushing those defines by 1 in the driver. Luca what do you think ?
Thanks,
Ohad.
>
> Thanks,
> Vitaly
>
^ permalink raw reply
* Re: 2.6.35-rc6-git6: Reported regressions from 2.6.34
From: Tejun Heo @ 2010-08-02 16:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rafael J. Wysocki, Jens Axboe, Linux Kernel Mailing List,
Maciej Rutecki, Andrew Morton, Kernel Testers List,
Network Development, Linux ACPI, Linux PM List, Linux SCSI List,
Linux Wireless List, DRI
In-Reply-To: <AANLkTimcH7+Bq1UEbaSU7SQpzArPgmSLegiqE13V8=CF@mail.gmail.com>
Hello, Linus.
On 08/01/2010 08:01 PM, Linus Torvalds wrote:
> This has a proposed patch. I don't know what the status of it is, though. Jens?
>
> http://marc.info/?l=linux-kernel&m=127950018204029&w=2
>
>> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=16393
>> Subject : kernel BUG at fs/block_dev.c:765!
>> Submitter : Markus Trippelsdorf <markus@trippelsdorf.de>
>> Date : 2010-07-14 13:52 (19 days old)
>> Message-ID : <20100714135217.GA1797@arch.tripp.de>
>> References : http://marc.info/?l=linux-kernel&m=127911564213748&w=2
>
> This one is interesting. And I think I perhaps see where it's coming from.
>
> bd_start_claiming() (through bd_prepare_to_claim()) has two separate
> success cases: either there was no holder (bd_claiming is NULL) or the
> new holder was already claiming it (bd_claiming == holder).
>
> Note in particular the case of the holder _already_ holding it. What happens is:
>
> - bd_start_claiming() succeeds because we had _already_ claimed it
> with the same holder
>
> - then some error happens, and we call bd_abort_claiming(), which
> does whole->bd_claiming = NULL;
>
> - the original holder thinks it still holds the bd, but it has been released!
>
> - a new claimer comes in, and succeeds because bd_claiming is now NULL.
>
> - we now have two "owners" of the bd, but bd_claiming only points to
> the second one.
>
> I think bd_start_claiming() needs to do some kind of refcount for the
> nested holder case, and bd_abort_claiming() needs to decrement the
> refcount and only clear the bd_claiming field when it goes down to
> zero.
>
> I dunno. Maybe there's something else going on, but it does look
> suspicious, and the above would explain the BUG_ON().
Yeah, that definitely sounds plausible. I think the condition check
in bd_prepare_to_claim() should have been "if (whole->bd_claiming)"
instead of "if (whole->bd_claiming && whole->bd_claiming != holder)".
It doesn't make much sense to allow multiple parallel claiming
operations anyway and the comment above already says - "This function
fails if @bdev is already claimed by another holder and waits if
another claiming is in progress."
I'll try to build a test case and verify it.
Thank you.
--
tejun
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox