Hello Johannes,
On Mon, Jan 09, 2012 at 02:04:54PM +0100, Johannes Berg wrote:
> On Thu, 2012-01-05 at 20:58 +0100, Simon Wunderlich wrote:
>
> > +static bool rate_idx_match_mcs_mask(struct ieee80211_tx_rate *rate,
> > + u8 mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
> > +{
> > + int i, j;
> > + int ridx, rbit;
> > +
> > + ridx = rate->idx / 8;
> > + rbit = rate->idx % 8;
> > +
> > + /* sanity check */
> > + if (ridx < 0 || ridx > IEEE80211_HT_MCS_MASK_LEN)
> > + return false;
> > +
> > + /* See whether the selected rate or anything below it is allowed. */
> > + for (i = ridx; i >= 0; i--) {
> > + for (j = rbit; j >= 0; j--)
> > + if (mcs_mask[i] & BIT(j)) {
> > + rate->idx = i * 8 + j;
> > + return true;
> > + }
> > + rbit = 7;
> > + }
> > +
> > + /* Try to find a higher rate that would be allowed */
> > + ridx = (rate->idx + 1) / 8;
> > + rbit = (rate->idx + 1) % 8;
> > +
> > + for (i = ridx; i < IEEE80211_HT_MCS_MASK_LEN; i++) {
> > + for (j = rbit; j < 8; j++)
> > + if (mcs_mask[i] & BIT(j)) {
> > + rate->idx = i * 8 + j;
> > + return true;
> > + }
> > + rbit = 0;
> > + }
> > + return false;
>
> All this logic is pretty complex, maybe we could translate the bitmap to
> an array of unsigned longs and use test_bit() instead of open-coding it
> for 8-bit words?
Mhm, I would rather not add further copies into this fast path function, and
simply casting will result in bad behaviour (alignment, endianess). Generally
changing the mcs_mask to u32 is possible, but then we have inconsistent mcs
fields (rx_mask and others are u8 arrays too) - do we want this? Or am I missing
something obvious right now? :)
Maybe we can simplify it to something like this:
for (i = rate->idx + 1; i < IEEE80211_HT_MCS_MASK_LEN * 8; i++) {
ridx = i / 8;
rbit = i % 8;
if (mcs_mask[ridx] & BIT(rbit))) {
rate->idx = i;
return true;
}
}
What do you think?
Cheers,
Simon
Attachment:
signature.asc
Description: Digital signature
