Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 18/24] sctp: Remove unnecessary OOM logging messages
From: Joe Perches @ 2011-08-30  1:21 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Vlad Yasevich, Sridhar Samudrala, David S. Miller, linux-sctp,
	netdev, linux-kernel
In-Reply-To: <1314654209.2563.7.camel@edumazet-laptop>

On Mon, 2011-08-29 at 23:43 +0200, Eric Dumazet wrote:
> Le lundi 29 août 2011 à 14:17 -0700, Joe Perches a écrit :
> > Removing unnecessary messages saves code and text.
> > Site specific OOM messages are duplications of a generic MM
> > out of memory message and aren't really useful, so just
> > delete them.
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> >  net/sctp/protocol.c |    3 ---
> >  1 files changed, 0 insertions(+), 3 deletions(-)
> > diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
[]
> > @@ -1326,7 +1326,6 @@ SCTP_STATIC __init int sctp_init(void)
> >  			__get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order);
> >  	} while (!sctp_assoc_hashtable && --order > 0);
> >  	if (!sctp_assoc_hashtable) {
> > -		pr_err("Failed association hash alloc\n");
> >  		status = -ENOMEM;
> >  		goto err_ahash_alloc;
> >  	}
[]
> > @@ -1359,7 +1357,6 @@ SCTP_STATIC __init int sctp_init(void)
> >  			__get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order);
> >  	} while (!sctp_port_hashtable && --order > 0);
> >  	if (!sctp_port_hashtable) {
> > -		pr_err("Failed bind hash alloc\n");
> >  		status = -ENOMEM;
> >  		goto err_bhash_alloc;
> >  	}
> It would be nice if you could avoid all these patches, that you dont
> even read.

Didn't read is not the same thing as didn't notice.

> As I already told you in the past, __GFP_NOWARN dont print generic OOM
> messages.

I didn't notice those had GFP_NOWARN.

> Its not because I told Wang Shaoyan not adding a useless "pr_err("Out of
> memory\n");" in last gianfar patch, that you have to remove all
> messages, with one hundred or more patches.

> If I remember well, you even disagreed at that time.

No, what I said was that it'd be better to get agreement
to delete them before deleting them.

https://lkml.org/lkml/2011/8/9/379

So I submitted an RFC and cc'd you.
You did not reply.

https://lkml.org/lkml/2011/8/25/580

> Furthermore, a failed vmalloc() is not guaranteed to emit an OOM
> message, is it ?

Doesn't seem to be, perhaps it should be
when __GFP_NOWARN is not set...

^ permalink raw reply

* Re: BQL crap and wireless
From: Dave Taht @ 2011-08-30  1:08 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Tom Herbert, linux-wireless, Andrew McGregor, Matt Smith,
	Kevin Hayes, Derek Smithies, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAB=NE6VwWu5+Hk7=3ghkgiXss9kCqGyS-RWPYtyHRhDsx5r2rA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

> 2. Constant small drops in throughput
> =============================

Huge drops in throughput, actually.

> How to explain this? I have no clue. Two current theories:
>
> a. Dynamic Power save
> b. Offchannel operations on bgscans
> c. Bufferbloat: large hw queue size and sw retries
>
> One can rule out (a) and (b) by disabling Dynamic Power Save (iw dev
> wlan0 power_save off) and also bg scans. If its (c) then we can work
> our way up to proving a solution with the same fixes for the first
> latency issue. But there are more subtle issues here.

(in part, your) recent analysis of the periodicity of bandwidth drops
does point
strongly to points a and b being mis-implemented by many vendors and OS-makers,
and shows a clear path toward further testing by several labs.

> Bufferbloat
> folks talk about "ants" and "elephants". They call "Elephants" as
> frames that are just data, but "ants" are small frames that build make
> the networks work --

This is an over-simplification of a complex issue, and wrong.

Elephants are very large *streams* of tcp data, the kind you get after
a few seconds of operation, and mice are short ones, the kind you
commonly get with non-pipelined http protocols and normal websites.

As noted elsewhere, with tons of reference material on google and citeseer,
"tcp mice and elephants" are a well researched and understood topic -
or at least were, back when network research still happened, which
was prior to the explosion and predominance of wireless.

The need for AQMs such as RED, Blue, etc, to manage elephants was
well demonstrated back in  the early 90s. Being able to shoot/slow down
the elephants on a sane basis is also required to have sane shared
network behavior.

There has been a huge shift in TCP flows with first the advent of the
web for all things, and more recently, back again, with the rise of video.

> so consider 802.11 management frames, and TCP
> ACKs, and so forth.

Um, actually, if TCP acks are 'ANT's, it's news to me, and I co-coined the term.

There is one special case, commonly implemented in cable modems, of
Syn and Syn/Ack optimization, that might pay off to some extent in wireless.

Upstream ACK prioritization does seem to help when it comes to
managing interactive
flows on asymmetric (e.g. home) networks as (for example) demonstrated by
wondershaper and other QoS mechanism.

But anything involving TCP fits into the well defined mice and
elephant categories
already, not ANTs.

I'm told that most management frames are in a separately managed queue already.

> They argue we should prioritize these more and
> ensure we use whatever techniques we can to ensure we reduce latency
> for them.

ARP, DHCP, ND, RA, and various forms of ipv6's ping based protocols,
most routing protocols
in many aspects, numerous other non-tcp protocols, all count as ANTs.

> At least on ath9k we only aggregate data frames, but that
> doesn't mean we are not aggregating other "ant" frames.

The approach that we are experimenting with now, is putting
ANT-like packets into the VO and VI queues as appropriate and out
of the BE or BK queues.

Too early to publish preliminary results, but I am encouraged by what
I've seen so far.

It's interesting to note that a lot of web traffic is actually marked
as bulk traffic (tos of bulk)
which ends up in the wireless BK queue, rather than BE.




-- 
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
http://the-edge.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Unicast hash for IXGBEVF driver
From: J.Hwan Kim @ 2011-08-30  1:05 UTC (permalink / raw)
  To: netdev
In-Reply-To: <43F901BD926A4E43B106BF17856F0755019454265E@orsmsx508.amr.corp.intel.com>

On 2011년 08월 30일 03:24, Rose, Gregory V wrote:
> The ROPE bits should not be set for any of the VFs and only on the PF when it is in promiscuous mode.
>
> The PFUTA bits are imperfect MAC address hash filters.  By setting them all to ones and then turning on the ROPE bit for each of the VFs you're essentially putting the VF into a sort of fake promiscuous mode.
>
> This is not recommended.  Older drivers that did this had a bug in them.
>
> - Greg

Hi, Greg,
Thank you for reply.
When I have two streams of which destination mac address, for example 
34:22:11:13:23:55 and 14:12:34:23:12:33,
and I have 2VFs, how can I set the PFUTA register for distributting the 
streams into 2VFs according to the MAC address?
I want to first MAC address stream to first VF and second MAC address 
stream to second VF.



>> -----Original Message-----
>> From: tarbal@gmail.com [mailto:tarbal@gmail.com] On Behalf Of Jeff Kirsher
>> Sent: Saturday, August 27, 2011 12:35 AM
>> To: J.Hwan Kim; Rose, Gregory V
>> Cc: netdev; e1000-devel@lists.sourceforge.net
>> Subject: Re: Unicast hash for IXGBEVF driver
>>
>> On Sat, Aug 27, 2011 at 00:23, J.Hwan Kim<frog1120@gmail.com>  wrote:
>>> Hi, everyone
>>>
>>> How can I distribute the packets according to destination MAC address
>>> into multi-virtual fucntion queue?
>>> Now, my setting is that all bit of PFUTA are '1' and ROPE bit is 1,
>>> so all mac packet is duplicated to all VF queue.
>>> I cannot understand the meaning of bits of PFUTA and the relation
>>> with mac address.
>>> I want to distribute the received packets to RX queues respectively,
>>> not duplicated.
>>>
>>>
>>> Thanks in advance.
>>>
>>> Best Regards,
>>> J.Hwan Kim
>>>
>> Adding Greg Rose, since he is the ixgbevf driver maintainer.  He
>> should be able to answer your questions early next week, unless he is
>> checking his email over the weekend.
>>
>> --
>> Cheers,
>> Jeff

^ permalink raw reply

* Re: BQL crap and wireless
From: Dave Taht @ 2011-08-30  0:24 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Tom Herbert, linux-wireless, Andrew McGregor, Matt Smith,
	Kevin Hayes, Derek Smithies, netdev
In-Reply-To: <CAB=NE6VwWu5+Hk7=3ghkgiXss9kCqGyS-RWPYtyHRhDsx5r2rA@mail.gmail.com>

On Mon, Aug 29, 2011 at 2:02 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> On Fri, Aug 26, 2011 at 4:27 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:

> Let me elaborate on 802.11 and bufferbloat as so far I see only crap
> documentation on this and also random crap adhoc patches.

I agree that the research into bufferbloat has been an evolving topic, and
the existing documentation and solutions throughout the web is inaccurate
 or just plan wrong in many respects. While I've been accumulating better
and more interesting results as research continues, we're not there yet...

> Given that I
> see effort on netdev to try to help with latency issues its important
> for netdev developers to be aware of what issues we do face today and
> what stuff is being mucked with.

Hear, Hear!

> As far as I see it I break down the issues into two categories:
>
>  * 1. High latencies on ping
>  * 2. Constant small drops in throughput

I'll take on 2, in a separate email.

>
>  1. High latencies on ping
> ===================

For starters, no, "high - and wildly varying - latencies on all sorts
of packets".

Ping is merely a diagnostic tool in this case.

If you would like several gb of packet captures of all sorts of streams
from various places and circumstances, ask. JG published a long
series about 7 months back, more are coming.

Regrettably most of the most recent traces come from irreproducible
circumstances, a flaw we are trying to fix after 'CeroWrt' is finished.

> It seems the bufferbloat folks are blaming the high latencies on our
> obsession on modern hardware to create huge queues and also with
> software retries. They assert that reducing the queue length
> (ATH_MAX_QDEPTH on ath9k) and software retries (ATH_MAX_SW_RETRIES on
> ath9k) helps with latencies. They have at least empirically tested
> this with ath9k with
> a simple patch:
>
> https://www.bufferbloat.net/attachments/43/580-ath9k_lowlatency.patch
>
> The obvious issue with this approach is it assumes STA mode of
> operation, with an AP you do not want to reduce the queue size like
> that. In fact because of the dynamic nature of 802.11 and the

If there is any one assumption about the bufferbloat issue that people
keep assuming we have, it's this one.

In article after article, in blog post after blog post, people keep
'fixing' bufferbloat by setting their queues to very low values,
and almost miraculously start seeing their  QoS start working
(which it does), and then they gleefully publish their results
 as recommendations, and then someone from the bufferbloat
effort has to go and comment on that piece, whenever we
notice, to straighten them out.

In no presentation, no documentation, anywhere I know of,
have we expressed  that queuing as it works today
is the right thing.

More recently, JG got fed up and wrote these...

