2007-12-17 00:29:22

by Mattias Nissler

[permalink] [raw]
Subject: [patch 0/9] Rate control rework

Hi,

this patchset is the result of my recent joint-venture with Stefano. It
introduces a new rate control algorithm based on a PID controller. First
tests show it works much better than the old simple rate control
algorithm.

We think the current patch set is clean enough so we can push it into
2.6.25. However, there are some issues that need some more work:

* The algorithm parameters need more tuning. The current defaults are
sane, but we hope to get a larger testing audience once this is in the
tree. Once there are more test results, tuning should be easier.

* It seems mac80211 fails to clean up the rate control algorithm. This
results in the debugfs entries created by patch 9 not being removed.

* Patch number 8 is actually a change to the debugfs code so I can add
files for signed variables. I've sent it to Greg KH and LKML, let's
see what happens. If it doesn't make 2.6.25, we could add similar
infrastructure locally.

Hm, anything I forgot? Stefano?

Mattias




2007-12-17 09:54:55

by Mattias Nissler

[permalink] [raw]
Subject: Re: [patch 0/9] Rate control rework

Hi,

I'm sorry most of my mails didn't make it to the list. I'll resubmit
tonight, also correcting issues that you (those that got the complete
original set via PM, you know who you are) find until then.

Mattias



2007-12-17 20:59:38

by Mattias Nissler

[permalink] [raw]
Subject: Re: [patch 0/9] Rate control rework


On Mon, 2007-12-17 at 15:05 +0100, Johannes Berg wrote:
> > * It seems mac80211 fails to clean up the rate control algorithm. This
> > results in the debugfs entries created by patch 9 not being removed.
>
> Probably a bug then, though I can't find it right now, the references
> all look balanced and if they weren't then we'd not have been able to
> rmmod rc80211_simple when it was still around. Not sure, can you try to
> see what the problem is and maybe even fix it?

Will do, after I've addressed the other issues.

>
> It does, however, look like the whole rate control code needs a revamp.
> Looks like Jiri got busy there with krefs too without giving it too much
> thought... It looks like all that can be converted to RCU just as well
> as all the other stuff I already converted, I don't see a reason to keep
> a reference count of the rate control algorithm for each sta.

What bothers me most is the fact that we have several ways to refer to a
rate: Either we use struct ieee80211_rate pointers or indices into
local->oper_hw_mode->rates[]. This is not very consistent. Furthermore,
there is oper_hw_mode vs scan_hw_mode. So potentially even to rates[].
That sucks.

>
> If you fix the free problem I'll take a look at fixing the locking/refs
> stuff there later.

I'll happily leave that to you, I don't have much spare time.

Mattias


2007-12-17 17:47:04

by Johannes Berg

[permalink] [raw]
Subject: Re: [patch 0/9] Rate control rework

> * It seems mac80211 fails to clean up the rate control algorithm. This
> results in the debugfs entries created by patch 9 not being removed.

Probably a bug then, though I can't find it right now, the references
all look balanced and if they weren't then we'd not have been able to
rmmod rc80211_simple when it was still around. Not sure, can you try to
see what the problem is and maybe even fix it?

It does, however, look like the whole rate control code needs a revamp.
Looks like Jiri got busy there with krefs too without giving it too much
thought... It looks like all that can be converted to RCU just as well
as all the other stuff I already converted, I don't see a reason to keep
a reference count of the rate control algorithm for each sta.

If you fix the free problem I'll take a look at fixing the locking/refs
stuff there later.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-12-18 13:19:02

by Johannes Berg

[permalink] [raw]
Subject: Re: [patch 0/9] Rate control rework


> > It does, however, look like the whole rate control code needs a revamp.
> > Looks like Jiri got busy there with krefs too without giving it too much
> > thought... It looks like all that can be converted to RCU just as well
> > as all the other stuff I already converted, I don't see a reason to keep
> > a reference count of the rate control algorithm for each sta.
>
> What bothers me most is the fact that we have several ways to refer to a
> rate: Either we use struct ieee80211_rate pointers or indices into
> local->oper_hw_mode->rates[]. This is not very consistent. Furthermore,
> there is oper_hw_mode vs scan_hw_mode. So potentially even to rates[].
> That sucks.

Yeah. That cfg80211 rate/channel stuff I had addressed some of it, but
not all, iirc.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part