* Re: 2.6.20->2.6.21 - networking dies after random time
From: Marcin Ślusarz @ 2007-08-06 6:58 UTC (permalink / raw)
To: Ingo Molnar
Cc: Jarek Poplawski, Thomas Gleixner, Linus Torvalds,
Jean-Baptiste Vignaud, linux-kernel, shemminger, linux-net,
netdev, Andrew Morton, Alan Cox
In-Reply-To: <20070801072741.GA21551@elte.hu>
2007/8/1, Ingo Molnar <mingo@elte.hu>:
> ok, it wasnt supposed to be _that_ easy i guess :-) Can you please
> (re-)confirm that the workaround below indeed fixes the hung card
> problem? (after producing a single WARN_ON message into the syslog)
yes, with this patch everything works fine
end of dmesg:
EXT3 FS on sda7, internal journal
EXT3-fs: mounted filesystem with ordered data mode.
Adding 1020112k swap on /dev/sda2. Priority:-1 extents:1 across:1020112k
skge eth1: enabling interface
NET: Registered protocol family 17
WARNING: at kernel/irq/resend.c:70 check_irq_resend()
Call Trace:
[<ffffffff8025e5c8>] check_irq_resend+0xa8/0xc0
[<ffffffff8025e1ca>] enable_irq+0xea/0xf0
[<ffffffff8800f21d>] :8390:ei_start_xmit+0x14d/0x30c
[<ffffffff8052b5ce>] dev_hard_start_xmit+0x26e/0x2d0
[<ffffffff80539b10>] __qdisc_run+0xc0/0x1f0
[<ffffffff8052db9f>] dev_queue_xmit+0x24f/0x310
[<ffffffff880d7ac9>] :af_packet:packet_sendmsg+0x259/0x2c0
[<ffffffff8051f0bf>] sock_sendmsg+0xdf/0x110
[<ffffffff8024b8c9>] trace_hardirqs_on+0xd9/0x180
[<ffffffff8024c1dd>] __lock_acquire+0x31d/0xff0
[<ffffffff80243290>] autoremove_wake_function+0x0/0x40
[<ffffffff803e3103>] __up_read+0x23/0xb0
[<ffffffff803e3125>] __up_read+0x45/0xb0
[<ffffffff805bd8f5>] _spin_unlock_irqrestore+0x65/0x80
[<ffffffff8024b8c9>] trace_hardirqs_on+0xd9/0x180
[<ffffffff803e3125>] __up_read+0x45/0xb0
[<ffffffff802464b6>] up_read+0x26/0x30
[<ffffffff8051f4f1>] sys_sendto+0x111/0x150
[<ffffffff8024b8c9>] trace_hardirqs_on+0xd9/0x180
[<ffffffff805bd93b>] _spin_unlock_irq+0x2b/0x60
[<ffffffff8023861a>] do_sigaction+0x11a/0x1d0
[<ffffffff802097fe>] system_call+0x7e/0x83
Marking TSC unstable due to cpufreq changes
Time: acpi_pm clocksource has been installed.
> also, does removing the ne2k-pci module and reinserting it again solve
> the problem too, or is your network card stuck forever once it got into
> that state?
it doesn't change anything - i tried reloading both modules (ne2k_pci and skge)
Marcin
^ permalink raw reply
* Re: 2.6.20->2.6.21 - networking dies after random time
From: Marcin Ślusarz @ 2007-08-06 7:00 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Ingo Molnar, Thomas Gleixner, Linus Torvalds,
Jean-Baptiste Vignaud, linux-kernel, shemminger, linux-net,
netdev, Andrew Morton, Alan Cox
In-Reply-To: <20070731132037.GC1046@ff.dom.local>
2007/7/31, Jarek Poplawski <jarkao2@o2.pl>:
> Marcin,
>
> I see you're quite busy, but if after testing this next Ingo's patch
> you are alive yet, maybe you could try one more "idea"? No patch this
> time, but if you could try this after adding boot option "noirqdebug"
> (I'd like to be sure it's not about timinig after all).
It didn't change anything. Network card still timed out.
Marcin
^ permalink raw reply
* Re: [PATCH] lro: eHEA example how to use LRO
From: Jan-Bernd Themann @ 2007-08-06 7:01 UTC (permalink / raw)
To: Kok, Auke
Cc: netdev, Christoph Raisch, Jan-Bernd Themann, linux-kernel,
linux-ppc, Marcus Eder, Thomas Klein, Stefan Roscher,
Andrew Gallatin, Jeff Garzik, David Miller
In-Reply-To: <46B39030.7030009@intel.com>
Hi Auke,
On Friday 03 August 2007 22:29, Kok, Auke wrote:
> Jan-Bernd Themann wrote:
> > This patch shows how the generic LRO interface is used for SKB mode
> >
> > Signed-off-by: Jan-Bernd Themann <themann@de.ibm.com>
> >
> > ---
> > drivers/net/Kconfig | 1 +
> > drivers/net/ehea/ehea.h | 9 ++++-
> > drivers/net/ehea/ehea_ethtool.c | 15 +++++++
> > drivers/net/ehea/ehea_main.c | 84 +++++++++++++++++++++++++++++++++++---
> > 4 files changed, 101 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > index f8a602c..fec4004 100644
> > --- a/drivers/net/Kconfig
> > +++ b/drivers/net/Kconfig
>
> <snip>
>
> > +module_param(use_lro, int, 0);
>
> Have you looked at my generic lro get/set patch that I posted this week? this
> adds a useless module parameter while ethtool has all the structure already to
> accomodate setting lro on/off.
>
Yes, I saw your patch. Looks very good to me. I will modify our driver soon
to use ethtool instead of this module parameter.
Thanks for adding this support to the ethtool interface.
^ permalink raw reply
* Re: 2.6.20->2.6.21 - networking dies after random time
From: Ingo Molnar @ 2007-08-06 7:03 UTC (permalink / raw)
To: Marcin Ślusarz
Cc: Jarek Poplawski, Thomas Gleixner, Linus Torvalds,
Jean-Baptiste Vignaud, linux-kernel, shemminger, linux-net,
netdev, Andrew Morton, Alan Cox
In-Reply-To: <4bacf17f0708060000n5a00bb77i74adc3b4b28ac42b@mail.gmail.com>
* Marcin Ślusarz <marcin.slusarz@gmail.com> wrote:
> 2007/7/31, Jarek Poplawski <jarkao2@o2.pl>:
> > Marcin,
> >
> > I see you're quite busy, but if after testing this next Ingo's patch
> > you are alive yet, maybe you could try one more "idea"? No patch this
> > time, but if you could try this after adding boot option "noirqdebug"
> > (I'd like to be sure it's not about timinig after all).
> It didn't change anything. Network card still timed out.
please try Jarek's second patch too - there was a missing unmask.
Ingo
-------------->
Subject: genirq: fix simple and fasteoi irq handlers
From: Jarek Poplawski <jarkao2@o2.pl>
After the "genirq: do not mask interrupts by default" patch interrupts
should be disabled not immediately upon request, but after they happen.
But, handle_simple_irq() and handle_fasteoi_irq() can skip this once or
more if an irq is just serviced (IRQ_INPROGRESS), possibly disrupting a
driver's work.
The main reason of problems here, pointing the broken patch and making
the first patch which can fix this was done by Marcin Slusarz.
Additional test patches of Thomas Gleixner and Ingo Molnar tested by
Marcin Slusarz helped to narrow possible reasons even more. Thanks.
PS: this patch fixes only one evident error here, but there could be
more places affected by above-mentioned change in irq handling.
PS 2:
After rethinking, IMHO, there are two most probable scenarios here:
1. After hw resend there could be a conflict between retriggered
edge type irq and the next level type one: e.g. if this level type
irq (io_apic is enabled then) is triggered while retriggered irq is
serviced (IRQ_INPROGRESS) there is goto out with eoi, and probably
the next such levels are triggered and looping, so probably kind of
flood in io_apic until this retriggered edge service has ended.
2. There is something wrong with ioapic_retrigger_irq (less probable
because this should be probably seen with 'normal' edge retriggers,
but on the other hand, they could be less common).
So, if there is #1, this fixed patch should work.
But, since level types don't need this retriggers too much I think
this "don't mask interrupts by default" idea should be rethinked:
is there enough gain to risk such hard to diagnose errors?
So, IMHO, there should be at least possibility to turn this off for
level types in config (it should be a visible option, so people could
find & try this before writing for help or changing a network card).
Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>
---
diff -Nurp 2.6.23-rc1-/kernel/irq/chip.c 2.6.23-rc1/kernel/irq/chip.c
--- 2.6.23-rc1-/kernel/irq/chip.c 2007-07-09 01:32:17.000000000 +0200
+++ 2.6.23-rc1/kernel/irq/chip.c 2007-08-05 21:49:46.000000000 +0200
@@ -295,12 +295,11 @@ handle_simple_irq(unsigned int irq, stru
spin_lock(&desc->lock);
- if (unlikely(desc->status & IRQ_INPROGRESS))
- goto out_unlock;
kstat_cpu(cpu).irqs[irq]++;
action = desc->action;
- if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
+ if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
+ IRQ_DISABLED)))) {
if (desc->chip->mask)
desc->chip->mask(irq);
desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);
@@ -318,6 +317,8 @@ handle_simple_irq(unsigned int irq, stru
spin_lock(&desc->lock);
desc->status &= ~IRQ_INPROGRESS;
+ if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
+ desc->chip->unmask(irq);
out_unlock:
spin_unlock(&desc->lock);
}
@@ -392,18 +393,16 @@ handle_fasteoi_irq(unsigned int irq, str
spin_lock(&desc->lock);
- if (unlikely(desc->status & IRQ_INPROGRESS))
- goto out;
-
desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);
kstat_cpu(cpu).irqs[irq]++;
/*
- * If its disabled or no action available
+ * If it's running, disabled or no action available
* then mask it and get out of here:
*/
action = desc->action;
- if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
+ if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
+ IRQ_DISABLED)))) {
desc->status |= IRQ_PENDING;
if (desc->chip->mask)
desc->chip->mask(irq);
@@ -420,6 +419,8 @@ handle_fasteoi_irq(unsigned int irq, str
spin_lock(&desc->lock);
desc->status &= ~IRQ_INPROGRESS;
+ if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
+ desc->chip->unmask(irq);
out:
desc->chip->eoi(irq);
^ permalink raw reply
* Re: [patch] genirq: fix simple and fasteoi irq handlers
From: Marcin Ślusarz @ 2007-08-06 7:05 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Ingo Molnar, Gabriel C, Linus Torvalds, Thomas Gleixner,
Jean-Baptiste Vignaud, linux-kernel, shemminger, linux-net,
netdev, Andrew Morton, Alan Cox
In-Reply-To: <20070803060733.GA1582@ff.dom.local>
2007/8/3, Jarek Poplawski <jarkao2@o2.pl>:
> Hi,
>
> I can't guarantee this is all needed to fix this bug, but I think
> this patch is necessary here.
>
> Regards,
> Jarek P.
> ------------>
>
> Subject: genirq: fix simple and fasteoi irq handlers
>
> After the "genirq: do not mask interrupts by default" patch interrupts
> should be disabled not immediately upon request, but after they happen.
> But, handle_simple_irq() and handle_fasteoi_irq() can skip this once or
> more if an irq is just serviced (IRQ_INPROGRESS), possibly disrupting a
> driver's work.
>
> The main reason of problems here, pointing the broken patch and making
> the first patch which can fix this was done by Marcin Slusarz.
> Additional test patches of Thomas Gleixner and Ingo Molnar tested by
> Marcin Slusarz helped to narrow possible reasons even more. Thanks.
>
> PS: this patch fixes only one evident error here, but there could be
> more places affected by above-mentioned change in irq handling.
>
>
> Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>
>
> ---
>
> diff -Nurp 2.6.23-rc1-/kernel/irq/chip.c 2.6.23-rc1/kernel/irq/chip.c
> --- 2.6.23-rc1-/kernel/irq/chip.c 2007-07-09 01:32:17.000000000 +0200
> +++ 2.6.23-rc1/kernel/irq/chip.c 2007-08-02 20:42:38.000000000 +0200
> @@ -295,12 +295,11 @@ handle_simple_irq(unsigned int irq, stru
>
> spin_lock(&desc->lock);
>
> - if (unlikely(desc->status & IRQ_INPROGRESS))
> - goto out_unlock;
> kstat_cpu(cpu).irqs[irq]++;
>
> action = desc->action;
> - if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
> + if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
> + IRQ_DISABLED)))) {
> if (desc->chip->mask)
> desc->chip->mask(irq);
> desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);
> @@ -392,18 +391,16 @@ handle_fasteoi_irq(unsigned int irq, str
>
> spin_lock(&desc->lock);
>
> - if (unlikely(desc->status & IRQ_INPROGRESS))
> - goto out;
> -
> desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);
> kstat_cpu(cpu).irqs[irq]++;
>
> /*
> - * If its disabled or no action available
> + * If it's running, disabled or no action available
> * then mask it and get out of here:
> */
> action = desc->action;
> - if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
> + if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
> + IRQ_DISABLED)))) {
> desc->status |= IRQ_PENDING;
> if (desc->chip->mask)
> desc->chip->mask(irq);
>
This patch didn't fix my NIC (tried on 2.6.22.1)
Marcin
^ permalink raw reply
* Re: [patch (take 2)] genirq: fix simple and fasteoi irq handlers
From: Marcin Ślusarz @ 2007-08-06 7:07 UTC (permalink / raw)
To: Ingo Molnar
Cc: Jarek Poplawski, Gabriel C, Linus Torvalds, Thomas Gleixner,
Jean-Baptiste Vignaud, linux-kernel, shemminger, linux-net,
netdev, Andrew Morton, Alan Cox
In-Reply-To: <20070806061459.GA26046@elte.hu>
2007/8/6, Ingo Molnar <mingo@elte.hu>:
>
> * Jarek Poplawski <jarkao2@o2.pl> wrote:
>
> > Subject: genirq: fix simple and fasteoi irq handlers
> >
> > After the "genirq: do not mask interrupts by default" patch interrupts
> > should be disabled not immediately upon request, but after they
> > happen. But, handle_simple_irq() and handle_fasteoi_irq() can skip
> > this once or more if an irq is just serviced (IRQ_INPROGRESS),
> > possibly disrupting a driver's work.
>
> nice fix. I think this is exactly the type of bug we were hoping to be
> able to identify and fix, and it could explain the regression in its
> entirety. The big question - does it fix Marcin's regression?
I'll try this patch tomorrow.
Marcin
^ permalink raw reply
* [RFC]: Driver internal queue mapping
From: David Miller @ 2007-08-06 7:16 UTC (permalink / raw)
To: netdev; +Cc: kaber, hadi
In order to put some energy back into the "device selects
TX queue internally" discussion I've cooked up an example
patch below.
While tossing this together I noticed that the current code
paths seem to not check the queue status at the beginnig of
running the queue, f.e. via dev_queue_xmit(), it just blindly
invokes qdisc_run() which only checks the global queue stopped
status, not the mapped one.
This is cumbersome and isn't going to work very well when we
split up all the locking to be per-queue.
The problem is that qdisc_run() has not enough information.
Proper checks can't be made until we know what packet we are
going to process.
Anyways, here is a patch giving an example interface to let
the driver override skb->queue_mapping. The drivers/net/sunvnet.c
driver would implement this as finding the port number for the
virtual channel that the packet should go over.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4a616d7..c2be6f7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -474,6 +474,8 @@ struct net_device
struct net_device *dev);
/* These may be needed for future network-power-down code. */
unsigned long trans_start; /* Time (in jiffies) of last Tx */
+ void (*tx_queue_select)(struct sk_buff *skb,
+ struct net_device *dev);
int watchdog_timeo; /* used by dev_watchdog() */
struct timer_list watchdog_timer;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index c81649c..ff8742c 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -141,6 +141,9 @@ static inline int qdisc_restart(struct net_device *dev)
if (unlikely((skb = dev_dequeue_skb(dev, q)) == NULL))
return 0;
+ if (dev->tx_queue_select)
+ dev->tx_queue_select(skb, dev);
+
/*
* When the driver has LLTX set, it does its own locking in
* start_xmit. These checks are worth it because even uncongested
^ permalink raw reply related
* Re: [patch (take 2)] genirq: fix simple and fasteoi irq handlers
From: Jarek Poplawski @ 2007-08-06 7:19 UTC (permalink / raw)
To: Ingo Molnar
Cc: Gabriel C, Linus Torvalds, Thomas Gleixner, Jean-Baptiste Vignaud,
linux-kernel, shemminger, linux-net, netdev, Andrew Morton,
Alan Cox, marcin.slusarz
In-Reply-To: <20070806061459.GA26046@elte.hu>
On Mon, Aug 06, 2007 at 08:14:59AM +0200, Ingo Molnar wrote:
>
> * Jarek Poplawski <jarkao2@o2.pl> wrote:
>
> > Subject: genirq: fix simple and fasteoi irq handlers
> >
> > After the "genirq: do not mask interrupts by default" patch interrupts
> > should be disabled not immediately upon request, but after they
> > happen. But, handle_simple_irq() and handle_fasteoi_irq() can skip
> > this once or more if an irq is just serviced (IRQ_INPROGRESS),
> > possibly disrupting a driver's work.
>
> nice fix. I think this is exactly the type of bug we were hoping to be
> able to identify and fix, and it could explain the regression in its
> entirety. The big question - does it fix Marcin's regression?
Alas, there still could be something more... To be more sure, even
with this, there should be some debug printk (which could mess too),
but I don't know how much patience (and similar boxes...) Marcin has.
Of course, this "temporary fix" in -rc2 should give us more time.
But, I think you should confirm this gain with levels (I mean there
could be some saving on flag setting/ checking too). E.g. I've thought
about adding another ioapic_chip struct for fasteoi without .retrigger
(and maybe with .disable = .mask) maybe with some #ifdef CONFIG_...,
but maybe there could be reconsidered IRQ_DELAYED_DISABLE too (but
with this, there probably was a possibility to run this hw ->retrigger
'by chance' too, so with some strange IO-APICS there would be still
an unnecessary risk here).
The big question for me is still why this isn't more common: it seems
some (most of?) IO-APICS have some safety against this?
BTW: Marcin, if you're still willing to test anything (and your box is
alive after my previous 'could not make any damage' patch - sorry!),
this should be done with something before -rc2, so 2.6.22 or .23-rc1.
Jarek P.
PS: I've just read Marcin's messages - so, happily, it seems
everybody's alive! Thanks.
^ permalink raw reply
* Re: [PATCH 1/1] lro: Generic Large Receive Offload for TCP traffic
From: Jan-Bernd Themann @ 2007-08-06 7:51 UTC (permalink / raw)
To: Jörn Engel
Cc: David Miller, Christoph Raisch, Jan-Bernd Themann, linux-kernel,
linux-ppc, Marcus Eder, Thomas Klein, netdev, Andrew Gallatin,
Jeff Garzik, Stefan Roscher
In-Reply-To: <20070803134150.GH19344@lazybastard.org>
Hi Jörn
On Friday 03 August 2007 15:41, Jörn Engel wrote:
> On Fri, 3 August 2007 14:41:19 +0200, Jan-Bernd Themann wrote:
> >
> > This patch provides generic Large Receive Offload (LRO) functionality
> > for IPv4/TCP traffic.
> >
> > LRO combines received tcp packets to a single larger tcp packet and
> > passes them then to the network stack in order to increase performance
> > (throughput). The interface supports two modes: Drivers can either pass
> > SKBs or fragment lists to the LRO engine.
>
> Maybe this is a stupid question, but why is LRO done at the device
> driver level?
>
> If it is a unversal performance benefit, I would have expected it to be
> done generically, i.e. have all packets moved into network layer pass
> through LRO instead.
The driver seems to be the right place:
- There is the "page mode" interface that accepts fragment lists instead of
SKBs and does generate SKBs only in the end (see Andrew Gallatins
mails where he described the advantages of this approach)
- Some drivers (in particular for 10G NICs which actually could benefit
from LRO) have multiple HW receive queues that do some sort of sorting,
thus using one lro_mgr per queue increases the likelyhood of beeing able
to do efficient LRO.
> > +void lro_flush_pkt(struct net_lro_mgr *lro_mgr,
> > + struct iphdr *iph, struct tcphdr *tcph);
> In particular this bit looks like it should be driven by a timeout,
> which would be settable via /proc/sys/net/core/lro_timeout or similar.
No, this function is needed for "page mode" as some HW provides
extra handling for small packets where packets are not stored in preallocated
pages but in extra queues. Thus the driver needs a way to flush old sessions
for this connection and handle these packets in a different way (for example
create a SKB and copy the data there).
Timeouts are not used at all. Experiments showed that flushing at the end
of a NAPI poll round seems to be sufficient (see Andrew's test results)
and does not affect the latency too badly.
Regards,
Jan-Bernd
^ permalink raw reply
* Re: Distributed storage.
From: Evgeniy Polyakov @ 2007-08-06 8:25 UTC (permalink / raw)
To: Daniel Phillips; +Cc: netdev, linux-kernel, linux-fsdevel, Peter Zijlstra
In-Reply-To: <200708051423.45484.phillips@phunq.net>
On Sun, Aug 05, 2007 at 02:23:45PM -0700, Daniel Phillips (phillips@phunq.net) wrote:
> On Sunday 05 August 2007 08:08, Evgeniy Polyakov wrote:
> > If we are sleeping in memory pool, then we already do not have memory
> > to complete previous requests, so we are in trouble.
>
> Not at all. Any requests in flight are guaranteed to get the resources
> they need to complete. This is guaranteed by the combination of memory
> reserve management and request queue throttling. In logical terms,
> reserve management plus queue throttling is necessary and sufficient to
> prevent these deadlocks. Conversely, the absence of either one allows
> deadlock.
Only if you have two, which must be closely related to each other (i.e.
each request must have network reserve big enough to store data).
> > This can work
> > for devices which do not require additional allocations (like usual
> > local storage), but not for network connected ones.
>
> It works for network devices too, and also for a fancy device like
> ddsnap, which is the moral equivalent of a filesystem implemented in
> user space.
With or without vm deadlock patches? I can not see how it can work, if
network does not have a reserve and there is not free memory completely.
If all systems have reserve then yes, it works good.
> > By default things will be like they are now, except additional
> > non-atomic increment and branch in generic_make_request() and
> > decrement and wake in bio_end_io()?
>
> ->endio is called in interrupt context, so the accounting needs to be
> atomic as far as I can see.
Actually we only care about if there is a place in the queue or not - so
it can be a flag. Actually non-atomic operations are ok, since having
plus/minus couple of requests in flight does not change the picture, but
allows not to introduce slow atomic operations in the fast path.
> We actually account the total number of bio pages in flight, otherwise
> you would need to assume the largest possible bio and waste a huge
> amount of reserve memory. A counting semaphore works fine for this
> purpose, with some slight inefficiency that is nigh on unmeasurable in
> the block IO path. What the semaphore does is make the patch small and
> easy to understand, which is important at this point.
Yes, it can be bio vectors.
> > I can cook up such a patch if idea worth efforts.
>
> It is. There are some messy details... You need a place to store the
> accounting variable/semaphore and need to be able to find that place
> again in ->endio. Trickier than it sounds, because of the unstructured
> way drivers rewrite ->bi_bdev. Peterz has already poked at this in a
> number of different ways, typically involving backing_dev_info, which
> seems like a good idea to me.
We can demand that reserve is not per virtual device, but per real one -
for example in case of distributed storage locally connected node should
have much higher limit than network one, but having a per-virtual device
reserve might end up with situation, when local node can proceed data,
but no requests will be queued sine all requests below limit are in
network node. In case of per real device limit there is no need to
increase bio.
--
Evgeniy Polyakov
^ permalink raw reply
* Re: Distributed storage.
From: Evgeniy Polyakov @ 2007-08-06 8:28 UTC (permalink / raw)
To: Daniel Phillips; +Cc: netdev, linux-kernel, linux-fsdevel
In-Reply-To: <200708051435.04542.phillips@phunq.net>
On Sun, Aug 05, 2007 at 02:35:04PM -0700, Daniel Phillips (phillips@phunq.net) wrote:
> On Sunday 05 August 2007 08:01, Evgeniy Polyakov wrote:
> > On Sun, Aug 05, 2007 at 01:06:58AM -0700, Daniel Phillips wrote:
> > > > DST original code worked as device mapper plugin too, but its two
> > > > additional allocations (io and clone) per block request ended up
> > > > for me as a show stopper.
> > >
> > > Ah, sorry, I misread. A show stopper in terms of efficiency, or in
> > > terms of deadlock?
> >
> > At least as in terms of efficiency. Device mapper lives in happy
> > world where memory does not end and allocations are fast.
>
> Are you saying that things are different for a network block device
> because it needs to do GFP_ATOMIC allocations? If so then that is just
> a misunderstanding. The global page reserve Peter and I use is
> available in interrupt context just like GFP_ATOMIC.
No, neither device needs atomic allocations, I just said that device
mapper is too expensive, since it performs alot of additional
allocations in the fast path and is not designed to cases when
allocation fails, since there is no recovery path and (maybe because of
this) mempool allocation waits forever until there is free memory
and can not fail.
--
Evgeniy Polyakov
^ permalink raw reply
* Re: [PATCH 1/4][SSB] EXTIF timing initialization
From: Michael Buesch @ 2007-08-06 8:42 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: netdev, Felix Fietkau
In-Reply-To: <20070805231910.GB13927@hall.aurel32.net>
On Monday 06 August 2007, Aurelien Jarno wrote:
> The patch below against 2.6.23-rc1-mm2 implements EXTIF timing
> initialization, currently marked as TODO.
>
> It originally comes from the OpenWrt patches.
>
> Cc: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>
> --- a/drivers/ssb/driver_mipscore.c
> +++ b/drivers/ssb/driver_mipscore.c
Please do a seperate file driver_extif.c to implement these functions
and call it from the mipscore driver.
Compilation can also depend on the mipscore kconfig option, so no
additional kconfig is needed.
> @@ -4,6 +4,7 @@
> *
> * Copyright 2005, Broadcom Corporation
> * Copyright 2006, 2007, Michael Buesch <mb@bu3sch.de>
> + * Copyright 2006, 2007, Felix Fietkau <nbd@openwrt.org>
> *
> * Licensed under the GNU/GPL. See COPYING for details.
> */
> @@ -31,6 +32,16 @@
> ssb_write32(mcore->dev, offset, value);
> }
>
> +static inline u32 extif_read32(struct ssb_extif *extif, u16 offset)
> +{
> + return ssb_read32(extif->dev, offset);
> +}
> +
> +static inline void extif_write32(struct ssb_extif *extif, u16 offset, u32 value)
> +{
> + ssb_write32(extif->dev, offset, value);
> +}
> +
> static const u32 ipsflag_irq_mask[] = {
> 0,
> SSB_IPSFLAG_IRQ1,
> @@ -183,6 +194,26 @@
> }
> }
>
> +static void ssb_extif_timing_init(struct ssb_extif *extif, u32 ns)
> +{
> + u32 tmp;
> +
> + /* Initialize extif so we can get to the LEDs and external UART */
> + extif_write32(extif, SSB_EXTIF_PROG_CFG, SSB_EXTCFG_EN);
> +
> + /* Set timing for the flash */
> + tmp = DIV_ROUND_UP(10, ns) << SSB_PROG_WCNT_3_SHIFT;
> + tmp |= DIV_ROUND_UP(40, ns) << SSB_PROG_WCNT_1_SHIFT;
> + tmp |= DIV_ROUND_UP(120, ns);
> + extif_write32(extif, SSB_EXTIF_PROG_WAITCNT, tmp);
> +
> + /* Set programmable interface timing for external uart */
> + tmp = DIV_ROUND_UP(10, ns) << SSB_PROG_WCNT_3_SHIFT;
> + tmp |= DIV_ROUND_UP(20, ns) << SSB_PROG_WCNT_2_SHIFT;
> + tmp |= DIV_ROUND_UP(100, ns) << SSB_PROG_WCNT_1_SHIFT;
> + tmp |= DIV_ROUND_UP(120, ns);
> + extif_write32(extif, SSB_EXTIF_PROG_WAITCNT, tmp);
> +}
>
> static void ssb_cpu_clock(struct ssb_mipscore *mcore)
> {
> @@ -205,27 +236,9 @@
> hz = 100000000;
> ns = 1000000000 / hz;
>
> -//TODO
> -#if 0
> - if (have EXTIF) {
> - /* Initialize extif so we can get to the LEDs and external UART */
> - W_REG(&eir->prog_config, CF_EN);
> -
> - /* Set timing for the flash */
> - tmp = CEIL(10, ns) << FW_W3_SHIFT; /* W3 = 10nS */
> - tmp = tmp | (CEIL(40, ns) << FW_W1_SHIFT); /* W1 = 40nS */
> - tmp = tmp | CEIL(120, ns); /* W0 = 120nS */
> - W_REG(&eir->prog_waitcount, tmp); /* 0x01020a0c for a 100Mhz clock */
> -
> - /* Set programmable interface timing for external uart */
> - tmp = CEIL(10, ns) << FW_W3_SHIFT; /* W3 = 10nS */
> - tmp = tmp | (CEIL(20, ns) << FW_W2_SHIFT); /* W2 = 20nS */
> - tmp = tmp | (CEIL(100, ns) << FW_W1_SHIFT); /* W1 = 100nS */
> - tmp = tmp | CEIL(120, ns); /* W0 = 120nS */
> - W_REG(&eir->prog_waitcount, tmp);
> - }
> -#endif
> - if (bus->chipco.dev)
> + if (bus->extif.dev)
> + ssb_extif_timing_init(&bus->extif, ns);
> + else if (bus->chipco.dev)
> ssb_chipco_timing_init(&bus->chipco, ns);
>
> /* Assign IRQs to all cores on the bus, start with irq line 2, because serial usually takes 1 */
>
^ permalink raw reply
* Re: [PATCH 2/4][SSB] EXTIF serial port initialization
From: Michael Buesch @ 2007-08-06 8:43 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: netdev, Felix Fietkau
In-Reply-To: <20070805231959.GC13927@hall.aurel32.net>
On Monday 06 August 2007, Aurelien Jarno wrote:
> The patch below against 2.6.23-rc1-mm2 implements EXTIF serial
> initialization, currently marked as TODO.
>
> It originally comes from the OpenWrt patches.
>
> Cc: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>
> --- a/drivers/ssb/driver_mipscore.c
> +++ b/drivers/ssb/driver_mipscore.c
Also to driver_extif.c, please.
> @@ -128,51 +128,52 @@
> ssb_write32(mdev, SSB_IPSFLAG, irqflag);
> }
>
> -/* XXX: leave here or move into separate extif driver? */
> -static int ssb_extif_serial_init(struct ssb_device *dev, struct ssb_serial_ports *ports)
> +static inline bool serial_exists(u8 *regs)
> {
> + u8 save_mcr, msr = 0;
>
> + if (regs) {
> + save_mcr = regs[UART_MCR];
> + regs[UART_MCR] = (UART_MCR_LOOP | UART_MCR_OUT2 | UART_MCR_RTS);
> + msr = regs[UART_MSR] & (UART_MSR_DCD | UART_MSR_RI \
> + | UART_MSR_CTS | UART_MSR_DSR);
> + regs[UART_MCR] = save_mcr;
> + }
> + return (msr == (UART_MSR_DCD | UART_MSR_CTS));
> }
>
> -
> -static void ssb_mips_serial_init(struct ssb_mipscore *mcore)
> +static int ssb_extif_serial_init(struct ssb_extif *dev, struct ssb_serial_port *ports)
> {
> - struct ssb_bus *bus = mcore->dev->bus;
> + u32 i, nr_ports = 0;
>
> - //TODO if (EXTIF available
> -#if 0
> - extifregs_t *eir = (extifregs_t *) regs;
> - sbconfig_t *sb;
> -
> - /* Determine external UART register base */
> - sb = (sbconfig_t *)((ulong) eir + SBCONFIGOFF);
> - base = EXTIF_CFGIF_BASE(sb_base(R_REG(&sb->sbadmatch1)));
> -
> - /* Determine IRQ */
> - irq = sb_irq(sbh);
> -
> - /* Disable GPIO interrupt initially */
> - W_REG(&eir->gpiointpolarity, 0);
> - W_REG(&eir->gpiointmask, 0);
> -
> - /* Search for external UARTs */
> - n = 2;
> - for (i = 0; i < 2; i++) {
> - regs = (void *) REG_MAP(base + (i * 8), 8);
> - if (BCMINIT(serial_exists)(regs)) {
> - /* Set GPIO 1 to be the external UART IRQ */
> - W_REG(&eir->gpiointmask, 2);
> - if (add)
> - add(regs, irq, 13500000, 0);
> - }
> + /* Disable GPIO interrupt initially */
> + extif_write32(dev, SSB_EXTIF_GPIO_INTPOL, 0);
> + extif_write32(dev, SSB_EXTIF_GPIO_INTMASK, 0);
> +
> + for (i = 0; i < 2; i++) {
> + void __iomem *uart_regs;
> +
> + uart_regs = ioremap_nocache(SSB_EUART, 16);
> + uart_regs += (i * 8);
> +
> + if (serial_exists(uart_regs) && ports) {
> + extif_write32(dev, SSB_EXTIF_GPIO_INTMASK, 2);
> +
> + nr_ports++;
> + ports[i].regs = uart_regs;
> + ports[i].irq = 2;
> + ports[i].baud_base = 13500000;
> + ports[i].reg_shift = 0;
> }
> + iounmap(uart_regs);
> + }
> + return nr_ports;
> +}
>
> - /* Add internal UART if enabled */
> - if (R_REG(&eir->corecontrol) & CC_UE)
> - if (add)
> - add((void *) &eir->uartdata, irq, sb_clock(sbh), 2);
> +static void ssb_mips_serial_init(struct ssb_mipscore *mcore)
> +{
> + struct ssb_bus *bus = mcore->dev->bus;
>
> -#endif
> if (bus->extif.dev)
> mcore->nr_serial_ports = ssb_extif_serial_init(&bus->extif, mcore->serial_ports);
> else if (bus->chipco.dev)
>
^ permalink raw reply
* phy_mii_ioctl - phydev->advertising ethtool.h vs. mii.h
From: Domen Puncer @ 2007-08-06 8:45 UTC (permalink / raw)
To: netdev; +Cc: macro
Hi!
I've noticed mii-tool doesn't work right for a driver I'm just
developing (it uses generic phy layer).
Driver's ioctl is wired directly to phy_mii_ioctl().
"mii-tool -A something" shows the problem is in
phy_mii_ioctl -> SIOCSMIIREG:
392 case MII_ADVERTISE:
393 phydev->advertising = val;
394 break;
phydev->advertising value should be combined with ones from
ethtool.h (at least in other usages it is):
437 #define ADVERTISED_10baseT_Half (1 << 0)
438 #define ADVERTISED_10baseT_Full (1 << 1)
439 #define ADVERTISED_100baseT_Half (1 << 2)
440 #define ADVERTISED_100baseT_Full (1 << 3)
but mii-tool sends a mask with values from mii.h:
70 #define ADVERTISE_10HALF 0x0020 /* Try for 10mbps half-duplex */
71 #define ADVERTISE_1000XFULL 0x0020 /* Try for 1000BASE-X full-duplex */
72 #define ADVERTISE_10FULL 0x0040 /* Try for 10mbps full-duplex */
73 #define ADVERTISE_1000XHALF 0x0040 /* Try for 1000BASE-X half-duplex */
74 #define ADVERTISE_100HALF 0x0080 /* Try for 100mbps half-duplex */
...
I'm I right that this is a bug?
Comments, suggestions how to fix it?
Domen
^ permalink raw reply
* Re: [PATCH 3/4][SSB] MIPS core flash driver
From: Michael Buesch @ 2007-08-06 8:45 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: netdev, Felix Fietkau
In-Reply-To: <20070805232044.GD13927@hall.aurel32.net>
On Monday 06 August 2007, Aurelien Jarno wrote:
> The patch below against 2.6.23-rc1-mm2 fixes the size of the flash
> window and detect the width of the flash bus (8 or 16 bits).
>
> It originally comes from the OpenWrt patches.
>
> Cc: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>
> --- a/drivers/ssb/driver_mipscore.c
> +++ b/drivers/ssb/driver_mipscore.c
> @@ -186,12 +186,16 @@
> {
> struct ssb_bus *bus = mcore->dev->bus;
>
> + mcore->flash_buswidth = 2;
> if (bus->chipco.dev) {
> mcore->flash_window = 0x1c000000;
> - mcore->flash_window_size = 0x800000;
> + mcore->flash_window_size = 0x02000000;
> + if ((ssb_read32(bus->chipco.dev, SSB_CHIPCO_FLASH_CFG)
> + & SSB_CHIPCO_CFG_DS16) == 0)
> + mcore->flash_buswidth = 1;
> } else {
> mcore->flash_window = 0x1fc00000;
> - mcore->flash_window_size = 0x400000;
> + mcore->flash_window_size = 0x00400000;
> }
> }
>
> --- a/include/linux/ssb/ssb_driver_mips.h
> +++ b/include/linux/ssb/ssb_driver_mips.h
> @@ -20,6 +20,7 @@
> int nr_serial_ports;
> struct ssb_serial_port serial_ports[4];
>
> + int flash_buswidth;
u8 please (although it doesn't save space in this case (padding)),
but the struct could be reorganized to save space later.
> u32 flash_window;
> u32 flash_window_size;
> };
>
^ permalink raw reply
* Re: [PATCH 4/4][SSB] Implement ssb_cpu_clock()
From: Michael Buesch @ 2007-08-06 8:46 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: netdev, Felix Fietkau
In-Reply-To: <20070805232129.GE13927@hall.aurel32.net>
On Monday 06 August 2007, Aurelien Jarno wrote:
> The patch below against 2.6.23-rc1-mm2 implements the ssb_cpu_clock()
> currently doing nothing, and export it. This function is needed to
> support the BCM947xx CPUs.
>
> It originally comes from the OpenWrt patches.
>
> Cc: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>
> --- a/drivers/ssb/driver_mipscore.c
> +++ b/drivers/ssb/driver_mipscore.c
> @@ -220,8 +220,37 @@
> extif_write32(extif, SSB_EXTIF_PROG_WAITCNT, tmp);
> }
>
> -static void ssb_cpu_clock(struct ssb_mipscore *mcore)
> +void ssb_extif_get_clockcontrol(struct ssb_extif *extif,
> + u32 *pll_type, u32 *n, u32 *m)
> {
> + *pll_type = SSB_PLLTYPE_1;
> + *n = extif_read32(extif, SSB_EXTIF_CLOCK_N);
> + *m = extif_read32(extif, SSB_EXTIF_CLOCK_SB);
> +}
The extif functions to driver_extif.c, please.
> +u32 ssb_cpu_clock(struct ssb_mipscore *mcore)
> +{
> + struct ssb_bus *bus = mcore->dev->bus;
> + u32 pll_type, n, m, rate = 0;
> +
> + if (bus->extif.dev) {
> + ssb_extif_get_clockcontrol(&bus->extif, &pll_type, &n, &m);
> + } else if (bus->chipco.dev) {
> + ssb_chipco_get_clockcpu(&bus->chipco, &pll_type, &n, &m);
> + } else
> + return 0;
> +
> + if ((pll_type == SSB_PLLTYPE_5) || (bus->chip_id == 0x5365)) {
> + rate = 200000000;
> + } else {
> + rate = ssb_calc_clock_rate(pll_type, n, m);
> + }
> +
> + if (pll_type == SSB_PLLTYPE_6) {
> + rate *= 2;
> + }
> +
> + return rate;
> }
>
> void ssb_mipscore_init(struct ssb_mipscore *mcore)
> --- a/drivers/ssb/main.c
> +++ b/drivers/ssb/main.c
> @@ -819,12 +819,12 @@
> u32 plltype;
> u32 clkctl_n, clkctl_m;
>
> - //TODO if EXTIF: PLLTYPE == 1, read n from clockcontrol_n, m from clockcontrol_sb
> -
> - if (bus->chipco.dev) {
> + if (bus->extif.dev)
> + ssb_extif_get_clockcontrol(&bus->extif, &plltype, &clkctl_n, &clkctl_m);
> + else if (bus->chipco.dev)
> ssb_chipco_get_clockcontrol(&bus->chipco, &plltype,
> &clkctl_n, &clkctl_m);
> - } else
> + else
> return 0;
>
> if (bus->chip_id == 0x5365) {
> --- a/include/linux/ssb/ssb_driver_extif.h
> +++ b/include/linux/ssb/ssb_driver_extif.h
> @@ -156,5 +156,6 @@
> /* watchdog */
> #define SSB_EXTIF_WATCHDOG_CLK 48000000 /* Hz */
>
> +extern void ssb_extif_get_clockcontrol(struct ssb_extif *, u32 *, u32 *, u32 *);
>
> #endif /* LINUX_SSB_EXTIFCORE_H_ */
> --- a/include/linux/ssb/ssb_driver_mips.h
> +++ b/include/linux/ssb/ssb_driver_mips.h
> @@ -26,6 +26,7 @@
> };
>
> extern void ssb_mipscore_init(struct ssb_mipscore *mcore);
> +extern u32 ssb_cpu_clock(struct ssb_mipscore *mcore);
>
> extern unsigned int ssb_mips_irq(struct ssb_device *dev);
>
>
^ permalink raw reply
* Re: [PATCH 2/4][SSB] EXTIF serial port initialization
From: Michael Buesch @ 2007-08-06 8:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: Aurelien Jarno, netdev, Felix Fietkau
In-Reply-To: <20070806002117.GJ2469@ghostprotocols.net>
On Monday 06 August 2007, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 06, 2007 at 01:19:59AM +0200, Aurelien Jarno escreveu:
> > The patch below against 2.6.23-rc1-mm2 implements EXTIF serial
> > initialization, currently marked as TODO.
> >
> > It originally comes from the OpenWrt patches.
>
> Comments below
>
> > Cc: Felix Fietkau <nbd@openwrt.org>
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >
> > --- a/drivers/ssb/driver_mipscore.c
> > +++ b/drivers/ssb/driver_mipscore.c
> > @@ -128,51 +128,52 @@
> > ssb_write32(mdev, SSB_IPSFLAG, irqflag);
> > }
> >
> > -/* XXX: leave here or move into separate extif driver? */
> > -static int ssb_extif_serial_init(struct ssb_device *dev, struct ssb_serial_ports *ports)
> > +static inline bool serial_exists(u8 *regs)
> > {
> > + u8 save_mcr, msr = 0;
>
> Why declare save_mcr here...
>
> >
> > + if (regs) {
>
> if it is just used here?
There is almost never an advantage when declaring variables at the beginning
of a statement block instead of the function start.
The only thing you "gain" is that you don't easily see anymore how much
stackspace is used by the funtions.
So I really suggest to declare variables at the beginning of functions,
except for some rare circumstances maybe. Which is not the case here.
But anyway. Just saw this: Drop the inline statement. There is no
need to inline this function.
^ permalink raw reply
* [PATCH 2.6.23 1/3]S2IO: Making MSIX as default intr_type
From: Sivakumar Subramani @ 2007-08-06 9:33 UTC (permalink / raw)
To: netdev, jeff; +Cc: support
- Making MSIX as default intr_type
- Driver will test MSI-X by issuing test MSI-X vector and if fails it will
fallback to INTA
Signed-off-by: Sivakumar Subramani <sivakumar.subramani@neterion.com>
Signed-off-by: Ramkrishna Vepa <ram.vepa@neterion.com>
---
diff -urpN org/drivers/net/s2io.c patch1/drivers/net/s2io.c
--- org/drivers/net/s2io.c 2007-07-31 10:50:04.000000000 +0530
+++ patch1/drivers/net/s2io.c 2007-07-31 11:04:50.000000000 +0530
@@ -37,7 +37,7 @@
* tx_fifo_len: This too is an array of 8. Each element defines the number of
* Tx descriptors that can be associated with each corresponding FIFO.
* intr_type: This defines the type of interrupt. The values can be 0(INTA),
- * 2(MSI_X). Default value is '0(INTA)'
+ * 2(MSI_X). Default value is '2(MSI_X)'
* lro: Specifies whether to enable Large Receive Offload (LRO) or not.
* Possible values '1' for enable '0' for disable. Default is '0'
* lro_max_pkts: This parameter defines maximum number of packets can be
@@ -428,7 +428,7 @@ S2IO_PARM_INT(l3l4hdr_size, 128);
/* Frequency of Rx desc syncs expressed as power of 2 */
S2IO_PARM_INT(rxsync_frequency, 3);
/* Interrupt type. Values can be 0(INTA), 2(MSI_X) */
-S2IO_PARM_INT(intr_type, 0);
+S2IO_PARM_INT(intr_type, 2);
/* Large receive offload feature */
S2IO_PARM_INT(lro, 0);
/* Max pkts to be aggregated by LRO at one time. If not specified,
@@ -3781,6 +3781,59 @@ static int s2io_enable_msi_x(struct s2io
return 0;
}
+/* Handle software interrupt used during MSI(X) test */
+static irqreturn_t __devinit s2io_test_intr(int irq, void *dev_id)
+{
+ struct s2io_nic *sp = dev_id;
+
+ sp->msi_detected = 1;
+ wake_up(&sp->msi_wait);
+
+ return IRQ_HANDLED;
+}
+
+/* Test interrupt path by forcing a a software IRQ */
+static int __devinit s2io_test_msi(struct s2io_nic *sp)
+{
+ struct pci_dev *pdev = sp->pdev;
+ struct XENA_dev_config __iomem *bar0 = sp->bar0;
+ int err;
+ u64 val64, saved64;
+
+ err = request_irq(sp->entries[1].vector, s2io_test_intr, 0,
+ sp->name, sp);
+ if (err) {
+ DBG_PRINT(ERR_DBG, "%s: PCI %s: cannot assign irq %d\n",
+ sp->dev->name, pci_name(pdev), pdev->irq);
+ return err;
+ }
+
+ init_waitqueue_head (&sp->msi_wait);
+ sp->msi_detected = 0;
+
+ saved64 = val64 = readq(&bar0->scheduled_int_ctrl);
+ val64 |= SCHED_INT_CTRL_ONE_SHOT;
+ val64 |= SCHED_INT_CTRL_TIMER_EN;
+ val64 |= SCHED_INT_CTRL_INT2MSI(1);
+ writeq(val64, &bar0->scheduled_int_ctrl);
+
+ wait_event_timeout(sp->msi_wait, sp->msi_detected, HZ/10);
+
+ if (!sp->msi_detected) {
+ /* MSI(X) test failed, go back to INTx mode */
+ DBG_PRINT(ERR_DBG, "%s: PCI %s: No interrupt was generated"
+ "using MSI(X) during test\n", sp->dev->name,
+ pci_name(pdev));
+
+ err = -EOPNOTSUPP;
+ }
+
+ free_irq(sp->entries[1].vector, sp);
+
+ writeq(saved64, &bar0->scheduled_int_ctrl);
+
+ return err;
+}
/* ********************************************************* *
* Functions defined below concern the OS part of the driver *
* ********************************************************* */
@@ -3809,6 +3862,42 @@ static int s2io_open(struct net_device *
netif_carrier_off(dev);
sp->last_link_state = 0;
+ if (sp->intr_type == MSI_X) {
+ int ret = s2io_enable_msi_x(sp);
+
+ if (!ret) {
+ u16 msi_control;
+
+ ret = s2io_test_msi(sp);
+
+ /* rollback MSI-X, will re-enable during add_isr() */
+ kfree(sp->entries);
+ sp->mac_control.stats_info->sw_stat.mem_freed +=
+ (MAX_REQUESTED_MSI_X *
+ sizeof(struct msix_entry));
+ kfree(sp->s2io_entries);
+ sp->mac_control.stats_info->sw_stat.mem_freed +=
+ (MAX_REQUESTED_MSI_X *
+ sizeof(struct s2io_msix_entry));
+ sp->entries = NULL;
+ sp->s2io_entries = NULL;
+
+ pci_read_config_word(sp->pdev, 0x42, &msi_control);
+ msi_control &= 0xFFFE; /* Disable MSI */
+ pci_write_config_word(sp->pdev, 0x42, msi_control);
+
+ pci_disable_msix(sp->pdev);
+
+ }
+ if (ret) {
+
+ DBG_PRINT(ERR_DBG,
+ "%s: MSI-X requested but failed to enable\n",
+ dev->name);
+ sp->intr_type = INTA;
+ }
+ }
+
/* Initialize H/W and enable interrupts */
err = s2io_card_up(sp);
if (err) {
diff -urpN org/drivers/net/s2io.h patch1/drivers/net/s2io.h
--- org/drivers/net/s2io.h 2007-07-31 10:50:04.000000000 +0530
+++ patch1/drivers/net/s2io.h 2007-07-31 10:47:06.000000000 +0530
@@ -412,6 +412,10 @@ struct config_param {
struct tx_fifo_config tx_cfg[MAX_TX_FIFOS]; /*Per-Tx FIFO config */
u32 max_txds; /*Max no. of Tx buffer descriptor per TxDL */
u64 tx_intr_type;
+#define INTA 0
+#define MSI_X 2
+ u8 intr_type;
+
/* Specifies if Tx Intr is UTILZ or PER_LIST type. */
/* Rx Side */
@@ -861,6 +865,8 @@ struct s2io_nic {
struct vlan_group *vlgrp;
#define MSIX_FLG 0xA5
struct msix_entry *entries;
+ int msi_detected;
+ wait_queue_head_t msi_wait;
struct s2io_msix_entry *s2io_entries;
char desc[MAX_REQUESTED_MSI_X][25];
diff -urpN org/drivers/net/s2io-regs.h patch1/drivers/net/s2io-regs.h
--- org/drivers/net/s2io-regs.h 2007-07-31 10:50:04.000000000 +0530
+++ patch1/drivers/net/s2io-regs.h 2007-07-31 10:47:32.000000000 +0530
@@ -220,7 +220,7 @@ struct XENA_dev_config {
u64 scheduled_int_ctrl;
#define SCHED_INT_CTRL_TIMER_EN BIT(0)
#define SCHED_INT_CTRL_ONE_SHOT BIT(1)
-#define SCHED_INT_CTRL_INT2MSI TBD
+#define SCHED_INT_CTRL_INT2MSI(val) vBIT(val,10,6)
#define SCHED_INT_PERIOD TBD
u64 txreqtimeout;
^ permalink raw reply
* [PATCH 2.6.23 2/3]S2IO: Fixes in MSIX related code.
From: Sivakumar Subramani @ 2007-08-06 9:36 UTC (permalink / raw)
To: netdev, jeff; +Cc: support
- Calling store_xmsi_data to store the MSI-X datas during initialization
in s2io-init_nic function
- Disabling NAPI when MSI-X is enabled
- Freeing sp->entries and sp->s2io_entries in s2io_rem_isr
Signed-off-by: Sivakumar Subramani <sivakumar.subramani@neterion.com>
Signed-off-by: Ramkrishna Vepa <ram.vepa@neterion.com>
---
diff -urpN patch1/drivers/net/s2io.c patch2/drivers/net/s2io.c
--- patch1/drivers/net/s2io.c 2007-07-31 11:04:50.000000000 +0530
+++ patch2/drivers/net/s2io.c 2007-08-02 16:51:57.000000000 +0530
@@ -3898,6 +3898,12 @@ static int s2io_open(struct net_device *
}
}
+ /* NAPI doesn't work well with MSI(X) */
+ if (sp->intr_type != INTA) {
+ if(sp->config.napi)
+ sp->config.napi = 0;
+ }
+
/* Initialize H/W and enable interrupts */
err = s2io_card_up(sp);
if (err) {
@@ -6472,6 +6478,7 @@ static void s2io_rem_isr(struct s2io_nic
{
int cnt = 0;
struct net_device *dev = sp->dev;
+ struct swStat *stats = &sp->mac_control.stats_info->sw_stat;
if (sp->intr_type == MSI_X) {
int i;
@@ -6484,6 +6491,16 @@ static void s2io_rem_isr(struct s2io_nic
free_irq(vector, arg);
}
+
+ kfree(sp->entries);
+ stats->mem_freed +=
+ (MAX_REQUESTED_MSI_X * sizeof(struct msix_entry));
+ kfree(sp->s2io_entries);
+ stats->mem_freed +=
+ (MAX_REQUESTED_MSI_X * sizeof(struct s2io_msix_entry));
+ sp->entries = NULL;
+ sp->s2io_entries = NULL;
+
pci_read_config_word(sp->pdev, 0x42, &msi_control);
msi_control &= 0xFFFE; /* Disable MSI */
pci_write_config_word(sp->pdev, 0x42, msi_control);
@@ -7379,6 +7396,8 @@ s2io_init_nic(struct pci_dev *pdev, cons
dev->addr_len = ETH_ALEN;
memcpy(dev->dev_addr, sp->def_mac_addr, ETH_ALEN);
+ /* Store the values of the MSIX table in the s2io_nic structure */
+ store_xmsi_data(sp);
/* reset Nic and bring it to known state */
s2io_reset(sp);
diff -urpN patch1/drivers/net/s2io.h patch2/drivers/net/s2io.h
--- patch1/drivers/net/s2io.h 2007-07-31 10:47:06.000000000 +0530
+++ patch2/drivers/net/s2io.h 2007-07-31 10:57:35.000000000 +0530
@@ -415,6 +415,7 @@ struct config_param {
#define INTA 0
#define MSI_X 2
u8 intr_type;
+ u8 napi;
/* Specifies if Tx Intr is UTILZ or PER_LIST type. */
^ permalink raw reply
* [PATCH 2.6.23 3/3]S2IO: Fixed Link LED issue when MSI-X is enabled
From: Sivakumar Subramani @ 2007-08-06 9:38 UTC (permalink / raw)
To: netdev, jeff; +Cc: support
-Fixed Link LED issue when MSI-X is enabled.
Signed-off-by: Sivakumar Subramani <sivakumar.subramani@neterion.com>
Signed-off-by: Ramkrishna Vepa <ram.vepa@neterion.com>
---
diff -urpN patch2/drivers/net/s2io.c patch3/drivers/net/s2io.c
--- patch2/drivers/net/s2io.c 2007-08-02 17:00:36.000000000 +0530
+++ patch3/drivers/net/s2io.c 2007-08-02 16:59:40.000000000 +0530
@@ -84,7 +84,7 @@
#include "s2io.h"
#include "s2io-regs.h"
-#define DRV_VERSION "2.0.25.1"
+#define DRV_VERSION "2.0.26.1"
/* S2io Driver name & version. */
static char s2io_driver_name[] = "Neterion";
@@ -6205,13 +6205,10 @@ static void s2io_set_link(struct work_st
netif_stop_queue(dev);
}
}
- val64 = readq(&bar0->adapter_status);
- if (!LINK_IS_UP(val64)) {
- DBG_PRINT(ERR_DBG, "%s:", dev->name);
- DBG_PRINT(ERR_DBG, " Link down after enabling ");
- DBG_PRINT(ERR_DBG, "device \n");
- } else
- s2io_link(nic, LINK_UP);
+ val64 = readq(&bar0->adapter_control);
+ val64 |= ADAPTER_LED_ON;
+ writeq(val64, &bar0->adapter_control);
+ s2io_link(nic, LINK_UP);
} else {
if (CARDS_WITH_FAULTY_LINK_INDICATORS(nic->device_type,
subid)) {
@@ -6220,6 +6217,10 @@ static void s2io_set_link(struct work_st
writeq(val64, &bar0->gpio_control);
val64 = readq(&bar0->gpio_control);
}
+ /* turn off LED */
+ val64 = readq(&bar0->adapter_control);
+ val64 = val64 &(~ADAPTER_LED_ON);
+ writeq(val64, &bar0->adapter_control);
s2io_link(nic, LINK_DOWN);
}
clear_bit(0, &(nic->link_state));
^ permalink raw reply
* Re: e1000 doesn't resume properly from standby (2.6.23-rc2)
From: Pavel Machek @ 2007-08-06 10:44 UTC (permalink / raw)
To: Kok, Auke; +Cc: Simon Arlott, linux-pm, Linux Kernel Mailing List, netdev
In-Reply-To: <46B68F08.60106@intel.com>
Hi!
> >00:0a.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet
> >Controller (Copper) (rev 01)
> > Subsystem: Intel Corp.: Unknown device 1012
> > Flags: bus master, 66Mhz, medium devsel, latency 32, IRQ 5
> > Memory at e3020000 (64-bit, non-prefetchable) [size=128K]
> > I/O ports at b000 [size=64]
> > Capabilities: [dc] Power Management version 2
> > Capabilities: [e4] PCI-X non-bridge device.
> > Capabilities: [f0] Message Signalled Interrupts: 64bit+ Queue=0/0
> > Enable-
> >
> >00:0a.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet
> >Controller (Copper) (rev 01)
> > Subsystem: Intel Corp.: Unknown device 1012
> > Flags: bus master, 66Mhz, medium devsel, latency 32, IRQ 12
> > Memory at e3000000 (64-bit, non-prefetchable) [size=128K]
> > I/O ports at b400 [size=64]
> > Capabilities: [dc] Power Management version 2
> > Capabilities: [e4] PCI-X non-bridge device.
> > Capabilities: [f0] Message Signalled Interrupts: 64bit+ Queue=0/0
> > Enable-
> >
> >[ 950.132046] Stopping tasks ... done.
> >[ 950.459794] Suspending console(s)
> >[ 951.776277] pnp: Device 00:0c disabled.
> >[ 951.776673] pnp: Device 00:0a disabled.
> >[ 951.776984] pnp: Device 00:09 disabled.
> >[ 951.777306] pnp: Device 00:08 disabled.
> >[ 951.777786] ACPI: PCI interrupt for device 0000:00:11.5 disabled
> >[ 951.995359] ACPI: PCI interrupt for device 0000:00:11.3 disabled
> >[ 952.006094] ACPI: PCI interrupt for device 0000:00:11.2 disabled
> >[ 952.022243] ACPI handle has no context!
> >[ 952.033068] ACPI: PCI interrupt for device 0000:00:0c.2 disabled
> >[ 952.044086] ACPI: PCI interrupt for device 0000:00:0c.1 disabled
> >[ 952.055083] ACPI: PCI interrupt for device 0000:00:0c.0 disabled
> >[ 952.282211] ACPI: PCI interrupt for device 0000:00:0a.1 disabled
> >[ 952.282221] ACPI handle has no context!
> >[ 952.537474] ACPI: PCI interrupt for device 0000:00:0a.0 disabled
> >[ 952.537495] ACPI handle has no context!
> >
> >[ 956.857085] Back to C!
Are you sure that is standby? Looks like suspend-to-RAM to me.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: e1000 doesn't resume properly from standby (2.6.23-rc2)
From: Simon Arlott @ 2007-08-06 11:37 UTC (permalink / raw)
To: Pavel Machek; +Cc: Kok, Auke, linux-pm, Linux Kernel Mailing List, netdev
In-Reply-To: <20070806104456.GA1901@elf.ucw.cz>
On Mon, August 6, 2007 11:44, Pavel Machek wrote:
> Hi!
>
>> >00:0a.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet
>> >Controller (Copper) (rev 01)
>> > Subsystem: Intel Corp.: Unknown device 1012
>> > Flags: bus master, 66Mhz, medium devsel, latency 32, IRQ 5
>> > Memory at e3020000 (64-bit, non-prefetchable) [size=128K]
>> > I/O ports at b000 [size=64]
>> > Capabilities: [dc] Power Management version 2
>> > Capabilities: [e4] PCI-X non-bridge device.
>> > Capabilities: [f0] Message Signalled Interrupts: 64bit+ Queue=0/0
>> > Enable-
>> >
>> >00:0a.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet
>> >Controller (Copper) (rev 01)
>> > Subsystem: Intel Corp.: Unknown device 1012
>> > Flags: bus master, 66Mhz, medium devsel, latency 32, IRQ 12
>> > Memory at e3000000 (64-bit, non-prefetchable) [size=128K]
>> > I/O ports at b400 [size=64]
>> > Capabilities: [dc] Power Management version 2
>> > Capabilities: [e4] PCI-X non-bridge device.
>> > Capabilities: [f0] Message Signalled Interrupts: 64bit+ Queue=0/0
>> > Enable-
>> >
>> >[ 950.132046] Stopping tasks ... done.
>> >[ 950.459794] Suspending console(s)
>> >[ 951.776277] pnp: Device 00:0c disabled.
>> >[ 951.776673] pnp: Device 00:0a disabled.
>> >[ 951.776984] pnp: Device 00:09 disabled.
>> >[ 951.777306] pnp: Device 00:08 disabled.
>> >[ 951.777786] ACPI: PCI interrupt for device 0000:00:11.5 disabled
>> >[ 951.995359] ACPI: PCI interrupt for device 0000:00:11.3 disabled
>> >[ 952.006094] ACPI: PCI interrupt for device 0000:00:11.2 disabled
>> >[ 952.022243] ACPI handle has no context!
>> >[ 952.033068] ACPI: PCI interrupt for device 0000:00:0c.2 disabled
>> >[ 952.044086] ACPI: PCI interrupt for device 0000:00:0c.1 disabled
>> >[ 952.055083] ACPI: PCI interrupt for device 0000:00:0c.0 disabled
>> >[ 952.282211] ACPI: PCI interrupt for device 0000:00:0a.1 disabled
>> >[ 952.282221] ACPI handle has no context!
>> >[ 952.537474] ACPI: PCI interrupt for device 0000:00:0a.0 disabled
>> >[ 952.537495] ACPI handle has no context!
>> >
>> >[ 956.857085] Back to C!
>
> Are you sure that is standby? Looks like suspend-to-RAM to me.
It's S1 (power-on suspend/standby), my BIOS/motherboard doesn't support S2 or S3.
--
Simon Arlott
^ permalink raw reply
* [PATCH] docs: note about select in kconfig-language.txt
From: Jarek Poplawski @ 2007-08-06 11:51 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Matt Mackall, Gabriel C, Andrew Morton, linux-kernel, netdev,
jason.wessel, amitkale, Satyam Sharma
In-Reply-To: <20070802093659.GB27748@uranus.ravnborg.org>
Hi,
If there are no other plans of kconfig docs update I think this
message of yours should be useful enough. I made minimal fixes,
so if they are wrong or you prefer it otherwise, I'm OK with any
change to this proposal, including replacement with something
else.
Thanks & regards,
Jarek P.
On Thu, Aug 02, 2007 at 11:36:59AM +0200, Sam Ravnborg wrote:
...
> > > seems to select NET_POLL_CONTROLLER after selecting NETPOLL, but
> > > still doesn't check for NETDEVICES dependency.
> >
> > That's odd. Adding Sam to the cc:.
>
> select is evil....
> select will by brute force set a symbol equal to 'y' without
> visiting the dependencies.
> So abusing select you are able to select a symbol FOO even
> if FOO depends on BAR that is not set.
>
> In general use select only for non-visible symbols (no promts anywhere)
> and for symbols with no dependencies.
> That will limit the suefullness but on the other hand avoid the illegal
> configurations all over.
>
> kconfig should one day warn about such things but I have not fel inclined
> to dive into the matters hoping that Roman does one day.
>
> Sam
>
----------->
Subject: docs: a warning note about select in kconfig-language.txt
A warning note of Sam Ravnborg about kconfig's select evilness,
dependencies and the future (slightly corrected).
Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
diff -Nu9r 2.6.23-rc1-/Documentation/kbuild/kconfig-language.txt 2.6.23-rc1/Documentation/kbuild/kconfig-language.txt
--- 2.6.23-rc1-/Documentation/kbuild/kconfig-language.txt 2007-07-09 01:32:17.000000000 +0200
+++ 2.6.23-rc1/Documentation/kbuild/kconfig-language.txt 2007-08-06 12:50:34.000000000 +0200
@@ -92,18 +92,27 @@
- reverse dependencies: "select" <symbol> ["if" <expr>]
While normal dependencies reduce the upper limit of a symbol (see
below), reverse dependencies can be used to force a lower limit of
another symbol. The value of the current menu symbol is used as the
minimal value <symbol> can be set to. If <symbol> is selected multiple
times, the limit is set to the largest selection.
Reverse dependencies can only be used with boolean or tristate
symbols.
+ Note:
+ select is evil.... select will by brute force set a symbol
+ equal to 'y' without visiting the dependencies. So abusing
+ select you are able to select a symbol FOO even if FOO depends
+ on BAR that is not set. In general use select only for
+ non-visible symbols (no promts anywhere) and for symbols with
+ no dependencies. That will limit the usefulness but on the
+ other hand avoid the illegal configurations all over. kconfig
+ should one day warn about such things.
- numerical ranges: "range" <symbol> <symbol> ["if" <expr>]
This allows to limit the range of possible input values for int
and hex symbols. The user can only input a value which is larger than
or equal to the first symbol and smaller than or equal to the second
symbol.
- help text: "help" or "---help---"
This defines a help text. The end of the help text is determined by
^ permalink raw reply
* [RESEND][PATCH 1/3] ehea: Fix workqueue handling
From: Thomas Klein @ 2007-08-06 11:54 UTC (permalink / raw)
To: Jeff Garzik
Cc: Christoph Raisch, Jan-Bernd Themann, Jan-Bernd Themann,
linux-kernel, linux-ppc, Marcus Eder, netdev, Thomas Klein,
Stefan Roscher
Fix: Workqueue ehea_driver_wq was not destroyed
Signed-off-by: Thomas Klein <tklein@de.ibm.com>
---
drivers/net/ehea/ehea.h | 2 +-
drivers/net/ehea/ehea_main.c | 1 +
2 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ehea/ehea.h b/drivers/net/ehea/ehea.h
index 8ee2c2c..d67f97b 100644
--- a/drivers/net/ehea/ehea.h
+++ b/drivers/net/ehea/ehea.h
@@ -39,7 +39,7 @@
#include <asm/io.h>
#define DRV_NAME "ehea"
-#define DRV_VERSION "EHEA_0072"
+#define DRV_VERSION "EHEA_0073"
/* eHEA capability flags */
#define DLPAR_PORT_ADD_REM 1
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index 58702f5..d43ab0f 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -3099,6 +3099,7 @@ out:
static void __exit ehea_module_exit(void)
{
+ destroy_workqueue(ehea_driver_wq);
driver_remove_file(&ehea_driver.driver, &driver_attr_capabilities);
ibmebus_unregister_driver(&ehea_driver);
ehea_destroy_busmap();
--
1.5.2
^ permalink raw reply related
* [RESEND][PATCH 2/3] ehea: Simplify resource usage check
From: Thomas Klein @ 2007-08-06 11:55 UTC (permalink / raw)
To: Jeff Garzik
Cc: Christoph Raisch, Jan-Bernd Themann, Jan-Bernd Themann,
linux-kernel, linux-ppc, Marcus Eder, netdev, Thomas Klein,
Stefan Roscher
Use shorter method to determine whether adapter has configured ports
Signed-off-by: Thomas Klein <tklein@de.ibm.com>
---
drivers/net/ehea/ehea_main.c | 18 ++++++------------
1 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index d43ab0f..36ca322 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -2165,24 +2165,18 @@ static int ehea_clean_all_portres(struct ehea_port *port)
return ret;
}
-static void ehea_remove_adapter_mr (struct ehea_adapter *adapter)
+static void ehea_remove_adapter_mr(struct ehea_adapter *adapter)
{
- int i;
-
- for (i=0; i < EHEA_MAX_PORTS; i++)
- if (adapter->port[i])
- return;
+ if (adapter->active_ports)
+ return;
ehea_rem_mr(&adapter->mr);
}
-static int ehea_add_adapter_mr (struct ehea_adapter *adapter)
+static int ehea_add_adapter_mr(struct ehea_adapter *adapter)
{
- int i;
-
- for (i=0; i < EHEA_MAX_PORTS; i++)
- if (adapter->port[i])
- return 0;
+ if (adapter->active_ports)
+ return 0;
return ehea_reg_kernel_mr(adapter, &adapter->mr);
}
--
1.5.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox