Re: [PATCH 08/29] rfkill.txt: standardize document format

2017-05-19 Thread Johannes Berg
On Fri, 2017-05-19 at 08:11 -0300, Mauro Carvalho Chehab wrote: > > Yes, it should work. Actually, you would need to use :depth: 2 to > produce this output: > > > Contents > > . rfkill - RF kill switch support > . Introduction > . Implementation

Re: [PATCH 08/29] rfkill.txt: standardize document format

2017-05-19 Thread Johannes Berg
On Fri, 2017-05-19 at 08:11 -0300, Mauro Carvalho Chehab wrote: > > Yes, it should work. Actually, you would need to use :depth: 2 to > produce this output: > > > Contents > > . rfkill - RF kill switch support > . Introduction > . Implementation

Re: [PATCH 08/29] rfkill.txt: standardize document format

2017-05-19 Thread Johannes Berg
On Thu, 2017-05-18 at 22:25 -0300, Mauro Carvalho Chehab wrote: > > +.. CONTENTS >   > +  1. Introduction > +  2. Implementation details > +  3. Kernel API > +  4. Userspace support Why not let this be auto-generated? .. contents:: :depth: 1 should work, no? johannes

Re: [PATCH 08/29] rfkill.txt: standardize document format

2017-05-19 Thread Johannes Berg
On Thu, 2017-05-18 at 22:25 -0300, Mauro Carvalho Chehab wrote: > > +.. CONTENTS >   > +  1. Introduction > +  2. Implementation details > +  3. Kernel API > +  4. Userspace support Why not let this be auto-generated? .. contents:: :depth: 1 should work, no? johannes

Re: [PATCH 1/2] wcn36xx: Pass used skb to ieee80211_tx_status()

2017-05-18 Thread Johannes Berg
On Wed, 2017-05-17 at 22:05 -0700, Bjorn Andersson wrote: > > It seems very important to a lot of people... I get blinking, I guess, but I don't get toggling for every packet :) The throughput thing we did in iwlwifi seems like a so much better idea. Not that it really matters for this

Re: [PATCH 1/2] wcn36xx: Pass used skb to ieee80211_tx_status()

2017-05-18 Thread Johannes Berg
On Wed, 2017-05-17 at 22:05 -0700, Bjorn Andersson wrote: > > It seems very important to a lot of people... I get blinking, I guess, but I don't get toggling for every packet :) The throughput thing we did in iwlwifi seems like a so much better idea. Not that it really matters for this

Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data

2017-05-17 Thread Johannes Berg
On Wed, 2017-05-17 at 15:21 +0200, Pali Rohár wrote: > On Wednesday 17 May 2017 15:04:50 Johannes Berg wrote: > > On Wed, 2017-05-17 at 14:53 +0200, Pali Rohár wrote: > > > > > > In fact, why should the *driver* care either? IOW - why should > > > > &quo

Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data

2017-05-17 Thread Johannes Berg
On Wed, 2017-05-17 at 15:21 +0200, Pali Rohár wrote: > On Wednesday 17 May 2017 15:04:50 Johannes Berg wrote: > > On Wed, 2017-05-17 at 14:53 +0200, Pali Rohár wrote: > > > > > > In fact, why should the *driver* care either? IOW - why should > > > > &quo

Re: [PATCH 1/2] wcn36xx: Pass used skb to ieee80211_tx_status()

2017-05-17 Thread Johannes Berg
On Thu, 2017-05-04 at 13:13 +, Kalle Valo wrote: > > > > This code intentionally checked if TX status was requested, and > > > if not then it doesn't go to the effort of building it. > > > > > > > What I'm finding puzzling is the fact that the only caller of > > ieee80211_led_tx() is

Re: [PATCH 1/2] wcn36xx: Pass used skb to ieee80211_tx_status()

2017-05-17 Thread Johannes Berg
On Thu, 2017-05-04 at 13:13 +, Kalle Valo wrote: > > > > This code intentionally checked if TX status was requested, and > > > if not then it doesn't go to the effort of building it. > > > > > > > What I'm finding puzzling is the fact that the only caller of > > ieee80211_led_tx() is

Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data

2017-05-17 Thread Johannes Berg
On Wed, 2017-05-17 at 14:53 +0200, Pali Rohár wrote: > > In fact, why should the *driver* care either? IOW - why should > > "request_firmware_prefer_user()" even exist? > > There are default/example NVS data, which are stored in /lib/firmware > and installed by linux-firmware package. [...] Oh,

Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data

2017-05-17 Thread Johannes Berg
On Wed, 2017-05-17 at 14:53 +0200, Pali Rohár wrote: > > In fact, why should the *driver* care either? IOW - why should > > "request_firmware_prefer_user()" even exist? > > There are default/example NVS data, which are stored in /lib/firmware > and installed by linux-firmware package. [...] Oh,

Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data

