Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next-2.6] net-caif: Added missing lock validator constants
From: David Miller @ 2010-06-07  8:01 UTC (permalink / raw)
  To: alex.lorca; +Cc: sjur.brandeland, netdev
In-Reply-To: <1275761704-8758-1-git-send-email-alex.lorca@gmail.com>

From: Alex Lorca <alex.lorca@gmail.com>
Date: Sat,  5 Jun 2010 18:15:04 +0000

> CAIF is using "xxx-AF_MAX" strings for the lock validator. It should use
> its own strings.
> 
> Signed-off-by: Alex Lorca <alex.lorca@gmail.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH] asix: check packet size against mtu+ETH_HLEN instead of ETH_FRAME_LEN
From: David Miller @ 2010-06-07  7:57 UTC (permalink / raw)
  To: jussi.kivilinna; +Cc: netdev, dhollis
In-Reply-To: <20100606233531.23585.10292.stgit@fate.lan>

From: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
Date: Mon, 07 Jun 2010 02:35:31 +0300

> Driver checks received packet is too large in asix_rx_fixup() and fails if it is. Problem is
> that MTU might be set larger than 1500 and asix fails to work correctly with VLAN tagged
> packets. The check should be 'dev->net->mtu + ETH_HLEN' instead.
> 
> Tested with AX88772.
> 
> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] tcp: Fix slowness in read /proc/net/tcp
From: David Miller @ 2010-06-07  7:55 UTC (permalink / raw)
  To: eric.dumazet; +Cc: therbert, netdev, iler.ml
In-Reply-To: <1275895167.2545.8.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 07 Jun 2010 09:19:27 +0200

> Le dimanche 06 juin 2010 à 20:27 -0700, Tom Herbert a écrit :
>> Signed-off-by: Tom Herbert <therbert@google.com>
>> ---
> 
> This problem raises every year,  (last attempt from Yakov Lerner :
> http://kerneltrap.org/mailarchive/linux-netdev/2009/9/26/6256119 )
> 
> And finally, someone motivated enough to use /proc/net/tcp found the
> right answer ;)
> 
> Most netdev people tend to push inet_diag (netlink) interface instead of
> old /proc/net/tcp, but it seems old interface will still be used in
> 2030, so :
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Indeed this is the best attempt of this I've seen so far,
applied to net-next-2.6, thanks Tom.

> BTW, another problem of /proc/net/tcp is the buffer size used by netstat
> utility : 1024 bytes instead of PAGE_SIZE, making O(N^2) behavior even
> more palpable.

Probably cap it at 8K like we do for netlink atomic reads, because
there are all sorts of crazy PAGE_SIZE configurations possible out
there, even as high as 512K.

^ permalink raw reply

* Re: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
From: Andi Kleen @ 2010-06-07  7:51 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: xiaohui.xin, netdev, kvm, linux-kernel, mst, mingo, davem,
	herbert, jdike
In-Reply-To: <20100606161348.427822fb@nehalam>

Stephen Hemminger <shemminger@vyatta.com> writes:

> Still not sure this is a good idea for a couple of reasons:
>
> 1. We already have lots of special cases with skb's (frags and fraglist),
>    and skb's travel through a lot of different parts of the kernel.  So any
>    new change like this creates lots of exposed points for new bugs. Look
>    at cases like MD5 TCP and netfilter, and forwarding these SKB's to ipsec
>    and ppp and ...
>
> 2. SKB's can have infinite lifetime in the kernel. If these buffers come from
>    a fixed size pool in an external device, they can easily all get tied up
>    if you have a slow listener. What happens then?

3. If they come from an internal pool what happens when the kernel runs 
low on memory? How is that pool balanced against other kernel
memory users?

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply

* Re: [PATCH] [ath5k][leds] Ability to disable leds support. If leds support enabled do not force mac802.11 leds layer selection.
From: Dmytro Milinevskyy @ 2010-06-07  7:39 UTC (permalink / raw)
  To: Pavel Roskin
  Cc: ath5k-devel, Jiri Slaby, Nick Kossifidis, Luis R. Rodriguez,
	Bob Copeland, John W. Linville, GeunSik Lim, Greg Kroah-Hartman,
	Lukas Turek, Mark Hindley, Johannes Berg, Jiri Kosina, Kalle Valo,
	Keng-Yu Lin, Luca Verdesca, Shahar Or, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <1275668535.17251.20.camel@mj>

Pavel, thank you for the response here.

I agree with all your comments and will adjust the patch according to them.

I'm new to the submitting patches into the community and I appreciate
telling criticism so that in future I will not cause that much
troubles.

Regards,

-- Dima

On Fri, Jun 4, 2010 at 7:22 PM, Pavel Roskin <proski@gnu.org> wrote:
> On Fri, 2010-06-04 at 16:43 +0300, Dmytro Milinevskyy wrote:
>> Hi!
>>
>> Here is the patch to disable ath5k leds support on build stage.
>> However if the leds support was enabled do not force selection of 802.11 leds layer.
>> Depency on LEDS_CLASS is kept.
>>
>> Suggestion given by Pavel Roskin and Bob Copeland applied.
>
> It's great that you did it.  The patch is much clearer now.  That makes
> smaller issues visible.  Please don't be discouraged by the criticism,
> you are on the right track.
>
> First of all, your patch doesn't apply cleanly to the current
> wireless-testing because of formatting changes in Makefile.  Please
> update.
>
>> +config ATH5K_LEDS
>> +       tristate "Atheros 5xxx wireless cards LEDs support"
>> +       depends on ATH5K
>> +       select NEW_LEDS
>> +       select LEDS_CLASS
>> +       ---help---
>> +         Atheros 5xxx LED support.
>
> "tristate" is wrong here.  "tristate" would allow users select "m",
> which is wrong, since LED support is not a separate module.  I think you
> want "bool" here.
>
>> +#ifdef CONFIG_ATH5K_LEDS
>>  /*
>>   * State for LED triggers
>>   */
>> @@ -95,6 +96,7 @@ struct ath5k_led
>>       struct ath5k_softc *sc;                 /* driver state */
>>       struct led_classdev led_dev;            /* led classdev */
>>  };
>> +#endif
>
> This shouldn't be needed.  I'll rather see a structure that is not used
> in some cases than an extra pair of preprocessor conditionals.
>
>> diff --git a/drivers/net/wireless/ath/ath5k/gpio.c b/drivers/net/wireless/ath/ath5k/gpio.c
>> index 64a27e7..9e757b3 100644
>> --- a/drivers/net/wireless/ath/ath5k/gpio.c
>> +++ b/drivers/net/wireless/ath/ath5k/gpio.c
>> @@ -25,6 +25,7 @@
>>  #include "debug.h"
>>  #include "base.h"
>>
>> +#ifdef CONFIG_ATH5K_LEDS
>>  /*
>>   * Set led state
>>   */
>> @@ -76,6 +77,7 @@ void ath5k_hw_set_ledstate(struct ath5k_hw *ah, unsigned int state)
>>       else
>>               AR5K_REG_ENABLE_BITS(ah, AR5K_PCICFG, led_5210);
>>  }
>> +#endif
>
> I would just move that function to led.c (and don't forget to include
> reg.h).  The Makefile should take care of the rest.
>
> --
> Regards,
> Pavel Roskin
>

^ permalink raw reply

* Re: [PATCH net-next 00/11] tg3: Bugfixes and 5719 support
From: David Miller @ 2010-06-07  7:28 UTC (permalink / raw)
  To: mcarlson; +Cc: netdev, andy
In-Reply-To: <20100607035934.GB11599@mcarlson.broadcom.com>

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Sun, 6 Jun 2010 20:59:34 -0700

> On Sun, Jun 06, 2010 at 06:00:29PM -0700, David Miller wrote:
> The programming manual says this bit prevents a tx state machine lockup
> due to tx mbuf corruption.  The conditions under which the tx mbufs get
> corrupted is complicated, but the net effect of this bit is that the
> RDMA engine acts a little more conservatively.

That last phrase is a good description and could have been used to
better name the register bit macro in question.  You're basically
confirming that you named the register bit too tersely.

> The problem is that register definitions can change from asic rev to
> asic rev.

Frankly, this kind of justification really ticks me off.

How the heck does that make a difference?  Describe which chip revs
the register bit is valid for in a comment for crying out loud.

Document your hardware properly, not selectively or where you see
it fit to do so.

It's bad enough that you guys don't publish your hardware specs.
As a consequence as much knowledge as possible must go into the
driver sources.  Any piece of information you take away is bad.

There are tons of chip register bits in this driver already which are
only valid on certain hardware revs.  In fact, there are many.  Yet
they are all still there in tg3.h whether they are used by the driver
or not.  In fact, if I recall correctly, the DMA burst size controls
are pretty much different on every single rev of the chip.

Documenting where for which chips a register bit is valid is
pervasively done elsewhere, and is nothing new.  Your driver and your
hardware is not special.


^ permalink raw reply

* Re: [PATCH] tcp: Fix slowness in read /proc/net/tcp
From: Eric Dumazet @ 2010-06-07  7:19 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev, Yakov Lerner
In-Reply-To: <alpine.DEB.1.00.1006062020180.24064@pokey.mtv.corp.google.com>

Le dimanche 06 juin 2010 à 20:27 -0700, Tom Herbert a écrit :
> This patch address a serious performance issue in reading the
> TCP sockets table (/proc/net/tcp).
> 
> Reading the full table is done by a number of sequential read
> operations.  At each read operation, a seek is done to find the
> last socket that was previously read.  This seek operation requires
> that the sockets in the table need to be counted up to the current
> file position, and to count each of these requires taking a lock for
> each non-empty bucket.  The whole algorithm is O(n^2).
> 
> The fix is to cache the last bucket value, offset within the bucket,
> and the file position returned by the last read operation.   On the
> next sequential read, the bucket and offset are used to find the
> last read socket immediately without needing ot scan the previous
> buckets  the table.  This algorithm t read the whole table is O(n).
> 
> The improvement offered by this patch is easily show by performing
> cat'ing /proc/net/tcp on a machine with a lot of connections.  With
> about 182K connections in the table, I see the following:
> 
> - Without patch
> time cat /proc/net/tcp > /dev/null
> 
> real	1m56.729s
> user	0m0.214s
> sys	1m56.344s
> 
> - With patch
> time cat /proc/net/tcp > /dev/null
> 
> real	0m0.894s
> user	0m0.290s
> sys	0m0.594s
> 
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---

This problem raises every year,  (last attempt from Yakov Lerner :
http://kerneltrap.org/mailarchive/linux-netdev/2009/9/26/6256119 )

And finally, someone motivated enough to use /proc/net/tcp found the
right answer ;)

Most netdev people tend to push inet_diag (netlink) interface instead of
old /proc/net/tcp, but it seems old interface will still be used in
2030, so :

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

BTW, another problem of /proc/net/tcp is the buffer size used by netstat
utility : 1024 bytes instead of PAGE_SIZE, making O(N^2) behavior even
more palpable.




^ permalink raw reply

* Re: [PATCH net-next 00/11] tg3: Bugfixes and 5719 support
From: Matt Carlson @ 2010-06-07  3:59 UTC (permalink / raw)
  To: David Miller; +Cc: Matthew Carlson, netdev@vger.kernel.org, andy@greyhouse.net
In-Reply-To: <20100606.180029.42789560.davem@davemloft.net>

On Sun, Jun 06, 2010 at 06:00:29PM -0700, David Miller wrote:
> From: "Matt Carlson" <mcarlson@broadcom.com>
> Date: Sat, 5 Jun 2010 20:24:29 -0700
> 
> > This patchset adds some bugfixes and adds 5719 device support.
> 
> All applied to net-next-2.6 but there are two things I'm very disappointed
> with in this series:
> 
> 1) Naming register bits things like "TX_MBUF_FIX" isn't descriptive,
>    and I suspect the actual bit name used in your programming manuals
>    is very different and would be more helpful to someone reading the
>    code and trying to understand exactly what that bit does.

Actually, the programming manual describes this register just as
tersely.

>    How does it change the chips internal MBUF handling behavior?  Does
>    it insert a delay in accesses or state changes?  Does it change
>    the MBUF arbitration?  What the heck does that bit do exactly?

The programming manual says this bit prevents a tx state machine lockup
due to tx mbuf corruption.  The conditions under which the tx mbufs get
corrupted is complicated, but the net effect of this bit is that the
RDMA engine acts a little more conservatively.

> 2) Removing register definitions is something we really shouldn't do.
>    Just because you're not using the register any more in the driver,
>    doesn't mean you should remove it's definition from tg3.h
> 
>    What if some other developer wants to play with that register and
>    use it for some other purpose or experiment?

The problem is that register definitions can change from asic rev to
asic rev.  Someone interested in playing with their hardware would have
to know more about their chip before it could even be considered safe to
use these bits.  Rather than have a bit do something other than appears
advertised, it would be safer to just remove it from view.

The patch that removed the register definition exists precisely because
that register definition was changing.  The motivating reason for the
patch was to make the code cleaner and more maintainable, but anyone
attempting to use that definition on a non-5717 device would be using it
incorrectly.

> You really have to handle situations like #1 and #2 better especially
> since you guys do not public post the full PDF hardware programming
> manuals of your chips online for other developers to use.
> 
> I wouldn't, therefore, impose these rules on Intel and their drivers
> because they do public the programming manuals for their networking
> chips.

Understood.


^ permalink raw reply

* [PATCH] tcp: Fix slowness in read /proc/net/tcp
From: Tom Herbert @ 2010-06-07  3:27 UTC (permalink / raw)
  To: davem; +Cc: netdev

This patch address a serious performance issue in reading the
TCP sockets table (/proc/net/tcp).

Reading the full table is done by a number of sequential read
operations.  At each read operation, a seek is done to find the
last socket that was previously read.  This seek operation requires
that the sockets in the table need to be counted up to the current
file position, and to count each of these requires taking a lock for
each non-empty bucket.  The whole algorithm is O(n^2).

The fix is to cache the last bucket value, offset within the bucket,
and the file position returned by the last read operation.   On the
next sequential read, the bucket and offset are used to find the
last read socket immediately without needing ot scan the previous
buckets  the table.  This algorithm t read the whole table is O(n).

The improvement offered by this patch is easily show by performing
cat'ing /proc/net/tcp on a machine with a lot of connections.  With
about 182K connections in the table, I see the following:

- Without patch
time cat /proc/net/tcp > /dev/null

real	1m56.729s
user	0m0.214s
sys	1m56.344s

- With patch
time cat /proc/net/tcp > /dev/null

real	0m0.894s
user	0m0.290s
sys	0m0.594s

Signed-off-by: Tom Herbert <therbert@google.com>
---
diff --git a/include/net/tcp.h b/include/net/tcp.h
index a144914..5731664 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1413,7 +1413,8 @@ struct tcp_iter_state {
 	sa_family_t		family;
 	enum tcp_seq_states	state;
 	struct sock		*syn_wait_sk;
-	int			bucket, sbucket, num, uid;
+	int			bucket, offset, sbucket, num, uid;
+	loff_t			last_pos;
 };
 
 extern int tcp_proc_register(struct net *net, struct tcp_seq_afinfo *afinfo);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 202cf09..4af6f20 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1977,6 +1977,11 @@ static inline struct inet_timewait_sock *tw_next(struct inet_timewait_sock *tw)
 		hlist_nulls_entry(tw->tw_node.next, typeof(*tw), tw_node) : NULL;
 }
 