http://gettys.wordpress.com/2011/07/06/rant-warning-there-is-no-single-right-answer-for-buffering-ever/

http://gettys.wordpress.com/2011/07/09/rant-warning-there-is-no-single-right-answer-for-buffering-ever-part-2/

There has been no time, since the inception of the bufferbloat
concept, have we had a fixed buffer size in any layer of the
stack as even a potential solution.

And you just did applied that preconception to us again.

My take on matters is that *unmanaged* buffer sizes > 1 is a
problem. Others set the number higher.

Of late, given what tools we have, we HAVE been trying to establish
what *good baseline* queue sizes (txqueues, driver queues, etc)
actually are for wireless under ANY circumstance that was
duplicate-able.

For the drivers JG was using last year, that answer was: 0.

Actually, less than 0  would have been good, but that
would have involved having tachyon emitters in the
architecture.

For the work that felix and andrew recently performed on the ath9k,
it looks to be about 37 for STA mode, but further testing is required
and is taking place... as well as instrumentation of the tcp stack, changes
to the system clock interrupt,  and a horde of other things.

As for AP performance... don't know. Am testing, capturing streams,
testing contention, building up labs, and creating a distro to deploy
to the field to test everything with.


> different modes of operation it is a hard question to solve on what
> queue size you should have. The BQL effort seems to try to unify a
> solution but obviously did not consider 802.11's complexities.

I only noticed the presentation and thread a few days ago. I do happen
to like byte, rather than packet limits, as a start towards sanity, but
framing overhead is still a problem, and an effective API and
set of servos more so.

> 802.11
> makes this very complicated given the PtP and PtMP support we have and
> random number of possible peers.
>
> Then -- we have Aggregation. At least AMPDU Aggregation seems to
> empirically deteriorate latency and bufferbloat guys seem to hate it.

I think aggregation is awesome, actually. I've said so on multiple occasions.

After reading the early work done on it, back in the early 2000s, I thought it
could not be made to work, at all. I was wrong.

The problems I have with aggregation as seemingly commenly
implemented today are:

0) Attempts to make the media look 100% perfect
1) Head of line blocking
2) Assumption that all packets are equal.
3) Co-existence with previous forms of 802.11 is difficult
4) Horrible behavior with every AQM algorithm, particularly with fair queuing

> Of course their statements are baseless and they are ignoring a lot of
> effort that went into this. Their current efforts have been to reduce
> segment size of a aggregates and this seems to help but the same

I note that some of the ongoing suggestions actually will make it statistically
more likely that stuff emerging from higher levels of the stack will be even
more aggregate-able than it is currently.

Also, it would be good to have some statistics as to how good aggregation
is currently, actually working under normal multiuser workloads.
A suggestion of felix's was to make that available via netlink broadcast.

> problem looms over this resolution -- the optimal aggregation segment
> size should be dynamic and my instincts tell me we likely need to also
> rely on a minstrel-like based algorithm for finding the optimal length.

We agree very much it should be dynamic.



-- 
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
http://the-edge.blogspot.com

^ permalink raw reply

* Re: [PATCH 19/24] sunrpc: Remove unnecessary OOM logging messages
From: Boaz Harrosh @ 2011-08-29 23:33 UTC (permalink / raw)
  To: Myklebust, Trond
  Cc: David Miller, joe-6d6DIl74uiNBDgjK7y7TUQ,
	bfields-uC3wQj2KruNg9hUCZPvPmw, neilb-l3A5Bk7waGM,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2E1EB2CF9ED1CB4AA966F0EB76EAB4430AEB4FAF-hX7t0kiaRRrlMGe9HJ1VYQK/GNPrWCqfQQ4Iyu8u01E@public.gmane.org>

On 08/29/2011 04:25 PM, Myklebust, Trond wrote:
>> -----Original Message-----
>> From: David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org]
<>
>> They've been doing this for years, so obviously they haven't bothered
>> you enough to care up to this point.
>>
>> All of this pushback is pure uneducated noise, please stop blocking
>> progress with poorly informed objections.
> 
> I can see that slub.c has the slab_out_of_memory() function that
> (although ratelimited) warns you if the allocation failed. However I
> can't find any equivalent for slab.c or slob.c.
> 

OK That would explain why I never saw it in my debugging. For some reason
I'm always using slab. 

Which one is the best for development and memory problems debugging?

> Trond

Thanks
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH 19/24] sunrpc: Remove unnecessary OOM logging messages
From: Myklebust, Trond @ 2011-08-29 23:25 UTC (permalink / raw)
  To: David Miller, bharrosh
  Cc: joe, bfields, neilb, linux-nfs, netdev, linux-kernel
In-Reply-To: <20110829.180348.1774953636752413432.davem@davemloft.net>

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Monday, August 29, 2011 6:04 PM
> To: bharrosh@panasas.com
> Cc: Myklebust, Trond; joe@perches.com; bfields@fieldses.org;
> neilb@suse.de; linux-nfs@vger.kernel.org; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 19/24] sunrpc: Remove unnecessary OOM logging
> messages
> 
> From: Boaz Harrosh <bharrosh@panasas.com>
> Date: Mon, 29 Aug 2011 14:54:59 -0700
> 
> > I have a question about that. Are the dprints going to show the
stack
> backtrace?
> 
> Yes, OOMs give full stack backtraces.
> 
> > If yes above? then I'm not sure I like it either, because am I'll be
> getting a full
> > stack backtrace for every failed allocation?
> 
> They've been doing this for years, so obviously they haven't bothered
> you
> enough to care up to this point.
> 
> All of this pushback is pure uneducated noise, please stop blocking
> progress
> with poorly informed objections.

I can see that slub.c has the slab_out_of_memory() function that
(although ratelimited) warns you if the allocation failed. However I
can't find any equivalent for slab.c or slob.c.

Trond

^ permalink raw reply

* Re: BQL crap and wireless
From: Andrew McGregor @ 2011-08-29 23:18 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Tom Herbert, linux-wireless, Matt Smith, Kevin Hayes, Dave Taht,
	Derek Smithies, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAB=NE6VwWu5+Hk7=3ghkgiXss9kCqGyS-RWPYtyHRhDsx5r2rA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>


On 30/08/2011, at 9:02 AM, Luis R. Rodriguez wrote:

> On Fri, Aug 26, 2011 at 4:27 PM, Luis R. Rodriguez <mcgrof-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> I've just read this thread:
>> 
>> http://marc.info/?t=131277868500001&r=1&w=2
>> 
>> Since its not linux-wireless I'll chime in here. It seems that you are
>> trying to write an algorithm that will work for all networking and
>> 802.11 devices. For networking is seems tough given driver
>> architecture and structure and the hope that all drivers will report
>> things in a fairly similar way. For 802.11 it was pointed out how we
>> have varying bandwidths and depending on the technology used for
>> connection (AP, 802.11s, IBSS) a different number of possible peers
>> need to be considered. 802.11 faced similar algorithmic complexities
>> with rate control and the way Andrew and Derek resolved this was to
>> not assume you could solve this problem and simply test out the water
>> by trial and error, that gave birth to the minstrel rate control
>> algorithm which Felix later rewrote for mac80211 with 802.11n support
>> [1]. Can the BQL algorithm make use of the same trial and error
>> mechanism and simply try different values and and use EWMA [2] to pick
>> the best size for the queue ?
>> 
>> [1] http://wireless.kernel.org/en/developers/Documentation/mac80211/RateControl/minstrel
>> [2] http://en.wikipedia.org/wiki/Moving_average#Exponential_moving_average
> 
> Let me elaborate on 802.11 and bufferbloat as so far I see only crap
> documentation on this and also random crap adhoc patches. Given that I
> see effort on netdev to try to help with latency issues its important
> for netdev developers to be aware of what issues we do face today and
> what stuff is being mucked with.
> 
> As far as I see it I break down the issues into two categories:
> 
> * 1. High latencies on ping
> * 2. Constant small drops in throughput
> 
> 1. High latencies on ping
> ===================
> 
> It seems the bufferbloat folks are blaming the high latencies on our
> obsession on modern hardware to create huge queues and also with
> software retries. They assert that reducing the queue length
> (ATH_MAX_QDEPTH on ath9k) and software retries (ATH_MAX_SW_RETRIES on
> ath9k) helps with latencies. They have at least empirically tested
> this with ath9k with
> a simple patch:
> 
> https://www.bufferbloat.net/attachments/43/580-ath9k_lowlatency.patch
> 
> The obvious issue with this approach is it assumes STA mode of
> operation, with an AP you do not want to reduce the queue size like
> that. In fact because of the dynamic nature of 802.11 and the
> different modes of operation it is a hard question to solve on what
> queue size you should have. The BQL effort seems to try to unify a
> solution but obviously did not consider 802.11's complexities. 802.11
> makes this very complicated given the PtP and PtMP support we have and
> random number of possible peers.
> 
> Then -- we have Aggregation. At least AMPDU Aggregation seems to
> empirically deteriorate latency and bufferbloat guys seem to hate it.
> Of course their statements are baseless and they are ignoring a lot of
> effort that went into this. Their current efforts have been to reduce
> segment size of a aggregates and this seems to help but the same
> problem looms over this resolution -- the optimal aggregation segment
> size should be dynamic and my instincts tell me we likely need to also
> rely on a minstrel-like based algorithm for finding the optimal length.

Luis, as the author of that patch... I agree entirely that we want something dynamic.  I was actually writing that assuming AP operation... and it works pretty well, but I wouldn't expect it to be anything like optimal in all situations.

It looks like the time limits (called segment_size in the code) in Minstrel-HT were just copied from Minstrel-classic, which was tuned for 11g timings.  The patch reduces the time limits and retry counts (the SW retry limit number goes up because of another patch that changes the accounting for SW retries from passes through the queue to transmitter shots, but 50 shots is way less than 20 passes through the queue).  Now, this is just a matter of having sensible defaults that are not crazily trying to use hundreds of transmit opportunities on one packet.

That patch also reduces the aggregation opportunities a bit; the driver has something approximating a target queue depth, ATH_MAX_QDEPTH, that by default was 123 packets, and I reduced it to 34.  Now, why 34?  Because at any smaller number, it's going to impact aggregation and therefore performance really badly, while at a much larger number it's going to affect latency.  I do understand why aggregation is important; the MAC has a lot of dead air in it, the scarce resource is transmit opportunities rather than bandwidth per se, and we need to get as much as reasonably possible done with each transmit opportunity.

What this patch does assume is that most of the clients have fairly decent coverage; it's going to be really unfair to .11b clients and those right out on the fringes of the APs coverage.  But, with a fairly busy AP and decent coverage, it does actually work well in practice on the AP (I run it at home, and there's a fair bit of stuff on my network...).  The demonstration is, I can have a few TCP streams running flat out from my laptop to something on the wired LAN, and still make a voip call from any device on the wireless... including the laptop (which runs OS X Lion and has a Broadcom radio, by the way, so Linux is not the only stack that does pretty well for latency).  The TCP throughput is barely affected by that patch, but the latency is dramatically improved; what was over 100 ms an
 d wildly varying (under load from TCP traffic) is now very stable and a little under 20 ms on the wireless hop.

So, yes, I do think some kind of dynamic queue sizing is called for.  Also, I think we need a queue bundle per active peer (just to cover all the operation modes... that would be just the AP in STA mode), that is dynamically sized per traffic class per peer.  Also, a decent qdisc that will do a reasonable job of getting latency-sensitive stuff into queues that are trying for latency rather than throughput.

And to the buffer bloat guys: yes, I do see how many buffers I'm proposing using here... it might be a couple or four thousand on a really busy AP.  But the point is not to eliminate buffering, it is to use it sensibly to help performance rather than hurt as it so often does at present.  The number of buffers in total doesn't matter, it's the depth of the queues seen by any particular packet passing through the network that is really critical... and too few is just as bad as too many, especially when it leads to wasting a scarce link resource, as too few buffers will do on an 802.11n AP.

> 
> 2. Constant small drops in throughput
> =============================
> 
> How to explain this? I have no clue. Two current theories:
> 
> a. Dynamic Power save
> b. Offchannel operations on bgscans
> c. Bufferbloat: large hw queue size and sw retries

d. Someone has a broken rate control algorithm, and that is causing slowdowns and packet loss.

The study that Dave was referring to earlier was done on Windows XP clients, using APs running who knows what (we know the best of them was ath9k and Minstrel-classic, because it was running code based on OpenWRT 8.something, but the others I have no idea).  There's all kinds of variables there, and I wouldn't rule out any kind of weirdness.

I think Linux in general is in pretty good shape with respect to most of these issues, what with your work on power save and scanning, and with minstrel for rate control.  Part of the bufferbloat effort is to get the awareness of queue size issues... and what we're doing here is, basically, documenting the tradeoffs and considerations in 802.11 land.

I think a distilled version of these conversations needs to be written up as an informational RFC so the IETC has something to refer to.

> 
> One can rule out (a) and (b) by disabling Dynamic Power Save (iw dev
> wlan0 power_save off) and also bg scans. If its (c) then we can work
> our way up to proving a solution with the same fixes for the first
> latency issue. But there are more subtle issues here. Bufferbloat
> folks talk about "ants" and "elephants". They call "Elephants" as
> frames that are just data, but "ants" are small frames that build make
> the networks work -- so consider 802.11 management frames, and TCP
> ACKs, and so forth. They argue we should prioritize these more and
> ensure we use whatever techniques we can to ensure we reduce latency
> for them. At least on ath9k we only aggregate data frames, but that
> doesn't mean we are not aggregating other "ant" frames. We at least
> now have in place code to not aggregate Voice Traffic -- that's good
> but we can do more. For example we can use AMSDU TX support for small
> frame. This means we'd need to prioritize AMSDU TX support, which we
> do not have support for in mac80211. I think this will help here, but
> consider queue size too -- we can likely get even better results here
> by ensuring we reduce latency further for them.
> 
> Hope this helps sum up the issue for 802.11 and what we are faced with.
> 
>  Luis

It sure does help, thanks Luis.

Andrew--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: BQL crap and wireless
From: Luis R. Rodriguez @ 2011-08-29 23:15 UTC (permalink / raw)
  To: Dave Taht
  Cc: Tom Herbert, linux-wireless, Andrew McGregor, Matt Smith,
	Kevin Hayes, Derek Smithies, netdev
In-Reply-To: <CAA93jw6Ude=x=5v2cq86UjnORXDhAQASp51zLvDDBYHB28T_Jw@mail.gmail.com>

On Mon, Aug 29, 2011 at 4:10 PM, Dave Taht <dave.taht@gmail.com> wrote:
> My goal in collecting these recordings is to get TRANSCRIPTS of what
> is important and what is not, so that ultimately better documentation
> can be written from those than exist today.
>
> Since the byte queuing issue is hot this week, I decided to make more
> people aware that these recordings existed in the hope that more
> available background information on how wireless works could be
> internalized by more people.
>
> I did note that the talk with felix was an attempt at capturing and
> summarizing some bufferbloat history and only towards the end did we
> start moving forward to things we planned to try, while the talk with
> andrew was an attempt at doing a more technical overview of quite a
> few issues.
>
> Multiple people have found these conversations useful. Certainly I
> find them useful, often reviewing them a few weeks after the fact to
> find subtleties that I missed originally, or needed further reading to
> really understand or write code for. The earliest conversation I had
> with felix (march?) still has some useful nuggets in it about how
> aggregation actually works that I hope to mine one day for better
> documentation. It also had some glaring errors in it...
> (mostly MINE!)
>
> http://mirrors.bufferbloat.net/podcasts/
>
> I would certainly like to have a (recorded!) conversation with you to
> have a constructive
> discussion of the issues you find important, correct, and essential,
> towards making wireless networking better.
>
> Many people are capable of doing otherwise useful work while having a
> podcast running in the background that might have a few percentage
> points of useful information.
>
> By all means, don't listen if you can't split the brain cells, as I
> wrote I will have transcript of that latest available and will try to
> summarize and extract the best of what was discussed, after we get
> some code written and some tests performed.
>
>  There are many other folk I would like to interview in this way, as well.

Honestly I do not have > 1 hour to review random audio on some topic
when the only meat of what I want is in 5 minutes of the > 1 hour
talk; the reason I stuck to hearing most of the talk though is I am
quite tired of the FUD around bufferbloat and also tired of the random
adhoc patches, e-mails and rants about it. My recommendation is edit
crap out and make a more condensed clip with the stuff technical folks
really care about. I'm not saying no one will find it useful, I'm
saying technical folks will puke all over it and likely not want to
even review more bufferbloat crap later. That's where I'm at least.

  Luis

^ permalink raw reply

* Re: BQL crap and wireless
From: Dave Taht @ 2011-08-29 23:10 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Tom Herbert, linux-wireless, Andrew McGregor, Matt Smith,
	Kevin Hayes, Derek Smithies, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAB=NE6V7jFgtK4HmkUnfY5MuWCJoTyo83p9KMVTvmXTsrGoTew-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

My goal in collecting these recordings is to get TRANSCRIPTS of what
is important and what is not, so that ultimately better documentation
can be written from those than exist today.

Since the byte queuing issue is hot this week, I decided to make more
people aware that these recordings existed in the hope that more
available background information on how wireless works could be
internalized by more people.

I did note that the talk with felix was an attempt at capturing and
summarizing some bufferbloat history and only towards the end did we
start moving forward to things we planned to try, while the talk with
andrew was an attempt at doing a more technical overview of quite a
few issues.

Multiple people have found these conversations useful. Certainly I
find them useful, often reviewing them a few weeks after the fact to
find subtleties that I missed originally, or needed further reading to
really understand or write code for. The earliest conversation I had
with felix (march?) still has some useful nuggets in it about how
aggregation actually works that I hope to mine one day for better
documentation. It also had some glaring errors in it...
(mostly MINE!)

http://mirrors.bufferbloat.net/podcasts/

I would certainly like to have a (recorded!) conversation with you to
have a constructive
discussion of the issues you find important, correct, and essential,
towards making wireless networking better.

Many people are capable of doing otherwise useful work while having a
podcast running in the background that might have a few percentage
points of useful information.

By all means, don't listen if you can't split the brain cells, as I
wrote I will have transcript of that latest available and will try to
summarize and extract the best of what was discussed, after we get
some code written and some tests performed.

 There are many other folk I would like to interview in this way, as well.

On Mon, Aug 29, 2011 at 3:54 PM, Luis R. Rodriguez <mcgrof-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Aug 29, 2011 at 3:45 PM, Dave Taht <dave.taht-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> http://huchra.bufferbloat.net/~d/talks/felix/
>> http://huchra.bufferbloat.net/~d/talks/andrew/
>
> I had already listened to Andrew's audio, and got up to 1 hour with
> your session with Felix but seriously considered that entire 1 hour a
> waste of my life. I think you digressed completely and rambled on
> about random crap instead of focusing on the core of the matter, I
> gave up after 1 hour.
>
>  Luis
>



-- 
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
http://the-edge.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH net-next 3/5] qlcnic: Add FLT entry for CO cards FW image region
From: Sony Chacko @ 2011-08-29 22:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Sritej Velaga
In-Reply-To: <1314658231-30735-1-git-send-email-sony.chacko@qlogic.com>

From: Sritej Velaga <sritej.velaga@qlogic.com>

The FLT entry for FW image region has changed for C0 cards.
Updated the driver to look at the right region in the FLT.

Signed-off-by: Sritej Velaga <sritej.velaga@qlogic.com>
Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h      |    4 +++-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c |    8 +++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 53c6e5d..4118502 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -73,6 +73,7 @@
 	(sizeof(struct cmd_desc_type0) * tx_ring->num_desc)
 
 #define QLCNIC_P3P_A0		0x50
+#define QLCNIC_P3P_C0		0x58
 
 #define QLCNIC_IS_REVISION_P3P(REVISION)     (REVISION >= QLCNIC_P3P_A0)
 
@@ -291,7 +292,8 @@ struct uni_data_desc{
 
 /* Flash Defines and Structures */
 #define QLCNIC_FLT_LOCATION	0x3F1000
-#define QLCNIC_FW_IMAGE_REGION	0x74
+#define QLCNIC_B0_FW_IMAGE_REGION 0x74
+#define QLCNIC_C0_FW_IMAGE_REGION 0x97
 #define QLCNIC_BOOTLD_REGION    0X72
 struct qlcnic_flt_header {
 	u16 version;
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
index b02859c..7f4b8e6 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
@@ -686,7 +686,13 @@ qlcnic_check_flash_fw_ver(struct qlcnic_adapter *adapter)
 	u32 ver = -1, min_ver;
 	int ret;
 
-	ret = qlcnic_get_flt_entry(adapter, QLCNIC_FW_IMAGE_REGION, &fw_entry);
+	if (adapter->ahw->revision_id == QLCNIC_P3P_C0)
+		ret = qlcnic_get_flt_entry(adapter, QLCNIC_C0_FW_IMAGE_REGION,
+						 &fw_entry);
+	else
+		ret = qlcnic_get_flt_entry(adapter, QLCNIC_B0_FW_IMAGE_REGION,
+						 &fw_entry);
+
 	if (!ret)
 		/* 0-4:-signature,  4-8:-fw version */
 		qlcnic_rom_fast_read(adapter, fw_entry.start_addr + 4,
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH net-next 0/5] qlcnic: Bug fixes
From: Sony Chacko @ 2011-08-29 22:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Sony Chacko
In-Reply-To: <1314658231-30735-1-git-send-email-sony.chacko@qlogic.com>

Please apply to net-next.

Thank you,
Sony Chacko

^ permalink raw reply

* [PATCH net-next 2/5] qlcnic: Change debug messages in loopback path
From: Sony Chacko @ 2011-08-29 22:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Manish chopra
In-Reply-To: <1314658231-30735-1-git-send-email-sony.chacko@qlogic.com>

From: Manish chopra <Manish.Chopra@qlogic.com>

Added more debug messages while loopback test in progress

Signed-off-by: Manish chopra <Manish.Chopra@qlogic.com>
Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
---
 .../net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c    |   15 ++++++++++-----
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c   |    6 +++---
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index 59d73f2..720b333 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -710,7 +710,7 @@ int qlcnic_check_loopback_buff(unsigned char *data, u8 mac[])
 	return memcmp(data, buff, QLCNIC_ILB_PKT_SIZE);
 }
 
-static int qlcnic_do_lb_test(struct qlcnic_adapter *adapter)
+static int qlcnic_do_lb_test(struct qlcnic_adapter *adapter, u8 mode)
 {
 	struct qlcnic_recv_context *recv_ctx = adapter->recv_ctx;
 	struct qlcnic_host_sds_ring *sds_ring = &recv_ctx->sds_rings[0];
@@ -736,13 +736,18 @@ static int qlcnic_do_lb_test(struct qlcnic_adapter *adapter)
 		dev_kfree_skb_any(skb);
 
 		if (!adapter->diag_cnt)
-			dev_warn(&adapter->pdev->dev, "LB Test: %dth packet"
-				" not recevied\n", i + 1);
+			QLCDB(adapter, DRV,
+			"LB Test: packet #%d was not received\n", i + 1);
 		else
 			cnt++;
 	}
 	if (cnt != i) {
 		dev_warn(&adapter->pdev->dev, "LB Test failed\n");
+		if (mode != QLCNIC_ILB_MODE) {
+			dev_warn(&adapter->pdev->dev,
+				"WARNING: Please make sure external"
+				"loopback connector is plugged in\n");
+		}
 		return -1;
 	}
 	return 0;
@@ -761,7 +766,7 @@ static int qlcnic_loopback_test(struct net_device *netdev, u8 mode)
 		return -EOPNOTSUPP;
 	}
 
-	netdev_info(netdev, "%s loopback test in progress\n",
+	QLCDB(adapter, DRV, "%s loopback test in progress\n",
 		   mode == QLCNIC_ILB_MODE ? "internal" : "external");
 	if (adapter->op_mode == QLCNIC_NON_PRIV_FUNC) {
 		netdev_warn(netdev, "Loopback test not supported for non "
@@ -797,7 +802,7 @@ static int qlcnic_loopback_test(struct net_device *netdev, u8 mode)
 		}
 	} while (!QLCNIC_IS_LB_CONFIGURED(adapter->ahw->loopback_state));
 
-	ret = qlcnic_do_lb_test(adapter);
+	ret = qlcnic_do_lb_test(adapter, mode);
 
 	qlcnic_clear_lb_mode(adapter);
 
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
index 3b6741e..b02859c 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
@@ -1779,14 +1779,14 @@ qlcnic_post_rx_buffers_nodb(struct qlcnic_adapter *adapter,
 	spin_unlock(&rds_ring->lock);
 }
 
-static void dump_skb(struct sk_buff *skb)
+static void dump_skb(struct sk_buff *skb, struct qlcnic_adapter *adapter)
 {
 	int i;
 	unsigned char *data = skb->data;
 
 	printk(KERN_INFO "\n");
 	for (i = 0; i < skb->len; i++) {
-		printk(KERN_INFO "%02x ", data[i]);
+		QLCDB(adapter, DRV, "%02x ", data[i]);
 		if ((i & 0x0f) == 8)
 			printk(KERN_INFO "\n");
 	}
@@ -1829,7 +1829,7 @@ void qlcnic_process_rcv_diag(struct qlcnic_adapter *adapter,
 	if (!qlcnic_check_loopback_buff(skb->data, adapter->mac_addr))
 		adapter->diag_cnt++;
 	else
-		dump_skb(skb);
+		dump_skb(skb, adapter);
 
 	dev_kfree_skb_any(skb);
 	adapter->stats.rx_pkts++;
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH net-next 5/5] qlcnic: add beacon test support.
From: Sony Chacko @ 2011-08-29 22:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Sucheta Chakraborty
In-Reply-To: <1314658231-30735-1-git-send-email-sony.chacko@qlogic.com>

From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>

Beacon test flashes both port LEDs instead of just 1 LED of a port.
Updated driver version to 5.0.23.

Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h        |   10 ++-
 .../net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c    |    5 +
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c   |   95 ++++++++++++++++++++
 3 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 2a3e552..04c7015 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -36,8 +36,8 @@
 
 #define _QLCNIC_LINUX_MAJOR 5
 #define _QLCNIC_LINUX_MINOR 0
-#define _QLCNIC_LINUX_SUBVERSION 22
-#define QLCNIC_LINUX_VERSIONID  "5.0.22"
+#define _QLCNIC_LINUX_SUBVERSION 23
+#define QLCNIC_LINUX_VERSIONID  "5.0.23"
 #define QLCNIC_DRV_IDC_VER  0x01
 #define QLCNIC_DRIVER_VERSION  ((_QLCNIC_LINUX_MAJOR << 16) |\
 		 (_QLCNIC_LINUX_MINOR << 8) | (_QLCNIC_LINUX_SUBVERSION))
@@ -457,6 +457,8 @@ struct qlcnic_hardware_context {
 	u16 port_type;
 	u16 board_type;
 
+	u8 beacon_state;
+
 	struct qlcnic_nic_intr_coalesce coal;
 	struct qlcnic_fw_dump fw_dump;
 };
@@ -931,6 +933,7 @@ struct qlcnic_ipaddr {
 #define __QLCNIC_START_FW 		4
 #define __QLCNIC_AER			5
 #define __QLCNIC_DIAG_RES_ALLOC		6
+#define __QLCNIC_LED_ENABLE		7
 
 #define QLCNIC_INTERRUPT_TEST		1
 #define QLCNIC_LOOPBACK_TEST		2
@@ -1397,6 +1400,9 @@ void qlcnic_pcie_sem_unlock(struct qlcnic_adapter *, int);
 #define crb_win_unlock(a)	\
 	qlcnic_pcie_sem_unlock((a), 7)
 
+#define __QLCNIC_MAX_LED_RATE	0xf
+#define __QLCNIC_MAX_LED_STATE	0x2
+
 int qlcnic_get_board_info(struct qlcnic_adapter *adapter);
 int qlcnic_wol_supported(struct qlcnic_adapter *adapter);
 int qlcnic_config_led(struct qlcnic_adapter *adapter, u32 state, u32 rate);
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index 2230a62..b127f80 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -939,6 +939,9 @@ static int qlcnic_set_led(struct net_device *dev,
 
 	switch (state) {
 	case ETHTOOL_ID_ACTIVE:
+		if (test_and_set_bit(__QLCNIC_LED_ENABLE, &adapter->state))
+			return -EBUSY;
+
 		if (!test_bit(__QLCNIC_DEV_UP, &adapter->state)) {
 			if (test_and_set_bit(__QLCNIC_RESETTING, &adapter->state))
 				return -EIO;
@@ -973,6 +976,8 @@ static int qlcnic_set_led(struct net_device *dev,
 		clear_bit(__QLCNIC_RESETTING, &adapter->state);
 	}
 
+	clear_bit(__QLCNIC_LED_ENABLE, &adapter->state);
+
 	return -EIO;
 }
 
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 998bb1d..690c93f 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -3466,6 +3466,98 @@ int qlcnic_set_max_rss(struct qlcnic_adapter *adapter, u8 data)
 }
 
 static int
+qlcnic_validate_beacon(struct qlcnic_adapter *adapter, u16 beacon, u8 *state,
+			u8 *rate)
+{
+	*rate = LSB(beacon);
+	*state = MSB(beacon);
+
+	QLCDB(adapter, DRV, "rate %x state %x\n", *rate, *state);
+
+	if (!*state) {
+		*rate = __QLCNIC_MAX_LED_RATE;
+		return 0;
+	} else if (*state > __QLCNIC_MAX_LED_STATE)
+		return -EINVAL;
+
+	if ((!*rate) || (*rate > __QLCNIC_MAX_LED_RATE))
+		return -EINVAL;
+
+	return 0;
+}
+
+static ssize_t
+qlcnic_store_beacon(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+{
+	struct qlcnic_adapter *adapter = dev_get_drvdata(dev);
+	int max_sds_rings = adapter->max_sds_rings;
+	int dev_down = 0;
+	u16 beacon;
+	u8 b_state, b_rate;
+	int err;
+
+	if (len != sizeof(u16))
+		return QL_STATUS_INVALID_PARAM;
+
+	memcpy(&beacon, buf, sizeof(u16));
+	err = qlcnic_validate_beacon(adapter, beacon, &b_state, &b_rate);
+	if (err)
+		return err;
+
+	if (adapter->ahw->beacon_state == b_state)
+		return len;
+
+	if (!adapter->ahw->beacon_state)
+		if (test_and_set_bit(__QLCNIC_LED_ENABLE, &adapter->state))
+			return -EBUSY;
+
+	if (!test_bit(__QLCNIC_DEV_UP, &adapter->state)) {
+		if (test_and_set_bit(__QLCNIC_RESETTING, &adapter->state))
+			return -EIO;
+		err = qlcnic_diag_alloc_res(adapter->netdev, QLCNIC_LED_TEST);
+		if (err) {
+			clear_bit(__QLCNIC_RESETTING, &adapter->state);
+			clear_bit(__QLCNIC_LED_ENABLE, &adapter->state);
+			return err;
+		}
+		dev_down = 1;
+	}
+
+	err = qlcnic_config_led(adapter, b_state, b_rate);
+
+	if (!err) {
+		adapter->ahw->beacon_state = b_state;
+		err = len;
+	}
+
+	if (dev_down) {
+		qlcnic_diag_free_res(adapter->netdev, max_sds_rings);
+		clear_bit(__QLCNIC_RESETTING, &adapter->state);
+	}
+
+	if (!b_state)
+		clear_bit(__QLCNIC_LED_ENABLE, &adapter->state);
+
+	return err;
+}
+
+static ssize_t
+qlcnic_show_beacon(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct qlcnic_adapter *adapter = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", adapter->ahw->beacon_state);
+}
+
+static struct device_attribute dev_attr_beacon = {
+	.attr = {.name = "beacon", .mode = (S_IRUGO | S_IWUSR)},
+	.show = qlcnic_show_beacon,
+	.store = qlcnic_store_beacon,
+};
+
+static int
 qlcnic_sysfs_validate_crb(struct qlcnic_adapter *adapter,
 		loff_t offset, size_t size)
 {
@@ -4162,6 +4254,8 @@ qlcnic_create_diag_entries(struct qlcnic_adapter *adapter)
 		return;
 	if (device_create_file(dev, &dev_attr_diag_mode))
 		dev_info(dev, "failed to create diag_mode sysfs entry\n");
+	if (device_create_file(dev, &dev_attr_beacon))
+		dev_info(dev, "failed to create beacon sysfs entry");
 	if (device_create_bin_file(dev, &bin_attr_crb))
 		dev_info(dev, "failed to create crb sysfs entry\n");
 	if (device_create_bin_file(dev, &bin_attr_mem))
@@ -4192,6 +4286,7 @@ qlcnic_remove_diag_entries(struct qlcnic_adapter *adapter)
 	if (adapter->op_mode == QLCNIC_NON_PRIV_FUNC)
 		return;
 	device_remove_file(dev, &dev_attr_diag_mode);
+	device_remove_file(dev, &dev_attr_beacon);
 	device_remove_bin_file(dev, &bin_attr_crb);
 	device_remove_bin_file(dev, &bin_attr_mem);
 	device_remove_bin_file(dev, &bin_attr_pci_config);
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH net-next 4/5] qlcnic: fix cdrp race condition
From: Sony Chacko @ 2011-08-29 22:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Sritej Velaga
In-Reply-To: <1314658231-30735-1-git-send-email-sony.chacko@qlogic.com>

From: Sritej Velaga <sritej.velaga@qlogic.com>

Reading CRB registers(if reqd) before releasing the api lock.

Signed-off-by: Sritej Velaga <sritej.velaga@qlogic.com>
Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h        |    3 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c    |  116 +++++++++++++++-----
 .../net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c    |    4 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c   |    4 +-
 4 files changed, 95 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 4118502..2a3e552 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -1465,7 +1465,8 @@ int qlcnic_check_loopback_buff(unsigned char *data, u8 mac[]);
 /* Functions from qlcnic_main.c */
 int qlcnic_reset_context(struct qlcnic_adapter *);
 u32 qlcnic_issue_cmd(struct qlcnic_adapter *adapter,
-	u32 pci_fn, u32 version, u32 arg1, u32 arg2, u32 arg3, u32 cmd);
+	u32 pci_fn, u32 version, u32 arg1, u32 arg2, u32 arg3, u32 cmd,
+		u32 *rd_args[3]);
 void qlcnic_diag_free_res(struct net_device *netdev, int max_sds_rings);
 int qlcnic_diag_alloc_res(struct net_device *netdev, int test);
 netdev_tx_t qlcnic_xmit_frame(struct sk_buff *skb, struct net_device *netdev);
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
index b0d32dd..e0c85a5 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
@@ -28,7 +28,8 @@ qlcnic_poll_rsp(struct qlcnic_adapter *adapter)
 
 u32
 qlcnic_issue_cmd(struct qlcnic_adapter *adapter,
-	u32 pci_fn, u32 version, u32 arg1, u32 arg2, u32 arg3, u32 cmd)
+	u32 pci_fn, u32 version, u32 arg1, u32 arg2, u32 arg3, u32 cmd,
+		u32 *rd_args[3])
 {
 	u32 rsp;
 	u32 signature;
@@ -56,7 +57,14 @@ qlcnic_issue_cmd(struct qlcnic_adapter *adapter,
 		rcode = QLCRD32(adapter, QLCNIC_ARG1_CRB_OFFSET);
 		dev_err(&pdev->dev, "failed card response code:0x%x\n",
 				rcode);
+	} else if (rsp == QLCNIC_CDRP_RSP_OK) {
+		if (rd_args[1])
+			*rd_args[1] = QLCRD32(adapter, QLCNIC_ARG2_CRB_OFFSET);
+		if (rd_args[2])
+			*rd_args[2] = QLCRD32(adapter, QLCNIC_ARG3_CRB_OFFSET);
 	}
+	if (rd_args[0])
+		*rd_args[0] = QLCRD32(adapter, QLCNIC_ARG1_CRB_OFFSET);
 
 	/* Release semaphore */
 	qlcnic_api_unlock(adapter);
@@ -80,28 +88,30 @@ int qlcnic_fw_cmd_get_minidump_temp(struct qlcnic_adapter *adapter)
 	int err, i;
 	u16 temp_size;
 	void *tmp_addr;
-	u32 version, csum, *template, *tmp_buf;
+	u32 version, csum, *template, *tmp_buf, rsp;
+	u32 *rd_args[3];
 	struct qlcnic_hardware_context *ahw;
 	struct qlcnic_dump_template_hdr *tmpl_hdr, *tmp_tmpl;
 	dma_addr_t tmp_addr_t = 0;
 
 	ahw = adapter->ahw;
+	rd_args[0] = &rsp;
+	rd_args[1] = (u32 *) &temp_size;
+	rd_args[2] = &version;
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			0,
 			0,
 			0,
-			QLCNIC_CDRP_CMD_TEMP_SIZE);
+			QLCNIC_CDRP_CMD_TEMP_SIZE,
+			rd_args);
 	if (err != QLCNIC_RCODE_SUCCESS) {
-		err = QLCRD32(adapter, QLCNIC_ARG1_CRB_OFFSET);
 		dev_info(&adapter->pdev->dev,
-			"Can't get template size %d\n", err);
+			"Can't get template size %d\n", rsp);
 		err = -EIO;
 		return err;
 	}