2017-05-17 Thread Johannes Berg
On Tue, 2017-05-16 at 01:13 +0200, Luis R. Rodriguez wrote: > > > Now for N900 case there is a similar scenario > > > alhtough it has additional requirement to go to user-space due to > > > need to use a proprietary library to obtain the NVS calibration > > > data. My thought: Why should

Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data

2017-05-17 Thread Johannes Berg
On Tue, 2017-05-16 at 01:13 +0200, Luis R. Rodriguez wrote: > > > Now for N900 case there is a similar scenario > > > alhtough it has additional requirement to go to user-space due to > > > need to use a proprietary library to obtain the NVS calibration > > > data. My thought: Why should

Re: [PATCH] rt2x00: improve calling conventions for register accessors

2017-05-16 Thread Johannes Berg
On Tue, 2017-05-16 at 13:55 +0200, Stanislaw Gruszka wrote: > > In rt2x00 driver we use poor convention in other kind of registers > accessors like bbp, mac, eeprom. I dislike to changing only rfcsr > accessors and leaving others in the old way. And changing all > accessors would be massive and

Re: [PATCH] rt2x00: improve calling conventions for register accessors

2017-05-16 Thread Johannes Berg
On Tue, 2017-05-16 at 13:55 +0200, Stanislaw Gruszka wrote: > > In rt2x00 driver we use poor convention in other kind of registers > accessors like bbp, mac, eeprom. I dislike to changing only rfcsr > accessors and leaving others in the old way. And changing all > accessors would be massive and

Re: [PATCH v7 0/5] skb_to_sgvec hardening

2017-05-09 Thread Johannes Berg
On Tue, 2017-05-09 at 15:50 +0200, Jason A. Donenfeld wrote: > The recent bug with macsec and historical one with virtio have > indicated that letting skb_to_sgvec trounce all over an sglist > without checking the length is probably a bad idea. And it's not > necessary either: an sglist already

Re: [PATCH v7 0/5] skb_to_sgvec hardening

2017-05-09 Thread Johannes Berg
On Tue, 2017-05-09 at 15:50 +0200, Jason A. Donenfeld wrote: > The recent bug with macsec and historical one with virtio have > indicated that letting skb_to_sgvec trounce all over an sglist > without checking the length is probably a bad idea. And it's not > necessary either: an sglist already

Re: [PATCH] wil6210: Replace five seq_puts() calls by seq_putc()

2017-05-09 Thread Johannes Berg
On Tue, 2017-05-09 at 09:50 +0200, SF Markus Elfring wrote: > From: Markus Elfring > Date: Mon, 8 May 2017 22:22:04 +0200 > > Five single characters (line breaks) should be put into a sequence. > Thus use the corresponding function "seq_putc". Please stop, this

Re: [PATCH] wil6210: Replace five seq_puts() calls by seq_putc()

2017-05-09 Thread Johannes Berg
On Tue, 2017-05-09 at 09:50 +0200, SF Markus Elfring wrote: > From: Markus Elfring > Date: Mon, 8 May 2017 22:22:04 +0200 > > Five single characters (line breaks) should be put into a sequence. > Thus use the corresponding function "seq_putc". Please stop, this isn't really an issue worth

Re: [PATCH] mac80211: Create ieee80211_if_process_skb from ieee80211_iface_work

2017-05-05 Thread Johannes Berg
On Fri, 2017-05-05 at 02:34 -0700, Joe Perches wrote: > On Fri, 2017-05-05 at 11:06 +0200, Johannes Berg wrote: > > > o Use explicit casts to proper types instead of casts to (void *) > > >   and have the compiler do the implicit cast > > > > I see no advantage

Re: [PATCH] mac80211: Create ieee80211_if_process_skb from ieee80211_iface_work

2017-05-05 Thread Johannes Berg
On Fri, 2017-05-05 at 02:34 -0700, Joe Perches wrote: > On Fri, 2017-05-05 at 11:06 +0200, Johannes Berg wrote: > > > o Use explicit casts to proper types instead of casts to (void *) > > >   and have the compiler do the implicit cast > > > > I see no advantage

Re: [PATCH] mac80211: Create ieee80211_if_process_skb from ieee80211_iface_work

2017-05-05 Thread Johannes Berg
> o Use explicit casts to proper types instead of casts to (void *) >   and have the compiler do the implicit cast I see no advantage in this, why? All it does is make the code longer, and if anything changes, you have to change it in multiple places now. johannes

Re: [PATCH] mac80211: Create ieee80211_if_process_skb from ieee80211_iface_work

2017-05-05 Thread Johannes Berg
> o Use explicit casts to proper types instead of casts to (void *) >   and have the compiler do the implicit cast I see no advantage in this, why? All it does is make the code longer, and if anything changes, you have to change it in multiple places now. johannes

Re: new warning at net/wireless/util.c:1236

