An integer overflow in the minstrel debug code prevented the
throughput to be displayed correctly. This patch fixes that,
by swaping the division and multiplication.
Signed-off-by: Arnd Hannemann <[email protected]>
---
net/mac80211/rc80211_minstrel_debugfs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
index 98f4807..caf9453 100644
--- a/net/mac80211/rc80211_minstrel_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_debugfs.c
@@ -83,7 +83,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
p += sprintf(p, "%3u%s", mr->bitrate / 2,
(mr->bitrate & 1 ? ".5" : " "));
- tp = ((mr->cur_tp * 96) / 18000) >> 10;
+ tp = ((mr->cur_tp / 18000) * 96) >> 10;
prob = mr->cur_prob / 18;
eprob = mr->probability / 18;
--
1.6.4.1
Joe Perches schrieb:
> On Mon, 2009-08-24 at 19:42 +0200, Arnd Hannemann wrote:
>> An integer overflow in the minstrel debug code prevented the
>> throughput to be displayed correctly. This patch fixes that,
>> by swaping the division and multiplication.
>>
>> Signed-off-by: Arnd Hannemann <[email protected]>
>> ---
>> net/mac80211/rc80211_minstrel_debugfs.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
>> index 98f4807..caf9453 100644
>> --- a/net/mac80211/rc80211_minstrel_debugfs.c
>> +++ b/net/mac80211/rc80211_minstrel_debugfs.c
>> @@ -83,7 +83,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
>> p += sprintf(p, "%3u%s", mr->bitrate / 2,
>> (mr->bitrate & 1 ? ".5" : " "));
>>
>> - tp = ((mr->cur_tp * 96) / 18000) >> 10;
>> + tp = ((mr->cur_tp / 18000) * 96) >> 10;
>
> Maybe do_div instead?
Do you mean the do_div from asm/div64h ?
It seems overly complicated to me. It would result in something like:
tp = mr->cur_tp;
do_div(tp, 18000);
tp = (tp * 96) >> 10;
Or am I missing something?
Probably
tp = mr->cur_tp * 96;
would not work..., as it would already overflow. Not sure if
do_div(tp * 96, 18000);
would work?
Best regards,
Arnd
Arnd Hannemann schrieb:
> Pavel Roskin schrieb:
>> On Mon, 2009-08-24 at 10:57 -0700, Joe Perches wrote:
>>> On Mon, 2009-08-24 at 19:42 +0200, Arnd Hannemann wrote:
>>>> An integer overflow in the minstrel debug code prevented the
>>>> throughput to be displayed correctly. This patch fixes that,
>>>> by swaping the division and multiplication.
>>>>
>>>> Signed-off-by: Arnd Hannemann <[email protected]>
>>>> ---
>>>> net/mac80211/rc80211_minstrel_debugfs.c | 2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
>>>> index 98f4807..caf9453 100644
>>>> --- a/net/mac80211/rc80211_minstrel_debugfs.c
>>>> +++ b/net/mac80211/rc80211_minstrel_debugfs.c
>>>> @@ -83,7 +83,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
>>>> p += sprintf(p, "%3u%s", mr->bitrate / 2,
>>>> (mr->bitrate & 1 ? ".5" : " "));
>>>>
>>>> - tp = ((mr->cur_tp * 96) / 18000) >> 10;
>>>> + tp = ((mr->cur_tp / 18000) * 96) >> 10;
>>> Maybe do_div instead?
>> How about this?
>>
>> tp = mr->cur_tp / ((18000 << 10) / 96);
>>
>> ((18000 << 10) / 96) is exactly 192000.
>>
>
> Hmm don't seems to be equivalent with the original statement:
>
> a = 61027734;
> b = ((a * 96) / 18000) >> 10;
> c = a / ((18000 << 10) / 96);
> printf("%u %u\n", b, c);
>
> Outputs:
> 84 317
Outch, sorry I took the overflowing formula not the fixed one!
Best regards,
Arnd
On Mon, 2009-08-24 at 10:57 -0700, Joe Perches wrote:
> On Mon, 2009-08-24 at 19:42 +0200, Arnd Hannemann wrote:
> > An integer overflow in the minstrel debug code prevented the
> > throughput to be displayed correctly. This patch fixes that,
> > by swaping the division and multiplication.
> >
> > Signed-off-by: Arnd Hannemann <[email protected]>
> > ---
> > net/mac80211/rc80211_minstrel_debugfs.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
> > index 98f4807..caf9453 100644
> > --- a/net/mac80211/rc80211_minstrel_debugfs.c
> > +++ b/net/mac80211/rc80211_minstrel_debugfs.c
> > @@ -83,7 +83,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
> > p += sprintf(p, "%3u%s", mr->bitrate / 2,
> > (mr->bitrate & 1 ? ".5" : " "));
> >
> > - tp = ((mr->cur_tp * 96) / 18000) >> 10;
> > + tp = ((mr->cur_tp / 18000) * 96) >> 10;
>
> Maybe do_div instead?
How about this?
tp = mr->cur_tp / ((18000 << 10) / 96);
((18000 << 10) / 96) is exactly 192000.
--
Regards,
Pavel Roskin
Pavel Roskin schrieb:
> On Mon, 2009-08-24 at 10:57 -0700, Joe Perches wrote:
>> On Mon, 2009-08-24 at 19:42 +0200, Arnd Hannemann wrote:
>>> An integer overflow in the minstrel debug code prevented the
>>> throughput to be displayed correctly. This patch fixes that,
>>> by swaping the division and multiplication.
>>>
>>> Signed-off-by: Arnd Hannemann <[email protected]>
>>> ---
>>> net/mac80211/rc80211_minstrel_debugfs.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
>>> index 98f4807..caf9453 100644
>>> --- a/net/mac80211/rc80211_minstrel_debugfs.c
>>> +++ b/net/mac80211/rc80211_minstrel_debugfs.c
>>> @@ -83,7 +83,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
>>> p += sprintf(p, "%3u%s", mr->bitrate / 2,
>>> (mr->bitrate & 1 ? ".5" : " "));
>>>
>>> - tp = ((mr->cur_tp * 96) / 18000) >> 10;
>>> + tp = ((mr->cur_tp / 18000) * 96) >> 10;
>> Maybe do_div instead?
>
> How about this?
>
> tp = mr->cur_tp / ((18000 << 10) / 96);
>
> ((18000 << 10) / 96) is exactly 192000.
>
Hmm don't seems to be equivalent with the original statement:
a = 61027734;
b = ((a * 96) / 18000) >> 10;
c = a / ((18000 << 10) / 96);
printf("%u %u\n", b, c);
Outputs:
84 317
Best regards,
Arnd
On Mon, 2009-08-24 at 19:42 +0200, Arnd Hannemann wrote:
> An integer overflow in the minstrel debug code prevented the
> throughput to be displayed correctly. This patch fixes that,
> by swaping the division and multiplication.
>
> Signed-off-by: Arnd Hannemann <[email protected]>
> ---
> net/mac80211/rc80211_minstrel_debugfs.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
> index 98f4807..caf9453 100644
> --- a/net/mac80211/rc80211_minstrel_debugfs.c
> +++ b/net/mac80211/rc80211_minstrel_debugfs.c
> @@ -83,7 +83,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
> p += sprintf(p, "%3u%s", mr->bitrate / 2,
> (mr->bitrate & 1 ? ".5" : " "));
>
> - tp = ((mr->cur_tp * 96) / 18000) >> 10;
> + tp = ((mr->cur_tp / 18000) * 96) >> 10;
Maybe do_div instead?
An integer overflow in the minstrel debug code prevented the
throughput to be displayed correctly. This patch fixes that,
by permutating operations like proposed by Pavel Roskin.
Signed-off-by: Arnd Hannemann <[email protected]>
---
net/mac80211/rc80211_minstrel_debugfs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
index 98f4807..3d72ec5 100644
--- a/net/mac80211/rc80211_minstrel_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_debugfs.c
@@ -83,7 +83,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
p += sprintf(p, "%3u%s", mr->bitrate / 2,
(mr->bitrate & 1 ? ".5" : " "));
- tp = ((mr->cur_tp * 96) / 18000) >> 10;
+ tp = mr->cur_tp / ((18000 << 10) / 96);
prob = mr->cur_prob / 18;
eprob = mr->probability / 18;
--
1.6.4.1
On Tue, Aug 25, 2009 at 04:51, Arnd
Hannemann<[email protected]> wrote:
> An integer overflow in the minstrel debug code prevented the
> throughput to be displayed correctly. This patch fixes that,
> by permutating operations like proposed by Pavel Roskin.
>
> Signed-off-by: Arnd Hannemann <[email protected]>
> ---
> ?net/mac80211/rc80211_minstrel_debugfs.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
> index 98f4807..3d72ec5 100644
> --- a/net/mac80211/rc80211_minstrel_debugfs.c
> +++ b/net/mac80211/rc80211_minstrel_debugfs.c
> @@ -83,7 +83,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
> ? ? ? ? ? ? ? ?p += sprintf(p, "%3u%s", mr->bitrate / 2,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?(mr->bitrate & 1 ? ".5" : " ?"));
>
> - ? ? ? ? ? ? ? tp = ((mr->cur_tp * 96) / 18000) >> 10;
> + ? ? ? ? ? ? ? tp = mr->cur_tp / ((18000 << 10) / 96);
Sorry about being so late, but wouldn't:
tp = ((mr->cur_tp * 2) / 375) >> 10;
also work? (Assuming that the numbers in the constant aren't important)
or even:
tp = (mr->cur_tp / 375) >> 9;
Thanks,
--
Julian Calaby
Email: [email protected]
.Plan: http://sites.google.com/site/juliancalaby/
Julian Calaby wrote:
> On Tue, Aug 25, 2009 at 04:51, Arnd
> Hannemann<[email protected]> wrote:
>> An integer overflow in the minstrel debug code prevented the
>> throughput to be displayed correctly. This patch fixes that,
>> by permutating operations like proposed by Pavel Roskin.
>>
>> Signed-off-by: Arnd Hannemann <[email protected]>
>> ---
>> net/mac80211/rc80211_minstrel_debugfs.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
>> index 98f4807..3d72ec5 100644
>> --- a/net/mac80211/rc80211_minstrel_debugfs.c
>> +++ b/net/mac80211/rc80211_minstrel_debugfs.c
>> @@ -83,7 +83,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
>> p += sprintf(p, "%3u%s", mr->bitrate / 2,
>> (mr->bitrate & 1 ? ".5" : " "));
>>
>> - tp = ((mr->cur_tp * 96) / 18000) >> 10;
>> + tp = mr->cur_tp / ((18000 << 10) / 96);
>
> Sorry about being so late, but wouldn't:
>
> tp = ((mr->cur_tp * 2) / 375) >> 10;
>
> also work? (Assuming that the numbers in the constant aren't important)
I needed a while to figure out why those constants are used, but finally
they made some sense, so I think its best to preserve them.
mr->cur_tp is the time to send one 1200 byte packet (9600 bits),
the loss probelity is scaled between 0-18000 to reduce rounding error.
>
> or even:
>
> tp = (mr->cur_tp / 375) >> 9;
See above.
Best regards,
Arnd