-	version = QLCRD32(adapter, QLCNIC_ARG3_CRB_OFFSET);
-	temp_size = QLCRD32(adapter, QLCNIC_ARG2_CRB_OFFSET);
 	if (!temp_size)
 		return -EIO;
 
@@ -112,13 +122,15 @@ int qlcnic_fw_cmd_get_minidump_temp(struct qlcnic_adapter *adapter)
 			"Can't get memory for FW dump template\n");
 		return -ENOMEM;
 	}
+	memset(rd_args, 0, sizeof(rd_args));
 	err = qlcnic_issue_cmd(adapter,
 		adapter->ahw->pci_func,
 		adapter->fw_hal_version,
 		LSD(tmp_addr_t),
 		MSD(tmp_addr_t),
 		temp_size,
-		QLCNIC_CDRP_CMD_GET_TEMP_HDR);
+		QLCNIC_CDRP_CMD_GET_TEMP_HDR,
+		rd_args);
 
 	if (err != QLCNIC_RCODE_SUCCESS) {
 		dev_err(&adapter->pdev->dev,
@@ -155,8 +167,10 @@ error:
 int
 qlcnic_fw_cmd_set_mtu(struct qlcnic_adapter *adapter, int mtu)
 {
+	u32 *rd_args[3];
 	struct qlcnic_recv_context *recv_ctx = adapter->recv_ctx;
 
+	memset(rd_args, 0, sizeof(rd_args));
 	if (recv_ctx->state == QLCNIC_HOST_CTX_STATE_ACTIVE) {
 		if (qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
@@ -164,7 +178,8 @@ qlcnic_fw_cmd_set_mtu(struct qlcnic_adapter *adapter, int mtu)
 			recv_ctx->context_id,
 			mtu,
 			0,
-			QLCNIC_CDRP_CMD_SET_MTU)) {
+			QLCNIC_CDRP_CMD_SET_MTU,
+			rd_args)) {
 
 			dev_err(&adapter->pdev->dev, "Failed to set mtu\n");
 			return -EIO;
@@ -193,6 +208,7 @@ qlcnic_fw_cmd_create_rx_ctx(struct qlcnic_adapter *adapter)
 	u8 i, nrds_rings, nsds_rings;
 	size_t rq_size, rsp_size;
 	u32 cap, reg, val, reg2;
+	u32 *rd_args[3];
 	int err;
 
 	struct qlcnic_recv_context *recv_ctx = adapter->recv_ctx;
@@ -274,13 +290,15 @@ qlcnic_fw_cmd_create_rx_ctx(struct qlcnic_adapter *adapter)
 	}
 
 	phys_addr = hostrq_phys_addr;
+	memset(rd_args, 0, sizeof(rd_args));
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			(u32)(phys_addr >> 32),
 			(u32)(phys_addr & 0xffffffff),
 			rq_size,
-			QLCNIC_CDRP_CMD_CREATE_RX_CTX);
+			QLCNIC_CDRP_CMD_CREATE_RX_CTX,
+			rd_args);
 	if (err) {
 		dev_err(&adapter->pdev->dev,
 			"Failed to create rx ctx in firmware%d\n", err);
@@ -326,15 +344,18 @@ out_free_rq:
 static void
 qlcnic_fw_cmd_destroy_rx_ctx(struct qlcnic_adapter *adapter)
 {
+	u32 *rd_args[3];
 	struct qlcnic_recv_context *recv_ctx = adapter->recv_ctx;
 
+	memset(rd_args, 0, sizeof(rd_args));
 	if (qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			recv_ctx->context_id,
 			QLCNIC_DESTROY_CTX_RESET,
 			0,
-			QLCNIC_CDRP_CMD_DESTROY_RX_CTX)) {
+			QLCNIC_CDRP_CMD_DESTROY_RX_CTX,
+			rd_args)) {
 
 		dev_err(&adapter->pdev->dev,
 			"Failed to destroy rx ctx in firmware\n");
@@ -352,6 +373,7 @@ qlcnic_fw_cmd_create_tx_ctx(struct qlcnic_adapter *adapter)
 	void	*rq_addr, *rsp_addr;
 	size_t	rq_size, rsp_size;
 	u32	temp;
+	u32 *rd_args[3];
 	int	err;
 	u64	phys_addr;
 	dma_addr_t	rq_phys_addr, rsp_phys_addr;
@@ -401,13 +423,15 @@ qlcnic_fw_cmd_create_tx_ctx(struct qlcnic_adapter *adapter)
 	prq_cds->ring_size = cpu_to_le32(tx_ring->num_desc);
 
 	phys_addr = rq_phys_addr;
+	memset(rd_args, 0, sizeof(rd_args));
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			(u32)(phys_addr >> 32),
 			((u32)phys_addr & 0xffffffff),
 			rq_size,
-			QLCNIC_CDRP_CMD_CREATE_TX_CTX);
+			QLCNIC_CDRP_CMD_CREATE_TX_CTX,
+			rd_args);
 
 	if (err == QLCNIC_RCODE_SUCCESS) {
 		temp = le32_to_cpu(prsp->cds_ring.host_producer_crb);
@@ -433,13 +457,17 @@ out_free_rq:
 static void
 qlcnic_fw_cmd_destroy_tx_ctx(struct qlcnic_adapter *adapter)
 {
+	u32 *rd_args[3];
+
+	memset(rd_args, 0, sizeof(rd_args));
 	if (qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			adapter->tx_context_id,
 			QLCNIC_DESTROY_CTX_RESET,
 			0,
-			QLCNIC_CDRP_CMD_DESTROY_TX_CTX)) {
+			QLCNIC_CDRP_CMD_DESTROY_TX_CTX,
+			rd_args)) {
 
 		dev_err(&adapter->pdev->dev,
 			"Failed to destroy tx ctx in firmware\n");
@@ -449,13 +477,17 @@ qlcnic_fw_cmd_destroy_tx_ctx(struct qlcnic_adapter *adapter)
 int
 qlcnic_fw_cmd_set_port(struct qlcnic_adapter *adapter, u32 config)
 {
+	u32 *rd_args[3];
+
+	memset(rd_args, 0, sizeof(rd_args));
 	return qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			config,
 			0,
 			0,
-			QLCNIC_CDRP_CMD_CONFIG_PORT);
+			QLCNIC_CDRP_CMD_CONFIG_PORT,
+			rd_args);
 }
 
 int qlcnic_alloc_hw_resources(struct qlcnic_adapter *adapter)