+/*
+ * Get next listener socket follow cur.  If cur is NULL, get first socket
+ * starting from bucket given in st->bucket; when st->bucket is zero the
+ * very first socket in the hash table is returned.
+ */
 static void *listening_get_next(struct seq_file *seq, void *cur)
 {
 	struct inet_connection_sock *icsk;
@@ -1987,14 +1992,15 @@ static void *listening_get_next(struct seq_file *seq, void *cur)
 	struct net *net = seq_file_net(seq);
 
 	if (!sk) {
-		st->bucket = 0;
-		ilb = &tcp_hashinfo.listening_hash[0];
+		ilb = &tcp_hashinfo.listening_hash[st->bucket];
 		spin_lock_bh(&ilb->lock);
 		sk = sk_nulls_head(&ilb->head);
+		st->offset = 0;
 		goto get_sk;
 	}
 	ilb = &tcp_hashinfo.listening_hash[st->bucket];
 	++st->num;
+	++st->offset;
 
 	if (st->state == TCP_SEQ_STATE_OPENREQ) {
 		struct request_sock *req = cur;
@@ -2009,6 +2015,7 @@ static void *listening_get_next(struct seq_file *seq, void *cur)
 				}
 				req = req->dl_next;
 			}
+			st->offset = 0;
 			if (++st->sbucket >= icsk->icsk_accept_queue.listen_opt->nr_table_entries)
 				break;
 get_req:
@@ -2044,6 +2051,7 @@ start_req:
 		read_unlock_bh(&icsk->icsk_accept_queue.syn_wait_lock);
 	}
 	spin_unlock_bh(&ilb->lock);
+	st->offset = 0;
 	if (++st->bucket < INET_LHTABLE_SIZE) {
 		ilb = &tcp_hashinfo.listening_hash[st->bucket];
 		spin_lock_bh(&ilb->lock);
@@ -2057,7 +2065,12 @@ out:
 
 static void *listening_get_idx(struct seq_file *seq, loff_t *pos)
 {
-	void *rc = listening_get_next(seq, NULL);
+	struct tcp_iter_state *st = seq->private;
+	void *rc;
+
+	st->bucket = 0;
+	st->offset = 0;
+	rc = listening_get_next(seq, NULL);
 
 	while (rc && *pos) {
 		rc = listening_get_next(seq, rc);
@@ -2072,13 +2085,18 @@ static inline int empty_bucket(struct tcp_iter_state *st)
 		hlist_nulls_empty(&tcp_hashinfo.ehash[st->bucket].twchain);
 }
 
+/*
+ * Get first established socket starting from bucket given in st->bucket.
+ * If st->bucket is zero, the very first socket in the hash is returned.
+ */
 static void *established_get_first(struct seq_file *seq)
 {
 	struct tcp_iter_state *st = seq->private;
 	struct net *net = seq_file_net(seq);
 	void *rc = NULL;
 
-	for (st->bucket = 0; st->bucket <= tcp_hashinfo.ehash_mask; ++st->bucket) {
+	st->offset = 0;
+	for (; st->bucket <= tcp_hashinfo.ehash_mask; ++st->bucket) {
 		struct sock *sk;
 		struct hlist_nulls_node *node;
 		struct inet_timewait_sock *tw;
@@ -2123,6 +2141,7 @@ static void *established_get_next(struct seq_file *seq, void *cur)
 	struct net *net = seq_file_net(seq);
 
 	++st->num;
+	++st->offset;
 
 	if (st->state == TCP_SEQ_STATE_TIME_WAIT) {
 		tw = cur;
@@ -2139,6 +2158,7 @@ get_tw:
 		st->state = TCP_SEQ_STATE_ESTABLISHED;
 
 		/* Look for next non empty bucket */
+		st->offset = 0;
 		while (++st->bucket <= tcp_hashinfo.ehash_mask &&
 				empty_bucket(st))
 			;
@@ -2166,7 +2186,11 @@ out:
 
 static void *established_get_idx(struct seq_file *seq, loff_t pos)
 {
-	void *rc = established_get_first(seq);
+	struct tcp_iter_state *st = seq->private;
+	void *rc;
+
+	st->bucket = 0;
+	rc = established_get_first(seq);
 
 	while (rc && pos) {
 		rc = established_get_next(seq, rc);
@@ -2191,24 +2215,72 @@ static void *tcp_get_idx(struct seq_file *seq, loff_t pos)
 	return rc;
 }
 
+static void *tcp_seek_last_pos(struct seq_file *seq)
+{
+	struct tcp_iter_state *st = seq->private;
+	int offset = st->offset;
+	int orig_num = st->num;
+	void *rc = NULL;
+	
+	switch (st->state) {
+	case TCP_SEQ_STATE_OPENREQ:
+	case TCP_SEQ_STATE_LISTENING:
+		if (st->bucket >= INET_LHTABLE_SIZE)
+			break;
+		st->state = TCP_SEQ_STATE_LISTENING;
+		rc = listening_get_next(seq, NULL);
+		while (offset-- && rc)
+			rc = listening_get_next(seq, rc);
+		if (rc)
+			break;
+		st->bucket = 0;
+		/* Fallthrough */
+	case TCP_SEQ_STATE_ESTABLISHED:
+	case TCP_SEQ_STATE_TIME_WAIT:
+		st->state = TCP_SEQ_STATE_ESTABLISHED;
+		if (st->bucket > tcp_hashinfo.ehash_mask)
+			break;
+		rc = established_get_first(seq);
+		while (offset-- && rc)
+			rc = established_get_next(seq, rc);
+	}
+
+	st->num = orig_num;
+
+	return rc;
+}
+
 static void *tcp_seq_start(struct seq_file *seq, loff_t *pos)
 {
 	struct tcp_iter_state *st = seq->private;
+	void *rc;
+
+	if (*pos && *pos == st->last_pos) {
+		rc = tcp_seek_last_pos(seq);
+		if (rc)
+			goto out;
+	}
+
 	st->state = TCP_SEQ_STATE_LISTENING;
 	st->num = 0;
-	return *pos ? tcp_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
+	st->bucket = 0;
+	st->offset = 0;
+	rc = *pos ? tcp_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
+
+out:
+	st->last_pos = *pos;
+	return rc;
 }
 
 static void *tcp_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 {
+	struct tcp_iter_state *st = seq->private;
 	void *rc = NULL;
-	struct tcp_iter_state *st;
 
 	if (v == SEQ_START_TOKEN) {
 		rc = tcp_get_idx(seq, 0);
 		goto out;
 	}
-	st = seq->private;
 
 	switch (st->state) {
 	case TCP_SEQ_STATE_OPENREQ:
@@ -2216,6 +2288,8 @@ static void *tcp_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 		rc = listening_get_next(seq, v);
 		if (!rc) {
 			st->state = TCP_SEQ_STATE_ESTABLISHED;
+			st->bucket = 0;
+			st->offset = 0;
 			rc	  = established_get_first(seq);
 		}
 		break;
@@ -2226,6 +2300,7 @@ static void *tcp_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 	}
 out:
 	++*pos;
+	st->last_pos = *pos;
 	return rc;
 }
 
@@ -2264,6 +2339,7 @@ static int tcp_seq_open(struct inode *inode, struct file *file)
 
 	s = ((struct seq_file *)file->private_data)->private;
 	s->family		= afinfo->family;
+	s->last_pos 		= 0;
 	return 0;
 }
 


^ permalink raw reply related

* Read Mail
From: Andrew Lu Sang @ 2010-06-06 10:40 UTC (permalink / raw)


My Dear Beloved,
I have an obscured business suggestion for you of $24,500,000USD.I will update you with further information 
as soon as I receive your complete name and phone number.Sincerely,Andrew

^ permalink raw reply

* Re: [RFC] pm_qos: get rid of the allocation in pm_qos_add_request()
From: James Bottomley @ 2010-06-07  2:50 UTC (permalink / raw)
  To: markgross; +Cc: pm list, netdev, alsa-devel, Takashi Iwai, Jaroslav Kysela
In-Reply-To: <20100606231021.GA17031@gvim.org>

On Sun, 2010-06-06 at 16:10 -0700, mark gross wrote:
> On Sat, Jun 05, 2010 at 02:20:14PM -0500, James Bottomley wrote:
> > [alsa-devel says it's a moderated list, so feel free to drop before
> > replying]
> > 
> > Since every caller has to squirrel away the returned pointer anyway,
> > they might as well supply the memory area.  This fixes a bug in a few of
> > the call sites where the returned pointer was dereferenced without
> > checking it for NULL (which gets returned if the kzalloc failed).
> > 
> > I'd like to hear how sound and netdev feels about this: it will add
> > about two more pointers worth of data to struct netdev and struct
> > snd_pcm_substream .. but I think it's worth it.  If you're OK, I'll add
> > your acks and send through the pm tree.
> > 
> > This also looks to me like an android independent clean up (even though
> > it renders the request_add atomically callable).  I also added include
> > guards to include/linux/pm_qos_params.h
> > 
> > James
> > 
> > ---
> > 
> >  drivers/net/e1000e/netdev.c            |   17 ++++------
> >  drivers/net/igbvf/netdev.c             |    9 ++---
> >  drivers/net/wireless/ipw2x00/ipw2100.c |   12 +++---
> >  include/linux/netdevice.h              |    2 +-
> >  include/linux/pm_qos_params.h          |   12 +++++--
> >  include/sound/pcm.h                    |    2 +-
> >  kernel/pm_qos_params.c                 |   55 ++++++++++++++++---------------
> >  sound/core/pcm_native.c                |   12 ++-----
> >  8 files changed, 60 insertions(+), 61 deletions(-)
> > 
> > diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> > index 24507f3..47ea62f 100644
> > --- a/drivers/net/e1000e/netdev.c
> > +++ b/drivers/net/e1000e/netdev.c
> > @@ -2901,10 +2901,10 @@ static void e1000_configure_rx(struct e1000_adapter *adapter)
> >  			 * dropped transactions.
> >  			 */
> >  			pm_qos_update_request(
> > -				adapter->netdev->pm_qos_req, 55);
> > +				&adapter->netdev->pm_qos_req, 55);
> >  		} else {
> >  			pm_qos_update_request(
> > -				adapter->netdev->pm_qos_req,
> > +				&adapter->netdev->pm_qos_req,
> >  				PM_QOS_DEFAULT_VALUE);
> >  		}
> >  	}
> > @@ -3196,9 +3196,9 @@ int e1000e_up(struct e1000_adapter *adapter)
> >  
> >  	/* DMA latency requirement to workaround early-receive/jumbo issue */
> >  	if (adapter->flags & FLAG_HAS_ERT)
> > -		adapter->netdev->pm_qos_req =
> > -			pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> > -				       PM_QOS_DEFAULT_VALUE);
> > +		pm_qos_add_request(&adapter->netdev->pm_qos_req,
> > +				   PM_QOS_CPU_DMA_LATENCY,
> > +				   PM_QOS_DEFAULT_VALUE);
> >  
> >  	/* hardware has been reset, we need to reload some things */
> >  	e1000_configure(adapter);
> > @@ -3263,11 +3263,8 @@ void e1000e_down(struct e1000_adapter *adapter)
> >  	e1000_clean_tx_ring(adapter);
> >  	e1000_clean_rx_ring(adapter);
> >  
> > -	if (adapter->flags & FLAG_HAS_ERT) {
> > -		pm_qos_remove_request(
> > -			      adapter->netdev->pm_qos_req);
> > -		adapter->netdev->pm_qos_req = NULL;
> > -	}
> > +	if (adapter->flags & FLAG_HAS_ERT)
> > +		pm_qos_remove_request(&adapter->netdev->pm_qos_req);
> >  
> >  	/*
> >  	 * TODO: for power management, we could drop the link and
> > diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
> > index 5e2b2a8..5da569f 100644
> > --- a/drivers/net/igbvf/netdev.c
> > +++ b/drivers/net/igbvf/netdev.c
> > @@ -48,7 +48,7 @@
> >  #define DRV_VERSION "1.0.0-k0"
> >  char igbvf_driver_name[] = "igbvf";
> >  const char igbvf_driver_version[] = DRV_VERSION;
> > -struct pm_qos_request_list *igbvf_driver_pm_qos_req;
> > +struct pm_qos_request_list igbvf_driver_pm_qos_req;
> >  static const char igbvf_driver_string[] =
> >  				"Intel(R) Virtual Function Network Driver";
> >  static const char igbvf_copyright[] = "Copyright (c) 2009 Intel Corporation.";
> > @@ -2902,8 +2902,8 @@ static int __init igbvf_init_module(void)
> >  	printk(KERN_INFO "%s\n", igbvf_copyright);
> >  
> >  	ret = pci_register_driver(&igbvf_driver);
> > -	igbvf_driver_pm_qos_req = pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> > -	                       PM_QOS_DEFAULT_VALUE);
> > +	pm_qos_add_request(igbvf_driver_pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> should be:
> 
> +	pm_qos_add_request(&igbvf_driver_pm_qos_req, PM_QOS_CPU_DMA_LATENCY,

Yes, thanks ... must be not compiling this driver for some reason in the
test case.

James



^ permalink raw reply

* Re: [RFC] pm_qos: get rid of the allocation in pm_qos_add_request()
From: James Bottomley @ 2010-06-07  2:47 UTC (permalink / raw)
  To: markgross; +Cc: pm list, netdev, alsa-devel, Takashi Iwai, Jaroslav Kysela
In-Reply-To: <20100606213224.GC8610@gvim.org>

On Sun, 2010-06-06 at 14:32 -0700, mark gross wrote:
> so the test for an un-registerd or in-initialized request is if list == null.

Actually I was using pm_qos_class == 0, but all current users use NULL
as the pointer test, so they must all be allocated in zero initialised
memory.

> >  
> > -struct pm_qos_request_list *pm_qos_add_request(int pm_qos_class, s32 value);
> > +void pm_qos_add_request(struct pm_qos_request_list *l, int pm_qos_class, s32 value);
> >  void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
> >  		s32 new_value);
> >  void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req);
> > @@ -25,3 +30,4 @@ int pm_qos_request(int pm_qos_class);
> >  int pm_qos_add_notifier(int pm_qos_class, struct notifier_block *notifier);
> >  int pm_qos_remove_notifier(int pm_qos_class, struct notifier_block *notifier);
> >  
> > +#endif
> > diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> > index dd76cde..6e3a297 100644
> > --- a/include/sound/pcm.h
> > +++ b/include/sound/pcm.h
> > @@ -366,7 +366,7 @@ struct snd_pcm_substream {
> >  	int number;
> >  	char name[32];			/* substream name */
> >  	int stream;			/* stream (direction) */
> > -	struct pm_qos_request_list *latency_pm_qos_req; /* pm_qos request */
> > +	struct pm_qos_request_list latency_pm_qos_req; /* pm_qos request */
> >  	size_t buffer_bytes_max;	/* limit ring buffer size */
> >  	struct snd_dma_buffer dma_buffer;
> >  	unsigned int dma_buf_id;
> > diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> > index 241fa79..f1d3d23 100644
> > --- a/kernel/pm_qos_params.c
> > +++ b/kernel/pm_qos_params.c
> > @@ -30,7 +30,6 @@
> >  /*#define DEBUG*/
> >  
> >  #include <linux/pm_qos_params.h>
> > -#include <linux/plist.h>
> ? plist pm_qos isn't yet in the code base yet. ;)
> Is this patch assumed after your RFC patch?
> It must be....

Yes, they're stacked.

> >  #include <linux/sched.h>
> >  #include <linux/spinlock.h>
> >  #include <linux/slab.h>
> > @@ -49,11 +48,6 @@
> >   * or pm_qos_object list and pm_qos_objects need to happen with pm_qos_lock
> >   * held, taken with _irqsave.  One lock to rule them all
> >   */
> > -struct pm_qos_request_list {
> > -	struct plist_node list;
> > -	int pm_qos_class;
> > -};
> > -
> >  enum pm_qos_type {
> >  	PM_QOS_MAX,		/* return the largest value */
> >  	PM_QOS_MIN,		/* return the smallest value */
> > @@ -195,6 +189,11 @@ int pm_qos_request(int pm_qos_class)
> >  }
> >  EXPORT_SYMBOL_GPL(pm_qos_request);
> >  
> > +static int pm_qos_request_active(struct pm_qos_request_list *req)
> > +{
> > +	return req->pm_qos_class != 0;
> > +}
> > +
> >  /**
> >   * pm_qos_add_request - inserts new qos request into the list
> >   * @pm_qos_class: identifies which list of qos request to us
> > @@ -206,25 +205,22 @@ EXPORT_SYMBOL_GPL(pm_qos_request);
> >   * element as a handle for use in updating and removal.  Call needs to save
> >   * this handle for later use.
> >   */
> > -struct pm_qos_request_list *pm_qos_add_request(int pm_qos_class, s32 value)
> > +void pm_qos_add_request(struct pm_qos_request_list *dep,
> > +			int pm_qos_class, s32 value)
> >  {
> > -	struct pm_qos_request_list *dep;
> > -
> > -	dep = kzalloc(sizeof(struct pm_qos_request_list), GFP_KERNEL);
> > -	if (dep) {
> > -		struct pm_qos_object *o =  pm_qos_array[pm_qos_class];
> > -		int new_value;
> > -
> > -		if (value == PM_QOS_DEFAULT_VALUE)
> > -			new_value = o->default_value;
> > -		else
> > -			new_value = value;
> > -		plist_node_init(&dep->list, new_value);
> > -		dep->pm_qos_class = pm_qos_class;
> > -		update_target(o, &dep->list, 0);
> > -	}
> > +	struct pm_qos_object *o =  pm_qos_array[pm_qos_class];
> > +	int new_value;
> > +
> > +	if (pm_qos_request_active(dep))
> > +		return;
> >  
> > -	return dep;
> > +	if (value == PM_QOS_DEFAULT_VALUE)
> > +		new_value = o->default_value;
> > +	else
> > +		new_value = value;
> > +	plist_node_init(&dep->list, new_value);
> > +	dep->pm_qos_class = pm_qos_class;
> > +	update_target(o, &dep->list, 0);
> >  }
> >  EXPORT_SYMBOL_GPL(pm_qos_add_request);
> >  
> > @@ -286,7 +282,7 @@ void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req)
> >  
> >  	o = pm_qos_array[pm_qos_req->pm_qos_class];
> >  	update_target(o, &pm_qos_req->list, 1);
> > -	kfree(pm_qos_req);
> > +	memset(pm_qos_req, 0, sizeof(*pm_qos_req));
> 
> I was wondering how to tell if a pm_qos_request was un-initialized
> un-registered request.

The assumption is zero initialised.

> >  }
> >  EXPORT_SYMBOL_GPL(pm_qos_remove_request);
> >  
> > @@ -334,8 +330,12 @@ static int pm_qos_power_open(struct inode *inode, struct file *filp)
> >  
> >  	pm_qos_class = find_pm_qos_object_by_minor(iminor(inode));
> >  	if (pm_qos_class >= 0) {
> > -		filp->private_data = (void *) pm_qos_add_request(pm_qos_class,
> > -				PM_QOS_DEFAULT_VALUE);
> > +		struct pm_qos_request_list *req = kzalloc(GFP_KERNEL, sizeof(*req));
> > +		if (!req)
> > +			return -ENOMEM;
> > +
> > +		pm_qos_add_request(req, pm_qos_class, PM_QOS_DEFAULT_VALUE);
> > +		filp->private_data = req;
> >  
> >  		if (filp->private_data)
> >  			return 0;
> > @@ -347,8 +347,9 @@ static int pm_qos_power_release(struct inode *inode, struct file *filp)
> >  {
> >  	struct pm_qos_request_list *req;
> >  
> > -	req = (struct pm_qos_request_list *)filp->private_data;
> > +	req = filp->private_data;
> >  	pm_qos_remove_request(req);
> > +	kfree(req);
> >  
> >  	return 0;
> >  }
> > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > index 303ac04..d3b8b51 100644
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -451,13 +451,10 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
> >  	snd_pcm_timer_resolution_change(substream);
> >  	runtime->status->state = SNDRV_PCM_STATE_SETUP;
> >  
> > -	if (substream->latency_pm_qos_req) {
> > -		pm_qos_remove_request(substream->latency_pm_qos_req);
> > -		substream->latency_pm_qos_req = NULL;
> > -	}
> > +	pm_qos_remove_request(&substream->latency_pm_qos_req);
> >  	if ((usecs = period_to_usecs(runtime)) >= 0)
> > -		substream->latency_pm_qos_req = pm_qos_add_request(
> > -					PM_QOS_CPU_DMA_LATENCY, usecs);
> > +		pm_qos_add_request(&substream->latency_pm_qos_req,
> > +				   PM_QOS_CPU_DMA_LATENCY, usecs);
> How are we going to avoid re-adding the latency_pm_qos_request multiple
> times to the list?  the last time I looked at this I really wanted to
> change this to update_request.  But, I got pushback so I added the file
> gard in pmqos_params.c instead. 

There's a guard check in add that does this ... it seemed better putting
it there than replicating through the subsystems.

James



^ permalink raw reply

* Re: [PATCH] virtio_net: indicate oom when addbuf returns failure
From: Rusty Russell @ 2010-06-07  2:24 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Michael S. Tsirkin, stable, Bruce Rogers, netdev, virtualization,
	Shirley Ma
In-Reply-To: <20100606222441.GA5992@gondor.apana.org.au>

On Mon, 7 Jun 2010 07:54:41 am Herbert Xu wrote:
> On Sun, Jun 06, 2010 at 11:13:00PM +0300, Michael S. Tsirkin wrote:
> >
> > Actually this code looks strange:
> > Note that add_buf inicates out of memory
> > condition with a positive return value, and ring full
> > (which is not an error!) with -ENOSPC.
> 
> Indeed, this ultimately came from
> 
> commit 9ab86bbcf8be755256f0a5e994e0b38af6b4d399
> Author: Shirley Ma <mashirle@us.ibm.com>
> Date:   Fri Jan 29 03:20:04 2010 +0000
> 
>     virtio_net: Defer skb allocation in receive path Date: Wed, 13 Jan 2010 12:53:38 -0800
> 
> (Greg, please don't apply this even though I've just given you
> the upstream commit ID that you were asking for :)
> 
> where it confuses a memory allocation error with an add_buf failure.
> 
> > Possibly the right thing to do is to
> > 1. handle ENOMEM specially
> > 2. fix add_buf to return ENOMEM on error
> 
> I think we should make it so that only a memory allocation error
> is returned as before.  There is no need for returning the add_buf
> error unless add_buf is now doing an allocation itself that needs
> to be retried.

With indirect bufs, this is indeed the case.  The code works except
for the bigpackets !mergable case, but should be clarified anyway.

See my other mail...

Thanks,
Rusty.

^ permalink raw reply

* Re: [PATCH] virtio_net: indicate oom when addbuf returns failure
From: Rusty Russell @ 2010-06-07  2:17 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: stable, Bruce Rogers, Herbert Xu, netdev, virtualization
In-Reply-To: <20100606201258.GA21196@redhat.com>

On Mon, 7 Jun 2010 05:43:00 am Michael S. Tsirkin wrote:
> On Fri, Jun 04, 2010 at 10:28:56AM +0930, Rusty Russell wrote:
> > This patch is a subset of an already upstream patch, but this portion
> > is useful in earlier releases.
> > 
> > Please consider for the 2.6.32 and 2.6.33 stable trees.
> > 
> > If the add_buf operation fails, indicate failure to the caller.
> > 
> > Signed-off-by: Bruce Rogers <brogers@novell.com>
> > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> Actually this code looks strange:
> Note that add_buf inicates out of memory
> condition with a positive return value, and ring full
> (which is not an error!) with -ENOSPC.
> 
> So it seems that this patch (and upstream code) will fill
> the ring and then end up setting oom = true and rescheduling the work
> forever.  And I suspect I actually saw this at some point
> on one of my systems: observed BW would drop
> with high CPU usage until reboot.
> Can't reproduce it now anymore ..

I thought that at first too, but it's subtler than that.

When the ring is exactly filled, err = 0.  With mergeable bufs and small
bufs that's the.  With big buffers, it's probably not, and the code will
indeed respond as if always out of memory, always trying to refill.

Our current code has the same error.  Probably a combination of noone using
big buffers, and noone noticing one timer every 1/2 second.

Want to fix that properly?  And comment it? :)

Thanks,
Rusty.

^ permalink raw reply

* Re: [PATCH net-next 00/11] tg3: Bugfixes and 5719 support
From: David Miller @ 2010-06-07  1:00 UTC (permalink / raw)
  To: mcarlson; +Cc: netdev, andy
In-Reply-To: <1275794679-11085-1-git-send-email-mcarlson@broadcom.com>

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Sat, 5 Jun 2010 20:24:29 -0700

> This patchset adds some bugfixes and adds 5719 device support.

All applied to net-next-2.6 but there are two things I'm very disappointed
with in this series:

1) Naming register bits things like "TX_MBUF_FIX" isn't descriptive,
   and I suspect the actual bit name used in your programming manuals
   is very different and would be more helpful to someone reading the
   code and trying to understand exactly what that bit does.

   How does it change the chips internal MBUF handling behavior?  Does
   it insert a delay in accesses or state changes?  Does it change
   the MBUF arbitration?  What the heck does that bit do exactly?

2) Removing register definitions is something we really shouldn't do.
   Just because you're not using the register any more in the driver,
   doesn't mean you should remove it's definition from tg3.h

   What if some other developer wants to play with that register and
   use it for some other purpose or experiment?

You really have to handle situations like #1 and #2 better especially
since you guys do not public post the full PDF hardware programming
manuals of your chips online for other developers to use.

I wouldn't, therefore, impose these rules on Intel and their drivers
because they do public the programming manuals for their networking
chips.

^ permalink raw reply

