* regd: sleeping in atomic
@ 2010-03-16 13:26 Jiri Slaby
2010-03-16 16:18 ` Luis R. Rodriguez
2010-03-16 19:49 ` [PATCH] wireless: convert reg_regdb_search_lock to mutex John W. Linville
0 siblings, 2 replies; 15+ messages in thread
From: Jiri Slaby @ 2010-03-16 13:26 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org; +Cc: John W. Linville, LKML
Hi,
Stanse found an atomic error in reg_copy_regd:
static int reg_copy_regd(const struct ieee80211_regdomain **dst_regd,
const struct ieee80211_regdomain *src_regd)
{
struct ieee80211_regdomain *regd;
int size_of_regd = 0;
unsigned int i;
size_of_regd = sizeof(struct ieee80211_regdomain) +
((src_regd->n_reg_rules + 1) * sizeof(struct
ieee80211_reg_rule));
regd = kzalloc(size_of_regd, GFP_KERNEL); <---- here
Called from:
static void reg_regdb_search(struct work_struct *work)
{
spin_lock(®_regdb_search_lock);
while (!list_empty(®_regdb_search_list)) {
...
for (i=0; i<reg_regdb_size; i++) {
curdom = reg_regdb[i];
if (!memcmp(request->alpha2, curdom->alpha2, 2)) {
r = reg_copy_regd(®dom, curdom);
...
spin_unlock(®_regdb_search_lock);
}
Whole error temporarily available at:
http://decibel.fi.muni.cz/~xslaby/stanse/error.cgi?db=34-rc&id=578
It is introduced by 3b377ea9d4efc94dc52fe41b4dfdb463635ab298.
Do you plan to extend it somehow or may the spinlock be converted to
mutex? If not how much may size_of_regd be -- can we safely switch to
GFP_ATOMIC?
--
js
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: regd: sleeping in atomic 2010-03-16 13:26 regd: sleeping in atomic Jiri Slaby @ 2010-03-16 16:18 ` Luis R. Rodriguez 2010-03-16 18:42 ` Jiri Slaby 2010-03-16 19:49 ` [PATCH] wireless: convert reg_regdb_search_lock to mutex John W. Linville 1 sibling, 1 reply; 15+ messages in thread From: Luis R. Rodriguez @ 2010-03-16 16:18 UTC (permalink / raw) To: Jiri Slaby; +Cc: linux-wireless@vger.kernel.org, John W. Linville, LKML 2010/3/16 Jiri Slaby <jirislaby@gmail.com>: > Hi, > > Stanse found an atomic error in reg_copy_regd: > > static int reg_copy_regd(const struct ieee80211_regdomain **dst_regd, > const struct ieee80211_regdomain *src_regd) > { > struct ieee80211_regdomain *regd; > int size_of_regd = 0; > unsigned int i; > > size_of_regd = sizeof(struct ieee80211_regdomain) + > ((src_regd->n_reg_rules + 1) * sizeof(struct ieee80211_reg_rule)); > > regd = kzalloc(size_of_regd, GFP_KERNEL); <---- here > > Called from: > > static void reg_regdb_search(struct work_struct *work) > { > spin_lock(®_regdb_search_lock); > while (!list_empty(®_regdb_search_list)) { > ... > for (i=0; i<reg_regdb_size; i++) { > curdom = reg_regdb[i]; > > if (!memcmp(request->alpha2, curdom->alpha2, 2)) { > r = reg_copy_regd(®dom, curdom); > ... > spin_unlock(®_regdb_search_lock); > } > > Whole error temporarily available at: > http://decibel.fi.muni.cz/~xslaby/stanse/error.cgi?db=34-rc&id=578 > > It is introduced by 3b377ea9d4efc94dc52fe41b4dfdb463635ab298. > > Do you plan to extend it somehow or may the spinlock be converted to mutex? I don't think you can convert this directly to a mutex. The spin_lock in question (®_regdb_search_lock) gets also used by reg_regdb_query() which in turn gets called by call_crda(). There is one iteration of call_crda() which happens during module initialization and from what I gather I don't think the kernel is happy when you mutex_lock on load routines, please correct my foggy memory if I am mistaken. So during module load we directly end up hitting the spin_lock in question, prior to the workqueue using it. Not sure yet how to address this, any ideas John? > If not how much may size_of_regd be -- can we safely switch to GFP_ATOMIC? The count is up to 117 right now. It should be around the number of countries of the world today, tomorrow even different countries could have different regulatory domains. This happened with Japan a while back when they required manufacturers to apply certification rules depending on the year the product went out.. and this changed many times in that country. But another reason why this number may also increase in any given country without regards to silly legislation is for experimenters who want to enhance the use of the spectrum in certain areas and would use sha1sums instead of alpha2s for the regdomains. But we're light years away from that happening. Luis ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: regd: sleeping in atomic 2010-03-16 16:18 ` Luis R. Rodriguez @ 2010-03-16 18:42 ` Jiri Slaby 2010-03-16 20:36 ` Luis R. Rodriguez 0 siblings, 1 reply; 15+ messages in thread From: Jiri Slaby @ 2010-03-16 18:42 UTC (permalink / raw) To: Luis R. Rodriguez; +Cc: linux-wireless@vger.kernel.org, John W. Linville, LKML On 03/16/2010 05:18 PM, Luis R. Rodriguez wrote: > one iteration of call_crda() which happens during module > initialization and from what I gather I don't think the kernel is > happy when you mutex_lock on load routines, please correct my foggy > memory if I am mistaken. No, using mutex in init/exit module routines is fine. Where do you have the information from? There might be problems only with request_module, flush_scheduled_work or similar. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: regd: sleeping in atomic 2010-03-16 18:42 ` Jiri Slaby @ 2010-03-16 20:36 ` Luis R. Rodriguez 0 siblings, 0 replies; 15+ messages in thread From: Luis R. Rodriguez @ 2010-03-16 20:36 UTC (permalink / raw) To: Jiri Slaby; +Cc: linux-wireless@vger.kernel.org, John W. Linville, LKML On Tue, Mar 16, 2010 at 11:42 AM, Jiri Slaby <jirislaby@gmail.com> wrote: > On 03/16/2010 05:18 PM, Luis R. Rodriguez wrote: >> >> one iteration of call_crda() which happens during module >> initialization and from what I gather I don't think the kernel is >> happy when you mutex_lock on load routines, please correct my foggy >> memory if I am mistaken. > > No, using mutex in init/exit module routines is fine. Where do you have the > information from? A snag I hit when trying this a while ago on reg.c. I suppose the real issue could have been something else. > There might be problems only with request_module, > flush_scheduled_work or similar. OK -- then in that case I think using a mutex would work here, unless John spots any other issues. Luis ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] wireless: convert reg_regdb_search_lock to mutex 2010-03-16 13:26 regd: sleeping in atomic Jiri Slaby 2010-03-16 16:18 ` Luis R. Rodriguez @ 2010-03-16 19:49 ` John W. Linville 2010-03-16 10:03 ` Senthil Balasubramanian 2010-03-17 19:33 ` [PATCH v2] " John W. Linville 1 sibling, 2 replies; 15+ messages in thread From: John W. Linville @ 2010-03-16 19:49 UTC (permalink / raw) To: linux-wireless; +Cc: Jiri Slaby, Luis R. Rodriguez, John W. Linville Stanse discovered that kmalloc can be called with GFP_KERNEL while holding this spinlock. It can be a mutex instead. Reported-by: Jiri Slaby <jirislaby@gmail.com> Signed-off-by: John W. Linville <linville@tuxdriver.com> --- Hmmm...I think I can get rid of the unlock of reg_regdb_search_mutex before calling set_regdom now as well? net/wireless/reg.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/net/wireless/reg.c b/net/wireless/reg.c index ed89c59..5f623ed 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -324,7 +324,7 @@ struct reg_regdb_search_request { }; static LIST_HEAD(reg_regdb_search_list); -static DEFINE_SPINLOCK(reg_regdb_search_lock); +static DEFINE_MUTEX(reg_regdb_search_mutex); static void reg_regdb_search(struct work_struct *work) { @@ -332,7 +332,7 @@ static void reg_regdb_search(struct work_struct *work) const struct ieee80211_regdomain *curdom, *regdom; int i, r; - spin_lock(®_regdb_search_lock); + mutex_lock(®_regdb_search_mutex); while (!list_empty(®_regdb_search_list)) { request = list_first_entry(®_regdb_search_list, struct reg_regdb_search_request, @@ -346,18 +346,18 @@ static void reg_regdb_search(struct work_struct *work) r = reg_copy_regd(®dom, curdom); if (r) break; - spin_unlock(®_regdb_search_lock); + mutex_unlock(®_regdb_search_mutex); mutex_lock(&cfg80211_mutex); set_regdom(regdom); mutex_unlock(&cfg80211_mutex); - spin_lock(®_regdb_search_lock); + mutex_lock(®_regdb_search_mutex); break; } } kfree(request); } - spin_unlock(®_regdb_search_lock); + mutex_unlock(®_regdb_search_mutex); } static DECLARE_WORK(reg_regdb_work, reg_regdb_search); -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] wireless: convert reg_regdb_search_lock to mutex 2010-03-16 19:49 ` [PATCH] wireless: convert reg_regdb_search_lock to mutex John W. Linville @ 2010-03-16 10:03 ` Senthil Balasubramanian 2010-03-17 0:24 ` John W. Linville 2010-03-17 19:33 ` [PATCH v2] " John W. Linville 1 sibling, 1 reply; 15+ messages in thread From: Senthil Balasubramanian @ 2010-03-16 10:03 UTC (permalink / raw) To: John W. Linville Cc: linux-wireless@vger.kernel.org, Jiri Slaby, Luis R. Rodriguez On Wed, Mar 17, 2010 at 01:19:00AM +0530, John W. Linville wrote: > Stanse discovered that kmalloc can be called with GFP_KERNEL while This commit log is confusing. It Should be "Stanse discovered kmalloc was called with GFP_KERNEL". Obviously kmalloc with GFP_KERNEL shouldn't be used while holding a spinlock. > holding this spinlock. It can be a mutex instead. > > Reported-by: Jiri Slaby <jirislaby@gmail.com> > Signed-off-by: John W. Linville <linville@tuxdriver.com> > --- > Hmmm...I think I can get rid of the unlock of reg_regdb_search_mutex > before calling set_regdom now as well? > > net/wireless/reg.c | 10 +++++----- > 1 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/net/wireless/reg.c b/net/wireless/reg.c > index ed89c59..5f623ed 100644 > --- a/net/wireless/reg.c > +++ b/net/wireless/reg.c > @@ -324,7 +324,7 @@ struct reg_regdb_search_request { > }; > > static LIST_HEAD(reg_regdb_search_list); > -static DEFINE_SPINLOCK(reg_regdb_search_lock); > +static DEFINE_MUTEX(reg_regdb_search_mutex); > > static void reg_regdb_search(struct work_struct *work) > { > @@ -332,7 +332,7 @@ static void reg_regdb_search(struct work_struct *work) > const struct ieee80211_regdomain *curdom, *regdom; > int i, r; > > - spin_lock(®_regdb_search_lock); > + mutex_lock(®_regdb_search_mutex); > while (!list_empty(®_regdb_search_list)) { > request = list_first_entry(®_regdb_search_list, > struct reg_regdb_search_request, > @@ -346,18 +346,18 @@ static void reg_regdb_search(struct work_struct *work) > r = reg_copy_regd(®dom, curdom); > if (r) > break; > - spin_unlock(®_regdb_search_lock); > + mutex_unlock(®_regdb_search_mutex); > mutex_lock(&cfg80211_mutex); > set_regdom(regdom); > mutex_unlock(&cfg80211_mutex); > - spin_lock(®_regdb_search_lock); > + mutex_lock(®_regdb_search_mutex); > break; > } > } > > kfree(request); > } > - spin_unlock(®_regdb_search_lock); > + mutex_unlock(®_regdb_search_mutex); > } > > static DECLARE_WORK(reg_regdb_work, reg_regdb_search); > -- > 1.6.2.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wireless: convert reg_regdb_search_lock to mutex 2010-03-16 10:03 ` Senthil Balasubramanian @ 2010-03-17 0:24 ` John W. Linville 2010-03-16 12:24 ` Senthil Balasubramanian 0 siblings, 1 reply; 15+ messages in thread From: John W. Linville @ 2010-03-17 0:24 UTC (permalink / raw) To: Senthil Balasubramanian Cc: linux-wireless@vger.kernel.org, Jiri Slaby, Luis R. Rodriguez On Tue, Mar 16, 2010 at 03:33:38PM +0530, Senthil Balasubramanian wrote: > On Wed, Mar 17, 2010 at 01:19:00AM +0530, John W. Linville wrote: > > Stanse discovered that kmalloc can be called with GFP_KERNEL while > This commit log is confusing. It Should be "Stanse discovered kmalloc > was called with GFP_KERNEL". Obviously kmalloc with GFP_KERNEL shouldn't > be used while holding a spinlock. > > holding this spinlock. It can be a mutex instead. Not half so confusing as your criticism... :-) Are you objecting to "can be" instead of "was"? John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wireless: convert reg_regdb_search_lock to mutex 2010-03-17 0:24 ` John W. Linville @ 2010-03-16 12:24 ` Senthil Balasubramanian 2010-03-17 2:56 ` John W. Linville 0 siblings, 1 reply; 15+ messages in thread From: Senthil Balasubramanian @ 2010-03-16 12:24 UTC (permalink / raw) To: John W. Linville Cc: Senthilkumar Balasubramanian, linux-wireless@vger.kernel.org, Jiri Slaby, Luis R. Rodriguez On Wed, Mar 17, 2010 at 05:54:16AM +0530, John W. Linville wrote: > On Tue, Mar 16, 2010 at 03:33:38PM +0530, Senthil Balasubramanian wrote: > > On Wed, Mar 17, 2010 at 01:19:00AM +0530, John W. Linville wrote: > > > Stanse discovered that kmalloc can be called with GFP_KERNEL while > > This commit log is confusing. It Should be "Stanse discovered kmalloc > > was called with GFP_KERNEL". Obviously kmalloc with GFP_KERNEL shouldn't > > be used while holding a spinlock. > > > holding this spinlock. It can be a mutex instead. > > Not half so confusing as your criticism... :-) sorry! if my mail wasn't proper. I didn't meant to blame/criticize. I was confused when I read the commit log and so I replied. > > Are you objecting to "can be" instead of "was"? I am not objecting anything... > > John > -- > John W. Linville Someday the world will need a hero, and you > linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wireless: convert reg_regdb_search_lock to mutex 2010-03-16 12:24 ` Senthil Balasubramanian @ 2010-03-17 2:56 ` John W. Linville 2010-03-17 3:24 ` Johannes Berg 0 siblings, 1 reply; 15+ messages in thread From: John W. Linville @ 2010-03-17 2:56 UTC (permalink / raw) To: Senthil Balasubramanian Cc: Senthilkumar Balasubramanian, linux-wireless@vger.kernel.org, Jiri Slaby, Luis R. Rodriguez On Tue, Mar 16, 2010 at 05:54:50PM +0530, Senthil Balasubramanian wrote: > On Wed, Mar 17, 2010 at 05:54:16AM +0530, John W. Linville wrote: > > On Tue, Mar 16, 2010 at 03:33:38PM +0530, Senthil Balasubramanian wrote: > > > On Wed, Mar 17, 2010 at 01:19:00AM +0530, John W. Linville wrote: > > > > Stanse discovered that kmalloc can be called with GFP_KERNEL while > > > This commit log is confusing. It Should be "Stanse discovered kmalloc > > > was called with GFP_KERNEL". Obviously kmalloc with GFP_KERNEL shouldn't > > > be used while holding a spinlock. > > > > holding this spinlock. It can be a mutex instead. > > > > Not half so confusing as your criticism... :-) > sorry! if my mail wasn't proper. I didn't meant to blame/criticize. > I was confused when I read the commit log and so I replied. > > > > Are you objecting to "can be" instead of "was"? > I am not objecting anything... Criticism is fine -- I just don't understand what you were saying or what you think I should have said in the commit log. John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wireless: convert reg_regdb_search_lock to mutex 2010-03-17 2:56 ` John W. Linville @ 2010-03-17 3:24 ` Johannes Berg 2010-03-17 1:26 ` Senthil Balasubramanian 2010-03-17 12:55 ` John W. Linville 0 siblings, 2 replies; 15+ messages in thread From: Johannes Berg @ 2010-03-17 3:24 UTC (permalink / raw) To: John W. Linville Cc: Senthil Balasubramanian, Senthilkumar Balasubramanian, linux-wireless@vger.kernel.org, Jiri Slaby, Luis R. Rodriguez On Tue, 2010-03-16 at 22:56 -0400, John W. Linville wrote: > On Tue, Mar 16, 2010 at 05:54:50PM +0530, Senthil Balasubramanian wrote: > > On Wed, Mar 17, 2010 at 05:54:16AM +0530, John W. Linville wrote: > > > On Tue, Mar 16, 2010 at 03:33:38PM +0530, Senthil Balasubramanian wrote: > > > > On Wed, Mar 17, 2010 at 01:19:00AM +0530, John W. Linville wrote: > > > > > Stanse discovered that kmalloc can be called with GFP_KERNEL while > > > > This commit log is confusing. It Should be "Stanse discovered kmalloc > > > > was called with GFP_KERNEL". Obviously kmalloc with GFP_KERNEL shouldn't > > > > be used while holding a spinlock. > > > > > holding this spinlock. It can be a mutex instead. > > > > > > Not half so confusing as your criticism... :-) > > sorry! if my mail wasn't proper. I didn't meant to blame/criticize. > > I was confused when I read the commit log and so I replied. > > > > > > Are you objecting to "can be" instead of "was"? > > I am not objecting anything... > > Criticism is fine -- I just don't understand what you were saying or > what you think I should have said in the commit log. English at work ... let me guess: For John: "can" == "it could happen that" For Senthil: "can" == "may" (when really you may NOT do GFP_KERNEL allocations in atomic context) johannes ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wireless: convert reg_regdb_search_lock to mutex 2010-03-17 3:24 ` Johannes Berg @ 2010-03-17 1:26 ` Senthil Balasubramanian 2010-03-17 12:55 ` John W. Linville 1 sibling, 0 replies; 15+ messages in thread From: Senthil Balasubramanian @ 2010-03-17 1:26 UTC (permalink / raw) To: Johannes Berg Cc: John W. Linville, Senthilkumar Balasubramanian, linux-wireless@vger.kernel.org, Jiri Slaby, Luis R. Rodriguez On Wed, Mar 17, 2010 at 08:54:35AM +0530, Johannes Berg wrote: > On Tue, 2010-03-16 at 22:56 -0400, John W. Linville wrote: > > On Tue, Mar 16, 2010 at 05:54:50PM +0530, Senthil Balasubramanian wrote: > > > On Wed, Mar 17, 2010 at 05:54:16AM +0530, John W. Linville wrote: > > > > On Tue, Mar 16, 2010 at 03:33:38PM +0530, Senthil Balasubramanian wrote: > > > > > On Wed, Mar 17, 2010 at 01:19:00AM +0530, John W. Linville wrote: > > > > > > Stanse discovered that kmalloc can be called with GFP_KERNEL while > > > > > This commit log is confusing. It Should be "Stanse discovered kmalloc > > > > > was called with GFP_KERNEL". Obviously kmalloc with GFP_KERNEL shouldn't > > > > > be used while holding a spinlock. > > > > > > holding this spinlock. It can be a mutex instead. > > > > > > > > Not half so confusing as your criticism... :-) > > > sorry! if my mail wasn't proper. I didn't meant to blame/criticize. > > > I was confused when I read the commit log and so I replied. > > > > > > > > Are you objecting to "can be" instead of "was"? > > > I am not objecting anything... > > > > Criticism is fine -- I just don't understand what you were saying or > > what you think I should have said in the commit log. > > English at work ... let me guess: Yes. I basically misread the commit log and understood like GFP_KENREL is allowed while holding a spinlock. So I replied. Sorry! for the unnecessary noise.. here... Please ignore my comment. > > For John: "can" == "it could happen that" > For Senthil: "can" == "may" > > (when really you may NOT do GFP_KERNEL allocations in atomic context) > > johannes > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wireless: convert reg_regdb_search_lock to mutex 2010-03-17 3:24 ` Johannes Berg 2010-03-17 1:26 ` Senthil Balasubramanian @ 2010-03-17 12:55 ` John W. Linville 2010-03-17 13:01 ` John W. Linville 1 sibling, 1 reply; 15+ messages in thread From: John W. Linville @ 2010-03-17 12:55 UTC (permalink / raw) To: Johannes Berg Cc: Senthil Balasubramanian, Senthilkumar Balasubramanian, linux-wireless@vger.kernel.org, Jiri Slaby, Luis R. Rodriguez On Tue, Mar 16, 2010 at 08:24:35PM -0700, Johannes Berg wrote: > On Tue, 2010-03-16 at 22:56 -0400, John W. Linville wrote: > > On Tue, Mar 16, 2010 at 05:54:50PM +0530, Senthil Balasubramanian wrote: > > > On Wed, Mar 17, 2010 at 05:54:16AM +0530, John W. Linville wrote: > > > > On Tue, Mar 16, 2010 at 03:33:38PM +0530, Senthil Balasubramanian wrote: > > > > > On Wed, Mar 17, 2010 at 01:19:00AM +0530, John W. Linville wrote: > > > > > > Stanse discovered that kmalloc can be called with GFP_KERNEL while > > > > > This commit log is confusing. It Should be "Stanse discovered kmalloc > > > > > was called with GFP_KERNEL". Obviously kmalloc with GFP_KERNEL shouldn't > > > > > be used while holding a spinlock. > > > > > > holding this spinlock. It can be a mutex instead. > > > > > > > > Not half so confusing as your criticism... :-) > > > sorry! if my mail wasn't proper. I didn't meant to blame/criticize. > > > I was confused when I read the commit log and so I replied. > > > > > > > > Are you objecting to "can be" instead of "was"? > > > I am not objecting anything... > > > > Criticism is fine -- I just don't understand what you were saying or > > what you think I should have said in the commit log. > > English at work ... let me guess: > > For John: "can" == "it could happen that" > For Senthil: "can" == "may" > > (when really you may NOT do GFP_KERNEL allocations in atomic context) Ah, that seems like a plausible explanation for the confusion. Bitte mein herr! John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wireless: convert reg_regdb_search_lock to mutex 2010-03-17 12:55 ` John W. Linville @ 2010-03-17 13:01 ` John W. Linville 0 siblings, 0 replies; 15+ messages in thread From: John W. Linville @ 2010-03-17 13:01 UTC (permalink / raw) To: Johannes Berg Cc: Senthil Balasubramanian, Senthilkumar Balasubramanian, linux-wireless@vger.kernel.org, Jiri Slaby, Luis R. Rodriguez On Wed, Mar 17, 2010 at 08:55:13AM -0400, John W. Linville wrote: > On Tue, Mar 16, 2010 at 08:24:35PM -0700, Johannes Berg wrote: > > English at work ... let me guess: > > > > For John: "can" == "it could happen that" > > For Senthil: "can" == "may" > > > > (when really you may NOT do GFP_KERNEL allocations in atomic context) > > Ah, that seems like a plausible explanation for the confusion. > Bitte mein herr! Err...danke mein herr! :-) John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] wireless: convert reg_regdb_search_lock to mutex 2010-03-16 19:49 ` [PATCH] wireless: convert reg_regdb_search_lock to mutex John W. Linville 2010-03-16 10:03 ` Senthil Balasubramanian @ 2010-03-17 19:33 ` John W. Linville 2010-03-18 20:47 ` [PATCH v3] " John W. Linville 1 sibling, 1 reply; 15+ messages in thread From: John W. Linville @ 2010-03-17 19:33 UTC (permalink / raw) To: linux-wireless; +Cc: Jiri Slaby, Luis R. Rodriguez, John W. Linville Stanse discovered that kmalloc is being called with GFP_KERNEL while holding this spinlock. The spinlock can be a mutex instead, which also enables the removal of the unlock/lock around the lock/unlock of cfg80211_mutex and the call to set_regdom. Reported-by: Jiri Slaby <jirislaby@gmail.com> Signed-off-by: John W. Linville <linville@tuxdriver.com> --- net/wireless/reg.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-) diff --git a/net/wireless/reg.c b/net/wireless/reg.c index ed89c59..1ef1639 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -324,7 +324,7 @@ struct reg_regdb_search_request { }; static LIST_HEAD(reg_regdb_search_list); -static DEFINE_SPINLOCK(reg_regdb_search_lock); +static DEFINE_MUTEX(reg_regdb_search_mutex); static void reg_regdb_search(struct work_struct *work) { @@ -332,7 +332,7 @@ static void reg_regdb_search(struct work_struct *work) const struct ieee80211_regdomain *curdom, *regdom; int i, r; - spin_lock(®_regdb_search_lock); + mutex_lock(®_regdb_search_mutex); while (!list_empty(®_regdb_search_list)) { request = list_first_entry(®_regdb_search_list, struct reg_regdb_search_request, @@ -346,18 +346,16 @@ static void reg_regdb_search(struct work_struct *work) r = reg_copy_regd(®dom, curdom); if (r) break; - spin_unlock(®_regdb_search_lock); mutex_lock(&cfg80211_mutex); set_regdom(regdom); mutex_unlock(&cfg80211_mutex); - spin_lock(®_regdb_search_lock); break; } } kfree(request); } - spin_unlock(®_regdb_search_lock); + mutex_unlock(®_regdb_search_mutex); } static DECLARE_WORK(reg_regdb_work, reg_regdb_search); -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3] wireless: convert reg_regdb_search_lock to mutex 2010-03-17 19:33 ` [PATCH v2] " John W. Linville @ 2010-03-18 20:47 ` John W. Linville 0 siblings, 0 replies; 15+ messages in thread From: John W. Linville @ 2010-03-18 20:47 UTC (permalink / raw) To: linux-wireless; +Cc: Jiri Slaby, Luis R. Rodriguez, John W. Linville Stanse discovered that kmalloc is being called with GFP_KERNEL while holding this spinlock. The spinlock can be a mutex instead, which also enables the removal of the unlock/lock around the lock/unlock of cfg80211_mutex and the call to set_regdom. Reported-by: Jiri Slaby <jirislaby@gmail.com> Signed-off-by: John W. Linville <linville@tuxdriver.com> --- Missed a hunk... net/wireless/reg.c | 12 +++++------- 1 files changed, 5 insertions(+), 7 deletions(-) diff --git a/net/wireless/reg.c b/net/wireless/reg.c index ed89c59..81fcafc 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -324,7 +324,7 @@ struct reg_regdb_search_request { }; static LIST_HEAD(reg_regdb_search_list); -static DEFINE_SPINLOCK(reg_regdb_search_lock); +static DEFINE_MUTEX(reg_regdb_search_mutex); static void reg_regdb_search(struct work_struct *work) { @@ -332,7 +332,7 @@ static void reg_regdb_search(struct work_struct *work) const struct ieee80211_regdomain *curdom, *regdom; int i, r; - spin_lock(®_regdb_search_lock); + mutex_lock(®_regdb_search_mutex); while (!list_empty(®_regdb_search_list)) { request = list_first_entry(®_regdb_search_list, struct reg_regdb_search_request, @@ -346,18 +346,16 @@ static void reg_regdb_search(struct work_struct *work) r = reg_copy_regd(®dom, curdom); if (r) break; - spin_unlock(®_regdb_search_lock); mutex_lock(&cfg80211_mutex); set_regdom(regdom); mutex_unlock(&cfg80211_mutex); - spin_lock(®_regdb_search_lock); break; } } kfree(request); } - spin_unlock(®_regdb_search_lock); + mutex_unlock(®_regdb_search_mutex); } static DECLARE_WORK(reg_regdb_work, reg_regdb_search); @@ -375,9 +373,9 @@ static void reg_regdb_query(const char *alpha2) memcpy(request->alpha2, alpha2, 2); - spin_lock(®_regdb_search_lock); + mutex_lock(®_regdb_search_mutex); list_add_tail(&request->list, ®_regdb_search_list); - spin_unlock(®_regdb_search_lock); + mutex_unlock(®_regdb_search_mutex); schedule_work(®_regdb_work); } -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2010-03-18 21:00 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-03-16 13:26 regd: sleeping in atomic Jiri Slaby 2010-03-16 16:18 ` Luis R. Rodriguez 2010-03-16 18:42 ` Jiri Slaby 2010-03-16 20:36 ` Luis R. Rodriguez 2010-03-16 19:49 ` [PATCH] wireless: convert reg_regdb_search_lock to mutex John W. Linville 2010-03-16 10:03 ` Senthil Balasubramanian 2010-03-17 0:24 ` John W. Linville 2010-03-16 12:24 ` Senthil Balasubramanian 2010-03-17 2:56 ` John W. Linville 2010-03-17 3:24 ` Johannes Berg 2010-03-17 1:26 ` Senthil Balasubramanian 2010-03-17 12:55 ` John W. Linville 2010-03-17 13:01 ` John W. Linville 2010-03-17 19:33 ` [PATCH v2] " John W. Linville 2010-03-18 20:47 ` [PATCH v3] " John W. Linville
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).