Re: [PATCH 01/10] mac80211: introduce channel context skeleton code

["Arend van Spriel" <arend@broadcom.com>]

On 09/15/2012 11:42 PM, Eliad Peller wrote:
> On Fri, Sep 14, 2012 at 3:37 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>> From: Michal Kazior <michal.kazior@tieto.com>
>>
>> Channel context are the foundation for multi-channel
>> operation. They are are immutable and are re-created
>> (or re-used if other interfaces are bound to a certain
>> channel and a compatible channel type) on channel
>> switching.
>>
>> This is an initial implementation and more features
>> will come in separate patches.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>> [some changes including RCU protection]
>> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>> ---
> [...]
>
>> +int ieee80211_vif_use_channel(struct ieee80211_sub_if_data *sdata,
>> +                             struct ieee80211_channel *channel,
>> +                             enum nl80211_channel_type channel_type,
>> +                             enum ieee80211_chanctx_mode mode)
>> +{
>> +       struct ieee80211_local *local = sdata->local;
>> +       struct ieee80211_chanctx *ctx;
>> +       int ret;
>> +
>> +       mutex_lock(&local->chanctx_mtx);
>> +       __ieee80211_vif_release_channel(sdata);
>> +
>> +       ctx = ieee80211_find_chanctx(local, channel, channel_type, mode);
> since you might use an existing ctx here...
>
>> +       if (!ctx)
>> +               ctx = ieee80211_new_chanctx(local, channel, channel_type, mode);
>> +       if (IS_ERR(ctx)) {
>> +               ret = PTR_ERR(ctx);
>> +               goto out;
>> +       }
>> +
>> +       ret = ieee80211_assign_vif_chanctx(sdata, ctx);
>> +       if (ret) {
>> +               ieee80211_free_chanctx(local, ctx);
> i think you should check ctx->refcount before freeing it.
>
> Eliad.

Would it make sense to have the refcount check in 
ieee80211_free_chanctx() instead of the WARN_ON_ONCE() in there.

Gr. AvS