2017-05-04 Thread Johannes Berg
Hi, > Things still work, but when it starts warning, it generates a *lot* > of noise (I got 36 of these within about ten minutes). Yeah, that's kinda dumb - I just sent a patch to make that just warn once and actually report the configuration. > I have no idea what triggered it, because when I

Re: new warning at net/wireless/util.c:1236

2017-05-04 Thread Johannes Berg
Hi, > Things still work, but when it starts warning, it generates a *lot* > of noise (I got 36 of these within about ten minutes). Yeah, that's kinda dumb - I just sent a patch to make that just warn once and actually report the configuration. > I have no idea what triggered it, because when I

Re: [RFC PATCH 8/9] debugfs: defer debugfs_fsdata allocation to first usage

2017-05-02 Thread Johannes Berg
On Tue, 2017-05-02 at 22:05 +0200, Nicolai Stange wrote: > > So either you could return some valid ops (perhaps > > debugfs_noop_file_operations although those don't have .name or > > .poll, so it doesn't cover everything), or you can just BUG_ON() > > here directly, saving the incomprehensible

Re: [RFC PATCH 8/9] debugfs: defer debugfs_fsdata allocation to first usage

2017-05-02 Thread Johannes Berg
On Tue, 2017-05-02 at 22:05 +0200, Nicolai Stange wrote: > > So either you could return some valid ops (perhaps > > debugfs_noop_file_operations although those don't have .name or > > .poll, so it doesn't cover everything), or you can just BUG_ON() > > here directly, saving the incomprehensible

Re: [PATCH 1/2] wcn36xx: Pass used skb to ieee80211_tx_status()

