| Message ID | 1420642040-4530-2-git-send-email-arik@wizery.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
Hi Arik, On Thu, Jan 8, 2015 at 1:47 AM, Arik Nemtsov <arik@wizery.com> wrote: > When a system contains only self-managed regulatory devices all hints > from the regulatory core are ignored. Stop hint processing early in this > case. These systems usually don't have CRDA deployed, which results in > endless (irrelevent) logs of the form: > cfg80211: Calling CRDA to update world regulatory domain > > Make sure there's at least one self-managed device before discarding a > hint, in order to prevent initial hints from disappearing on CRDA > managed systems. > > Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com> > --- > net/wireless/reg.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/net/wireless/reg.c b/net/wireless/reg.c > index 521f3a4..588e45f 100644 > --- a/net/wireless/reg.c > +++ b/net/wireless/reg.c > @@ -2120,6 +2120,26 @@ out_free: > reg_free_request(reg_request); > } > > +static bool reg_only_self_managed_wiphys(void) > +{ > + struct cfg80211_registered_device *rdev; > + struct wiphy *wiphy; > + bool self_managed_found = false; > + > + ASSERT_RTNL(); Would it make sense to quickly return false here if the list is empty rather than the whole mess with the new variable? > + > + list_for_each_entry(rdev, &cfg80211_rdev_list, list) { > + wiphy = &rdev->wiphy; > + if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) > + self_managed_found = true; > + else > + return false; > + } > + > + /* make sure at least one self-managed wiphy exists */ > + return self_managed_found; > +} > + > /* > * Processes regulatory hints, this is all the NL80211_REGDOM_SET_BY_* > * Regulatory hints come on a first come first serve basis and we Thanks,
On Tue, Jan 13, 2015 at 6:26 AM, Julian Calaby <julian.calaby@gmail.com> wrote: > Hi Arik, > > On Thu, Jan 8, 2015 at 1:47 AM, Arik Nemtsov <arik@wizery.com> wrote: >> When a system contains only self-managed regulatory devices all hints >> from the regulatory core are ignored. Stop hint processing early in this >> case. These systems usually don't have CRDA deployed, which results in >> endless (irrelevent) logs of the form: >> cfg80211: Calling CRDA to update world regulatory domain >> >> Make sure there's at least one self-managed device before discarding a >> hint, in order to prevent initial hints from disappearing on CRDA >> managed systems. >> >> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com> >> --- >> net/wireless/reg.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/net/wireless/reg.c b/net/wireless/reg.c >> index 521f3a4..588e45f 100644 >> --- a/net/wireless/reg.c >> +++ b/net/wireless/reg.c >> @@ -2120,6 +2120,26 @@ out_free: >> reg_free_request(reg_request); >> } >> >> +static bool reg_only_self_managed_wiphys(void) >> +{ >> + struct cfg80211_registered_device *rdev; >> + struct wiphy *wiphy; >> + bool self_managed_found = false; >> + >> + ASSERT_RTNL(); > > Would it make sense to quickly return false here if the list is empty > rather than the whole mess with the new variable? I'm thinking the "mess" isn't really such a mess - are you expecting a real performance hit? Also this is a corner case - you won't really get regulatory updates when no network cards are presents. You only get a single update to the core. Arik -- 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
Hi Arik, On Tue, Jan 13, 2015 at 9:11 PM, Arik Nemtsov <arik@wizery.com> wrote: > On Tue, Jan 13, 2015 at 6:26 AM, Julian Calaby <julian.calaby@gmail.com> wrote: >> Hi Arik, >> >> On Thu, Jan 8, 2015 at 1:47 AM, Arik Nemtsov <arik@wizery.com> wrote: >>> When a system contains only self-managed regulatory devices all hints >>> from the regulatory core are ignored. Stop hint processing early in this >>> case. These systems usually don't have CRDA deployed, which results in >>> endless (irrelevent) logs of the form: >>> cfg80211: Calling CRDA to update world regulatory domain >>> >>> Make sure there's at least one self-managed device before discarding a >>> hint, in order to prevent initial hints from disappearing on CRDA >>> managed systems. >>> >>> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com> >>> --- >>> net/wireless/reg.c | 25 +++++++++++++++++++++++++ >>> 1 file changed, 25 insertions(+) >>> >>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c >>> index 521f3a4..588e45f 100644 >>> --- a/net/wireless/reg.c >>> +++ b/net/wireless/reg.c >>> @@ -2120,6 +2120,26 @@ out_free: >>> reg_free_request(reg_request); >>> } >>> >>> +static bool reg_only_self_managed_wiphys(void) >>> +{ >>> + struct cfg80211_registered_device *rdev; >>> + struct wiphy *wiphy; >>> + bool self_managed_found = false; >>> + >>> + ASSERT_RTNL(); >> >> Would it make sense to quickly return false here if the list is empty >> rather than the whole mess with the new variable? > > I'm thinking the "mess" isn't really such a mess - are you expecting a > real performance hit? Personally, purely for readability, I prefer the style of checking and returning as early as possible. Arguably doing it my way trades speed for memory efficiency and your way trades memory efficiency for speed, but it's so small (and this is so far from a hot path) that there's no real argument either way whatsoever. > Also this is a corner case - you won't really get regulatory updates > when no network cards are presents. You only get a single update to > the core. Very true. Thanks,
On Tue, Jan 13, 2015 at 12:30 PM, Julian Calaby <julian.calaby@gmail.com> wrote: > Hi Arik, > > On Tue, Jan 13, 2015 at 9:11 PM, Arik Nemtsov <arik@wizery.com> wrote: >> On Tue, Jan 13, 2015 at 6:26 AM, Julian Calaby <julian.calaby@gmail.com> wrote: >>> Hi Arik, >>> >>> On Thu, Jan 8, 2015 at 1:47 AM, Arik Nemtsov <arik@wizery.com> wrote: >>>> When a system contains only self-managed regulatory devices all hints >>>> from the regulatory core are ignored. Stop hint processing early in this >>>> case. These systems usually don't have CRDA deployed, which results in >>>> endless (irrelevent) logs of the form: >>>> cfg80211: Calling CRDA to update world regulatory domain >>>> >>>> Make sure there's at least one self-managed device before discarding a >>>> hint, in order to prevent initial hints from disappearing on CRDA >>>> managed systems. >>>> >>>> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com> >>>> --- >>>> net/wireless/reg.c | 25 +++++++++++++++++++++++++ >>>> 1 file changed, 25 insertions(+) >>>> >>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c >>>> index 521f3a4..588e45f 100644 >>>> --- a/net/wireless/reg.c >>>> +++ b/net/wireless/reg.c >>>> @@ -2120,6 +2120,26 @@ out_free: >>>> reg_free_request(reg_request); >>>> } >>>> >>>> +static bool reg_only_self_managed_wiphys(void) >>>> +{ >>>> + struct cfg80211_registered_device *rdev; >>>> + struct wiphy *wiphy; >>>> + bool self_managed_found = false; >>>> + >>>> + ASSERT_RTNL(); >>> >>> Would it make sense to quickly return false here if the list is empty >>> rather than the whole mess with the new variable? >> >> I'm thinking the "mess" isn't really such a mess - are you expecting a >> real performance hit? > > Personally, purely for readability, I prefer the style of checking and > returning as early as possible. Actually I thought readability was on my side - wanted to point out the shorter version of the code is more legible. I guess it's a matter of personal taste :) > > Arguably doing it my way trades speed for memory efficiency and your > way trades memory efficiency for speed, but it's so small (and this is > so far from a hot path) that there's no real argument either way > whatsoever. Agree it doesn't matter for these reasons. Arik -- 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
Hi Arik, On Tue, Jan 13, 2015 at 9:35 PM, Arik Nemtsov <arik@wizery.com> wrote: > On Tue, Jan 13, 2015 at 12:30 PM, Julian Calaby <julian.calaby@gmail.com> wrote: >> Hi Arik, >> >> On Tue, Jan 13, 2015 at 9:11 PM, Arik Nemtsov <arik@wizery.com> wrote: >>> On Tue, Jan 13, 2015 at 6:26 AM, Julian Calaby <julian.calaby@gmail.com> wrote: >>>> Hi Arik, >>>> >>>> On Thu, Jan 8, 2015 at 1:47 AM, Arik Nemtsov <arik@wizery.com> wrote: >>>>> When a system contains only self-managed regulatory devices all hints >>>>> from the regulatory core are ignored. Stop hint processing early in this >>>>> case. These systems usually don't have CRDA deployed, which results in >>>>> endless (irrelevent) logs of the form: >>>>> cfg80211: Calling CRDA to update world regulatory domain >>>>> >>>>> Make sure there's at least one self-managed device before discarding a >>>>> hint, in order to prevent initial hints from disappearing on CRDA >>>>> managed systems. >>>>> >>>>> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com> >>>>> --- >>>>> net/wireless/reg.c | 25 +++++++++++++++++++++++++ >>>>> 1 file changed, 25 insertions(+) >>>>> >>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c >>>>> index 521f3a4..588e45f 100644 >>>>> --- a/net/wireless/reg.c >>>>> +++ b/net/wireless/reg.c >>>>> @@ -2120,6 +2120,26 @@ out_free: >>>>> reg_free_request(reg_request); >>>>> } >>>>> >>>>> +static bool reg_only_self_managed_wiphys(void) >>>>> +{ >>>>> + struct cfg80211_registered_device *rdev; >>>>> + struct wiphy *wiphy; >>>>> + bool self_managed_found = false; >>>>> + >>>>> + ASSERT_RTNL(); >>>> >>>> Would it make sense to quickly return false here if the list is empty >>>> rather than the whole mess with the new variable? >>> >>> I'm thinking the "mess" isn't really such a mess - are you expecting a >>> real performance hit? >> >> Personally, purely for readability, I prefer the style of checking and >> returning as early as possible. > > Actually I thought readability was on my side - wanted to point out > the shorter version of the code is more legible. > I guess it's a matter of personal taste :) Indeed it is. In terms of length, your version is a variable definition and two lines in the internal if statement. Mine is a blank line and two lines in a separate if statement. (Assuming the condition doesn't run over 80 chars) >> Arguably doing it my way trades speed for memory efficiency and your >> way trades memory efficiency for speed, but it's so small (and this is >> so far from a hot path) that there's no real argument either way >> whatsoever. > > Agree it doesn't matter for these reasons. At the end of the day, it's a stalemate for a lot of reasons. You prefer this way, it's your code, so leave it as it is. I apologise for the noise =) Thanks,
diff --git a/net/wireless/reg.c b/net/wireless/reg.c index 521f3a4..588e45f 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -2120,6 +2120,26 @@ out_free: reg_free_request(reg_request); } +static bool reg_only_self_managed_wiphys(void) +{ + struct cfg80211_registered_device *rdev; + struct wiphy *wiphy; + bool self_managed_found = false; + + ASSERT_RTNL(); + + list_for_each_entry(rdev, &cfg80211_rdev_list, list) { + wiphy = &rdev->wiphy; + if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) + self_managed_found = true; + else + return false; + } + + /* make sure at least one self-managed wiphy exists */ + return self_managed_found; +} + /* * Processes regulatory hints, this is all the NL80211_REGDOM_SET_BY_* * Regulatory hints come on a first come first serve basis and we @@ -2151,6 +2171,11 @@ static void reg_process_pending_hints(void) spin_unlock(®_requests_lock); + if (reg_only_self_managed_wiphys()) { + reg_free_request(reg_request); + return; + } + reg_process_hint(reg_request); }
When a system contains only self-managed regulatory devices all hints from the regulatory core are ignored. Stop hint processing early in this case. These systems usually don't have CRDA deployed, which results in endless (irrelevent) logs of the form: cfg80211: Calling CRDA to update world regulatory domain Make sure there's at least one self-managed device before discarding a hint, in order to prevent initial hints from disappearing on CRDA managed systems. Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com> --- net/wireless/reg.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)