On 10 March 2014 10:08, Luca Coelho <lrothc@xxxxxxxxx> wrote:
> On Mon, 2014-03-10 at 08:03 +0100, Michal Kazior wrote:
>> On 7 March 2014 07:48, Luca Coelho <luca@xxxxxxxxx> wrote:
>> > On Wed, 2014-03-05 at 13:04 +0100, Michal Kazior wrote:
>> >> On 5 March 2014 12:11, Luca Coelho <luca@xxxxxxxxx> wrote:
>> >>
>> >> [...]
>> >>
>> >> > + /* unref our reservation before assigning */
>> >> > + ctx->refcount--;
>> >> > + sdata->reserved_chanctx = NULL;
>> >> > + ret = ieee80211_assign_vif_chanctx(sdata, ctx);
>> >> > + if (ret) {
>> >> > + /* if assign fails refcount stays the same */
>> >> > + if (ctx->refcount == 0)
>> >> > + ieee80211_free_chanctx(local, ctx);
>> >> > + goto out_wake;
>> >> > + }
>> >>
>> >> This actually won't work if there are AP VLANs (I noticed that yesterday).
>> >>
>> >> You can't call ieee80211_vif_copy_chanctx_to_vlans() here due to
>> >> current locking requirements. I have a patch for that though ;-) I'll
>> >> send it soon.
>> >
>> > How is this related to this patch?
>> >
>> > Anyway, good to know that you have fixed it. :)
>>
>> If you re-assign a chanctx of an AP that has VLANs you leave VLANs
>> with the old chanctx pointer. Those pointers should be updated or else
>> you'll end up dereferencing an invalid pointer.
>
> I see. So, I need to call ieee80211_vif_copy_chanctx_to_vlans() here,
> but to do that I need your patch that allows it to be called without the
> RTNL.
>
> So, now this series will depend on that patch, I'll talk to Johannes.
There's also another catch with having unassign-assign chanctx
swapping. There's a brief period of time where chanctx_mtx unprotected
code (think rcu based sections) access the chanctx_conf to get
band/sband. This ends up with bloody NULL dereferences if you have a
5GHz-only hw. I'm currently patching those up. This would be another
(although indirect) dependency for the patchset.
We might also want to have a ieee80211_vif_chanctx() wrapper that
would return either assigned chanctx or reserved chanctx before the
reservation patchset. Otherwise, even with those NULL dereferences
patched you risk WARN_ONs and other (logic) failures.
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