2017-04-27 Thread Johannes Berg
> @@ -371,7 +371,7 @@ static void reap_tx_dxes(struct wcn36xx *wcn, > struct wcn36xx_dxe_ch *ch) >   info = IEEE80211_SKB_CB(ctl->skb); >   if (!(info->flags & > IEEE80211_TX_CTL_REQ_TX_STATUS)) { >   /* Keep frame until TX

Re: [PATCH 1/2] wcn36xx: Pass used skb to ieee80211_tx_status()

2017-04-27 Thread Johannes Berg
> @@ -371,7 +371,7 @@ static void reap_tx_dxes(struct wcn36xx *wcn, > struct wcn36xx_dxe_ch *ch) >   info = IEEE80211_SKB_CB(ctl->skb); >   if (!(info->flags & > IEEE80211_TX_CTL_REQ_TX_STATUS)) { >   /* Keep frame until TX

Re: [PATCH 1/1] cfg80211: add return value validation

2017-04-23 Thread Johannes Berg
This is not a cfg80211 patch, please resend with the correct subject. Thanks, johannes

Re: [PATCH 1/1] cfg80211: add return value validation

2017-04-23 Thread Johannes Berg
This is not a cfg80211 patch, please resend with the correct subject. Thanks, johannes

Re: [PATCH] nl80211: fix enumeration type

2017-04-20 Thread Johannes Berg
On Wed, 2017-04-19 at 23:55 -0700, Stefan Agner wrote: > Use type enum nl80211_rate_info for bitrate information. This fixes > a warning when compiling with clang: >   warning: implicit conversion from enumeration type 'enum > nl80211_rate_info' >   to different enumeration type 'enum

Re: [PATCH] nl80211: fix enumeration type

2017-04-20 Thread Johannes Berg
On Wed, 2017-04-19 at 23:55 -0700, Stefan Agner wrote: > Use type enum nl80211_rate_info for bitrate information. This fixes > a warning when compiling with clang: >   warning: implicit conversion from enumeration type 'enum > nl80211_rate_info' >   to different enumeration type 'enum

Re: [RFC PATCH 9/9] debugfs: free debugfs_fsdata instances

2017-04-18 Thread Johannes Berg
On Tue, 2017-04-18 at 08:17 -0700, Paul E. McKenney wrote: > > Again, no (S)RCU abuse here, just an ABBA deadlock. > > OK, please accept my apologies for failing to follow the thread. No worries - just wanted to clarify this in case I was missing something. > I nevertheless reiterate my advice

Re: [RFC PATCH 9/9] debugfs: free debugfs_fsdata instances

2017-04-18 Thread Johannes Berg
On Tue, 2017-04-18 at 08:17 -0700, Paul E. McKenney wrote: > > Again, no (S)RCU abuse here, just an ABBA deadlock. > > OK, please accept my apologies for failing to follow the thread. No worries - just wanted to clarify this in case I was missing something. > I nevertheless reiterate my advice

Re: [RFC PATCH 9/9] debugfs: free debugfs_fsdata instances

2017-04-18 Thread Johannes Berg
On Tue, 2017-04-18 at 06:31 -0700, Paul E. McKenney wrote: > On Tue, Apr 18, 2017 at 11:39:27AM +0200, Johannes Berg wrote: > > On Mon, 2017-04-17 at 09:01 -0700, Paul E. McKenney wrote: > > > > > If you have not already done so, please run this with debug > &g

Re: [RFC PATCH 9/9] debugfs: free debugfs_fsdata instances

2017-04-18 Thread Johannes Berg
On Tue, 2017-04-18 at 06:31 -0700, Paul E. McKenney wrote: > On Tue, Apr 18, 2017 at 11:39:27AM +0200, Johannes Berg wrote: > > On Mon, 2017-04-17 at 09:01 -0700, Paul E. McKenney wrote: > > > > > If you have not already done so, please run this with debug > &g

Re: [RFC PATCH 9/9] debugfs: free debugfs_fsdata instances

2017-04-18 Thread Johannes Berg
On Mon, 2017-04-17 at 09:01 -0700, Paul E. McKenney wrote: > If you have not already done so, please run this with debug enabled, > especially CONFIG_PROVE_LOCKING=y (which implies CONFIG_PROVE_RCU=y). > This is important because there are configurations for which the > deadlocks you saw with

Re: [RFC PATCH 9/9] debugfs: free debugfs_fsdata instances

2017-04-18 Thread Johannes Berg
On Mon, 2017-04-17 at 09:01 -0700, Paul E. McKenney wrote: > If you have not already done so, please run this with debug enabled, > especially CONFIG_PROVE_LOCKING=y (which implies CONFIG_PROVE_RCU=y). > This is important because there are configurations for which the > deadlocks you saw with

Re: [RFC PATCH 8/9] debugfs: defer debugfs_fsdata allocation to first usage

2017-04-18 Thread Johannes Berg
On Sun, 2017-04-16 at 11:51 +0200, Nicolai Stange wrote: > > +++ b/fs/debugfs/file.c > @@ -53,6 +53,7 @@ const struct file_operations > *debugfs_real_fops(const struct file *filp) >  { >   struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata; >   > + WARN_ON((unsigned long)fsd & >

Re: [RFC PATCH 8/9] debugfs: defer debugfs_fsdata allocation to first usage

2017-04-18 Thread Johannes Berg
On Sun, 2017-04-16 at 11:51 +0200, Nicolai Stange wrote: > > +++ b/fs/debugfs/file.c > @@ -53,6 +53,7 @@ const struct file_operations > *debugfs_real_fops(const struct file *filp) >  { >   struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata; >   > + WARN_ON((unsigned long)fsd & >

Re: [PATCH] nl80211: Fix enum type of variable in nl80211_put_sta_rate()

2017-04-18 Thread Johannes Berg
On Mon, 2017-04-17 at 15:59 -0700, Matthias Kaehlcke wrote: > rate_flg is of type 'enum nl80211_attrs', however it is assigned with > 'enum nl80211_rate_info' values. Change the type of rate_flg > accordingly. Applied this, and the other two patches you had (IBSS enum and array- bounds) johannes

Re: [PATCH] nl80211: Fix enum type of variable in nl80211_put_sta_rate()

2017-04-18 Thread Johannes Berg
On Mon, 2017-04-17 at 15:59 -0700, Matthias Kaehlcke wrote: > rate_flg is of type 'enum nl80211_attrs', however it is assigned with > 'enum nl80211_rate_info' values. Change the type of rate_flg > accordingly. Applied this, and the other two patches you had (IBSS enum and array- bounds) johannes

Re: linux-next: build failure after merge of the staging tree

2017-04-18 Thread Johannes Berg
On Tue, 2017-04-18 at 15:53 +1000, Stephen Rothwell wrote: > Caused by commit > >   554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver") Oh, another new driver :) > interacting with commit > >   818a986e4eba ("cfg80211: move add/change interface monitor flags > into params") > > from the

Re: linux-next: build failure after merge of the staging tree

2017-04-18 Thread Johannes Berg
On Tue, 2017-04-18 at 15:53 +1000, Stephen Rothwell wrote: > Caused by commit > >   554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver") Oh, another new driver :) > interacting with commit > >   818a986e4eba ("cfg80211: move add/change interface monitor flags > into params") > > from the

Re: [PATCH v2] mac80211: Fix clang warning about constant operand in logical operation

2017-04-13 Thread Johannes Berg
On Thu, 2017-04-06 at 16:31 -0700, Matthias Kaehlcke wrote: > When clang detects a non-boolean constant in a logical operation it > generates a 'constant-logical-operand' warning. In > ieee80211_try_rate_control_ops_get() the result of strlen( str>) > is used in a logical operation, clang resolves

Re: [PATCH v2] mac80211: Fix clang warning about constant operand in logical operation

2017-04-13 Thread Johannes Berg
On Thu, 2017-04-06 at 16:31 -0700, Matthias Kaehlcke wrote: > When clang detects a non-boolean constant in a logical operation it > generates a 'constant-logical-operand' warning. In > ieee80211_try_rate_control_ops_get() the result of strlen( str>) > is used in a logical operation, clang resolves

Re: [PATCH v2] cfg80211: Fix array-bounds warning in fragment copy

2017-04-10 Thread Johannes Berg
On Mon, 2017-04-10 at 14:36 -0700, Matthias Kaehlcke wrote: > Ping, any feedback on this patch? You didn't cc linux-wireless, so it fell through the cracks ... I'll try to remember it when I merge later. johannes

Re: [PATCH v2] cfg80211: Fix array-bounds warning in fragment copy

2017-04-10 Thread Johannes Berg
On Mon, 2017-04-10 at 14:36 -0700, Matthias Kaehlcke wrote: > Ping, any feedback on this patch? You didn't cc linux-wireless, so it fell through the cracks ... I'll try to remember it when I merge later. johannes

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

2017-04-06 Thread Johannes Berg
On Thu, 2017-04-06 at 15:42 -0700, Matthias Kaehlcke wrote: > > Thanks, it would also require to move the initialization of > ieee80211_default_rc_algo into an ifdef. If you can live with such a > solution I'm happy to change it. I think that'd be something I can live with, yeah. > > git

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

2017-04-06 Thread Johannes Berg
On Thu, 2017-04-06 at 15:42 -0700, Matthias Kaehlcke wrote: > > Thanks, it would also require to move the initialization of > ieee80211_default_rc_algo into an ifdef. If you can live with such a > solution I'm happy to change it. I think that'd be something I can live with, yeah. > > git

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

2017-04-06 Thread Johannes Berg
On Thu, 2017-04-06 at 12:24 -0700, Matthias Kaehlcke wrote: > I agree that the code looks worse :( I hoped to find a fix using a > preprocessor condition but wasn't successful. It's actually easy - just remove the 'default ""' from Kconfig, and then the symbol won't be defined at all if it

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

2017-04-06 Thread Johannes Berg
On Thu, 2017-04-06 at 12:24 -0700, Matthias Kaehlcke wrote: > I agree that the code looks worse :( I hoped to find a fix using a > preprocessor condition but wasn't successful. It's actually easy - just remove the 'default ""' from Kconfig, and then the symbol won't be defined at all if it

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

2017-04-06 Thread Johannes Berg
On Thu, 2017-04-06 at 11:56 -0700, Matthias Kaehlcke wrote: > Clang raises a warning about the expression 'strlen(CONFIG_XXX)' > being > used in a logical operation. Clangs' builtin strlen function resolves > the > expression to a constant at compile time, which causes clang to > generate > a

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

2017-04-06 Thread Johannes Berg
On Thu, 2017-04-06 at 11:56 -0700, Matthias Kaehlcke wrote: > Clang raises a warning about the expression 'strlen(CONFIG_XXX)' > being > used in a logical operation. Clangs' builtin strlen function resolves > the > expression to a constant at compile time, which causes clang to > generate > a

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-31 Thread Johannes Berg
On Fri, 2017-03-31 at 11:03 +0200, Nicolai Stange wrote: > > 2) > > There's a complete deadlock situation if this happens: > > > > CPU1CPU2 > > > > debugfs_file_read(file="foo") mutex_lock(); > > srcu_read_lock(_srcu);

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-31 Thread Johannes Berg
On Fri, 2017-03-31 at 11:03 +0200, Nicolai Stange wrote: > > 2) > > There's a complete deadlock situation if this happens: > > > > CPU1CPU2 > > > > debugfs_file_read(file="foo") mutex_lock(); > > srcu_read_lock(_srcu);

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-30 Thread Johannes Berg
On Thu, 2017-03-30 at 12:27 +0200, Nicolai Stange wrote: > So, please correct me if I'm wrong, there are two problems with > indefinitely blocking debugfs files' fops: > > 1. The one which actually hung your system: >    An indefinitely blocking debugfs_remove() while holding a lock. >    Other

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-30 Thread Johannes Berg
On Thu, 2017-03-30 at 12:27 +0200, Nicolai Stange wrote: > So, please correct me if I'm wrong, there are two problems with > indefinitely blocking debugfs files' fops: > > 1. The one which actually hung your system: >    An indefinitely blocking debugfs_remove() while holding a lock. >    Other

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-30 Thread Johannes Berg
On Thu, 2017-03-30 at 09:32 +0200, Nicolai Stange wrote: > > I wonder if holding the RTNL during the debugfs file removal is > really needed. I'll try to have a look in the next couple of days. Yes, I'm pretty much convinced that it is. I considered doing a deferred debugfs_remove() by holding

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-30 Thread Johannes Berg
On Thu, 2017-03-30 at 09:32 +0200, Nicolai Stange wrote: > > I wonder if holding the RTNL during the debugfs file removal is > really needed. I'll try to have a look in the next couple of days. Yes, I'm pretty much convinced that it is. I considered doing a deferred debugfs_remove() by holding

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-27 Thread Johannes Berg
Hi, > > Before I go hunting - has anyone seen a deadlock in > > synchronize_srcu() in debugfs_remove() before? > > Not yet. How reproducible is this? So ... this turned out to be a livelock of sorts. We have a debugfs file (not upstream (yet?), it seems) that basically blocks reading data. At

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-27 Thread Johannes Berg
Hi, > > Before I go hunting - has anyone seen a deadlock in > > synchronize_srcu() in debugfs_remove() before? > > Not yet. How reproducible is this? So ... this turned out to be a livelock of sorts. We have a debugfs file (not upstream (yet?), it seems) that basically blocks reading data. At

Re: [PATCH] cfg80211: Fix array-bounds warning in fragment copy

2017-03-27 Thread Johannes Berg
> > > - const skb_frag_t *frag = >frags[-1]; > > > + const skb_frag_t *frag = >frags[0]; [...] > > > + frag--; > > > > Isn't it just a question of time until the compiler will see > > through this trick and warn about it? > > Frag is incremented again before being accessed, so there is nothing

Re: [PATCH] cfg80211: Fix array-bounds warning in fragment copy

2017-03-27 Thread Johannes Berg
> > > - const skb_frag_t *frag = >frags[-1]; > > > + const skb_frag_t *frag = >frags[0]; [...] > > > + frag--; > > > > Isn't it just a question of time until the compiler will see > > through this trick and warn about it? > > Frag is incremented again before being accessed, so there is nothing

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-27 Thread Johannes Berg
On Fri, 2017-03-24 at 13:20 -0700, Paul E. McKenney wrote: > > And I cannot resist adding this one: > > CPU 1 CPU 2 > i = srcu_read_lock();mutex_lock(); > mutex_lock();synchronize_srcu(); > mutex_unlock();

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-27 Thread Johannes Berg
On Fri, 2017-03-24 at 13:20 -0700, Paul E. McKenney wrote: > > And I cannot resist adding this one: > > CPU 1 CPU 2 > i = srcu_read_lock();mutex_lock(); > mutex_lock();synchronize_srcu(); > mutex_unlock();

Re: [PATCH] cfg80211: Fix array-bounds warning in fragment copy

2017-03-27 Thread Johannes Berg
On Fri, 2017-03-24 at 18:06 -0700, Matthias Kaehlcke wrote: > __ieee80211_amsdu_copy_frag intentionally initializes a pointer to > array[-1] to increment it later to valid values. clang rightfully > generates an array-bounds warning on the initialization statement. > Work around this by

Re: [PATCH] cfg80211: Fix array-bounds warning in fragment copy

2017-03-27 Thread Johannes Berg
On Fri, 2017-03-24 at 18:06 -0700, Matthias Kaehlcke wrote: > __ieee80211_amsdu_copy_frag intentionally initializes a pointer to > array[-1] to increment it later to valid values. clang rightfully > generates an array-bounds warning on the initialization statement. > Work around this by

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-24 Thread Johannes Berg
> Yes.  CPU2 has a pre-existing reader that CPU1's synchronize_srcu() > must wait for.  But CPU2's reader cannot end until CPU1 releases > its lock, which it cannot do until after CPU2's reader ends.  Thus, > as you say, deadlock. > > The rule is that if you are within any kind of RCU read-side

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-24 Thread Johannes Berg
> Yes.  CPU2 has a pre-existing reader that CPU1's synchronize_srcu() > must wait for.  But CPU2's reader cannot end until CPU1 releases > its lock, which it cannot do until after CPU2's reader ends.  Thus, > as you say, deadlock. > > The rule is that if you are within any kind of RCU read-side

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-24 Thread Johannes Berg
Hi, On Fri, 2017-03-24 at 09:56 +0100, Johannes Berg wrote: > On Thu, 2017-03-23 at 16:29 +0100, Johannes Berg wrote: > > Isn't it possible for the following to happen? > > > > CPU1CPU2 > > > >

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-24 Thread Johannes Berg
Hi, On Fri, 2017-03-24 at 09:56 +0100, Johannes Berg wrote: > On Thu, 2017-03-23 at 16:29 +0100, Johannes Berg wrote: > > Isn't it possible for the following to happen? > > > > CPU1CPU2 > > > >

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-24 Thread Johannes Berg
On Thu, 2017-03-23 at 16:29 +0100, Johannes Berg wrote: > Isn't it possible for the following to happen? > > CPU1 CPU2 > > mutex_lock(); > full_proxy_xyz(); >

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-24 Thread Johannes Berg
On Thu, 2017-03-23 at 16:29 +0100, Johannes Berg wrote: > Isn't it possible for the following to happen? > > CPU1 CPU2 > > mutex_lock(); > full_proxy_xyz(); >

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-23 Thread Johannes Berg
Hi, > Not yet. How reproducible is this? Apparently quite. I haven't tried myself - it happens during some automated test that I need to analyse further. > > We're observing that with our (backported, but very recent) driver > > against 4.9 (and 4.10, I think), > > Do I understand it correctly

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-23 Thread Johannes Berg
Hi, > Not yet. How reproducible is this? Apparently quite. I haven't tried myself - it happens during some automated test that I need to analyse further. > > We're observing that with our (backported, but very recent) driver > > against 4.9 (and 4.10, I think), > > Do I understand it correctly

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-23 Thread Johannes Berg
On Thu, 2017-03-23 at 08:37 -0700, Paul E. McKenney wrote: > I have not seen this, but my usual question for __synchronize_srcu() > is if some other task is blocked holding srcu_read_lock() for that > same srcu_struct. > Not as far as I can see - but that was the scenario I was outlining in my

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-23 Thread Johannes Berg
On Thu, 2017-03-23 at 08:37 -0700, Paul E. McKenney wrote: > I have not seen this, but my usual question for __synchronize_srcu() > is if some other task is blocked holding srcu_read_lock() for that > same srcu_struct. > Not as far as I can see - but that was the scenario I was outlining in my

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-23 Thread Johannes Berg
On Thu, 2017-03-23 at 15:54 +0100, Johannes Berg wrote: > Before I go hunting - has anyone seen a deadlock in > synchronize_srcu() in debugfs_remove() before? Isn't it possible for the following to happen? CPU1CPU2 mute

Re: deadlock in synchronize_srcu() in debugfs?

2017-03-23 Thread Johannes Berg
On Thu, 2017-03-23 at 15:54 +0100, Johannes Berg wrote: > Before I go hunting - has anyone seen a deadlock in > synchronize_srcu() in debugfs_remove() before? Isn't it possible for the following to happen? CPU1CPU2 mute

deadlock in synchronize_srcu() in debugfs?

2017-03-23 Thread Johannes Berg
Hi, Before I go hunting - has anyone seen a deadlock in synchronize_srcu() in debugfs_remove() before? We're observing that with our (backported, but very recent) driver against 4.9 (and 4.10, I think), but there are no backports of any debugfs things so the backport itself doesn't seem like a

deadlock in synchronize_srcu() in debugfs?

2017-03-23 Thread Johannes Berg
Hi, Before I go hunting - has anyone seen a deadlock in synchronize_srcu() in debugfs_remove() before? We're observing that with our (backported, but very recent) driver against 4.9 (and 4.10, I think), but there are no backports of any debugfs things so the backport itself doesn't seem like a

Re: [PATCH] mac80211: Use setup_timer instead of init_timer

2017-03-06 Thread Johannes Berg
On Fri, 2017-03-03 at 13:45 +0100, Jiri Slaby wrote: > From: Ondřej Lysoněk > > Use setup_timer() and setup_deferrable_timer() to set the data and > function timer fields. It makes the code cleaner and will allow for > easier change of the timer struct internals.

Re: [PATCH] mac80211: Use setup_timer instead of init_timer

2017-03-06 Thread Johannes Berg
On Fri, 2017-03-03 at 13:45 +0100, Jiri Slaby wrote: > From: Ondřej Lysoněk > > Use setup_timer() and setup_deferrable_timer() to set the data and > function timer fields. It makes the code cleaner and will allow for > easier change of the timer struct internals. Applied. johannes

Re: [PATCH] mac80211: Use setup_timer instead of init_timer

2017-03-06 Thread Johannes Berg
On Fri, 2017-03-03 at 13:45 +0100, Jiri Slaby wrote: > From: Ondřej Lysoněk > > Use setup_timer() and setup_deferrable_timer() to set the data and > function timer fields. It makes the code cleaner and will allow for > easier change of the timer struct internals. Btw,

Re: [PATCH] mac80211: Use setup_timer instead of init_timer

2017-03-06 Thread Johannes Berg
On Fri, 2017-03-03 at 13:45 +0100, Jiri Slaby wrote: > From: Ondřej Lysoněk > > Use setup_timer() and setup_deferrable_timer() to set the data and > function timer fields. It makes the code cleaner and will allow for > easier change of the timer struct internals. Btw, I suspect you generated

Re: [PATCH] mac80211: Use setup_timer instead of init_timer

2017-03-06 Thread Johannes Berg
On Mon, 2017-03-06 at 13:25 +0100, Johannes Berg wrote: > On Fri, 2017-03-03 at 13:45 +0100, Jiri Slaby wrote: > > From: Ondřej Lysoněk <ondrej.lyso...@seznam.cz> > > > > Use setup_timer() and setup_deferrable_timer() to set the data and > > function timer

Re: [PATCH] mac80211: Use setup_timer instead of init_timer

2017-03-06 Thread Johannes Berg
On Mon, 2017-03-06 at 13:25 +0100, Johannes Berg wrote: > On Fri, 2017-03-03 at 13:45 +0100, Jiri Slaby wrote: > > From: Ondřej Lysoněk > > > > Use setup_timer() and setup_deferrable_timer() to set the data and > > function timer fields. It makes the code cleaner an

Re: [PATCH] mac80211: Use setup_timer instead of init_timer

2017-03-06 Thread Johannes Berg
> Not really. This is one of assignments for students I lead, so this > is done by hand every end of winter semester (Note the From line.) You really should teach them about coccinelle then :-) > > Care to send a patch for that one too? > > I am just a forwarder, he received this request too,

Re: [PATCH] mac80211: Use setup_timer instead of init_timer

2017-03-06 Thread Johannes Berg
> Not really. This is one of assignments for students I lead, so this > is done by hand every end of winter semester (Note the From line.) You really should teach them about coccinelle then :-) > > Care to send a patch for that one too? > > I am just a forwarder, he received this request too,

Re: [RFC 0/5] iwlwifi: enhance final opmode work

2017-02-28 Thread Johannes Berg
> One of the limitations of using async_schedule() though is we cannot > request_module() synchronously on async calls given that the module > initialization code will call async_synchronize_full() if the module > being initialized happened to have used async work on its > initialization routine,

Re: [RFC 0/5] iwlwifi: enhance final opmode work

2017-02-28 Thread Johannes Berg
> One of the limitations of using async_schedule() though is we cannot > request_module() synchronously on async calls given that the module > initialization code will call async_synchronize_full() if the module > being initialized happened to have used async work on its > initialization routine,

Re: KASAN+netlink, was: [PATCH] [net-next?] hns: avoid stack overflow with CONFIG_KASAN

2017-02-08 Thread Johannes Berg
On Wed, 2017-02-08 at 13:03 +0100, Arnd Bergmann wrote: > > - Moving nla_put_{u8,u16,u32} out of line is probably uncontroversial > and >   it helps enough with br_netlink.c, but nl820211 is worse and needs > some >   additional fiddling. Why would that not be sufficient by itself for nl80211?

Re: KASAN+netlink, was: [PATCH] [net-next?] hns: avoid stack overflow with CONFIG_KASAN

2017-02-08 Thread Johannes Berg
On Wed, 2017-02-08 at 13:03 +0100, Arnd Bergmann wrote: > > - Moving nla_put_{u8,u16,u32} out of line is probably uncontroversial > and >   it helps enough with br_netlink.c, but nl820211 is worse and needs > some >   additional fiddling. Why would that not be sufficient by itself for nl80211?

Re: rtlwifi: rtl8192c_common: "BUG: KASAN: slab-out-of-bounds"

2017-02-06 Thread Johannes Berg
On Sat, 2017-02-04 at 12:41 -0600, Larry Finger wrote: > On 02/04/2017 10:58 AM, Dmitry Osipenko wrote: > > Seems the problem is caused by rtl92c_dm_*() casting .priv to > > "struct > > rtl_pci_priv", while it is "struct rtl_usb_priv". > > Those routines are shared by rtl8192ce and rtl8192cu,

Re: rtlwifi: rtl8192c_common: "BUG: KASAN: slab-out-of-bounds"

2017-02-06 Thread Johannes Berg
On Sat, 2017-02-04 at 12:41 -0600, Larry Finger wrote: > On 02/04/2017 10:58 AM, Dmitry Osipenko wrote: > > Seems the problem is caused by rtl92c_dm_*() casting .priv to > > "struct > > rtl_pci_priv", while it is "struct rtl_usb_priv". > > Those routines are shared by rtl8192ce and rtl8192cu,

Re: [PATCH] cfg80211 debugfs: Cleanup some checkpatch issues

2017-01-27 Thread Johannes Berg
On Fri, 2017-01-27 at 22:26 +0300, Pichugin Dmitry wrote: > This fixes the checkpatch.pl warnings: > * Macros should not use a trailing semicolon. > * Spaces required around that '='. > * Symbolic permissions 'S_IRUGO' are not preferred. > * Macro argument reuse 'buflen' - possible side-effects I

Re: [PATCH] cfg80211 debugfs: Cleanup some checkpatch issues

2017-01-27 Thread Johannes Berg
On Fri, 2017-01-27 at 22:26 +0300, Pichugin Dmitry wrote: > This fixes the checkpatch.pl warnings: > * Macros should not use a trailing semicolon. > * Spaces required around that '='. > * Symbolic permissions 'S_IRUGO' are not preferred. > * Macro argument reuse 'buflen' - possible side-effects I

<    2   3   4   5   6   7   8   9   10   11   >