* Re: [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used
From: Grant Likely @ 2010-06-07  0:47 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev, netdev, Brian Hill, michal.simek, John Linn,
	john.williams
In-Reply-To: <1275861445.1931.2974.camel@pasglop>

On Sun, Jun 6, 2010 at 3:57 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2010-05-26 at 11:29 -0600, John Linn wrote:
>> The code is not checking the interrupt for DMA correctly so that an
>> interrupt number of 0 will cause a false error.
>>
>> Signed-off-by: Brian Hill <brian.hill@xilinx.com>
>> Signed-off-by: John Linn <john.linn@xilinx.com>
>> ---
>>  drivers/net/ll_temac_main.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
>> index fa7620e..0615737 100644
>> --- a/drivers/net/ll_temac_main.c
>> +++ b/drivers/net/ll_temac_main.c
>> @@ -950,7 +950,7 @@ temac_of_probe(struct of_device *op, const struct of_device_id *match)
>>
>>       lp->rx_irq = irq_of_parse_and_map(np, 0);
>>       lp->tx_irq = irq_of_parse_and_map(np, 1);
>> -     if (!lp->rx_irq || !lp->tx_irq) {
>> +     if ((lp->rx_irq == NO_IRQ) || (lp->tx_irq == NO_IRQ)) {
>>               dev_err(&op->dev, "could not determine irqs\n");
>>               rc = -ENOMEM;
>>               goto nodev;
>
> Hasn't NO_IRQ been deprecated ?

ARM still uses it, plus less than a handful of other arches.  There is
no reason for Microblaze to use NO_IRQ as -1.

In fact, on ARM, as part of the device tree work I'm hoping to piggy
back in irq remapping and make 0 no longer a valid irq.

> Linus made it clear a while back that interrupt 0 was not valid and
> that's the way it should be.
>
> We now have an interrupt remapping scheme on powerpc, so we ensure that
> 0 always mean no interrupt. Other archs might need some fixups. Which
> are specifically needs this patch ?

Microblaze.

BTW jlinn, for xilinx ARM support we should make sure 0 is never used
as a valid IRQ from day one.  It will result it far less pain in the
future.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* [PATCH] asix: check packet size against mtu+ETH_HLEN instead of ETH_FRAME_LEN
From: Jussi Kivilinna @ 2010-06-06 23:35 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, David Hollis

Driver checks received packet is too large in asix_rx_fixup() and fails if it is. Problem is
that MTU might be set larger than 1500 and asix fails to work correctly with VLAN tagged
packets. The check should be 'dev->net->mtu + ETH_HLEN' instead.

Tested with AX88772.

Signed-off-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
---
 drivers/net/usb/asix.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index 7e797ed..aea4645 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -344,7 +344,7 @@ static int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 			return 2;
 		}
 
-		if (size > ETH_FRAME_LEN) {
+		if (size > dev->net->mtu + ETH_HLEN) {
 			netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
 				   size);
 			return 0;


^ permalink raw reply related

* Re: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
From: Stephen Hemminger @ 2010-06-06 23:13 UTC (permalink / raw)
  To: xiaohui.xin; +Cc: netdev, kvm, linux-kernel, mst, mingo, davem, herbert, jdike
In-Reply-To: <1275732899-5423-1-git-send-email-xiaohui.xin@intel.com>

Still not sure this is a good idea for a couple of reasons:

1. We already have lots of special cases with skb's (frags and fraglist),
   and skb's travel through a lot of different parts of the kernel.  So any
   new change like this creates lots of exposed points for new bugs. Look
   at cases like MD5 TCP and netfilter, and forwarding these SKB's to ipsec
   and ppp and ...

2. SKB's can have infinite lifetime in the kernel. If these buffers come from
   a fixed size pool in an external device, they can easily all get tied up
   if you have a slow listener. What happens then?

^ permalink raw reply

* Re: [RFC] pm_qos: get rid of the allocation in pm_qos_add_request()
From: mark gross @ 2010-06-06 23:10 UTC (permalink / raw)
  To: James Bottomley
  Cc: pm list, markgross, netdev, alsa-devel, Takashi Iwai,
	Jaroslav Kysela
In-Reply-To: <1275765614.7227.42.camel@mulgrave.site>

On Sat, Jun 05, 2010 at 02:20:14PM -0500, James Bottomley wrote:
> [alsa-devel says it's a moderated list, so feel free to drop before
> replying]
> 
> Since every caller has to squirrel away the returned pointer anyway,
> they might as well supply the memory area.  This fixes a bug in a few of
> the call sites where the returned pointer was dereferenced without
> checking it for NULL (which gets returned if the kzalloc failed).
> 
> I'd like to hear how sound and netdev feels about this: it will add
> about two more pointers worth of data to struct netdev and struct
> snd_pcm_substream .. but I think it's worth it.  If you're OK, I'll add
> your acks and send through the pm tree.
> 
> This also looks to me like an android independent clean up (even though
> it renders the request_add atomically callable).  I also added include
> guards to include/linux/pm_qos_params.h
> 
> James
> 
> ---
> 
>  drivers/net/e1000e/netdev.c            |   17 ++++------
>  drivers/net/igbvf/netdev.c             |    9 ++---
>  drivers/net/wireless/ipw2x00/ipw2100.c |   12 +++---
>  include/linux/netdevice.h              |    2 +-
>  include/linux/pm_qos_params.h          |   12 +++++--
>  include/sound/pcm.h                    |    2 +-
>  kernel/pm_qos_params.c                 |   55 ++++++++++++++++---------------
>  sound/core/pcm_native.c                |   12 ++-----
>  8 files changed, 60 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 24507f3..47ea62f 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -2901,10 +2901,10 @@ static void e1000_configure_rx(struct e1000_adapter *adapter)
>  			 * dropped transactions.
>  			 */
>  			pm_qos_update_request(
> -				adapter->netdev->pm_qos_req, 55);
> +				&adapter->netdev->pm_qos_req, 55);
>  		} else {
>  			pm_qos_update_request(
> -				adapter->netdev->pm_qos_req,
> +				&adapter->netdev->pm_qos_req,
>  				PM_QOS_DEFAULT_VALUE);
>  		}
>  	}
> @@ -3196,9 +3196,9 @@ int e1000e_up(struct e1000_adapter *adapter)
>  
>  	/* DMA latency requirement to workaround early-receive/jumbo issue */
>  	if (adapter->flags & FLAG_HAS_ERT)
> -		adapter->netdev->pm_qos_req =
> -			pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> -				       PM_QOS_DEFAULT_VALUE);
> +		pm_qos_add_request(&adapter->netdev->pm_qos_req,
> +				   PM_QOS_CPU_DMA_LATENCY,
> +				   PM_QOS_DEFAULT_VALUE);
>  
>  	/* hardware has been reset, we need to reload some things */
>  	e1000_configure(adapter);
> @@ -3263,11 +3263,8 @@ void e1000e_down(struct e1000_adapter *adapter)
>  	e1000_clean_tx_ring(adapter);
>  	e1000_clean_rx_ring(adapter);
>  
> -	if (adapter->flags & FLAG_HAS_ERT) {
> -		pm_qos_remove_request(
> -			      adapter->netdev->pm_qos_req);
> -		adapter->netdev->pm_qos_req = NULL;
> -	}
> +	if (adapter->flags & FLAG_HAS_ERT)
> +		pm_qos_remove_request(&adapter->netdev->pm_qos_req);
>  
>  	/*
>  	 * TODO: for power management, we could drop the link and
> diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
> index 5e2b2a8..5da569f 100644
> --- a/drivers/net/igbvf/netdev.c
> +++ b/drivers/net/igbvf/netdev.c
> @@ -48,7 +48,7 @@
>  #define DRV_VERSION "1.0.0-k0"
>  char igbvf_driver_name[] = "igbvf";
>  const char igbvf_driver_version[] = DRV_VERSION;
> -struct pm_qos_request_list *igbvf_driver_pm_qos_req;
> +struct pm_qos_request_list igbvf_driver_pm_qos_req;
>  static const char igbvf_driver_string[] =
>  				"Intel(R) Virtual Function Network Driver";
>  static const char igbvf_copyright[] = "Copyright (c) 2009 Intel Corporation.";
> @@ -2902,8 +2902,8 @@ static int __init igbvf_init_module(void)
>  	printk(KERN_INFO "%s\n", igbvf_copyright);
>  
>  	ret = pci_register_driver(&igbvf_driver);
> -	igbvf_driver_pm_qos_req = pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> -	                       PM_QOS_DEFAULT_VALUE);
> +	pm_qos_add_request(igbvf_driver_pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
should be:

+	pm_qos_add_request(&igbvf_driver_pm_qos_req, PM_QOS_CPU_DMA_LATENCY,


--mgross

> +			   PM_QOS_DEFAULT_VALUE);
>  
>  	return ret;
>  }
> @@ -2918,8 +2918,7 @@ module_init(igbvf_init_module);
>  static void __exit igbvf_exit_module(void)
>  {
>  	pci_unregister_driver(&igbvf_driver);
> -	pm_qos_remove_request(igbvf_driver_pm_qos_req);
> -	igbvf_driver_pm_qos_req = NULL;
> +	pm_qos_remove_request(&igbvf_driver_pm_qos_req);
>  }
>  module_exit(igbvf_exit_module);
>  
> diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
> index 0bd4dfa..7f0d98b 100644
> --- a/drivers/net/wireless/ipw2x00/ipw2100.c
> +++ b/drivers/net/wireless/ipw2x00/ipw2100.c
> @@ -174,7 +174,7 @@ that only one external action is invoked at a time.
>  #define DRV_DESCRIPTION	"Intel(R) PRO/Wireless 2100 Network Driver"
>  #define DRV_COPYRIGHT	"Copyright(c) 2003-2006 Intel Corporation"
>  
> -struct pm_qos_request_list *ipw2100_pm_qos_req;
> +struct pm_qos_request_list ipw2100_pm_qos_req;
>  
>  /* Debugging stuff */
>  #ifdef CONFIG_IPW2100_DEBUG
> @@ -1741,7 +1741,7 @@ static int ipw2100_up(struct ipw2100_priv *priv, int deferred)
>  	/* the ipw2100 hardware really doesn't want power management delays
>  	 * longer than 175usec
>  	 */
> -	pm_qos_update_request(ipw2100_pm_qos_req, 175);
> +	pm_qos_update_request(&ipw2100_pm_qos_req, 175);
>  
>  	/* If the interrupt is enabled, turn it off... */
>  	spin_lock_irqsave(&priv->low_lock, flags);
> @@ -1889,7 +1889,7 @@ static void ipw2100_down(struct ipw2100_priv *priv)
>  	ipw2100_disable_interrupts(priv);
>  	spin_unlock_irqrestore(&priv->low_lock, flags);
>  
> -	pm_qos_update_request(ipw2100_pm_qos_req, PM_QOS_DEFAULT_VALUE);
> +	pm_qos_update_request(&ipw2100_pm_qos_req, PM_QOS_DEFAULT_VALUE);
>  
>  	/* We have to signal any supplicant if we are disassociating */
>  	if (associated)
> @@ -6669,8 +6669,8 @@ static int __init ipw2100_init(void)
>  	if (ret)
>  		goto out;
>  
> -	ipw2100_pm_qos_req = pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> -			PM_QOS_DEFAULT_VALUE);
> +	pm_qos_add_request(&ipw2100_pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> +			   PM_QOS_DEFAULT_VALUE);
>  #ifdef CONFIG_IPW2100_DEBUG
>  	ipw2100_debug_level = debug;
>  	ret = driver_create_file(&ipw2100_pci_driver.driver,
> @@ -6692,7 +6692,7 @@ static void __exit ipw2100_exit(void)
>  			   &driver_attr_debug_level);
>  #endif
>  	pci_unregister_driver(&ipw2100_pci_driver);
> -	pm_qos_remove_request(ipw2100_pm_qos_req);
> +	pm_qos_remove_request(&ipw2100_pm_qos_req);
>  }
>  
>  module_init(ipw2100_init);
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 40291f3..393555a 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -779,7 +779,7 @@ struct net_device {
>  	 */
>  	char			name[IFNAMSIZ];
>  
> -	struct pm_qos_request_list *pm_qos_req;
> +	struct pm_qos_request_list pm_qos_req;
>  
>  	/* device name hash chain */
>  	struct hlist_node	name_hlist;
> diff --git a/include/linux/pm_qos_params.h b/include/linux/pm_qos_params.h
> index 8ba440e..d823cc0 100644
> --- a/include/linux/pm_qos_params.h
> +++ b/include/linux/pm_qos_params.h
> @@ -1,8 +1,10 @@
> +#ifndef _LINUX_PM_QOS_PARAMS_H
> +#define _LINUX_PM_QOS_PARAMS_H
>  /* interface for the pm_qos_power infrastructure of the linux kernel.
>   *
>   * Mark Gross <mgross@linux.intel.com>
>   */
> -#include <linux/list.h>
> +#include <linux/plist.h>
>  #include <linux/notifier.h>
>  #include <linux/miscdevice.h>
>  
> @@ -14,9 +16,12 @@
>  #define PM_QOS_NUM_CLASSES 4
>  #define PM_QOS_DEFAULT_VALUE -1
>  
> -struct pm_qos_request_list;
> +struct pm_qos_request_list {
> +	struct plist_node list;
> +	int pm_qos_class;
> +};
>  
> -struct pm_qos_request_list *pm_qos_add_request(int pm_qos_class, s32 value);
> +void pm_qos_add_request(struct pm_qos_request_list *l, int pm_qos_class, s32 value);
>  void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
>  		s32 new_value);
>  void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req);
> @@ -25,3 +30,4 @@ int pm_qos_request(int pm_qos_class);
>  int pm_qos_add_notifier(int pm_qos_class, struct notifier_block *notifier);
>  int pm_qos_remove_notifier(int pm_qos_class, struct notifier_block *notifier);
>  
> +#endif
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index dd76cde..6e3a297 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -366,7 +366,7 @@ struct snd_pcm_substream {
>  	int number;
>  	char name[32];			/* substream name */
>  	int stream;			/* stream (direction) */
> -	struct pm_qos_request_list *latency_pm_qos_req; /* pm_qos request */
> +	struct pm_qos_request_list latency_pm_qos_req; /* pm_qos request */
>  	size_t buffer_bytes_max;	/* limit ring buffer size */
>  	struct snd_dma_buffer dma_buffer;
>  	unsigned int dma_buf_id;
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index 241fa79..f1d3d23 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -30,7 +30,6 @@
>  /*#define DEBUG*/
>  
>  #include <linux/pm_qos_params.h>
> -#include <linux/plist.h>
>  #include <linux/sched.h>
>  #include <linux/spinlock.h>
>  #include <linux/slab.h>
> @@ -49,11 +48,6 @@
>   * or pm_qos_object list and pm_qos_objects need to happen with pm_qos_lock
>   * held, taken with _irqsave.  One lock to rule them all
>   */
> -struct pm_qos_request_list {
> -	struct plist_node list;
> -	int pm_qos_class;
> -};
> -
>  enum pm_qos_type {
>  	PM_QOS_MAX,		/* return the largest value */
>  	PM_QOS_MIN,		/* return the smallest value */
> @@ -195,6 +189,11 @@ int pm_qos_request(int pm_qos_class)
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_request);
>  
> +static int pm_qos_request_active(struct pm_qos_request_list *req)
> +{
> +	return req->pm_qos_class != 0;
> +}
> +
>  /**
>   * pm_qos_add_request - inserts new qos request into the list
>   * @pm_qos_class: identifies which list of qos request to us
> @@ -206,25 +205,22 @@ EXPORT_SYMBOL_GPL(pm_qos_request);
>   * element as a handle for use in updating and removal.  Call needs to save
>   * this handle for later use.
>   */
> -struct pm_qos_request_list *pm_qos_add_request(int pm_qos_class, s32 value)
> +void pm_qos_add_request(struct pm_qos_request_list *dep,
> +			int pm_qos_class, s32 value)
>  {
> -	struct pm_qos_request_list *dep;
> -
> -	dep = kzalloc(sizeof(struct pm_qos_request_list), GFP_KERNEL);
> -	if (dep) {
> -		struct pm_qos_object *o =  pm_qos_array[pm_qos_class];
> -		int new_value;
> -
> -		if (value == PM_QOS_DEFAULT_VALUE)
> -			new_value = o->default_value;
> -		else
> -			new_value = value;
> -		plist_node_init(&dep->list, new_value);
> -		dep->pm_qos_class = pm_qos_class;
> -		update_target(o, &dep->list, 0);
> -	}
> +	struct pm_qos_object *o =  pm_qos_array[pm_qos_class];
> +	int new_value;
> +
> +	if (pm_qos_request_active(dep))
> +		return;
>  
> -	return dep;
> +	if (value == PM_QOS_DEFAULT_VALUE)
> +		new_value = o->default_value;
> +	else
> +		new_value = value;
> +	plist_node_init(&dep->list, new_value);
> +	dep->pm_qos_class = pm_qos_class;
> +	update_target(o, &dep->list, 0);
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_add_request);
>  
> @@ -286,7 +282,7 @@ void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req)
>  
>  	o = pm_qos_array[pm_qos_req->pm_qos_class];
>  	update_target(o, &pm_qos_req->list, 1);
> -	kfree(pm_qos_req);
> +	memset(pm_qos_req, 0, sizeof(*pm_qos_req));
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_remove_request);
>  
> @@ -334,8 +330,12 @@ static int pm_qos_power_open(struct inode *inode, struct file *filp)
>  
>  	pm_qos_class = find_pm_qos_object_by_minor(iminor(inode));
>  	if (pm_qos_class >= 0) {
> -		filp->private_data = (void *) pm_qos_add_request(pm_qos_class,
> -				PM_QOS_DEFAULT_VALUE);
> +		struct pm_qos_request_list *req = kzalloc(GFP_KERNEL, sizeof(*req));
> +		if (!req)
> +			return -ENOMEM;
> +
> +		pm_qos_add_request(req, pm_qos_class, PM_QOS_DEFAULT_VALUE);
> +		filp->private_data = req;
>  
>  		if (filp->private_data)
>  			return 0;
> @@ -347,8 +347,9 @@ static int pm_qos_power_release(struct inode *inode, struct file *filp)
>  {
>  	struct pm_qos_request_list *req;
>  
> -	req = (struct pm_qos_request_list *)filp->private_data;
> +	req = filp->private_data;
>  	pm_qos_remove_request(req);
> +	kfree(req);
>  
>  	return 0;
>  }
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 303ac04..d3b8b51 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -451,13 +451,10 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
>  	snd_pcm_timer_resolution_change(substream);
>  	runtime->status->state = SNDRV_PCM_STATE_SETUP;
>  
> -	if (substream->latency_pm_qos_req) {
> -		pm_qos_remove_request(substream->latency_pm_qos_req);
> -		substream->latency_pm_qos_req = NULL;
> -	}
> +	pm_qos_remove_request(&substream->latency_pm_qos_req);
>  	if ((usecs = period_to_usecs(runtime)) >= 0)
> -		substream->latency_pm_qos_req = pm_qos_add_request(
> -					PM_QOS_CPU_DMA_LATENCY, usecs);
> +		pm_qos_add_request(&substream->latency_pm_qos_req,
> +				   PM_QOS_CPU_DMA_LATENCY, usecs);
>  	return 0;
>   _error:
>  	/* hardware might be unuseable from this time,
> @@ -512,8 +509,7 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream)
>  	if (substream->ops->hw_free)
>  		result = substream->ops->hw_free(substream);
>  	runtime->status->state = SNDRV_PCM_STATE_OPEN;
> -	pm_qos_remove_request(substream->latency_pm_qos_req);
> -	substream->latency_pm_qos_req = NULL;
> +	pm_qos_remove_request(&substream->latency_pm_qos_req);
>  	return result;
>  }
>  
> 
> 

^ permalink raw reply

* Re: [PATCH] r8169: fix random mdio_write failures
From: David Miller @ 2010-06-06 22:39 UTC (permalink / raw)
  To: romieu; +Cc: timo.teras, netdev, edward_hsu, hayeswang
In-Reply-To: <20100605124103.GA3213@electric-eye.fr.zoreil.com>

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sat, 5 Jun 2010 14:41:03 +0200

> Timo Teräs <timo.teras@iki.fi> :
> [...]
>> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
>> index 217e709..03a8318 100644
>> --- a/drivers/net/r8169.c
>> +++ b/drivers/net/r8169.c
>> @@ -559,6 +559,11 @@ static void mdio_write(void __iomem *ioaddr, int reg_addr, int value)
>>  			break;
>>  		udelay(25);
>>  	}
>> +	/*
>> +	 * Some configurations require a small delay even after the write
>> +	 * completed indication or the next write might fail.
>> +	 */
>> +	udelay(25);
> 
> Acked-off-by: Francois Romieu <romieu@fr.zoreil.com>

Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH] virtio_net: indicate oom when addbuf returns failure
From: Herbert Xu @ 2010-06-06 22:24 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Rusty Russell, stable, Bruce Rogers, netdev, virtualization,
	Shirley Ma
