* [RFC 1/2] mac80211: iterate over vif using RCU @ 2013-12-10 16:39 Antonio Quartulli 2013-12-10 16:39 ` [RFC 2/2] mac80211: Use RCU to handle local->key_list Antonio Quartulli 2013-12-10 17:05 ` [RFC 1/2] mac80211: iterate over vif using RCU Eliad Peller 0 siblings, 2 replies; 6+ messages in thread From: Antonio Quartulli @ 2013-12-10 16:39 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, Antonio Quartulli From: Antonio Quartulli <antonio@open-mesh.com> I need to invoke ieee80211_iter_keys() from a periodic worker in a driver and therefore I would prefer to get rid of any of locks to avoid problems. These two patches try to use rcu lock to protect the iteration, but I'd like to get a feedback before sending this stuff as a patch :-) Moreover, why do we use list_for_each_entry_safe() is ieee80211_iter_keys() if the list cannot be altered (pointer to key is not passed to iter() so we should be sure that nobody is going to invoke list_del())? Cheers, In ieee80211_iter_keys the local->interfaces list is accessed for reading only. RCU can be used instead of pretending to be under RTNL lock. This can simplify future users of this function. Signed-off-by: Antonio Quartulli <antonio@open-mesh.com> --- net/mac80211/key.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/mac80211/key.c b/net/mac80211/key.c index 3e51dd7..04c885a 100644 --- a/net/mac80211/key.c +++ b/net/mac80211/key.c @@ -550,8 +550,6 @@ void ieee80211_iter_keys(struct ieee80211_hw *hw, struct ieee80211_key *key, *tmp; struct ieee80211_sub_if_data *sdata; - ASSERT_RTNL(); - mutex_lock(&local->key_mtx); if (vif) { sdata = vif_to_sdata(vif); @@ -560,12 +558,14 @@ void ieee80211_iter_keys(struct ieee80211_hw *hw, key->sta ? &key->sta->sta : NULL, &key->conf, iter_data); } else { - list_for_each_entry(sdata, &local->interfaces, list) + rcu_read_lock(); + list_for_each_entry_rcu(sdata, &local->interfaces, list) list_for_each_entry_safe(key, tmp, &sdata->key_list, list) iter(hw, &sdata->vif, key->sta ? &key->sta->sta : NULL, &key->conf, iter_data); + rcu_read_unlock(); } mutex_unlock(&local->key_mtx); } -- 1.8.5.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC 2/2] mac80211: Use RCU to handle local->key_list 2013-12-10 16:39 [RFC 1/2] mac80211: iterate over vif using RCU Antonio Quartulli @ 2013-12-10 16:39 ` Antonio Quartulli 2013-12-10 17:05 ` [RFC 1/2] mac80211: iterate over vif using RCU Eliad Peller 1 sibling, 0 replies; 6+ messages in thread From: Antonio Quartulli @ 2013-12-10 16:39 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, Antonio Quartulli From: Antonio Quartulli <antonio@open-mesh.com> By using RCU it is now possible to iterate over the key list without holding lock (other than rcu_read). This allows to simplify ieee80211_iter_keys() so that it can now run without holding any lock (other than rcu_read) Signed-off-by: Antonio Quartulli <antonio@open-mesh.com> --- net/mac80211/key.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/net/mac80211/key.c b/net/mac80211/key.c index 04c885a..e2a1e3c 100644 --- a/net/mac80211/key.c +++ b/net/mac80211/key.c @@ -261,7 +261,7 @@ static void ieee80211_key_replace(struct ieee80211_sub_if_data *sdata, bool defunikey, defmultikey, defmgmtkey; if (new) - list_add_tail(&new->list, &sdata->key_list); + list_add_tail_rcu(&new->list, &sdata->key_list); if (sta && pairwise) { rcu_assign_pointer(sta->ptk, new); @@ -309,7 +309,7 @@ static void ieee80211_key_replace(struct ieee80211_sub_if_data *sdata, } if (old) - list_del(&old->list); + list_del_rcu(&old->list); } struct ieee80211_key *ieee80211_key_alloc(u32 cipher, int idx, size_t key_len, @@ -537,6 +537,10 @@ void ieee80211_enable_keys(struct ieee80211_sub_if_data *sdata) mutex_unlock(&sdata->local->key_mtx); } +/* + * This function is supposed to be used for reading operations only. + * Iterations over the key list are performed with RCU_read lock only + */ void ieee80211_iter_keys(struct ieee80211_hw *hw, struct ieee80211_vif *vif, void (*iter)(struct ieee80211_hw *hw, @@ -547,27 +551,24 @@ void ieee80211_iter_keys(struct ieee80211_hw *hw, void *iter_data) { struct ieee80211_local *local = hw_to_local(hw); - struct ieee80211_key *key, *tmp; struct ieee80211_sub_if_data *sdata; mutex_lock(&local->key_mtx); + rcu_read_lock(); if (vif) { sdata = vif_to_sdata(vif); - list_for_each_entry_safe(key, tmp, &sdata->key_list, list) + list_for_each_entry(key, &sdata->key_list, list) iter(hw, &sdata->vif, key->sta ? &key->sta->sta : NULL, &key->conf, iter_data); } else { - rcu_read_lock(); list_for_each_entry_rcu(sdata, &local->interfaces, list) - list_for_each_entry_safe(key, tmp, - &sdata->key_list, list) + list_for_each_entry_rcu(key, &sdata->key_list, list) iter(hw, &sdata->vif, key->sta ? &key->sta->sta : NULL, &key->conf, iter_data); - rcu_read_unlock(); } - mutex_unlock(&local->key_mtx); + rcu_read_unlock(); } EXPORT_SYMBOL(ieee80211_iter_keys); -- 1.8.5.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC 1/2] mac80211: iterate over vif using RCU 2013-12-10 16:39 [RFC 1/2] mac80211: iterate over vif using RCU Antonio Quartulli 2013-12-10 16:39 ` [RFC 2/2] mac80211: Use RCU to handle local->key_list Antonio Quartulli @ 2013-12-10 17:05 ` Eliad Peller 2013-12-10 17:08 ` Antonio Quartulli 1 sibling, 1 reply; 6+ messages in thread From: Eliad Peller @ 2013-12-10 17:05 UTC (permalink / raw) To: Antonio Quartulli Cc: Johannes Berg, linux-wireless@vger.kernel.org, Antonio Quartulli hi, On Tue, Dec 10, 2013 at 6:39 PM, Antonio Quartulli <antonio@meshcoding.com> wrote: > I need to invoke ieee80211_iter_keys() from a periodic worker in a driver and > therefore I would prefer to get rid of any of locks to avoid problems. > These two patches try to use rcu lock to protect the iteration, but I'd like to > get a feedback before sending this stuff as a patch :-) > at least iwlwifi might sleep inside the iterator, so you can't just convert it to rcu (atomic). > Moreover, why do we use list_for_each_entry_safe() is ieee80211_iter_keys() if > the list cannot be altered (pointer to key is not passed to iter() so we should > be sure that nobody is going to invoke list_del())? see the documentation of ieee80211_remove_key(): /** * ieee80211_remove_key - remove the given key * @keyconf: the parameter passed with the set key * * Remove the given key. If the key was uploaded to the hardware at the * time this function is called, it is not deleted in the hardware but * instead assumed to have been removed already. * * Note that due to locking considerations this function can (currently) * only be called during key iteration (ieee80211_iter_keys().) */ Eliad. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 1/2] mac80211: iterate over vif using RCU 2013-12-10 17:05 ` [RFC 1/2] mac80211: iterate over vif using RCU Eliad Peller @ 2013-12-10 17:08 ` Antonio Quartulli 2013-12-10 17:23 ` Eliad Peller 0 siblings, 1 reply; 6+ messages in thread From: Antonio Quartulli @ 2013-12-10 17:08 UTC (permalink / raw) To: Eliad Peller, Antonio Quartulli Cc: Johannes Berg, linux-wireless@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 951 bytes --] On 10/12/13 18:05, Eliad Peller wrote: > hi, > > On Tue, Dec 10, 2013 at 6:39 PM, Antonio Quartulli > <antonio@meshcoding.com> wrote: >> I need to invoke ieee80211_iter_keys() from a periodic worker in a driver and >> therefore I would prefer to get rid of any of locks to avoid problems. >> These two patches try to use rcu lock to protect the iteration, but I'd like to >> get a feedback before sending this stuff as a patch :-) >> > at least iwlwifi might sleep inside the iterator, so you can't just > convert it to rcu (atomic). > mh, this "might sleep" could be an issue, ok. >> Moreover, why do we use list_for_each_entry_safe() is ieee80211_iter_keys() if >> the list cannot be altered (pointer to key is not passed to iter() so we should >> be sure that nobody is going to invoke list_del())? > > see the documentation of ieee80211_remove_key(): This function does not exist anymore -- Antonio Quartulli [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 1/2] mac80211: iterate over vif using RCU 2013-12-10 17:08 ` Antonio Quartulli @ 2013-12-10 17:23 ` Eliad Peller 2013-12-10 17:34 ` Antonio Quartulli 0 siblings, 1 reply; 6+ messages in thread From: Eliad Peller @ 2013-12-10 17:23 UTC (permalink / raw) To: Antonio Quartulli Cc: Antonio Quartulli, Johannes Berg, linux-wireless@vger.kernel.org On Tue, Dec 10, 2013 at 7:08 PM, Antonio Quartulli <antonio@open-mesh.com> wrote: >>> Moreover, why do we use list_for_each_entry_safe() is ieee80211_iter_keys() if >>> the list cannot be altered (pointer to key is not passed to iter() so we should >>> be sure that nobody is going to invoke list_del())? >> >> see the documentation of ieee80211_remove_key(): > > This function does not exist anymore > it does seem to exist here :) https://git.kernel.org/cgit/linux/kernel/git/linville/wireless-next.git/tree/include/net/mac80211.h Eliad. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 1/2] mac80211: iterate over vif using RCU 2013-12-10 17:23 ` Eliad Peller @ 2013-12-10 17:34 ` Antonio Quartulli 0 siblings, 0 replies; 6+ messages in thread From: Antonio Quartulli @ 2013-12-10 17:34 UTC (permalink / raw) To: Eliad Peller, Antonio Quartulli Cc: Johannes Berg, linux-wireless@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 839 bytes --] On 10/12/13 18:23, Eliad Peller wrote: > On Tue, Dec 10, 2013 at 7:08 PM, Antonio Quartulli > <antonio@open-mesh.com> wrote: >>>> Moreover, why do we use list_for_each_entry_safe() is ieee80211_iter_keys() if >>>> the list cannot be altered (pointer to key is not passed to iter() so we should >>>> be sure that nobody is going to invoke list_del())? >>> >>> see the documentation of ieee80211_remove_key(): >> >> This function does not exist anymore >> > it does seem to exist here :) > https://git.kernel.org/cgit/linux/kernel/git/linville/wireless-next.git/tree/include/net/mac80211.h > ops you are right. I got confused because I was discussing about that function some days ago: it is not used by any driver, so it could be removed. Therefore it should not really be a problem here. -- Antonio Quartulli [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-10 17:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-10 16:39 [RFC 1/2] mac80211: iterate over vif using RCU Antonio Quartulli 2013-12-10 16:39 ` [RFC 2/2] mac80211: Use RCU to handle local->key_list Antonio Quartulli 2013-12-10 17:05 ` [RFC 1/2] mac80211: iterate over vif using RCU Eliad Peller 2013-12-10 17:08 ` Antonio Quartulli 2013-12-10 17:23 ` Eliad Peller 2013-12-10 17:34 ` Antonio Quartulli
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).