Re: [PATCH v2 3/3] rt2x00: do not print error when queue is full

From: Stanislaw Gruszka <sgruszka@redhat.com>
To: Tom Psyborg <pozega.tomislav@gmail.com>
Cc: linux-wireless@vger.kernel.org,
	Randy Oostdyk <linux-kernel@oostdyk.com>,
	Daniel Golle <daniel@makrotopia.org>,
	Felix Fietkau <nbd@nbd.name>, Mathias Kresin <dev@kresin.me>
Subject: Re: [PATCH v2 3/3] rt2x00: do not print error when queue is full
Date: Thu, 27 Dec 2018 11:25:36 +0100	[thread overview]
Message-ID: <20181227102535.GA25885@redhat.com> (raw)
In-Reply-To: <CAKR_QVKQMcGGpY_3ur9kFjuYpaKgca=YfUJxjSjLEfP1HELuJw@mail.gmail.com>

On Sat, Dec 22, 2018 at 02:12:13PM +0100, Tom Psyborg wrote:
> On 21/12/2018, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> > On Thu, Dec 20, 2018 at 06:52:03PM +0100, Tom Psyborg wrote:
> >> Shouldn't you now also revert the commits from
> >> https://git.openwrt.org/?p=openwrt/staging/dangole.git;a=commitdiff;h=de1c58a64bd66319e770d2587da07d8c9c90174a
> >> since they caused throughput regression?
> >
> > For the record: this is not about currently posted patch and you
> > reference this:
> > https://lore.kernel.org/linux-wireless/1538697102-3764-1-git-send-email-pozega.tomislav@gmail.com/
> >
> > I'm not convinced for reverting. I would rater fix the problem
> > on top of the patches.
> >
> 
> Let me get this straight: You made these patches to fix wlan stalls
> when "arrived at non-free entry" error message appears, but in turn
> they result in lower throughput. Now you are moving err printks to dbg
> level since the printks themselfs are causing wlan problems. Why would
> you waste life fixing patches that don't do any good? Just revert all
> of them!

I move prints to debug level for for "arrived at non-free entry" from
"Dropping frame due to full tx queue" . For some users those 5 patches
improve things, they fix problem of router connection hung completely.
Printk problem is connection stall due to CPU being busy, but 
is possible to reconnect to AP after that. It's different issue.

Also 'result in lower throughput' is over-generalized statement. More
precise would be 'it lower throughput on some test cases', but even
in those specific cases, throughput vary randomly with and without
patches.

> > Looking at those images, the performance vary also on "good" case and
> > there is entry 12.6 Mbits/s there, where the lowest entry on "bad" case
> > is 32.5 Mbits/s . However, yes, if performance do not drop for unknown
> > reason, it is 51.4 Mbits/s with patches and 56.6 Mbits/s on good case.
> >
> > Could you specify after applying which one of those 5 patches
> > throughput regression starts ?
> >
> > Thanks
> > Stanislaw
> >
> 
> I cannot test that ATM but the best would be to revert all of them.

Ok, could anyone else confirm this throughput regression and point
offending commit ?

> And, for the future reference, please try sending them to the
> openwrt-devel list first for testing before deciding to go upstream.

Sure, I can send patches to openwrt-devel. However, for the record, 
I informed about those patches interested people including you
in August:
https://lore.kernel.org/linux-wireless/20180815114029.GA1862@redhat.com/
month before I submitted them upstream.

Thanks
Stanislaw