@@ -620,20 +652,24 @@ void qlcnic_free_hw_resources(struct qlcnic_adapter *adapter)
 int qlcnic_get_mac_address(struct qlcnic_adapter *adapter, u8 *mac)
 {
 	int err;
-	u32 arg1;
+	u32 arg1, rd_arg1, rd_arg2;
+	u32 *rd_args[3];
 
 	arg1 = adapter->ahw->pci_func | BIT_8;
+	rd_args[0] = &rd_arg1;
+	rd_args[1] = &rd_arg2;
+	rd_args[2] = 0;
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			arg1,
 			0,
 			0,
-			QLCNIC_CDRP_CMD_MAC_ADDRESS);
+			QLCNIC_CDRP_CMD_MAC_ADDRESS,
+			rd_args);
 
 	if (err == QLCNIC_RCODE_SUCCESS)
-		qlcnic_fetch_mac(adapter, QLCNIC_ARG1_CRB_OFFSET,
-				QLCNIC_ARG2_CRB_OFFSET, 0, mac);
+		qlcnic_fetch_mac(adapter, rd_arg1, rd_arg2, 0, mac);
 	else {
 		dev_err(&adapter->pdev->dev,
 			"Failed to get mac address%d\n", err);
@@ -651,6 +687,7 @@ int qlcnic_get_nic_info(struct qlcnic_adapter *adapter,
 	dma_addr_t nic_dma_t;
 	struct qlcnic_info *nic_info;
 	void *nic_info_addr;
+	u32 *rd_args[3];
 	size_t	nic_size = sizeof(struct qlcnic_info);
 
 	nic_info_addr = dma_alloc_coherent(&adapter->pdev->dev, nic_size,
@@ -660,13 +697,15 @@ int qlcnic_get_nic_info(struct qlcnic_adapter *adapter,
 	memset(nic_info_addr, 0, nic_size);
 
 	nic_info = nic_info_addr;
+	memset(rd_args, 0, sizeof(rd_args));
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			MSD(nic_dma_t),
 			LSD(nic_dma_t),
 			(func_id << 16 | nic_size),
-			QLCNIC_CDRP_CMD_GET_NIC_INFO);
+			QLCNIC_CDRP_CMD_GET_NIC_INFO,
+			rd_args);
 
 	if (err == QLCNIC_RCODE_SUCCESS) {
 		npar_info->pci_func = le16_to_cpu(nic_info->pci_func);
@@ -705,6 +744,7 @@ int qlcnic_set_nic_info(struct qlcnic_adapter *adapter, struct qlcnic_info *nic)
 	int err = -EIO;
 	dma_addr_t nic_dma_t;
 	void *nic_info_addr;
+	u32 *rd_args[3];
 	struct qlcnic_info *nic_info;
 	size_t nic_size = sizeof(struct qlcnic_info);
 
@@ -730,13 +770,15 @@ int qlcnic_set_nic_info(struct qlcnic_adapter *adapter, struct qlcnic_info *nic)
 	nic_info->min_tx_bw = cpu_to_le16(nic->min_tx_bw);
 	nic_info->max_tx_bw = cpu_to_le16(nic->max_tx_bw);
 
+	memset(rd_args, 0, sizeof(rd_args));
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			MSD(nic_dma_t),
 			LSD(nic_dma_t),
 			((nic->pci_func << 16) | nic_size),
-			QLCNIC_CDRP_CMD_SET_NIC_INFO);
+			QLCNIC_CDRP_CMD_SET_NIC_INFO,
+			rd_args);
 
 	if (err != QLCNIC_RCODE_SUCCESS) {
 		dev_err(&adapter->pdev->dev,
@@ -754,6 +796,7 @@ int qlcnic_get_pci_info(struct qlcnic_adapter *adapter,
 				struct qlcnic_pci_info *pci_info)
 {
 	int err = 0, i;
+	u32 *rd_args[3];
 	dma_addr_t pci_info_dma_t;
 	struct qlcnic_pci_info *npar;
 	void *pci_info_addr;
@@ -767,13 +810,15 @@ int qlcnic_get_pci_info(struct qlcnic_adapter *adapter,
 	memset(pci_info_addr, 0, pci_size);
 
 	npar = pci_info_addr;
+	memset(rd_args, 0, sizeof(rd_args));
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			MSD(pci_info_dma_t),
 			LSD(pci_info_dma_t),
 			pci_size,
-			QLCNIC_CDRP_CMD_GET_PCI_INFO);
+			QLCNIC_CDRP_CMD_GET_PCI_INFO,
+			rd_args);
 
 	if (err == QLCNIC_RCODE_SUCCESS) {
 		for (i = 0; i < QLCNIC_MAX_PCI_FUNC; i++, npar++, pci_info++) {
@@ -805,6 +850,7 @@ int qlcnic_config_port_mirroring(struct qlcnic_adapter *adapter, u8 id,
 {
 	int err = -EIO;
 	u32 arg1;
+	u32 *rd_args[3];
 
 	if (adapter->op_mode != QLCNIC_MGMT_FUNC ||
 		!(adapter->eswitch[id].flags & QLCNIC_SWITCH_ENABLE))
@@ -813,13 +859,15 @@ int qlcnic_config_port_mirroring(struct qlcnic_adapter *adapter, u8 id,
 	arg1 = id | (enable_mirroring ? BIT_4 : 0);
 	arg1 |= pci_func << 8;
 
+	memset(rd_args, 0, sizeof(rd_args));
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			arg1,
 			0,
 			0,
-			QLCNIC_CDRP_CMD_SET_PORTMIRRORING);
+			QLCNIC_CDRP_CMD_SET_PORTMIRRORING,
+			rd_args);
 
 	if (err != QLCNIC_RCODE_SUCCESS) {
 		dev_err(&adapter->pdev->dev,
@@ -842,6 +890,7 @@ int qlcnic_get_port_stats(struct qlcnic_adapter *adapter, const u8 func,
 	dma_addr_t stats_dma_t;
 	void *stats_addr;
 	u32 arg1;
+	u32 *rd_args[3];
 	int err;
 
 	if (esw_stats == NULL)
@@ -865,13 +914,15 @@ int qlcnic_get_port_stats(struct qlcnic_adapter *adapter, const u8 func,
 	arg1 = func | QLCNIC_STATS_VERSION << 8 | QLCNIC_STATS_PORT << 12;
 	arg1 |= rx_tx << 15 | stats_size << 16;
 
+	memset(rd_args, 0, sizeof(rd_args));
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			arg1,
 			MSD(stats_dma_t),
 			LSD(stats_dma_t),
-			QLCNIC_CDRP_CMD_GET_ESWITCH_STATS);
+			QLCNIC_CDRP_CMD_GET_ESWITCH_STATS,
+			rd_args);
 
 	if (!err) {
 		stats = stats_addr;
@@ -952,6 +1003,7 @@ int qlcnic_clear_esw_stats(struct qlcnic_adapter *adapter, const u8 func_esw,
 {
 
 	u32 arg1;
+	u32 *rd_args[3];
 
 	if (adapter->op_mode != QLCNIC_MGMT_FUNC)
 		return -EIO;
@@ -972,13 +1024,15 @@ int qlcnic_clear_esw_stats(struct qlcnic_adapter *adapter, const u8 func_esw,
 	arg1 = port | QLCNIC_STATS_VERSION << 8 | func_esw << 12;
 	arg1 |= BIT_14 | rx_tx << 15;
 
+	memset(rd_args, 0, sizeof(rd_args));
 	return qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			arg1,
 			0,
 			0,
-			QLCNIC_CDRP_CMD_GET_ESWITCH_STATS);
+			QLCNIC_CDRP_CMD_GET_ESWITCH_STATS,
+			rd_args);
 
 err_ret:
 	dev_err(&adapter->pdev->dev, "Invalid argument func_esw=%d port=%d"
@@ -991,19 +1045,22 @@ __qlcnic_get_eswitch_port_config(struct qlcnic_adapter *adapter,
 					u32 *arg1, u32 *arg2)
 {
 	int err = -EIO;
+	u32 *rd_args[3];
 	u8 pci_func;
 	pci_func = (*arg1 >> 8);
+	rd_args[0] = arg1;
+	rd_args[1] = arg2;
+	rd_args[2] = 0;
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			*arg1,
 			0,
 			0,
-			QLCNIC_CDRP_CMD_GET_ESWITCH_PORT_CONFIG);
+			QLCNIC_CDRP_CMD_GET_ESWITCH_PORT_CONFIG,
+			rd_args);
 
 	if (err == QLCNIC_RCODE_SUCCESS) {
-		*arg1 = QLCRD32(adapter, QLCNIC_ARG1_CRB_OFFSET);
-		*arg2 = QLCRD32(adapter, QLCNIC_ARG2_CRB_OFFSET);
 		dev_info(&adapter->pdev->dev,
 			"eSwitch port config for pci func %d\n", pci_func);
 	} else {
@@ -1025,6 +1082,7 @@ int qlcnic_config_switch_port(struct qlcnic_adapter *adapter,
 {
 	int err = -EIO;
 	u32 arg1, arg2 = 0;
+	u32 *rd_args[3];
 	u8 pci_func;
 
 	if (adapter->op_mode != QLCNIC_MGMT_FUNC)
@@ -1071,13 +1129,15 @@ int qlcnic_config_switch_port(struct qlcnic_adapter *adapter,
 		return err;
 	}
 
+	memset(rd_args, 0, sizeof(rd_args));
 	err = qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
 			arg1,
 			arg2,
 			0,
-			QLCNIC_CDRP_CMD_CONFIGURE_ESWITCH);
+			QLCNIC_CDRP_CMD_CONFIGURE_ESWITCH,
+			rd_args);
 
 	if (err != QLCNIC_RCODE_SUCCESS) {
 		dev_err(&adapter->pdev->dev,
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index 720b333..2230a62 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -659,6 +659,7 @@ static int qlcnic_irq_test(struct net_device *netdev)
 	struct qlcnic_adapter *adapter = netdev_priv(netdev);
 	int max_sds_rings = adapter->max_sds_rings;
 	int ret;
+	u32 *rd_args[3];
 
 	if (test_and_set_bit(__QLCNIC_RESETTING, &adapter->state))
 		return -EIO;
@@ -668,9 +669,10 @@ static int qlcnic_irq_test(struct net_device *netdev)
 		goto clear_it;
 
 	adapter->diag_cnt = 0;
+	memset(rd_args, 0, sizeof(rd_args));
 	ret = qlcnic_issue_cmd(adapter, adapter->ahw->pci_func,
 			adapter->fw_hal_version, adapter->ahw->pci_func,
-			0, 0, 0x00000011);
+			0, 0, 0x00000011, rd_args);
 	if (ret)
 		goto done;
 
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
index 7f4b8e6..312c1c3 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
@@ -1889,8 +1889,8 @@ qlcnic_fetch_mac(struct qlcnic_adapter *adapter, u32 off1, u32 off2,
 	u32 mac_low, mac_high;
 	int i;
 
-	mac_low = QLCRD32(adapter, off1);
-	mac_high = QLCRD32(adapter, off2);
+	mac_low = off1;
+	mac_high = off2;
 
 	if (alt_mac) {
 		mac_low |= (mac_low >> 16) | (mac_high << 16);
-- 
1.6.0.2

^ permalink raw reply related

* [PATCH net-next 1/5] qlcnic: detect fan failure
From: Sony Chacko @ 2011-08-29 22:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Sony Chacko

Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
---
 .../net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c    |    1 -
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_hdr.h    |    4 ++-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c   |   29 ++++++++++++++++---
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index 7c64f2f..59d73f2 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -832,7 +832,6 @@ qlcnic_diag_test(struct net_device *dev, struct ethtool_test *eth_test,
 		data[3] = qlcnic_loopback_test(dev, QLCNIC_ILB_MODE);
 		if (data[3])
 			eth_test->flags |= ETH_TEST_FL_FAILED;
-
 		if (eth_test->flags & ETH_TEST_FL_EXTERNAL_LB) {
 			data[4] = qlcnic_loopback_test(dev, QLCNIC_ELB_MODE);
 			if (data[4])
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hdr.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hdr.h
index d14506f..92bc8ce 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hdr.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hdr.h
@@ -609,6 +609,7 @@ enum {
 	QLCNIC_TEMP_PANIC	/* Fatal error, hardware has shut down. */
 };
 
+
 /* Lock IDs for PHY lock */
 #define PHY_LOCK_DRIVER		0x44524956
 
@@ -723,7 +724,8 @@ enum {
 #define QLCNIC_RCODE_DRIVER_CAN_RELOAD		BIT_30
 #define QLCNIC_RCODE_FATAL_ERROR		BIT_31
 #define QLCNIC_FWERROR_PEGNUM(code)		((code) & 0xff)
-#define QLCNIC_FWERROR_CODE(code)		((code >> 8) & 0xfffff)
+#define QLCNIC_FWERROR_CODE(code)		((code >> 8) & 0x1fffff)
+#define QLCNIC_FWERROR_FAN_FAILURE		0x16
 
 #define FW_POLL_DELAY		(1 * HZ)
 #define FW_FAIL_THRESH		2
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index b447cc5..998bb1d 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -2928,15 +2928,36 @@ qlcnic_detach_work(struct work_struct *work)
 
 	status = QLCRD32(adapter, QLCNIC_PEG_HALT_STATUS1);
 
-	if (status & QLCNIC_RCODE_FATAL_ERROR)
+	if (status & QLCNIC_RCODE_FATAL_ERROR) {
+		dev_err(&adapter->pdev->dev,
+			"Detaching the device: peg halt status1=0x%x\n",
+					status);
+
+		if (QLCNIC_FWERROR_CODE(status) == QLCNIC_FWERROR_FAN_FAILURE) {
+			dev_err(&adapter->pdev->dev,
+			"On board active cooling fan failed. "
+				"Device has been halted.\n");
+			dev_err(&adapter->pdev->dev,
+				"Replace the adapter.\n");
+		}
+
 		goto err_ret;
+	}
 
-	if (adapter->temp == QLCNIC_TEMP_PANIC)
+	if (adapter->temp == QLCNIC_TEMP_PANIC) {
+		dev_err(&adapter->pdev->dev, "Detaching the device: temp=%d\n",
+			adapter->temp);
 		goto err_ret;
+	}
+
 	/* Dont ack if this instance is the reset owner */
 	if (!(adapter->flags & QLCNIC_FW_RESET_OWNER)) {
-		if (qlcnic_set_drv_state(adapter, adapter->dev_state))
+		if (qlcnic_set_drv_state(adapter, adapter->dev_state)) {
+			dev_err(&adapter->pdev->dev,
+				"Failed to set driver state,"
+					"detaching the device.\n");
 			goto err_ret;
+		}
 	}
 
 	adapter->fw_wait_cnt = 0;
@@ -2946,8 +2967,6 @@ qlcnic_detach_work(struct work_struct *work)
 	return;
 
 err_ret:
-	dev_err(&adapter->pdev->dev, "detach failed; status=%d temp=%d\n",
-			status, adapter->temp);
 	netif_device_attach(netdev);
 	qlcnic_clr_all_drv_state(adapter, 1);
 }
-- 
1.6.0.2

^ permalink raw reply related

* Re: linux-next: Tree for Aug 29 (bnx2x)
From: Randy Dunlap @ 2011-08-29 22:56 UTC (permalink / raw)
  To: dmitry
  Cc: Stephen Rothwell, netdev, linux-next@vger.kernel.org, LKML,
	Eilon Greenstein
In-Reply-To: <1314653744.3085.2.camel@lb-tlvb-dmitry>

On Tue, 30 Aug 2011 00:35:44 +0300 Dmitry Kravkov wrote:

> On Mon, 2011-08-29 at 13:28 -0700, Randy Dunlap wrote:
> > On Mon, 29 Aug 2011 16:19:07 +1000 Stephen Rothwell wrote:
> > 
> > > Hi all,
> > 
> > (on i386 or x86_64)
> > 
> > drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:10148: error: 'bnx2x_fcoe_get_wwn' undeclared here (not in a function)
> > 
> > 
> > Full randconfig file is attached.
> > 
> > ---
> > ~Randy
> > *** Remember to use Documentation/SubmitChecklist when testing your code ***
> This should sync #define structures between definition and declaration
> ---
> 
> Reported-by: Randy Dunlap <rdunlap@xenotime.net>
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>

Acked-by: Randy Dunlap <rdunlap@xenotime.net>

Thanks.

> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
> index f290b23..5b1f9b5 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
> @@ -522,7 +522,7 @@ void bnx2x_free_mem_bp(struct bnx2x *bp);
>   */
>  int bnx2x_change_mtu(struct net_device *dev, int new_mtu);
>  
> -#if defined(BCM_CNIC) && (defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE))
> +#if defined(NETDEV_FCOE_WWNN) && defined(BCM_CNIC)
>  /**
>   * bnx2x_fcoe_get_wwn - return the requested WWN value for this port
>   *
> -- 


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply

* Re: BQL crap and wireless
From: Luis R. Rodriguez @ 2011-08-29 22:54 UTC (permalink / raw)
  To: Dave Taht
  Cc: Tom Herbert, linux-wireless, Andrew McGregor, Matt Smith,
	Kevin Hayes, Derek Smithies, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAA93jw5pTHpUOX=iUnck8Eun2iNqkuKwG-Jd0X1oNxD_s9_prA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Mon, Aug 29, 2011 at 3:45 PM, Dave Taht <dave.taht-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> http://huchra.bufferbloat.net/~d/talks/felix/
> http://huchra.bufferbloat.net/~d/talks/andrew/

I had already listened to Andrew's audio, and got up to 1 hour with
your session with Felix but seriously considered that entire 1 hour a
waste of my life. I think you digressed completely and rambled on
about random crap instead of focusing on the core of the matter, I
gave up after 1 hour.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: BQL crap and wireless
From: Dave Taht @ 2011-08-29 22:45 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Tom Herbert, linux-wireless, Andrew McGregor, Matt Smith,
	Kevin Hayes, Derek Smithies, netdev
In-Reply-To: <CAB=NE6XrkdiZcGEDGuYe=SwLBhTm=Mt4NaPzjV9j_W-8sVosOA@mail.gmail.com>

I have a few other things to correct and comment on from the earlier
postings on this topic, but I'll start here, and work backwards.

On Mon, Aug 29, 2011 at 2:10 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> On Mon, Aug 29, 2011 at 2:02 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>> Hope this helps sum up the issue for 802.11 and what we are faced with.
>
> I should elaborate a bit more here on ensuring people understand that
> the "bufferbloat" issue assumes simply not retrying frames is a good
> thing. This is incorrect.

No, we don't assume that "simply not retrying frames is a good thing".
The particular patch to which you are referring is part of a series of
patches still under development and test and is already obsolete.

In particular, the bug we were stomping in that thread involved
excessive retries in the packet aggregation queue on the ath9k driver,
where a ping at distance, was taking 1.6 seconds to get there.

http://www.bufferbloat.net/issues/216 - I note there was a lot of
confusing activity around this bug as this was the final piece of a
solution to why my mesh network in Nica went to h*ll in rain, and I
was trapped in a hotel at the time.

Far worse ping times have been observed in the field - 20+ seconds, or
more - and as most internet protocols were designed with a little over
a lunar diameter in mind at most (~2 seconds of latency) - induced
latency of over a certain point is a very bad thing as it introduces
major problems/timeouts in what we now call the 'ANT' protocols -
DHCP, DNS, ARP, ND, etc - as well as begin to serious muck with the
servo mechanisms within TCP itself.

Please note that ping is merely a diagnostic - all sorts of packets
are exhibiting unbounded latency across most wireless standards.

Retrying wireless frames with bounded latency is a good thing.
Dropping packets to signal congestion is a good thing, also. Knowing
when to drop a packet is a very good thing. Preserving all packets, no
matter the cost, leads to RFC970.

> TCP's congestion algorithm is designed to
> help with the network conditions, not the dynamic PHY conditions.

Agreed, although things like TCP westwood and the earlier vegas,
attempt to also measure latencies more dynamically.

Also, I have always been an advocate of using "split tcp" when making
the jump to from wired to wireless.

>The
> dyanmic PHY conditions are handled through a slew of different means:
>
>  * Rate control
>  * Adaptive Noise Immunity effort
>
> Rate control is addressed either in firmware or by the driver.
> Typically rate control algorithms use some sort of metrics to do best
> guess at what rate a frame should be transmitted at. Minstrel was the
> first to say -- ahhh the hell with it, I give up and simply do trial
> and error and keep using the most reliable one but keep testing
> different rates as you go on. You fixate on the best one by using
> EWMA.

Which I like, very much.

I note that the gargoyle traffic shaper attempts to use the number of
packets in a conntracked connection as a measure of the TCP
mice/elephant transition to determine what traffic class the stream
should be in.

It cannot, however, detect a elephant/mouse transition, and perhaps
if there was also EWMA in conntrack, it may help the shaping problem
somewhat.

The concepts of "TCP mice and elephants" are well established in the
literature (see google), however the concept of an 'Ant' is not, it's
a new usage we have tried to establish to raise the importance of
lower latency needed, system critical packets on local wireless lans.

> What I was arguing early was that perhaps the same approach can be
> taken for the latency issues under the assumption the resolution is
> queue size and software retries. In fact this same principle might be
> able to be applicable to the aggregation segment size as well.

EWMA time and feedback to higher layers, of how long it takes packets
to make a given next-hop destination

Also somehow passing up the stack that 'this (wireless-n) destination
can handle an aggregate of 32 packets or XX bytes', and this
destination (g or b), can't, would make
applying higher level traffic shaping and fairness algorithms such as
SFB, RED, SFQ, HSFC, actually somewhat useful.

There is also the huge weight of wireless multicast packets on
wireless-n, which can be well over 300x1 vs normal packets at present,
to somehow account for throughout the stack. It only takes a little
multicast to mess up your whole day.

> Now, ANI is specific to hardware and does adjustments on hardware
> based on some known metrics. Today we have fixed thresholds for these
> but I wouldn't be surprised if taking minstrel-like guesses and doing
> trial and error and EWMA based fixation would help here as well.

In this 80 minute discussion of the interaction of wireless stack
between myself and felix feitkau, we attempt to summarize all the work
re bufferbloat and wireless specific to the ath9k done to date, and in
the last 10 minutes or so, speculate as to further solutions,based on
and expanding on andrew's input from the previous week.

http://huchra.bufferbloat.net/~d/talks/felix/

There will be a transcript available soon.

Felix has (as of this morning) already implemented pieces of what we discussed.

There is an earlier discussion with Andrew McGregor, as well as
transcript, here:

http://huchra.bufferbloat.net/~d/talks/andrew/

The transcriptionist struggles mightily with the acronyms and I
haven't got around to trying to fix it yet. I'd like very much to
capture andrew's descriptions of how minstrel actually works and one
day fold all this stuff together, along with how power management
actually works on wireless, etc, so more people internalize how
wireless really works.





--
Dave Täht
http://www.bufferbloat.net

^ permalink raw reply

* Re: [PATCH 04/24] ax25: Remove unnecessary OOM logging messages
From: Joerg Reuter @ 2011-08-29 22:42 UTC (permalink / raw)
  To: Joe Perches
  Cc: Ralf Baechle, David S. Miller, linux-hams, netdev, linux-kernel
In-Reply-To: <a426e678426cf2be1552edfbbb0de7ce08e9a7ef.1314650069.git.joe@perches.com>

On Mon, Aug 29, 2011 at 02:17:23PM -0700, Joe Perches wrote:

> Removing unnecessary messages saves code and text.

Not really that much of an impact, but yes, I agree it just swamps
syslog with duplicate messages... It was more debug code than
anything else originally. Beats me why it is logged as KERN_ERR or
KERN_CRIT, being out of buffers is not really fatal or even
unusual.

Still alive with just not enough time for hamradio stuff nowadays,
	Joerg DL1BKE

-- 
Joerg Reuter                                    http://yaina.de/jreuter
And I make my way to where the warm scent of soil fills the evening air. 
Everything is waiting quietly out there....                 (Anne Clark)


^ permalink raw reply

* Re: [PATCH 18/24] sctp: Remove unnecessary OOM logging messages
From: David Miller @ 2011-08-29 22:15 UTC (permalink / raw)
  To: eric.dumazet
  Cc: joe, vladislav.yasevich, sri, linux-sctp, netdev, linux-kernel
In-Reply-To: <1314654681.2563.10.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 29 Aug 2011 23:51:21 +0200

> Le lundi 29 août 2011 à 23:43 +0200, Eric Dumazet a écrit :
> 
>> Furthermore, a failed vmalloc() is not guaranteed to emit an OOM
>> message, is it ?
> 
> It currently displays a message without context :
> 
> vmap allocation for size XXXXXX failed: use vmalloc=<size> to increase
> size.
> 
> So we dont know which part of the kernel asked this allocation.
> 
> Please dont remove existing error messages after failed vmalloc() calls.

Indeed.

Joe, these vmalloc() and also the __GFP_NOWARN cases will need to be
attended to and this series resubmitted as such.

Thanks.

^ permalink raw reply

* RE: [PATCH 19/24] sunrpc: Remove unnecessary OOM logging messages
From: Myklebust, Trond @ 2011-08-29 22:08 UTC (permalink / raw)
  To: Boaz Harrosh, David Miller
  Cc: joe-6d6DIl74uiNBDgjK7y7TUQ, bfields-uC3wQj2KruNg9hUCZPvPmw,
	neilb-l3A5Bk7waGM, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4E5C0AB3.90401-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1665 bytes --]

> -----Original Message-----
> From: Boaz Harrosh [mailto:bharrosh@panasas.com]
> Sent: Monday, August 29, 2011 5:55 PM
> To: David Miller
> Cc: Myklebust, Trond; joe@perches.com; bfields@fieldses.org;
> neilb@suse.de; linux-nfs@vger.kernel.org; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 19/24] sunrpc: Remove unnecessary OOM logging
> messages
> 
> On 08/29/2011 02:37 PM, David Miller wrote:
> > From: "Myklebust, Trond" <Trond.Myklebust@netapp.com>
> > Date: Mon, 29 Aug 2011 14:36:17 -0700
> >
> >> Big NACK...
> >>
> >> By whose standard are those "not useful"?
> >
> > By mine, that's for sure.  It's duplicating something that the
> allocation
> > layers are already going to print.
> 
> I have a question about that. Are the dprints going to show the stack
> backtrace?
> Otherwise how can I see which exact allocation failed and was not
> properly handled?
> 
> If yes above? then I'm not sure I like it either, because am I'll be
> getting a full
> stack backtrace for every failed allocation?
> 
> But I might like it if I try. How do I turn on allocation failures
> prints?
> Can I filter out to print only GFP_KERNEL failures and or other GFP
> combinations?

Right. If every GFP_ATOMIC or GFP_NOWAIT is going to print out stack traces, then we're heading for absolute insanity. If not, then the existing dprintk()s make a lot more sense, 'cos they are turned on only when the administrator notices a problem, and is trying to debug it.

Trond

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±û"žØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br	šê+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹\x1e®w¥¢¸?™¨è­Ú&¢)ߢ^[f

^ permalink raw reply

* Re: [PATCH 19/24] sunrpc: Remove unnecessary OOM logging messages
From: David Miller @ 2011-08-29 22:03 UTC (permalink / raw)
  To: bharrosh-C4P08NqkoRlBDgjK7y7TUQ
  Cc: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	joe-6d6DIl74uiNBDgjK7y7TUQ, bfields-uC3wQj2KruNg9hUCZPvPmw,
	neilb-l3A5Bk7waGM, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4E5C0AB3.90401-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>

From: Boaz Harrosh <bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
Date: Mon, 29 Aug 2011 14:54:59 -0700

> I have a question about that. Are the dprints going to show the stack backtrace?

Yes, OOMs give full stack backtraces.

> If yes above? then I'm not sure I like it either, because am I'll be getting a full
> stack backtrace for every failed allocation?

They've been doing this for years, so obviously they haven't bothered you
enough to care up to this point.

All of this pushback is pure uneducated noise, please stop blocking progress
with poorly informed objections.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 19/24] sunrpc: Remove unnecessary OOM logging messages
From: Boaz Harrosh @ 2011-08-29 21:54 UTC (permalink / raw)
  To: David Miller
  Cc: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	joe-6d6DIl74uiNBDgjK7y7TUQ, bfields-uC3wQj2KruNg9hUCZPvPmw,
	neilb-l3A5Bk7waGM, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110829.143751.1153162956919885670.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On 08/29/2011 02:37 PM, David Miller wrote:
> From: "Myklebust, Trond" <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
> Date: Mon, 29 Aug 2011 14:36:17 -0700
> 
>> Big NACK...
>>
>> By whose standard are those "not useful"?
> 
> By mine, that's for sure.  It's duplicating something that the allocation
> layers are already going to print.

I have a question about that. Are the dprints going to show the stack backtrace?
Otherwise how can I see which exact allocation failed and was not properly handled?

If yes above? then I'm not sure I like it either, because am I'll be getting a full
stack backtrace for every failed allocation?

But I might like it if I try. How do I turn on allocation failures prints?
Can I filter out to print only GFP_KERNEL failures and or other GFP combinations?

Thanks 
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 18/24] sctp: Remove unnecessary OOM logging messages
From: Eric Dumazet @ 2011-08-29 21:51 UTC (permalink / raw)
  To: Joe Perches
  Cc: Vlad Yasevich, Sridhar Samudrala, David S. Miller, linux-sctp,
	netdev, linux-kernel
In-Reply-To: <1314654209.2563.7.camel@edumazet-laptop>

Le lundi 29 août 2011 à 23:43 +0200, Eric Dumazet a écrit :

> Furthermore, a failed vmalloc() is not guaranteed to emit an OOM
> message, is it ?

It currently displays a message without context :

vmap allocation for size XXXXXX failed: use vmalloc=<size> to increase
size.

So we dont know which part of the kernel asked this allocation.

Please dont remove existing error messages after failed vmalloc() calls.

^ permalink raw reply

* Re: [PATCH 18/24] sctp: Remove unnecessary OOM logging messages
From: Eric Dumazet @ 2011-08-29 21:43 UTC (permalink / raw)
  To: Joe Perches
  Cc: Vlad Yasevich, Sridhar Samudrala, David S. Miller, linux-sctp,
	netdev, linux-kernel
In-Reply-To: <21b4d996c8861373ac77d427914ec7882fe0c83e.1314650069.git.joe@perches.com>

Le lundi 29 août 2011 à 14:17 -0700, Joe Perches a écrit :
> Removing unnecessary messages saves code and text.
> 
> Site specific OOM messages are duplications of a generic MM
> out of memory message and aren't really useful, so just
> delete them.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  net/sctp/protocol.c |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 91784f4..0801444 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1326,7 +1326,6 @@ SCTP_STATIC __init int sctp_init(void)
>  			__get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order);
>  	} while (!sctp_assoc_hashtable && --order > 0);
>  	if (!sctp_assoc_hashtable) {
> -		pr_err("Failed association hash alloc\n");
>  		status = -ENOMEM;
>  		goto err_ahash_alloc;
>  	}
> @@ -1340,7 +1339,6 @@ SCTP_STATIC __init int sctp_init(void)
>  	sctp_ep_hashtable = (struct sctp_hashbucket *)
>  		kmalloc(64 * sizeof(struct sctp_hashbucket), GFP_KERNEL);
>  	if (!sctp_ep_hashtable) {
> -		pr_err("Failed endpoint_hash alloc\n");
>  		status = -ENOMEM;
>  		goto err_ehash_alloc;
>  	}
> @@ -1359,7 +1357,6 @@ SCTP_STATIC __init int sctp_init(void)
>  			__get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order);
>  	} while (!sctp_port_hashtable && --order > 0);
>  	if (!sctp_port_hashtable) {
> -		pr_err("Failed bind hash alloc\n");
>  		status = -ENOMEM;
>  		goto err_bhash_alloc;
>  	}


It would be nice if you could avoid all these patches, that you dont
even read.

As I already told you in the past, __GFP_NOWARN dont print generic OOM
messages.

Its not because I told Wang Shaoyan not adding a useless "pr_err("Out of
memory\n");" in last gianfar patch, that you have to remove all
messages, with one hundred or more patches.

If I remember well, you even disagreed at that time.

Furthermore, a failed vmalloc() is not guaranteed to emit an OOM
message, is it ?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox