* [PATCH] mac80211: Fix output of minstrels rc_stats
@ 2009-08-24 17:42 Arnd Hannemann
2009-08-24 17:57 ` Joe Perches
0 siblings, 1 reply; 9+ messages in thread
From: Arnd Hannemann @ 2009-08-24 17:42 UTC (permalink / raw)
To: linux-wireless; +Cc: Arnd Hannemann
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 <hannemann@nets.rwth-aachen.de>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mac80211: Fix output of minstrels rc_stats
2009-08-24 17:42 [PATCH] mac80211: Fix output of minstrels rc_stats Arnd Hannemann
@ 2009-08-24 17:57 ` Joe Perches
2009-08-24 18:19 ` Arnd Hannemann
2009-08-24 18:20 ` Pavel Roskin
0 siblings, 2 replies; 9+ messages in thread
From: Joe Perches @ 2009-08-24 17:57 UTC (permalink / raw)
To: Arnd Hannemann; +Cc: linux-wireless
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 <hannemann@nets.rwth-aachen.de>
> ---
> 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?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mac80211: Fix output of minstrels rc_stats
2009-08-24 17:57 ` Joe Perches
@ 2009-08-24 18:19 ` Arnd Hannemann
2009-08-24 18:20 ` Pavel Roskin
1 sibling, 0 replies; 9+ messages in thread
From: Arnd Hannemann @ 2009-08-24 18:19 UTC (permalink / raw)
To: Joe Perches; +Cc: Arnd Hannemann, linux-wireless@vger.kernel.org
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 <hannemann@nets.rwth-aachen.de>
>> ---
>> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mac80211: Fix output of minstrels rc_stats
2009-08-24 17:57 ` Joe Perches
2009-08-24 18:19 ` Arnd Hannemann
@ 2009-08-24 18:20 ` Pavel Roskin
2009-08-24 18:36 ` Arnd Hannemann
2009-08-24 18:51 ` [PATCH v2] " Arnd Hannemann
1 sibling, 2 replies; 9+ messages in thread
From: Pavel Roskin @ 2009-08-24 18:20 UTC (permalink / raw)
To: Joe Perches; +Cc: Arnd Hannemann, linux-wireless
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 <hannemann@nets.rwth-aachen.de>
> > ---
> > 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mac80211: Fix output of minstrels rc_stats
2009-08-24 18:20 ` Pavel Roskin
@ 2009-08-24 18:36 ` Arnd Hannemann
2009-08-24 18:38 ` Arnd Hannemann
2009-08-24 18:51 ` [PATCH v2] " Arnd Hannemann
1 sibling, 1 reply; 9+ messages in thread
From: Arnd Hannemann @ 2009-08-24 18:36 UTC (permalink / raw)
To: Pavel Roskin; +Cc: Joe Perches, Arnd Hannemann, linux-wireless@vger.kernel.org
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 <hannemann@nets.rwth-aachen.de>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] mac80211: Fix output of minstrels rc_stats
2009-08-24 18:36 ` Arnd Hannemann
@ 2009-08-24 18:38 ` Arnd Hannemann
0 siblings, 0 replies; 9+ messages in thread
From: Arnd Hannemann @ 2009-08-24 18:38 UTC (permalink / raw)
To: Pavel Roskin; +Cc: Joe Perches, Arnd Hannemann, linux-wireless@vger.kernel.org
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 <hannemann@nets.rwth-aachen.de>
>>>> ---
>>>> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] mac80211: Fix output of minstrels rc_stats
2009-08-24 18:20 ` Pavel Roskin
2009-08-24 18:36 ` Arnd Hannemann
@ 2009-08-24 18:51 ` Arnd Hannemann
2009-09-01 0:54 ` Julian Calaby
1 sibling, 1 reply; 9+ messages in thread
From: Arnd Hannemann @ 2009-08-24 18:51 UTC (permalink / raw)
To: linux-wireless; +Cc: joe, proski, Arnd Hannemann
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 <hannemann@nets.rwth-aachen.de>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mac80211: Fix output of minstrels rc_stats
2009-08-24 18:51 ` [PATCH v2] " Arnd Hannemann
@ 2009-09-01 0:54 ` Julian Calaby
2009-09-01 8:05 ` Arnd Hannemann
0 siblings, 1 reply; 9+ messages in thread
From: Julian Calaby @ 2009-09-01 0:54 UTC (permalink / raw)
To: Arnd Hannemann; +Cc: linux-wireless, joe, proski
On Tue, Aug 25, 2009 at 04:51, Arnd
Hannemann<hannemann@nets.rwth-aachen.de> 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 <hannemann@nets.rwth-aachen.de>
> ---
> 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: julian.calaby@gmail.com
.Plan: http://sites.google.com/site/juliancalaby/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mac80211: Fix output of minstrels rc_stats
2009-09-01 0:54 ` Julian Calaby
@ 2009-09-01 8:05 ` Arnd Hannemann
0 siblings, 0 replies; 9+ messages in thread
From: Arnd Hannemann @ 2009-09-01 8:05 UTC (permalink / raw)
To: Julian Calaby
Cc: Arnd Hannemann, linux-wireless@vger.kernel.org, joe@perches.com,
proski@gnu.org
Julian Calaby wrote:
> On Tue, Aug 25, 2009 at 04:51, Arnd
> Hannemann<hannemann@nets.rwth-aachen.de> 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 <hannemann@nets.rwth-aachen.de>
>> ---
>> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-09-01 8:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-24 17:42 [PATCH] mac80211: Fix output of minstrels rc_stats Arnd Hannemann
2009-08-24 17:57 ` Joe Perches
2009-08-24 18:19 ` Arnd Hannemann
2009-08-24 18:20 ` Pavel Roskin
2009-08-24 18:36 ` Arnd Hannemann
2009-08-24 18:38 ` Arnd Hannemann
2009-08-24 18:51 ` [PATCH v2] " Arnd Hannemann
2009-09-01 0:54 ` Julian Calaby
2009-09-01 8:05 ` Arnd Hannemann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).