In-Reply-To: <20100606201258.GA21196@redhat.com>

On Sun, Jun 06, 2010 at 11:13:00PM +0300, Michael S. Tsirkin wrote:
>
> Actually this code looks strange:
> Note that add_buf inicates out of memory
> condition with a positive return value, and ring full
> (which is not an error!) with -ENOSPC.

Indeed, this ultimately came from

commit 9ab86bbcf8be755256f0a5e994e0b38af6b4d399
Author: Shirley Ma <mashirle@us.ibm.com>
Date:   Fri Jan 29 03:20:04 2010 +0000

    virtio_net: Defer skb allocation in receive path Date: Wed, 13 Jan 2010 12:53:38 -0800

(Greg, please don't apply this even though I've just given you
the upstream commit ID that you were asking for :)

where it confuses a memory allocation error with an add_buf failure.

> Possibly the right thing to do is to
> 1. handle ENOMEM specially
> 2. fix add_buf to return ENOMEM on error

I think we should make it so that only a memory allocation error
is returned as before.  There is no need for returning the add_buf
error unless add_buf is now doing an allocation itself that needs
to be retried.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 1/2] net: ll_temac: fix interrupt bug when interrupt 0 is used
From: Benjamin Herrenschmidt @ 2010-06-06 21:57 UTC (permalink / raw)
  To: John Linn
  Cc: netdev, linuxppc-dev, grant.likely, jwboyer, michal.simek,
	john.williams, Brian Hill
In-Reply-To: <6f264a7a-e5da-494a-a24d-1578ca422807@VA3EHSMHS016.ehs.local>

On Wed, 2010-05-26 at 11:29 -0600, John Linn wrote:
> The code is not checking the interrupt for DMA correctly so that an
> interrupt number of 0 will cause a false error.
> 
> Signed-off-by: Brian Hill <brian.hill@xilinx.com>
> Signed-off-by: John Linn <john.linn@xilinx.com>
> ---
>  drivers/net/ll_temac_main.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
> index fa7620e..0615737 100644
> --- a/drivers/net/ll_temac_main.c
> +++ b/drivers/net/ll_temac_main.c
> @@ -950,7 +950,7 @@ temac_of_probe(struct of_device *op, const struct of_device_id *match)
>  
>  	lp->rx_irq = irq_of_parse_and_map(np, 0);
>  	lp->tx_irq = irq_of_parse_and_map(np, 1);
> -	if (!lp->rx_irq || !lp->tx_irq) {
> +	if ((lp->rx_irq == NO_IRQ) || (lp->tx_irq == NO_IRQ)) {
>  		dev_err(&op->dev, "could not determine irqs\n");
>  		rc = -ENOMEM;
>  		goto nodev;

Hasn't NO_IRQ been deprecated ?

Linus made it clear a while back that interrupt 0 was not valid and
that's the way it should be.

We now have an interrupt remapping scheme on powerpc, so we ensure that
0 always mean no interrupt. Other archs might need some fixups. Which
are specifically needs this patch ?

Cheers,
Ben.



^ permalink raw reply

* Re: [RFC] pm_qos: get rid of the allocation in pm_qos_add_request()
From: mark gross @ 2010-06-06 21:32 UTC (permalink / raw)
  To: James Bottomley
  Cc: pm list, markgross, netdev, alsa-devel, Takashi Iwai,
	Jaroslav Kysela
In-Reply-To: <1275765614.7227.42.camel@mulgrave.site>

On Sat, Jun 05, 2010 at 02:20:14PM -0500, James Bottomley wrote:
> [alsa-devel says it's a moderated list, so feel free to drop before
> replying]
> 
> Since every caller has to squirrel away the returned pointer anyway,
> they might as well supply the memory area.  This fixes a bug in a few of
> the call sites where the returned pointer was dereferenced without
> checking it for NULL (which gets returned if the kzalloc failed).
> 
> I'd like to hear how sound and netdev feels about this: it will add
> about two more pointers worth of data to struct netdev and struct
> snd_pcm_substream .. but I think it's worth it.  If you're OK, I'll add
> your acks and send through the pm tree.
> 
> This also looks to me like an android independent clean up (even though
> it renders the request_add atomically callable).  I also added include
> guards to include/linux/pm_qos_params.h
> 
> James
> 
> ---
> 
>  drivers/net/e1000e/netdev.c            |   17 ++++------
>  drivers/net/igbvf/netdev.c             |    9 ++---
>  drivers/net/wireless/ipw2x00/ipw2100.c |   12 +++---
>  include/linux/netdevice.h              |    2 +-
>  include/linux/pm_qos_params.h          |   12 +++++--
>  include/sound/pcm.h                    |    2 +-
>  kernel/pm_qos_params.c                 |   55 ++++++++++++++++---------------
>  sound/core/pcm_native.c                |   12 ++-----
>  8 files changed, 60 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 24507f3..47ea62f 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -2901,10 +2901,10 @@ static void e1000_configure_rx(struct e1000_adapter *adapter)
>  			 * dropped transactions.
>  			 */
>  			pm_qos_update_request(
> -				adapter->netdev->pm_qos_req, 55);
> +				&adapter->netdev->pm_qos_req, 55);
>  		} else {
>  			pm_qos_update_request(
> -				adapter->netdev->pm_qos_req,
> +				&adapter->netdev->pm_qos_req,
>  				PM_QOS_DEFAULT_VALUE);
>  		}
>  	}
> @@ -3196,9 +3196,9 @@ int e1000e_up(struct e1000_adapter *adapter)
>  
>  	/* DMA latency requirement to workaround early-receive/jumbo issue */
>  	if (adapter->flags & FLAG_HAS_ERT)
> -		adapter->netdev->pm_qos_req =
> -			pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> -				       PM_QOS_DEFAULT_VALUE);
> +		pm_qos_add_request(&adapter->netdev->pm_qos_req,
> +				   PM_QOS_CPU_DMA_LATENCY,
> +				   PM_QOS_DEFAULT_VALUE);
>  
>  	/* hardware has been reset, we need to reload some things */
>  	e1000_configure(adapter);
> @@ -3263,11 +3263,8 @@ void e1000e_down(struct e1000_adapter *adapter)
>  	e1000_clean_tx_ring(adapter);
>  	e1000_clean_rx_ring(adapter);
>  
> -	if (adapter->flags & FLAG_HAS_ERT) {
> -		pm_qos_remove_request(
> -			      adapter->netdev->pm_qos_req);
> -		adapter->netdev->pm_qos_req = NULL;
> -	}
> +	if (adapter->flags & FLAG_HAS_ERT)
> +		pm_qos_remove_request(&adapter->netdev->pm_qos_req);
>  
>  	/*
>  	 * TODO: for power management, we could drop the link and
> diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
> index 5e2b2a8..5da569f 100644
> --- a/drivers/net/igbvf/netdev.c
> +++ b/drivers/net/igbvf/netdev.c
> @@ -48,7 +48,7 @@
>  #define DRV_VERSION "1.0.0-k0"
>  char igbvf_driver_name[] = "igbvf";
>  const char igbvf_driver_version[] = DRV_VERSION;
> -struct pm_qos_request_list *igbvf_driver_pm_qos_req;
> +struct pm_qos_request_list igbvf_driver_pm_qos_req;
>  static const char igbvf_driver_string[] =
>  				"Intel(R) Virtual Function Network Driver";
>  static const char igbvf_copyright[] = "Copyright (c) 2009 Intel Corporation.";
> @@ -2902,8 +2902,8 @@ static int __init igbvf_init_module(void)
>  	printk(KERN_INFO "%s\n", igbvf_copyright);
>  
>  	ret = pci_register_driver(&igbvf_driver);
> -	igbvf_driver_pm_qos_req = pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> -	                       PM_QOS_DEFAULT_VALUE);
> +	pm_qos_add_request(igbvf_driver_pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> +			   PM_QOS_DEFAULT_VALUE);
>  
>  	return ret;
>  }
> @@ -2918,8 +2918,7 @@ module_init(igbvf_init_module);
>  static void __exit igbvf_exit_module(void)
>  {
>  	pci_unregister_driver(&igbvf_driver);
> -	pm_qos_remove_request(igbvf_driver_pm_qos_req);
> -	igbvf_driver_pm_qos_req = NULL;
> +	pm_qos_remove_request(&igbvf_driver_pm_qos_req);
>  }
>  module_exit(igbvf_exit_module);
>  
> diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
> index 0bd4dfa..7f0d98b 100644
> --- a/drivers/net/wireless/ipw2x00/ipw2100.c
> +++ b/drivers/net/wireless/ipw2x00/ipw2100.c
> @@ -174,7 +174,7 @@ that only one external action is invoked at a time.
>  #define DRV_DESCRIPTION	"Intel(R) PRO/Wireless 2100 Network Driver"
>  #define DRV_COPYRIGHT	"Copyright(c) 2003-2006 Intel Corporation"
>  
> -struct pm_qos_request_list *ipw2100_pm_qos_req;
> +struct pm_qos_request_list ipw2100_pm_qos_req;
>  
>  /* Debugging stuff */
>  #ifdef CONFIG_IPW2100_DEBUG
> @@ -1741,7 +1741,7 @@ static int ipw2100_up(struct ipw2100_priv *priv, int deferred)
>  	/* the ipw2100 hardware really doesn't want power management delays
>  	 * longer than 175usec
>  	 */
> -	pm_qos_update_request(ipw2100_pm_qos_req, 175);
> +	pm_qos_update_request(&ipw2100_pm_qos_req, 175);
>  
>  	/* If the interrupt is enabled, turn it off... */
>  	spin_lock_irqsave(&priv->low_lock, flags);
> @@ -1889,7 +1889,7 @@ static void ipw2100_down(struct ipw2100_priv *priv)
>  	ipw2100_disable_interrupts(priv);
>  	spin_unlock_irqrestore(&priv->low_lock, flags);
>  
> -	pm_qos_update_request(ipw2100_pm_qos_req, PM_QOS_DEFAULT_VALUE);
> +	pm_qos_update_request(&ipw2100_pm_qos_req, PM_QOS_DEFAULT_VALUE);
>  
>  	/* We have to signal any supplicant if we are disassociating */
>  	if (associated)
> @@ -6669,8 +6669,8 @@ static int __init ipw2100_init(void)
>  	if (ret)
>  		goto out;
>  
> -	ipw2100_pm_qos_req = pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> -			PM_QOS_DEFAULT_VALUE);
> +	pm_qos_add_request(&ipw2100_pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> +			   PM_QOS_DEFAULT_VALUE);
>  #ifdef CONFIG_IPW2100_DEBUG
>  	ipw2100_debug_level = debug;
>  	ret = driver_create_file(&ipw2100_pci_driver.driver,
> @@ -6692,7 +6692,7 @@ static void __exit ipw2100_exit(void)
>  			   &driver_attr_debug_level);
>  #endif
>  	pci_unregister_driver(&ipw2100_pci_driver);
> -	pm_qos_remove_request(ipw2100_pm_qos_req);
> +	pm_qos_remove_request(&ipw2100_pm_qos_req);
>  }
>  
>  module_init(ipw2100_init);
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 40291f3..393555a 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -779,7 +779,7 @@ struct net_device {
>  	 */
>  	char			name[IFNAMSIZ];
>  
> -	struct pm_qos_request_list *pm_qos_req;
> +	struct pm_qos_request_list pm_qos_req;
>  
>  	/* device name hash chain */
>  	struct hlist_node	name_hlist;
> diff --git a/include/linux/pm_qos_params.h b/include/linux/pm_qos_params.h
> index 8ba440e..d823cc0 100644
> --- a/include/linux/pm_qos_params.h
> +++ b/include/linux/pm_qos_params.h
> @@ -1,8 +1,10 @@
> +#ifndef _LINUX_PM_QOS_PARAMS_H
> +#define _LINUX_PM_QOS_PARAMS_H
>  /* interface for the pm_qos_power infrastructure of the linux kernel.
>   *
>   * Mark Gross <mgross@linux.intel.com>
>   */
> -#include <linux/list.h>
> +#include <linux/plist.h>
>  #include <linux/notifier.h>
>  #include <linux/miscdevice.h>
>  
> @@ -14,9 +16,12 @@
>  #define PM_QOS_NUM_CLASSES 4
>  #define PM_QOS_DEFAULT_VALUE -1
>  
> -struct pm_qos_request_list;
> +struct pm_qos_request_list {
> +	struct plist_node list;
> +	int pm_qos_class;
> +};

so the test for an un-registerd or in-initialized request is if list == null.

>  
> -struct pm_qos_request_list *pm_qos_add_request(int pm_qos_class, s32 value);
> +void pm_qos_add_request(struct pm_qos_request_list *l, int pm_qos_class, s32 value);
>  void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
>  		s32 new_value);
>  void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req);
> @@ -25,3 +30,4 @@ int pm_qos_request(int pm_qos_class);
>  int pm_qos_add_notifier(int pm_qos_class, struct notifier_block *notifier);
>  int pm_qos_remove_notifier(int pm_qos_class, struct notifier_block *notifier);
>  
> +#endif
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index dd76cde..6e3a297 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -366,7 +366,7 @@ struct snd_pcm_substream {
>  	int number;
>  	char name[32];			/* substream name */
>  	int stream;			/* stream (direction) */
> -	struct pm_qos_request_list *latency_pm_qos_req; /* pm_qos request */
> +	struct pm_qos_request_list latency_pm_qos_req; /* pm_qos request */
>  	size_t buffer_bytes_max;	/* limit ring buffer size */
>  	struct snd_dma_buffer dma_buffer;
>  	unsigned int dma_buf_id;
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index 241fa79..f1d3d23 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -30,7 +30,6 @@
>  /*#define DEBUG*/
>  
>  #include <linux/pm_qos_params.h>
> -#include <linux/plist.h>
? plist pm_qos isn't yet in the code base yet. ;)
Is this patch assumed after your RFC patch?
It must be....


>  #include <linux/sched.h>
>  #include <linux/spinlock.h>
>  #include <linux/slab.h>
> @@ -49,11 +48,6 @@
>   * or pm_qos_object list and pm_qos_objects need to happen with pm_qos_lock
>   * held, taken with _irqsave.  One lock to rule them all
>   */
> -struct pm_qos_request_list {
> -	struct plist_node list;
> -	int pm_qos_class;
> -};
> -
>  enum pm_qos_type {
>  	PM_QOS_MAX,		/* return the largest value */
>  	PM_QOS_MIN,		/* return the smallest value */
> @@ -195,6 +189,11 @@ int pm_qos_request(int pm_qos_class)
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_request);
>  
> +static int pm_qos_request_active(struct pm_qos_request_list *req)
> +{
> +	return req->pm_qos_class != 0;
> +}
> +
>  /**
>   * pm_qos_add_request - inserts new qos request into the list
>   * @pm_qos_class: identifies which list of qos request to us
> @@ -206,25 +205,22 @@ EXPORT_SYMBOL_GPL(pm_qos_request);
>   * element as a handle for use in updating and removal.  Call needs to save
>   * this handle for later use.
>   */
> -struct pm_qos_request_list *pm_qos_add_request(int pm_qos_class, s32 value)
> +void pm_qos_add_request(struct pm_qos_request_list *dep,
> +			int pm_qos_class, s32 value)
>  {
> -	struct pm_qos_request_list *dep;
> -
> -	dep = kzalloc(sizeof(struct pm_qos_request_list), GFP_KERNEL);
> -	if (dep) {
> -		struct pm_qos_object *o =  pm_qos_array[pm_qos_class];
> -		int new_value;
> -
> -		if (value == PM_QOS_DEFAULT_VALUE)
> -			new_value = o->default_value;
> -		else
> -			new_value = value;
> -		plist_node_init(&dep->list, new_value);
> -		dep->pm_qos_class = pm_qos_class;
> -		update_target(o, &dep->list, 0);
> -	}
> +	struct pm_qos_object *o =  pm_qos_array[pm_qos_class];
> +	int new_value;
> +
> +	if (pm_qos_request_active(dep))
> +		return;
>  
> -	return dep;
> +	if (value == PM_QOS_DEFAULT_VALUE)
> +		new_value = o->default_value;
> +	else
> +		new_value = value;
> +	plist_node_init(&dep->list, new_value);
> +	dep->pm_qos_class = pm_qos_class;
> +	update_target(o, &dep->list, 0);
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_add_request);
>  
> @@ -286,7 +282,7 @@ void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req)
>  
>  	o = pm_qos_array[pm_qos_req->pm_qos_class];
>  	update_target(o, &pm_qos_req->list, 1);
> -	kfree(pm_qos_req);
> +	memset(pm_qos_req, 0, sizeof(*pm_qos_req));

