On 3 March 2014 14:42, Luca Coelho <luca@xxxxxxxxx> wrote: > On Mon, 2014-03-03 at 14:26 +0100, Michal Kazior wrote: >> On 3 March 2014 13:37, Luca Coelho <luca@xxxxxxxxx> wrote: >> > On Mon, 2014-03-03 at 11:38 +0100, Michal Kazior wrote: >> >> On 3 March 2014 10:57, Luca Coelho <luca@xxxxxxxxx> wrote: [...] >> >> > If more vifs came and are changing the chanctx at the same time, it will >> >> > be fine too because the channel will only change when the last reserver >> >> > uses the reservation. >> >> > >> >> > Does this make sense? >> >> >> >> Does the following match your idea of multi-vif reservation with the >> >> refcount idea? >> >> >> >> [2 vifs on chanctx->refcount==2] >> >> * vif1 reserve (refcount==3) >> >> * vif2 reserve (refcount==4) >> >> * vif1 use reservation: (refcount==3) >> >> * stop queues >> >> * unassign vif (refcount==2) >> >> * since refcount!=0 do nothing more >> >> * vif3 use reservation: (refcount==1) >> >> * unassign vif (refcount==0) >> >> * since refcount==0 do: >> >> * chanctx channel change >> >> * vif1 assign (refcount==1) >> >> * vif2 assign (refcount==2) >> >> * wake queues >> > >> > Right, this is a good idea (better than what I wrote in my previous >> > reply). I just need to iterate all the vifs and assign the ones with >> > matching reserved_chanctx (and no assigned chanctx) to this chanctx when >> > refcount reaches 0. >> >> Yes. In theory you can even handle cases where ctx->refcount is still >> >0 if you have multi-channel hw. Consider the following: >> >> hw: 2 channels max >> ctx1 (chan 1): vif1 >> ctx2 (chan 2): vif2 vif3 >> >> * vif2 wants to switch to chan 3, but can't create new chanctx so use >> current one for "in place" hoping vif3 will also switch soon enough >> * vif1 decides it wants to follow vif2 to chan 3 >> * vif3 doesn't want to do anything >> * vif1 and vif2 both call use_reserved (order doesn't matter if you >> perform checks properly) >> * ctx1 should be freed now since vif1 has been unassigned >> * ctx2->refcount > 0, but num_chanctx < max_num_chanctx, so create new >> ctx1 for chan3 >> * assign vif1 and vif2 to ctx1 on chan 3 >> >> (this would depend on my ieee80211_max_num_channels() patch) > > Sounds good! But where do the channel combinations come in here? I think > instead of just checking for max_num_channels we should do a full > combination check. The channel counting uses combination checks. It simply iterates over all *matching* ifcombs and picks the greatest max_num_channels. CSA doesn't change iftype and regular ifcomb checks (with your patch) is protected by chanctx_mtx, so it should be safe enough. >> >> Additionally you'd have to check after each "use reservation": if all >> >> vifs with matching reserved_chanctx have no assigned chanctx but the >> >> reserved_chanctx->refcount > 0, then disconnect all vifs with the >> >> matching reserved_chanctx. >> > >> > I'm not sure I understood this part. I think that when refcount reaches >> > 0 I should disconnect the ones that are still using this chanctx, right? >> > All the ones that wanted to switch together, will have already done so >> > (since the refcount reached 0). If there's any remaining vif in the >> > chanctx we want to change, disconnect them. >> >> There are two approaches here: >> a) first use_reservation implies actual channel change >> b) last use_reservation implies actual channel change >> >> You probably keep model (a) in mind, while I use (b). > > Actually I was thinking about (b) as well. > > My question here is how do we discern vifs that are still in the channel > but didn't want to change (and thus we should disconnect)? We can't rely > on refcount. They simply will have a NULL reserved_chanctx. The only thing missing (while including your patches) is a boolean per-sdata that will indicate whether use_reserved() was already called or not so you can check if all vifs with the same reserved_chanctx have "synchronized" and if that chanctx is suitable to use (i.e. refcount==0 when switching in-place, or it's possible to create a new chanctx which wasn't possible when original reservation was done). 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
