Re: [PATCH 13/17] iwlwifi: mvm: Convert timers to use timer_setup()

2017-11-06 Thread Kees Cook 
On Mon, Nov 6, 2017 at 11:48 AM, Luca Coelho  wrote:
> On Mon, 2017-11-06 at 11:45 -0800, Kees Cook wrote:
>> On Sun, Oct 29, 2017 at 5:28 AM, Luca Coelho  wrote:
>> > From: Kees Cook 
>> >
>> > In preparation for unconditionally passing the struct timer_list
>> > pointer to
>> > all timer callbacks, switch to using the new timer_setup() and
>> > from_timer()
>> > to pass the timer pointer explicitly.
>> >
>> > The RCU lifetime on baid_data is unclear, so this adds a direct
>> > copy of the
>> > rcu_ptr passed to the original callback. It may be possible to
>> > improve this
>> > to just use baid_data->mvm->baid_map[baid_data->baid] instead.
>> >
>> > Cc: Johannes Berg 
>> > Cc: Emmanuel Grumbach 
>> > Cc: Luca Coelho 
>> > Cc: Intel Linux Wireless 
>> > Cc: Kalle Valo 
>> > Cc: Sara Sharon 
>> > Cc: linux-wirel...@vger.kernel.org
>> > Cc: netdev@vger.kernel.org
>> > Signed-off-by: Kees Cook 
>> > Signed-off-by: Luca Coelho 
>> > ---
>> >  drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  3 ++-
>> >  drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c |  4 ++--
>> >  drivers/net/wireless/intel/iwlwifi/mvm/sta.c  | 18 +
>> > -
>> >  3 files changed, 13 insertions(+), 12 deletions(-)
>>
>> Hi,
>>
>> Thanks for taking this! I had a question on timing: is this expected
>> to land for 4.15? If not, I would like to take this via the timers
>> tree, since it is one of the few remaining conversions.
>
> Hi Kees,
>
> Yes, this should land for 4.15.  Kalle just pulled my pull-request
> (which includes this) to wireless-drivers-next.  He told me he'll send
> a pull-request for 4.15 during this week and hopefully Dave will pull
> from him too.
>
> I'll let you know if something doesn't go as planned.

Awesome, thanks very much!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 13/17] iwlwifi: mvm: Convert timers to use timer_setup()

2017-11-06 Thread Luca Coelho 
On Mon, 2017-11-06 at 11:45 -0800, Kees Cook wrote:
> On Sun, Oct 29, 2017 at 5:28 AM, Luca Coelho  wrote:
> > From: Kees Cook 
> > 
> > In preparation for unconditionally passing the struct timer_list
> > pointer to
> > all timer callbacks, switch to using the new timer_setup() and
> > from_timer()
> > to pass the timer pointer explicitly.
> > 
> > The RCU lifetime on baid_data is unclear, so this adds a direct
> > copy of the
> > rcu_ptr passed to the original callback. It may be possible to
> > improve this
> > to just use baid_data->mvm->baid_map[baid_data->baid] instead.
> > 
> > Cc: Johannes Berg 
> > Cc: Emmanuel Grumbach 
> > Cc: Luca Coelho 
> > Cc: Intel Linux Wireless 
> > Cc: Kalle Valo 
> > Cc: Sara Sharon 
> > Cc: linux-wirel...@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Kees Cook 
> > Signed-off-by: Luca Coelho 
> > ---
> >  drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  3 ++-
> >  drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c |  4 ++--
> >  drivers/net/wireless/intel/iwlwifi/mvm/sta.c  | 18 +
> > -
> >  3 files changed, 13 insertions(+), 12 deletions(-)
> 
> Hi,
> 
> Thanks for taking this! I had a question on timing: is this expected
> to land for 4.15? If not, I would like to take this via the timers
> tree, since it is one of the few remaining conversions.

Hi Kees,

Yes, this should land for 4.15.  Kalle just pulled my pull-request
(which includes this) to wireless-drivers-next.  He told me he'll send
a pull-request for 4.15 during this week and hopefully Dave will pull
from him too.

I'll let you know if something doesn't go as planned.

--
Cheers,
Luca.

Re: [PATCH 13/17] iwlwifi: mvm: Convert timers to use timer_setup()

2017-11-06 Thread Kees Cook 
On Sun, Oct 29, 2017 at 5:28 AM, Luca Coelho  wrote:
> From: Kees Cook 
>
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
>
> The RCU lifetime on baid_data is unclear, so this adds a direct copy of the
> rcu_ptr passed to the original callback. It may be possible to improve this
> to just use baid_data->mvm->baid_map[baid_data->baid] instead.
>
> Cc: Johannes Berg 
> Cc: Emmanuel Grumbach 
> Cc: Luca Coelho 
> Cc: Intel Linux Wireless 
> Cc: Kalle Valo 
> Cc: Sara Sharon 
> Cc: linux-wirel...@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook 
> Signed-off-by: Luca Coelho 
> ---
>  drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  3 ++-
>  drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c |  4 ++--
>  drivers/net/wireless/intel/iwlwifi/mvm/sta.c  | 18 +-
>  3 files changed, 13 insertions(+), 12 deletions(-)

Hi,

Thanks for taking this! I had a question on timing: is this expected
to land for 4.15? If not, I would like to take this via the timers
tree, since it is one of the few remaining conversions.

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

[PATCH 13/17] iwlwifi: mvm: Convert timers to use timer_setup()

2017-10-29 Thread Luca Coelho 
From: Kees Cook 

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

The RCU lifetime on baid_data is unclear, so this adds a direct copy of the
rcu_ptr passed to the original callback. It may be possible to improve this
to just use baid_data->mvm->baid_map[baid_data->baid] instead.

Cc: Johannes Berg 
Cc: Emmanuel Grumbach 
Cc: Luca Coelho 
Cc: Intel Linux Wireless 
Cc: Kalle Valo 
Cc: Sara Sharon 
Cc: linux-wirel...@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook 
Signed-off-by: Luca Coelho 
---
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  3 ++-
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c |  4 ++--
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c  | 18 +-
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h 
b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index 2f3d5bef4b9e..0e18c5066f04 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -652,6 +652,7 @@ struct iwl_mvm_baid_data {
u16 entries_per_queue;
unsigned long last_rx;
struct timer_list session_timer;
+   struct iwl_mvm_baid_data __rcu **rcu_ptr;
struct iwl_mvm *mvm;
struct iwl_mvm_reorder_buffer reorder_buf[IWL_MAX_RX_HW_QUEUES];
struct iwl_mvm_reorder_buf_entry entries[];
@@ -1853,7 +1854,7 @@ void iwl_mvm_tdls_ch_switch_work(struct work_struct 
*work);
 void iwl_mvm_sync_rx_queues_internal(struct iwl_mvm *mvm,
 struct iwl_mvm_internal_rxq_notif *notif,
 u32 size);
-void iwl_mvm_reorder_timer_expired(unsigned long data);
+void iwl_mvm_reorder_timer_expired(struct timer_list *t);
 struct ieee80211_vif *iwl_mvm_get_bss_vif(struct iwl_mvm *mvm);
 bool iwl_mvm_is_vif_assoc(struct iwl_mvm *mvm);
 
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index 9852a4d62337..343bdc4266cd 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -460,9 +460,9 @@ static void iwl_mvm_release_frames(struct iwl_mvm *mvm,
}
 }
 
-void iwl_mvm_reorder_timer_expired(unsigned long data)
+void iwl_mvm_reorder_timer_expired(struct timer_list *t)
 {
-   struct iwl_mvm_reorder_buffer *buf = (void *)data;
+   struct iwl_mvm_reorder_buffer *buf = from_timer(buf, t, reorder_timer);
struct iwl_mvm_baid_data *baid_data =
iwl_mvm_baid_data_from_reorder_buf(buf);
struct iwl_mvm_reorder_buf_entry *entries =
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index 039efcd2735d..c19f98489d4e 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -252,9 +252,11 @@ int iwl_mvm_sta_send_to_fw(struct iwl_mvm *mvm, struct 
ieee80211_sta *sta,
return ret;
 }
 
-static void iwl_mvm_rx_agg_session_expired(unsigned long data)
+static void iwl_mvm_rx_agg_session_expired(struct timer_list *t)
 {
-   struct iwl_mvm_baid_data __rcu **rcu_ptr = (void *)data;
+   struct iwl_mvm_baid_data *data =
+   from_timer(data, t, session_timer);
+   struct iwl_mvm_baid_data __rcu **rcu_ptr = data->rcu_ptr;
struct iwl_mvm_baid_data *ba_data;
struct ieee80211_sta *sta;
struct iwl_mvm_sta *mvm_sta;
@@ -2160,10 +2162,8 @@ static void iwl_mvm_init_reorder_buffer(struct iwl_mvm 
*mvm,
reorder_buf->head_sn = ssn;
reorder_buf->buf_size = buf_size;
/* rx reorder timer */
-   reorder_buf->reorder_timer.function =
-   iwl_mvm_reorder_timer_expired;
-   reorder_buf->reorder_timer.data = (unsigned long)reorder_buf;
-   init_timer(_buf->reorder_timer);
+   timer_setup(_buf->reorder_timer,
+   iwl_mvm_reorder_timer_expired, 0);
spin_lock_init(_buf->lock);
reorder_buf->mvm = mvm;
reorder_buf->queue = i;
@@ -2286,9 +2286,9 @@ int iwl_mvm_sta_rx_agg(struct iwl_mvm *mvm, struct 
ieee80211_sta *sta,
baid_data->baid = baid;
baid_data->timeout = timeout;
baid_data->last_rx = jiffies;
-   setup_timer(_data->session_timer,
-   iwl_mvm_rx_agg_session_expired,
-   (unsigned long)>baid_map[baid]);
+   baid_data->rcu_ptr = >baid_map[baid];
+