netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mattias Nissler <mattias.nissler@gmx.de>
To: Vegard Nossum <vegard.nossum@gmail.com>
Cc: Stefano Brivio <stefano.brivio@polimi.it>,
	"John W. Linville" <linville@tuxdriver.com>,
	"David S. Miller" <davem@davemloft.net>,
	Ingo Molnar <mingo@elte.hu>,
	Pekka Enberg <penberg@cs.helsinki.fi>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rc80211_pid: Fix fast_start parameter handling
Date: Mon, 07 Jul 2008 22:39:05 +0200	[thread overview]
Message-ID: <1215463145.4416.11.camel@localhost> (raw)
In-Reply-To: <19f34abd0807071331s52dc994ao6ac70e787d88227d@mail.gmail.com>

On Mon, 2008-07-07 at 22:31 +0200, Vegard Nossum wrote:
> Hi,
> 
> On Mon, Jul 7, 2008 at 10:23 PM, Mattias Nissler <mattias.nissler@gmx.de> wrote:
> > This removes the fast_start parameter from the rc_pid parameters information
> > and instead uses the parameter macro when initializing the rc_pid state. Since the
> > parameter is only used on initialization, there is no point of making exporting
> > it via debugfs. This also fixes uninitialized memory references to the
> > fast_start and norm_offset parameters.
> >
> > Signed-off-by: Mattias Nissler <mattias.nissler@gmx.de>
> > ---
> >
> > This should fix the problem. I think this should go into mainline ASAP.
> 
> Was this code actually causing some problem/regression? If so, can we
> please have a reference to bugzilla/prior conversations in the patch
> description?

I think the problem probably didn't cause any major problem at all. At
most, the rate scaling didn't work properly. Maybe the algorithm would
even recover from the incorrect initialization (not too sure about this
one, better ask Stefano about the rate sorting implications). And no, I
have never seen any problem reports that relate to this bug. I wouldn't
call it a regression either, since this is exactly the code that went
mainline with the first version of rc80211_pid.

> 
> Was the reason for the problem known before I posted the "use of
> uninitialized memory" e-mail earlier today? If no, can you please
> sneak in a little reference to kmemcheck there in the patch
> description? The acknowledgement is sorely needed :-)

It was unknown before. Just a sec, I'll post new version of the patch
that mentions kmemcheck ;-)

Mattias


  reply	other threads:[~2008-07-07 20:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-07 11:27 Use of uninitialized memory in rate_control_pid_alloc() Vegard Nossum
2008-07-07 20:23 ` [PATCH] rc80211_pid: Fix fast_start parameter handling Mattias Nissler
2008-07-07 20:31   ` Vegard Nossum
2008-07-07 20:39     ` Mattias Nissler [this message]
2008-07-07 21:08 ` [PATCH v2] " Mattias Nissler
2008-07-10  9:58   ` Vegard Nossum
2008-07-10 10:40   ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1215463145.4416.11.camel@localhost \
    --to=mattias.nissler@gmx.de \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=mingo@elte.hu \
    --cc=netdev@vger.kernel.org \
    --cc=penberg@cs.helsinki.fi \
    --cc=stefano.brivio@polimi.it \
    --cc=vegard.nossum@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).