I was wondering how to tell if a pm_qos_request was un-initialized
un-registered request.

>  }
>  EXPORT_SYMBOL_GPL(pm_qos_remove_request);
>  
> @@ -334,8 +330,12 @@ static int pm_qos_power_open(struct inode *inode, struct file *filp)
>  
>  	pm_qos_class = find_pm_qos_object_by_minor(iminor(inode));
>  	if (pm_qos_class >= 0) {
> -		filp->private_data = (void *) pm_qos_add_request(pm_qos_class,
> -				PM_QOS_DEFAULT_VALUE);
> +		struct pm_qos_request_list *req = kzalloc(GFP_KERNEL, sizeof(*req));
> +		if (!req)
> +			return -ENOMEM;
> +
> +		pm_qos_add_request(req, pm_qos_class, PM_QOS_DEFAULT_VALUE);
> +		filp->private_data = req;
>  
>  		if (filp->private_data)
>  			return 0;
> @@ -347,8 +347,9 @@ static int pm_qos_power_release(struct inode *inode, struct file *filp)
>  {
>  	struct pm_qos_request_list *req;
>  
> -	req = (struct pm_qos_request_list *)filp->private_data;
> +	req = filp->private_data;
>  	pm_qos_remove_request(req);
> +	kfree(req);
>  
>  	return 0;
>  }
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 303ac04..d3b8b51 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -451,13 +451,10 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
>  	snd_pcm_timer_resolution_change(substream);
>  	runtime->status->state = SNDRV_PCM_STATE_SETUP;
>  
> -	if (substream->latency_pm_qos_req) {
> -		pm_qos_remove_request(substream->latency_pm_qos_req);
> -		substream->latency_pm_qos_req = NULL;
> -	}
> +	pm_qos_remove_request(&substream->latency_pm_qos_req);
>  	if ((usecs = period_to_usecs(runtime)) >= 0)
> -		substream->latency_pm_qos_req = pm_qos_add_request(
> -					PM_QOS_CPU_DMA_LATENCY, usecs);
> +		pm_qos_add_request(&substream->latency_pm_qos_req,
> +				   PM_QOS_CPU_DMA_LATENCY, usecs);
How are we going to avoid re-adding the latency_pm_qos_request multiple
times to the list?  the last time I looked at this I really wanted to
change this to update_request.  But, I got pushback so I added the file
gard in pmqos_params.c instead. 

--mgross

>  	return 0;
>   _error:
>  	/* hardware might be unuseable from this time,
> @@ -512,8 +509,7 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream)
>  	if (substream->ops->hw_free)
>  		result = substream->ops->hw_free(substream);
>  	runtime->status->state = SNDRV_PCM_STATE_OPEN;
> -	pm_qos_remove_request(substream->latency_pm_qos_req);
> -	substream->latency_pm_qos_req = NULL;
> +	pm_qos_remove_request(&substream->latency_pm_qos_req);
>  	return result;
>  }
>  
> 
> 

^ permalink raw reply

* Re: Bug#584724: linux-image-2.6.32-5-amd64: NETDEV WATCHDOG: (via-rhine): transmit queue 0 timed out
From: Ben Hutchings @ 2010-06-06 21:23 UTC (permalink / raw)
  To: Roger Luethi; +Cc: netdev, R. Scott Bailey, 584724
In-Reply-To: <20100606024158.12393.82960.reportbug@buzz.bailey>

[-- Attachment #1: Type: text/plain, Size: 40499 bytes --]

Roger,

There's some kind of TX lock-up bug in via-rhine (or more likely certain
variants of the hardware).

On Sat, 2010-06-05 at 22:41 -0400, R. Scott Bailey wrote:
> Package: linux-2.6
> Version: 2.6.32-15
> Severity: normal
> 
> This appears to be the same bug as:
>  https://bugzilla.kernel.org/show_bug.cgi?id=11663
> 
> I have attached kernel output; once the timeout occurs, the interface is
> unusable and bounces spastically (without becoming operational) until the
> system is rebooted. This most often is triggered by samba traffic, but that
> could be because that is just the heaviest traffic on my system. ;-)
> 
> BTW, this interface is reported by lspci as:
> 00:12.0 Ethernet controller: VIA Technologies, Inc. VT6102 [Rhine-II] (rev 7c)
> 
> Double-BTW, this is not a new problem with 2.6.32, but our activity levels
> have picked up recently and trigger it more frequently.

Here's that attachment inline:

> Jun  5 21:01:57 buzz kernel: [104288.805044] WARNING: at /build/buildd-linux-2.6_2.6.32-15-i386-fb7Hfg/linux-2.6-2.6.32/debian/build/source_i386_none/net/sched/sch_generic.c:261 dev_watchdog+0xd2/0x16f()
> Jun  5 21:01:57 buzz kernel: [104288.805052] Hardware name: MS-7253
> Jun  5 21:01:57 buzz kernel: [104288.805057] NETDEV WATCHDOG: eth0 (via-rhine): transmit queue 0 timed out
> Jun  5 21:01:57 buzz kernel: [104288.805062] Modules linked in: tcp_diag inet_diag ufs qnx4 hfsplus hfs minix ntfs vfat msdos fat jfs xfs via drm ip6_tables ppdev lp xt_multiport iptable_filter ip_tables x_tables kvm_amd kvm powernow_k8 cpufreq_stats cpufreq_powersave cpufreq_userspace cpufreq_conservative dm_snapshot dm_mirror dm_region_hash dm_log fuse nfsd exportfs nfs lockd fscache nfs_acl auth_rpcgss sunrpc ext3 jbd ext2 snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer snd_seq_device snd soundcore snd_page_alloc psmouse i2c_viapro tpm_tis evdev tpm k8temp tpm_bios edac_core pcspkr serio_raw i2c_core edac_mce_amd parport_pc parport shpchp pci_hotplug button processor sha256_generic aes_x86_64 aes_generic cbc ext4 mbcache jbd2 crc16 dm_crypt raid1 md_mod reiserfs dm_mod usb_storage usbhid hid sg sr_mod cdrom st sd_mod crc_t10dif osst ata_generic sata_via thermal uhci_
>  hcd via_rhine pata_via atp870u tul
> Jun  5 21:01:57 buzz kernel: p libata ehci_hcd thermal_sys mii usbcore nls_base scsi_mod [last unloaded: scsi_wait_scan]
> Jun  5 21:01:57 buzz kernel: [104288.805243] Pid: 0, comm: swapper Not tainted 2.6.32-5-amd64 #1
> Jun  5 21:01:57 buzz kernel: [104288.805248] Call Trace:
> Jun  5 21:01:57 buzz kernel: [104288.805253]  <IRQ>  [<ffffffff8125defb>] ? dev_watchdog+0xd2/0x16f
> Jun  5 21:01:57 buzz kernel: [104288.805268]  [<ffffffff8125defb>] ? dev_watchdog+0xd2/0x16f
> Jun  5 21:01:57 buzz kernel: [104288.805278]  [<ffffffff8104db48>] ? warn_slowpath_common+0x77/0xa3
> Jun  5 21:01:57 buzz kernel: [104288.805285]  [<ffffffff8125de29>] ? dev_watchdog+0x0/0x16f
> Jun  5 21:01:57 buzz kernel: [104288.805292]  [<ffffffff8104dbd0>] ? warn_slowpath_fmt+0x51/0x59
> Jun  5 21:01:57 buzz kernel: [104288.805304]  [<ffffffff8104a046>] ? try_to_wake_up+0x249/0x259
> Jun  5 21:01:57 buzz kernel: [104288.805314]  [<ffffffff81064853>] ? autoremove_wake_function+0x9/0x2e
> Jun  5 21:01:57 buzz kernel: [104288.805321]  [<ffffffff8125ddfd>] ? netif_tx_lock+0x3d/0x69
> Jun  5 21:01:57 buzz kernel: [104288.805330]  [<ffffffff81248dec>] ? netdev_drivername+0x3b/0x40
> Jun  5 21:01:57 buzz kernel: [104288.805338]  [<ffffffff8125defb>] ? dev_watchdog+0xd2/0x16f
> Jun  5 21:01:57 buzz kernel: [104288.805347]  [<ffffffff81030065>] ? amd_iommu_map_range+0x13/0x8a
> Jun  5 21:01:57 buzz kernel: [104288.805356]  [<ffffffff81061834>] ? __queue_work+0x22/0x32
> Jun  5 21:01:57 buzz kernel: [104288.805366]  [<ffffffff8105a05f>] ? run_timer_softirq+0x1c9/0x268
> Jun  5 21:01:57 buzz kernel: [104288.805377]  [<ffffffff810538b2>] ? __do_softirq+0xdd/0x19f
> Jun  5 21:01:57 buzz kernel: [104288.805387]  [<ffffffff81024c92>] ? lapic_next_event+0x18/0x1d
> Jun  5 21:01:57 buzz kernel: [104288.805396]  [<ffffffff81011cac>] ? call_softirq+0x1c/0x30
> Jun  5 21:01:57 buzz kernel: [104288.805403]  [<ffffffff81013903>] ? do_softirq+0x3f/0x7c
> Jun  5 21:01:57 buzz kernel: [104288.805411]  [<ffffffff81053721>] ? irq_exit+0x36/0x76
> Jun  5 21:01:57 buzz kernel: [104288.805419]  [<ffffffff81025757>] ? smp_apic_timer_interrupt+0x87/0x95
> Jun  5 21:01:57 buzz kernel: [104288.805426]  [<ffffffff81011673>] ? apic_timer_interrupt+0x13/0x20
> Jun  5 21:01:57 buzz kernel: [104288.805431]  <EOI>  [<ffffffff8102c558>] ? native_safe_halt+0x2/0x3
> Jun  5 21:01:57 buzz kernel: [104288.805448]  [<ffffffff81017c6d>] ? default_idle+0x34/0x51
> Jun  5 21:01:57 buzz kernel: [104288.805456]  [<ffffffff81017ff9>] ? c1e_idle+0xf5/0xfb
> Jun  5 21:01:57 buzz kernel: [104288.805466]  [<ffffffff8100feb1>] ? cpu_idle+0xa2/0xda
> Jun  5 21:01:57 buzz kernel: [104288.805472] ---[ end trace 6b516d8045567b1e ]---
> Jun  5 21:01:57 buzz kernel: [104288.805621] eth0: Transmit timed out, status 0003, PHY status 786d, resetting...
> Jun  5 21:01:57 buzz kernel: [104288.806347] eth0: link up, 100Mbps, full-duplex, lpa 0x45E1
> Jun  5 21:02:00 buzz kernel: [104292.805163] eth0: Transmit timed out, status 0003, PHY status 786d, resetting...
> Jun  5 21:02:00 buzz kernel: [104292.805934] eth0: link up, 100Mbps, full-duplex, lpa 0x45E1

More system information follows:

> -- Package-specific info:
> ** Version:
> Linux version 2.6.32-5-amd64 (Debian 2.6.32-15) (ben@decadent.org.uk) (gcc version 4.3.5 (Debian 4.3.5-1) ) #1 SMP Tue Jun 1 06:11:58 UTC 2010
> 
> ** Command line:
> BOOT_IMAGE=/vmlinuz-2.6.32-5-amd64 root=/dev/mapper/vg00-root ro ipv6.disable=1 rootdelay=10
> 
> ** Not tainted
> 
> ** Kernel log:
> [   20.917471] shpchp: Standard Hot Plug PCI Controller Driver version: 0.4
> [   20.924598] udev: renamed network interface eth0 to eth1
> [   20.953407] udev: renamed network interface eth1_rename to eth0
> [   21.014759] parport_pc 00:0a: reported by Plug and Play ACPI
> [   21.014871] parport0: PC-style at 0x378, irq 7 [PCSPP]
> [   21.018448] input: PC Speaker as /devices/platform/pcspkr/input/input5
> [   21.032277] k8temp 0000:00:18.3: Temperature readouts might be wrong - check erratum #141
> [   21.209202] EDAC MC: Ver: 2.1.0 Jun  1 2010
> [   21.226787] st0: Block limits 2 - 16777214 bytes.
> [   21.229458] EDAC amd64_edac:  Ver: 3.2.0 Jun  1 2010
> [   21.229633] EDAC amd64: This node reports that Memory ECC is currently disabled, set F3x44[22] (0000:00:18.3).
> [   21.229720] EDAC amd64: ECC disabled in the BIOS or no ECC capability, module will not load.
> [   21.229722]  Either enable ECC checking or force module loading by setting 'ecc_enable_override'.
> [   21.229723]  (Note that use of the override may cause unknown side effects.)
> [   21.229934] amd64_edac: probe of 0000:00:18.2 failed with error -22
> [   21.373141] HDA Intel 0000:04:01.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
> [   21.373280] HDA Intel 0000:04:01.0: setting latency timer to 64
> [   21.373285] HDA Intel 0000:04:01.0: PCI: Disallowing DAC for device
> [   21.461004] hda_codec: ALC883: BIOS auto-probing.
> [   21.462907] input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:13.0/0000:04:01.0/input/input6
> [   22.156505] Adding 262136k swap on /dev/mapper/vg00-swap.  Priority:-1 extents:1 across:262136k 
> [   32.897030] kjournald starting.  Commit interval 5 seconds
> [   32.909096] EXT3 FS on md0, internal journal
> [   32.909200] EXT3-fs: mounted filesystem with ordered data mode.
> [   33.073055] kjournald starting.  Commit interval 5 seconds
> [   33.156713] EXT3 FS on dm-5, internal journal
> [   33.156817] EXT3-fs: mounted filesystem with ordered data mode.
> [   33.181588] kjournald starting.  Commit interval 5 seconds
> [   33.182243] EXT3 FS on dm-4, internal journal
> [   33.182343] EXT3-fs: mounted filesystem with ordered data mode.
> [   33.232237] EXT4-fs (dm-9): mounted filesystem with ordered data mode
> [   33.287448] EXT4-fs (dm-6): mounted filesystem with ordered data mode
> [   33.348953] EXT4-fs (dm-3): mounted filesystem with ordered data mode
> [   33.414076] EXT4-fs (dm-1): mounted filesystem with ordered data mode
> [   33.497949] EXT4-fs (dm-2): mounted filesystem with ordered data mode
> [   33.672536] EXT4-fs (dm-12): mounted filesystem with ordered data mode
> [   33.725336] EXT4-fs (dm-7): mounted filesystem with ordered data mode
> [   33.791361] EXT4-fs (dm-10): mounted filesystem with ordered data mode
> [   33.865425] EXT4-fs (dm-11): mounted filesystem with ordered data mode
> [   33.964434] EXT4-fs (dm-13): mounted filesystem with ordered data mode
> [   50.233632] RPC: Registered udp transport module.
> [   50.233703] RPC: Registered tcp transport module.
> [   50.233759] RPC: Registered tcp NFSv4.1 backchannel transport module.
> [   50.261049] Slow work thread pool: Starting up
> [   50.261195] Slow work thread pool: Ready
> [   50.261332] FS-Cache: Loaded
> [   50.317395] FS-Cache: Netfs 'nfs' registered for caching
> [   50.349999] Installing knfsd (copyright (C) 1996 okir@monad.swb.de).
> [   50.925510] fuse init (API version 7.13)
> [   52.467290] powernow-k8: Found 1 AMD Athlon(tm) 64 X2 Dual Core Processor 4200+ processors (2 cpu cores) (version 2.20.00)
> [   52.467426] powernow-k8:    0 : fid 0xe (2200 MHz), vid 0xc
> [   52.467483] powernow-k8:    1 : fid 0xc (2000 MHz), vid 0xe
> [   52.467541] powernow-k8:    2 : fid 0xa (1800 MHz), vid 0x10
> [   52.467599] powernow-k8:    3 : fid 0x2 (1000 MHz), vid 0x12
> [   52.678554] kvm: Nested Virtualization enabled
> [   53.153019] Clocksource tsc unstable (delta = -269844223 ns)
> [   55.712610] st0: Failed to read 64512 byte block with 80 byte transfer.
> [   71.320893] ip_tables: (C) 2000-2006 Netfilter Core Team
> [   73.046091] lp0: using parport0 (interrupt-driven).
> [   73.057427] ppdev: user-space parallel port driver
> [   76.243406] ip6_tables: (C) 2000-2006 Netfilter Core Team
> [   78.057768] [drm] Initialized drm 1.1.0 20060810
> [   78.162620]   alloc irq_desc for 16 on node 0
> [   78.162625]   alloc kstat_irqs on node 0
> [   78.162636] pci 0000:01:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
> [   78.164716] [drm] Initialized via 2.11.1 20070202 for 0000:01:00.0 on minor 0
> [   78.165039] ioctl32(Xorg:3623): Unknown cmd fd(11) cmd(c0106407){t:'d';sz:16} arg(ff894e10) on /dev/dri/card0
> [   78.165056] ioctl32(Xorg:3623): Unknown cmd fd(11) cmd(c0086401){t:'d';sz:8} arg(ff894e08) on /dev/dri/card0
> [   78.228349] ioctl32(Xorg:3623): Unknown cmd fd(11) cmd(c0246400){t:'d';sz:36} arg(0869c910) on /dev/dri/card0
> [   78.234915] ioctl32(Xorg:3623): Unknown cmd fd(11) cmd(c0246400){t:'d';sz:36} arg(0869c910) on /dev/dri/card0
> [   82.424895] NFSD: Using /var/lib/nfs/v4recovery as the NFSv4 state recovery directory
> [   82.437939] NFSD: starting 90-second grace period
> [  306.805357] SGI XFS with ACLs, security attributes, realtime, large block/inode numbers, no debug enabled
> [  306.807862] SGI XFS Quota Management subsystem
> [  306.825141] JFS: nTxBlock = 8192, nTxLock = 65536
> [  306.851135] NTFS driver 2.1.29 [Flags: R/W MODULE].
> [  306.872372] QNX4 filesystem 0.2.3 registered.
> [  686.805028] ata4: lost interrupt (Status 0x51)
> [  691.805047] ata4.01: qc timeout (cmd 0xa0)
> [  691.805064] ata4.01: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> [  691.805092] sr 4:0:1:0: CDB: Test Unit Ready: 00 00 00 00 00 00
> [  691.805105] ata4.01: cmd a0/00:00:00:00:00/00:00:00:00:00/b0 tag 0
> [  691.805106]          res 51/20:03:00:00:00/00:00:00:00:00/b0 Emask 0x5 (timeout)
> [  691.805110] ata4.01: status: { DRDY ERR }
> [  691.805136] ata4: soft resetting link
> [  692.193313] ata4.01: configured for UDMA/66
> [  697.193028] ata4.01: qc timeout (cmd 0xa0)
> [  697.193037] ata4.01: TEST_UNIT_READY failed (err_mask=0x5)
> [  697.193060] ata4: soft resetting link
> [  697.585251] ata4.01: configured for UDMA/66
> [  702.585058] ata4.01: qc timeout (cmd 0xa0)
> [  702.585071] ata4.01: TEST_UNIT_READY failed (err_mask=0x5)
> [  702.585076] ata4.01: limiting speed to UDMA/66:PIO3
> [  702.585103] ata4: soft resetting link
> [  702.977291] ata4.01: configured for UDMA/66
> [  707.977030] ata4.01: qc timeout (cmd 0xa0)
> [  707.977040] ata4.01: TEST_UNIT_READY failed (err_mask=0x5)
> [  707.977043] ata4.01: disabled
> [  707.977081] ata4: soft resetting link
> [  708.301095] ata4: EH complete
> 
> ** Model information
> sys_vendor: MICRO-STAR INTERNATIONAL CO., LTD
> product_name: MS-7253
> product_version: 1.00
> chassis_vendor:  
> chassis_version:  
> bios_vendor: Phoenix Technologies, LTD
> bios_version: 6.00 PG
> board_vendor: MICRO-STAR INTERNATIONAL CO., LTD
> board_name: MS-7253
> board_version: 1.00
> 
> ** Loaded modules:
> Module                  Size  Used by
> ufs                    56458  0 
> qnx4                    6194  0 
> hfsplus                64966  0 
> hfs                    37199  0 
> minix                  21117  0 
> ntfs                  162540  0 
> vfat                    7836  0 
> msdos                   5914  0 
> fat                    39990  2 vfat,msdos
> jfs                   140115  0 
> xfs                   436189  0 
> via                    32847  0 
> drm                   142199  1 via
> ip6_tables             14867  0 
> ppdev                   5030  0 
> lp                      7462  0 
> xt_multiport            2267  1 
> iptable_filter          2258  1 
> ip_tables              13675  1 iptable_filter
> x_tables               12685  3 ip6_tables,xt_multiport,ip_tables
> kvm_amd                31222  0 
> kvm                   213448  1 kvm_amd
> powernow_k8            10978  1 
> cpufreq_stats           2659  0 
> cpufreq_powersave        902  0 
> cpufreq_userspace       1992  0 
> cpufreq_conservative     5162  0 
> dm_snapshot            18513  0 
> dm_mirror              10843  0 
> dm_region_hash          6680  1 dm_mirror
> dm_log                  7381  2 dm_mirror,dm_region_hash
> fuse                   49982  1 
> nfsd                  253350  13 
> exportfs                3122  2 xfs,nfsd
> nfs                   239786  0 
> lockd                  57299  2 nfsd,nfs
> fscache                29818  1 nfs
> nfs_acl                 2031  2 nfsd,nfs
> auth_rpcgss            33428  2 nfsd,nfs
> sunrpc                160117  12 nfsd,nfs,lockd,nfs_acl,auth_rpcgss
> ext3                  106326  3 
> jbd                    36861  1 ext3
> ext2                   52953  1 
> snd_hda_codec_realtek   235026  1 
> snd_hda_intel          19907  0 
> snd_hda_codec          53892  2 snd_hda_codec_realtek,snd_hda_intel
> snd_hwdep               5236  1 snd_hda_codec
> snd_pcm_oss            32415  0 
> snd_mixer_oss          12478  1 snd_pcm_oss
> snd_pcm                60119  3 snd_hda_intel,snd_hda_codec,snd_pcm_oss
> snd_seq_midi            4256  0 
> snd_rawmidi            15323  1 snd_seq_midi
> snd_seq_midi_event      4628  1 snd_seq_midi
> snd_seq                41281  2 snd_seq_midi,snd_seq_midi_event
> snd_timer              15486  2 snd_pcm,snd_seq
> snd_seq_device          4493  3 snd_seq_midi,snd_rawmidi,snd_seq
> snd                    45918  11 snd_hda_codec_realtek,snd_hda_intel,snd_hda_codec,snd_hwdep,snd_pcm_oss,snd_mixer_oss,snd_pcm,snd_rawmidi,snd_seq,snd_timer,snd_seq_device
> edac_core              29197  0 
> psmouse                49777  0 
> soundcore               4566  1 snd
> edac_mce_amd            6433  0 
> evdev                   7352  14 
> tpm_tis                 7336  0 
> snd_page_alloc          6217  2 snd_hda_intel,snd_pcm
> tpm                     9549  1 tpm_tis
> pcspkr                  1699  0 
> tpm_bios                4489  1 tpm
> serio_raw               3752  0 
> k8temp                  3139  0 
> parport_pc             18855  1 
> i2c_viapro              5483  0 
> i2c_core               15328  2 drm,i2c_viapro
> parport                27682  3 ppdev,lp,parport_pc
> shpchp                 26232  0 
> pci_hotplug            21203  1 shpchp
> button                  4650  0 
> processor              30167  1 powernow_k8
> sha256_generic          8644  2 
> aes_x86_64              7340  2 
> aes_generic            25714  1 aes_x86_64
> cbc                     2507  1 
> ext4                  285387  11 
> mbcache                 5050  3 ext3,ext2,ext4
> jbd2                   66823  1 ext4
> crc16                   1319  1 ext4
> dm_crypt               10491  1 
> raid1                  17999  2 
> md_mod                 73040  3 raid1
> reiserfs              193756  0 
> dm_mod                 53434  58 dm_snapshot,dm_mirror,dm_log,dm_crypt
> usb_storage            39337  1 
> usbhid                 33244  2 
> hid                    62793  1 usbhid
> sg                     18632  0 
> sr_mod                 12250  0 
> sd_mod                 29553  8 
> crc_t10dif              1276  1 sd_mod
> cdrom                  28631  1 sr_mod
> st                     30404  2 
> ata_generic             2983  0 
> osst                   44725  0 
> pata_via                7461  0 
> uhci_hcd               18521  0 
> sata_via                7789  4 
> via_rhine              17371  0 
> atp870u                24161  1 
> thermal                11610  0 
> tulip                  41368  0 
> thermal_sys            11942  2 processor,thermal
> ehci_hcd               31023  0 
> libata                133008  3 ata_generic,pata_via,sata_via
> scsi_mod              121717  8 usb_storage,sg,sr_mod,sd_mod,st,osst,atp870u,libata
> mii                     3210  1 via_rhine
> usbcore               121671  7 usb_storage,usbhid,uhci_hcd,ehci_hcd
> nls_base                6361  7 hfsplus,hfs,ntfs,vfat,fat,jfs,usbcore
> 
> ** PCI devices:
> 00:00.0 Host bridge [0600]: VIA Technologies, Inc. K8M890CE Host Bridge [1106:0336]
>         Subsystem: VIA Technologies, Inc. K8M890CE Host Bridge [1106:0336]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort+ >SERR- <PERR- INTx-
>         Latency: 8, Cache Line Size: 32 bytes
>         Region 0: Memory at <ignored> (32-bit, prefetchable)
>         Capabilities: <access denied>
>         Kernel driver in use: agpgart-amd64
> 
> 00:00.1 Host bridge [0600]: VIA Technologies, Inc. K8M890CE Host Bridge [1106:1336]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 32 bytes
> 
> 00:00.2 Host bridge [0600]: VIA Technologies, Inc. K8M890CE Host Bridge [1106:2336]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 32 bytes
> 
> 00:00.3 Host bridge [0600]: VIA Technologies, Inc. K8M890CE Host Bridge [1106:3336]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 32 bytes
> 
> 00:00.4 Host bridge [0600]: VIA Technologies, Inc. K8M890CE Host Bridge [1106:4336]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 32 bytes
> 
> 00:00.5 PIC [0800]: VIA Technologies, Inc. K8M890CE I/O APIC Interrupt Controller [1106:5336] (prog-if 20 [IO(X)-APIC])
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 32 bytes
> 
> 00:00.7 Host bridge [0600]: VIA Technologies, Inc. K8M890CE Host Bridge [1106:7336]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 32 bytes
> 
> 00:01.0 PCI bridge [0604]: VIA Technologies, Inc. VT8237 PCI bridge [K8T800/K8T890 South] [1106:b188] (prog-if 00 [Normal decode])
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 32 bytes
>         Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>         I/O behind bridge: 0000e000-0000efff
>         Memory behind bridge: dd000000-deffffff
>         Prefetchable memory behind bridge: c0000000-cfffffff
>         Secondary status: 66MHz+ FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- <SERR- <PERR-
>         BridgeCtl: Parity- SERR- NoISA- VGA+ MAbort- >Reset- FastB2B-
>                 PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
>         Capabilities: <access denied>
> 
> 00:02.0 PCI bridge [0604]: VIA Technologies, Inc. K8T890 PCI to PCI Bridge Controller [1106:a238] (prog-if 00 [Normal decode])
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 32 bytes
>         Bus: primary=00, secondary=02, subordinate=02, sec-latency=0
>         I/O behind bridge: 0000d000-0000dfff
>         Memory behind bridge: dfc00000-dfcfffff
>         Prefetchable memory behind bridge: 00000000dfb00000-00000000dfbfffff
>         Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR-
>         BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
>                 PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
>         Capabilities: <access denied>
>         Kernel driver in use: pcieport
> 
> 00:03.0 PCI bridge [0604]: VIA Technologies, Inc. K8T890 PCI to PCI Bridge Controller [1106:c238] (prog-if 00 [Normal decode])
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 32 bytes
>         Bus: primary=00, secondary=03, subordinate=03, sec-latency=0
>         I/O behind bridge: 0000c000-0000cfff
>         Memory behind bridge: dfe00000-dfefffff
>         Prefetchable memory behind bridge: 00000000dfd00000-00000000dfdfffff
>         Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR-
>         BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
>                 PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
>         Capabilities: <access denied>
>         Kernel driver in use: pcieport
> 
> 00:0f.0 IDE interface [0101]: VIA Technologies, Inc. VT8237A SATA 2-Port Controller [1106:0591] (rev 80) (prog-if 8f [Master SecP SecO PriP PriO])
>         Subsystem: Micro-Star International Co., Ltd. Device [1462:7253]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 64
>         Interrupt: pin B routed to IRQ 21
>         Region 0: I/O ports at ff00 [size=8]
>         Region 1: I/O ports at fe00 [size=4]
>         Region 2: I/O ports at fd00 [size=8]
>         Region 3: I/O ports at fc00 [size=4]
>         Region 4: I/O ports at fb00 [size=16]
>         Region 5: I/O ports at f400 [size=256]
>         Capabilities: <access denied>
>         Kernel driver in use: sata_via
> 
> 00:0f.1 IDE interface [0101]: VIA Technologies, Inc. VT82C586A/B/VT82C686/A/B/VT823x/A/C PIPC Bus Master IDE [1106:0571] (rev 07) (prog-if 8a [Master SecP PriP])
>         Subsystem: Micro-Star International Co., Ltd. Device [1462:7253]
>         Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx+
>         Latency: 64
>         Region 0: [virtual] Memory at 000001f0 (32-bit, non-prefetchable) [size=8]
>         Region 1: [virtual] Memory at 000003f0 (type 3, non-prefetchable) [size=1]
>         Region 2: [virtual] Memory at 00000170 (32-bit, non-prefetchable) [size=8]
>         Region 3: [virtual] Memory at 00000370 (type 3, non-prefetchable) [size=1]
>         Region 4: I/O ports at fa00 [size=16]
>         Capabilities: <access denied>
>         Kernel driver in use: pata_via
> 
> 00:10.0 USB Controller [0c03]: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller [1106:3038] (rev a0) (prog-if 00 [UHCI])
>         Subsystem: Micro-Star International Co., Ltd. Device [1462:7253]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 64, Cache Line Size: 32 bytes
>         Interrupt: pin A routed to IRQ 20
>         Region 4: I/O ports at f900 [size=32]
>         Capabilities: <access denied>
>         Kernel driver in use: uhci_hcd
> 
> 00:10.1 USB Controller [0c03]: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller [1106:3038] (rev a0) (prog-if 00 [UHCI])
>         Subsystem: Micro-Star International Co., Ltd. Device [1462:7253]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 64, Cache Line Size: 32 bytes
>         Interrupt: pin B routed to IRQ 22
>         Region 4: I/O ports at f800 [size=32]
>         Capabilities: <access denied>
>         Kernel driver in use: uhci_hcd
> 
> 00:10.2 USB Controller [0c03]: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller [1106:3038] (rev a0) (prog-if 00 [UHCI])
>         Subsystem: Micro-Star International Co., Ltd. Device [1462:7253]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 64, Cache Line Size: 32 bytes
>         Interrupt: pin C routed to IRQ 21
>         Region 4: I/O ports at f700 [size=32]
>         Capabilities: <access denied>
>         Kernel driver in use: uhci_hcd
> 
> 00:10.3 USB Controller [0c03]: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller [1106:3038] (rev a0) (prog-if 00 [UHCI])
>         Subsystem: Micro-Star International Co., Ltd. Device [1462:7253]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 64, Cache Line Size: 32 bytes
>         Interrupt: pin D routed to IRQ 23
>         Region 4: I/O ports at f600 [size=32]
>         Capabilities: <access denied>
>         Kernel driver in use: uhci_hcd
> 
> 00:10.4 USB Controller [0c03]: VIA Technologies, Inc. USB 2.0 [1106:3104] (rev 86) (prog-if 20 [EHCI])
>         Subsystem: Micro-Star International Co., Ltd. Device [1462:7253]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 64, Cache Line Size: 64 bytes
>         Interrupt: pin C routed to IRQ 21
>         Region 0: Memory at dffff000 (32-bit, non-prefetchable) [size=256]
>         Capabilities: <access denied>
>         Kernel driver in use: ehci_hcd
> 
> 00:11.0 ISA bridge [0601]: VIA Technologies, Inc. VT8237A PCI to ISA Bridge [1106:3337]
>         Subsystem: VIA Technologies, Inc. VT8237A PCI to ISA Bridge [1106:3337]
>         Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Capabilities: <access denied>
> 
> 00:11.7 Host bridge [0600]: VIA Technologies, Inc. VT8251 Ultra VLINK Controller [1106:287e]
>         Subsystem: VIA Technologies, Inc. Device [1106:337e]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort+ >SERR- <PERR- INTx-
>         Latency: 64
>         Capabilities: <access denied>
> 
> 00:12.0 Ethernet controller [0200]: VIA Technologies, Inc. VT6102 [Rhine-II] [1106:3065] (rev 7c)
>         Subsystem: Micro-Star International Co., Ltd. Device [1462:7253]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 64 (750ns min, 2000ns max), Cache Line Size: 32 bytes
>         Interrupt: pin A routed to IRQ 23
>         Region 0: I/O ports at f200 [size=256]
>         Region 1: Memory at dfffe000 (32-bit, non-prefetchable) [size=256]
>         Capabilities: <access denied>
>         Kernel driver in use: via-rhine
> 
> 00:13.0 PCI bridge [0604]: VIA Technologies, Inc. VT8237A Host Bridge [1106:337b] (prog-if 00 [Normal decode])
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ >SERR- <PERR- INTx-
>         Latency: 0
>         Bus: primary=00, secondary=04, subordinate=04, sec-latency=0
>         I/O behind bridge: 0000b000-0000bfff
>         Memory behind bridge: dfa00000-dfafffff
>         Prefetchable memory behind bridge: 00000000df900000-00000000df9fffff
>         Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- <SERR- <PERR-
>         BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
>                 PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
>         Capabilities: <access denied>
> 
> 00:13.1 PCI bridge [0604]: VIA Technologies, Inc. VT8237A PCI to PCI Bridge [1106:337a] (prog-if 01 [Subtractive decode])
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ >SERR- <PERR- INTx-
>         Latency: 0
>         Bus: primary=00, secondary=05, subordinate=05, sec-latency=0
>         I/O behind bridge: 0000a000-0000afff
>         Memory behind bridge: df800000-df8fffff
>         Prefetchable memory behind bridge: 00000000df700000-00000000df7fffff
>         Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- <SERR- <PERR-
>         BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
>                 PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
>         Capabilities: <access denied>
> 
> 00:18.0 Host bridge [0600]: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] HyperTransport Technology Configuration [1022:1100]
>         Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Capabilities: <access denied>
> 
> 00:18.1 Host bridge [0600]: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] Address Map [1022:1101]
>         Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> 
> 00:18.2 Host bridge [0600]: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] DRAM Controller [1022:1102]
>         Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> 
> 00:18.3 Host bridge [0600]: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] Miscellaneous Control [1022:1103]
>         Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Capabilities: <access denied>
>         Kernel driver in use: k8temp
> 
> 01:00.0 VGA compatible controller [0300]: VIA Technologies, Inc. K8M890CE/K8N890CE [Chrome 9] [1106:3230] (rev 11) (prog-if 00 [VGA controller])
>         Subsystem: Micro-Star International Co., Ltd. Device [1462:7253]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 64 (500ns min)
>         Interrupt: pin A routed to IRQ 16
>         Region 0: Memory at c0000000 (32-bit, prefetchable) [size=256M]
>         Region 1: Memory at dd000000 (32-bit, non-prefetchable) [size=16M]
>         [virtual] Expansion ROM at de000000 [disabled] [size=64K]
>         Capabilities: <access denied>
> 
> 04:01.0 Audio device [0403]: VIA Technologies, Inc. VT1708/A [Azalia HDAC] (VIA High Definition Audio Controller) [1106:3288] (rev 10)
>         Subsystem: Micro-Star International Co., Ltd. Device [1462:7253]
>         Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 32 bytes
>         Interrupt: pin A routed to IRQ 17
>         Region 0: Memory at dfafc000 (64-bit, non-prefetchable) [size=16K]
>         Capabilities: <access denied>
>         Kernel driver in use: HDA Intel
> 
> 05:04.0 SCSI storage controller [0100]: Artop Electronic Corp AEC6712UW SCSI [1191:8010] (rev 01)
>         Subsystem: Artop Electronic Corp AEC6712UW SCSI [1191:8010]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 254
>         Interrupt: pin A routed to IRQ 17
>         Region 0: I/O ports at af00 [size=64]
>         [virtual] Expansion ROM at df740000 [disabled] [size=64K]
>         Capabilities: <access denied>
>         Kernel driver in use: atp870u
> 
> 05:05.0 Ethernet controller [0200]: Digital Equipment Corporation DECchip 21142/43 [1011:0019] (rev 30)
>         Subsystem: Digital Equipment Corporation DE500B Fast Ethernet [1011:500b]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 64 (5000ns min, 10000ns max), Cache Line Size: 32 bytes
>         Interrupt: pin A routed to IRQ 18
>         Region 0: I/O ports at ae00 [size=128]
>         Region 1: Memory at df8ff000 (32-bit, non-prefetchable) [size=128]
>         [virtual] Expansion ROM at df700000 [disabled] [size=256K]
>         Kernel driver in use: tulip
> 
> 
> ** USB devices:
> Bus 005 Device 003: ID 08ec:0010 M-Systems Flash Disk Pioneers DiskOnKey
> Bus 005 Device 002: ID 058f:6362 Alcor Micro Corp. Flash Card Reader/Writer
> Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 003 Device 004: ID 045e:005f Microsoft Corp. Wireless MultiMedia Keyboard
> Bus 003 Device 003: ID 0557:7000 ATEN International Co., Ltd Hub
> Bus 003 Device 002: ID 051d:0002 American Power Conversion Uninterruptible Power Supply
> Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> 
> 
> -- System Information:
> Debian Release: squeeze/sid
>   APT prefers testing
>   APT policy: (900, 'testing'), (500, 'stable'), (400, 'oldstable'), (50, 'unstable')
> Architecture: i386 (x86_64)
> 
> Kernel: Linux 2.6.32-5-amd64 (SMP w/2 CPU cores)
> Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968)
> Shell: /bin/sh linked to /bin/bash
> 
> Versions of packages linux-image-2.6.32-5-amd64 depends on:
> ii  debconf [debconf-2.0]        1.5.32      Debian configuration management sy
> ii  initramfs-tools [linux-initr 0.94.4      tools for generating an initramfs
> ii  linux-base                   2.6.32-15   Linux image base package
> ii  module-init-tools            3.12~pre2-3 tools for managing Linux kernel mo
> 
> Versions of packages linux-image-2.6.32-5-amd64 recommends:
> ii  firmware-linux-free           2.6.32-9   Binary firmware for various driver
> ii  libc6-i686                    2.10.2-9   GNU C Library: Shared libraries [i
> 
> Versions of packages linux-image-2.6.32-5-amd64 suggests:
> pn  grub | lilo                   <none>     (no description available)
> pn  linux-doc-2.6.32              <none>     (no description available)
> 
> Versions of packages linux-image-2.6.32-5-amd64 is related to:
> pn  firmware-bnx2                 <none>     (no description available)
> pn  firmware-bnx2x                <none>     (no description available)
> pn  firmware-ipw2x00              <none>     (no description available)
> pn  firmware-ivtv                 <none>     (no description available)
> pn  firmware-iwlwifi              <none>     (no description available)
> pn  firmware-linux                <none>     (no description available)
> pn  firmware-linux-nonfree        <none>     (no description available)
> pn  firmware-qlogic               <none>     (no description available)
> pn  firmware-ralink               <none>     (no description available)
> pn  xen-hypervisor                <none>     (no description available)
> 
> -- debconf information:
>   linux-image-2.6.32-5-amd64/postinst/missing-firmware-2.6.32-5-amd64:
>   linux-image-2.6.32-5-amd64/postinst/bootloader-error-2.6.32-5-amd64:
>   linux-image-2.6.32-5-amd64/prerm/would-invalidate-boot-loader-2.6.32-5-amd64: true
>   shared/kernel-image/really-run-bootloader: true
>   linux-image-2.6.32-5-amd64/postinst/depmod-error-initrd-2.6.32-5-amd64: false
>   linux-image-2.6.32-5-amd64/prerm/removing-running-kernel-2.6.32-5-amd64: true
>   linux-image-2.6.32-5-amd64/postinst/bootloader-test-error-2.6.32-5-amd64:

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [PATCH] virtio_net: indicate oom when addbuf returns failure
From: Michael S. Tsirkin @ 2010-06-06 20:13 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Herbert Xu, netdev, virtualization, stable, Bruce Rogers
In-Reply-To: <201006041028.56798.rusty@rustcorp.com.au>

On Fri, Jun 04, 2010 at 10:28:56AM +0930, Rusty Russell wrote:
> This patch is a subset of an already upstream patch, but this portion
> is useful in earlier releases.
> 
> Please consider for the 2.6.32 and 2.6.33 stable trees.
> 
> If the add_buf operation fails, indicate failure to the caller.
> 
> Signed-off-by: Bruce Rogers <brogers@novell.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Actually this code looks strange:
Note that add_buf inicates out of memory
condition with a positive return value, and ring full
(which is not an error!) with -ENOSPC.

So it seems that this patch (and upstream code) will fill
the ring and then end up setting oom = true and rescheduling the work
forever.  And I suspect I actually saw this at some point
on one of my systems: observed BW would drop
with high CPU usage until reboot.
Can't reproduce it now anymore ..


> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> 
> @@ -318,6 +318,7 @@ static bool try_fill_recv_maxbufs(struct
>                         skb_unlink(skb, &vi->recv);
>                         trim_pages(vi, skb);
>                         kfree_skb(skb);
> +                       oom = true;
>                         break;
>                 }
>                 vi->num++;
> @@ -368,6 +369,7 @@ static bool try_fill_recv(struct virtnet
>                 if (err < 0) {
>                         skb_unlink(skb, &vi->recv);
>                         kfree_skb(skb);
> +                       oom = true;
>                         break;
>                 }
>                 vi->num++;


Possibly the right thing to do is to
1. handle ENOMEM specially
2. fix add_buf to return ENOMEM on error

Something like the below for upstream (warning: compile
tested only) and a similar one later for stable:


diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 06c30df..85615a3 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -416,7 +416,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, gfp_t gfp)
 static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
 {
 	int err;
-	bool oom = false;
+	bool oom;
 
 	do {
 		if (vi->mergeable_rx_bufs)
@@ -426,10 +426,9 @@ static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
 		else
 			err = add_recvbuf_small(vi, gfp);
 
-		if (err < 0) {
-			oom = true;
+		oom = err == -ENOMEM;
+		if (err < 0)
 			break;
-		}
 		++vi->num;
 	} while (err > 0);
 	if (unlikely(vi->num > vi->max))
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index f9e6fbb..3c7f10a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -119,7 +119,7 @@ static int vring_add_indirect(struct vring_virtqueue *vq,
 
 	desc = kmalloc((out + in) * sizeof(struct vring_desc), gfp);
 	if (!desc)
-		return vq->vring.num;
+		return -ENOMEM;
 
 	/* Transfer entries from the sg list into the indirect page */
 	for (i = 0; i < out; i++) {

^ permalink raw reply related


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