* Re: SO_REUSEPORT - can it be done in kernel?
From: Thomas Graf @ 2011-03-01 15:07 UTC (permalink / raw)
To: Eric Dumazet
Cc: Herbert Xu, David Miller, rick.jones2, therbert, wsommerfeld,
daniel.baluta, netdev
In-Reply-To: <1298991160.3284.108.camel@edumazet-laptop>
On Tue, Mar 01, 2011 at 03:52:40PM +0100, Eric Dumazet wrote:
> Le mardi 01 mars 2011 à 09:30 -0500, Thomas Graf a écrit :
> > On Tue, Mar 01, 2011 at 09:22:35AM -0500, Thomas Graf wrote:
> > > On Tue, Mar 01, 2011 at 03:06:59PM +0100, Eric Dumazet wrote:
> > > > Would be nice to cpu affine named to _not_ run on CPU11, just to
> > > > specialize it for TX completions and have softirq time percentage and
> > > > "perf top -C 11 " results
> >
> > CPU 1 isolated as well (named running with mask 0,2-10)
> >
> > ----------------------------------------------------------------------------------------------------------------------
> > PerfTop: 580 irqs/sec kernel:100.0% exact: 0.0% [1000Hz cpu-clock-msecs], (all, CPU: 1)
> > ----------------------------------------------------------------------------------------------------------------------
> >
> > samples pcnt function DSO
> > _______ _____ ___________________________ ___________________________________________________________
> >
> > 283.00 9.2% get_rx_page_info /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
> > 256.00 8.4% _raw_spin_unlock_irqrestore /lib/modules/2.6.38-rc5+/build/vmlinux
> > 190.00 6.2% be_poll_rx /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
> > 182.00 5.9% get_page_from_freelist /lib/modules/2.6.38-rc5+/build/vmlinux
> > 157.00 5.1% intel_idle /lib/modules/2.6.38-rc5+/build/vmlinux
> > 143.00 4.7% __do_softirq /lib/modules/2.6.38-rc5+/build/vmlinux
> > 133.00 4.3% sock_queue_rcv_skb /lib/modules/2.6.38-rc5+/build/vmlinux
> > 133.00 4.3% __udp4_lib_lookup /lib/modules/2.6.38-rc5+/build/vmlinux
> > 131.00 4.3% sk_run_filter /lib/modules/2.6.38-rc5+/build/vmlinux
>
> sk_run_filter ? Do you have a packet filter running ?
dhclient was running. With dhclient killed:
----------------------------------------------------------------------------------------------------------------------
PerfTop: 726 irqs/sec kernel:99.9% exact: 0.0% [1000Hz cpu-clock-msecs], (all, CPU: 1)
----------------------------------------------------------------------------------------------------------------------
samples pcnt function DSO
_______ _____ ___________________________ ___________________________________________________________
472.00 10.6% get_rx_page_info /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
419.00 9.4% _raw_spin_unlock_irqrestore /lib/modules/2.6.38-rc5+/build/vmlinux
280.00 6.3% be_poll_rx /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
259.00 5.8% get_page_from_freelist /lib/modules/2.6.38-rc5+/build/vmlinux
248.00 5.6% __do_softirq /lib/modules/2.6.38-rc5+/build/vmlinux
238.00 5.4% intel_idle /lib/modules/2.6.38-rc5+/build/vmlinux
204.00 4.6% sock_queue_rcv_skb /lib/modules/2.6.38-rc5+/build/vmlinux
189.00 4.3% __udp4_lib_lookup /lib/modules/2.6.38-rc5+/build/vmlinux
178.00 4.0% getnstimeofday /lib/modules/2.6.38-rc5+/build/vmlinux
169.00 3.8% __alloc_skb /lib/modules/2.6.38-rc5+/build/vmlinux
144.00 3.2% read_tsc /lib/modules/2.6.38-rc5+/build/vmlinux
143.00 3.2% sock_def_readable /lib/modules/2.6.38-rc5+/build/vmlinux
138.00 3.1% udp_queue_rcv_skb /lib/modules/2.6.38-rc5+/build/vmlinux
115.00 2.6% kmem_cache_alloc_node_trace /lib/modules/2.6.38-rc5+/build/vmlinux
114.00 2.6% __netif_receive_skb /lib/modules/2.6.38-rc5+/build/vmlinux
109.00 2.5% _raw_spin_lock /lib/modules/2.6.38-rc5+/build/vmlinux
100.00 2.3% is_swiotlb_buffer /lib/modules/2.6.38-rc5+/build/vmlinux
90.00 2.0% __phys_addr /lib/modules/2.6.38-rc5+/build/vmlinux
82.00 1.8% __udp4_lib_rcv /lib/modules/2.6.38-rc5+/build/vmlinux
80.00 1.8% kmem_cache_alloc_node /lib/modules/2.6.38-rc5+/build/vmlinux
73.00 1.6% ip_route_input_common /lib/modules/2.6.38-rc5+/build/vmlinux
60.00 1.4% memcpy /lib/modules/2.6.38-rc5+/build/vmlinux
59.00 1.3% dma_issue_pending_all /lib/modules/2.6.38-rc5+/build/vmlinux
58.00 1.3% ip_rcv /lib/modules/2.6.38-rc5+/build/vmlinux
57.00 1.3% be_post_rx_frags /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
49.00 1.1% __alloc_pages_nodemask /lib/modules/2.6.38-rc5+/build/vmlinux
45.00 1.0% alloc_pages_current /lib/modules/2.6.38-rc5+/build/vmlinux
27.00 0.6% get_rps_cpu /lib/modules/2.6.38-rc5+/build/vmlinux
23.00 0.5% napi_complete /lib/modules/2.6.38-rc5+/build/vmlinux
22.00 0.5% ip_local_deliver /lib/modules/2.6.38-rc5+/build/vmlinux
18.00 0.4% selinux_socket_sock_rcv_skb /lib/modules/2.6.38-rc5+/build/vmlinux
17.00 0.4% native_read_tsc /lib/modules/2.6.38-rc5+/build/vmlinux
16.00 0.4% local_bh_enable /lib/modules/2.6.38-rc5+/build/vmlinux
16.00 0.4% next_zones_zonelist /lib/modules/2.6.38-rc5+/build/vmlinux
14.00 0.3% sk_filter /lib/modules/2.6.38-rc5+/build/vmlinux
13.00 0.3% eth_type_trans /lib/modules/2.6.38-rc5+/build/vmlinux
10.00 0.2% __kmalloc_node_track_caller /lib/modules/2.6.38-rc5+/build/vmlinux
10.00 0.2% _raw_spin_lock_irqsave /lib/modules/2.6.38-rc5+/build/vmlinux
9.00 0.2% raw_local_deliver /lib/modules/2.6.38-rc5+/build/vmlinux
8.00 0.2% __udp_queue_rcv_skb /lib/modules/2.6.38-rc5+/build/vmlinux
8.00 0.2% netif_receive_skb /lib/modules/2.6.38-rc5+/build/vmlinux
8.00 0.2% ip_queue_rcv_skb /lib/modules/2.6.38-rc5+/build/vmlinux
7.00 0.2% net_rx_action /lib/modules/2.6.38-rc5+/build/vmlinux
6.00 0.1% swiotlb_map_page /lib/modules/2.6.38-rc5+/build/vmlinux
6.00 0.1% __sk_mem_schedule /lib/modules/2.6.38-rc5+/build/vmlinux
6.00 0.1% dso__find_symbol /usr/bin/perf
6.00 0.1% __netdev_alloc_skb /lib/modules/2.6.38-rc5+/build/vmlinux
^ permalink raw reply
* Re: [PATCH 2/4] slub,rcu: don't assume the size of struct rcu_head
From: Christoph Lameter @ 2011-03-01 15:11 UTC (permalink / raw)
To: Pekka Enberg
Cc: Lai Jiangshan, Ingo Molnar, Paul E. McKenney, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
In-Reply-To: <AANLkTimXy2Yaj+NTDMNTWuLqHHfKZJhVDpeXj3CfMvBf@mail.gmail.com>
On Tue, 1 Mar 2011, Pekka Enberg wrote:
> The SLAB and SLUB patches are fine by me if there are going to be real
> users for this. Christoph, Paul?
The solution is a bit overkill. It would be much simpler to add a union to
struct page that has lru and the rcu in there similar things can be done
for SLAB and the network layer. A similar issue already exists for the
spinlock in struct page. Lets follow the existing way of handling this.
Struct page may be larger for debugging purposes already because of the
need for extended spinlock data.
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: remove skb_share_check in handle_frame
From: Changli Gao @ 2011-03-01 15:12 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, fubar, eric.dumazet, nicolas.2p.debian, andy
In-Reply-To: <20110301092907.GG2855@psychotron.redhat.com>
On Tue, Mar 1, 2011 at 5:29 PM, Jiri Pirko <jpirko@redhat.com> wrote:
> Unapplicable, sorry (wrong branch :(). Here's corrected patch:
>
> Subject: [PATCH net-next-2.6 v2] bonding: remove skb_share_check in handle_frame
>
> No need to do share check here.
>
I don't think so. Although you avoid netif_rx(), you can't avoid
ptype_all handlers. In fact, all the RX handlers should has this
check(), if they may modify the skb.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* [PATCH 1/2 net-next][v2] bonding: fix incorrect transmit queue offset
From: Andy Gospodarek @ 2011-03-01 15:31 UTC (permalink / raw)
To: Phil Oester; +Cc: David Miller, bhutchings, andy, netdev, fubar
In-Reply-To: <20110225225636.GA18792@linuxace.com>
On Fri, Feb 25, 2011 at 02:56:36PM -0800, Phil Oester wrote:
> On Wed, Feb 23, 2011 at 03:54:51PM -0800, David Miller wrote:
> > From: Ben Hutchings <bhutchings@solarflare.com>
> > Date: Wed, 23 Feb 2011 23:37:49 +0000
> >
> > > On Wed, 2011-02-23 at 15:13 -0800, David Miller wrote:
> > >> From: Phil Oester <kernel@linuxace.com>
> > >> Date: Wed, 23 Feb 2011 15:08:44 -0800
> > >>
> > >> > On Wed, Feb 23, 2011 at 02:42:49PM -0500, Andy Gospodarek wrote:
> > >> >> + while (txq >= dev->real_num_tx_queues) {
> > >> >> + /* let the user know if we do not have enough tx queues */
> > >> >> + if (net_ratelimit())
> > >> >> + pr_warning("%s selects invalid tx queue %d. Consider"
> > >> >> + " setting module option tx_queues > %d.",
> > >> >> + dev->name, txq, dev->real_num_tx_queues);
> > >> >> + txq -= dev->real_num_tx_queues;
> > >> >> + }
> > >> >
> > >> > Think this would be better as a WARN_ONCE, as otherwise syslog will still
> > >> > get flooded with this - even when ratelimited. See get_rps_cpu in
> > >> > net/core/dev.c as an example.o
> > >>
> > >> Agreed.
> > >
> > > This shouldn't WARN at all. It is perfectly valid (though non-optimal)
> > > to have different numbers of queues on two different multiqueue devices.
> >
> > That's also a good point.
>
> The patch works as expected. Do we have any agreement on a final version?
>
Thanks for the testing, Phil.
I'm in favor of this patch as it does alert the admin that bonding may
not have enough default queues, but it is not as verbose (backtrace et
al) and likely to create bug reports as a message from WARN_ON.
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
---
drivers/net/bonding/bond_main.c | 26 +++++++++++++++++++-------
1 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 584f97b..acc05d6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4643,15 +4643,27 @@ out:
return res;
}
+/*
+ * This helper function exists to help dev_pick_tx get the correct
+ * destination queue. Using a helper function skips the a call to
+ * skb_tx_hash and will put the skbs in the queue we expect on their
+ * way down to the bonding driver.
+ */
static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
{
- /*
- * This helper function exists to help dev_pick_tx get the correct
- * destination queue. Using a helper function skips the a call to
- * skb_tx_hash and will put the skbs in the queue we expect on their
- * way down to the bonding driver.
- */
- return skb->queue_mapping;
+ u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
+
+ if (txq >= dev->real_num_tx_queues) {
+ /* let the user know if we do not have enough tx queues */
+ if (net_ratelimit())
+ pr_warning("%s selects invalid tx queue %d. Consider"
+ " setting module option tx_queues > %d.",
+ dev->name, txq, dev->real_num_tx_queues);
+ do
+ txq -= dev->real_num_tx_queues;
+ while (txq >= dev->real_num_tx_queues);
+ }
+ return txq;
}
static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)
^ permalink raw reply related
* Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure
From: Stuart Menefy @ 2011-03-01 15:20 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Peppe CAVALLARO, linux-sh@vger.kernel.org, netdev@vger.kernel.org,
John Stultz, Thomas Gleixner, linux-kernel@vger.kernel.org
In-Reply-To: <201102241820.55873.arnd@arndb.de>
Hi Arnd
Thanks for the comments.
On 24/02/11 17:20, Arnd Bergmann wrote:
> On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
>> From: Stuart Menefy <stuart.menefy@st.com>
>>
>> Many devices targeted at the embedded market provide a number of
>> generic timers which are capable of generating interrupts at a
>> requested rate. These can then be used in the implementation of drivers
>> for other peripherals which require a timer interrupt, without having
>> to provide an additional timer as part of that peripheral.
>>
>> A code provides a simple abstraction layer which allows a timer to be
>> registered, and for a driver to request a timer.
>>
>> Currently this doesn't provide any of the additional information, such
>> as precision or position in clock framework which might be required
>> for a fully featured driver.
>
> This code should probably be discussed on a more broader
> platform than the netdev and linux-sh mailing lists,
> as the scope is neither sh nor network specific.
>
> You should at least add linux-kernel@vger.kernel.org, possibly
> also linux-arch@vger.kernel.org.
>
> Further, John and Thomas are responsible for the timekeeping
> infrastructure, and they are probably interested in this
> as well.
>
> Why is this code useful to you? In the scenarios I've seen, the
> board can always assign a timer to a specific device in a fixed
> way that can be describe in a board file or device tree.
What we were trying to do was separate the code which actually manipulates
the timer hardware from the code which wants that timer service. In this
case it was a network device driver which is used on multiple SoC devices,
while the timer hardware tends to differ from device to device.
The other user of this code which we have is an OProfile driver, which
with this change can now be independent of the hardware it is running on,
while the previous version manipulated the timer hardware directly.
> Also, what is the difference between this and clkdev?
clkdev can be used to find a struct clk, which is fine if you just want to
read the time. In this instance we want to get interrupts from the timer
hardware, which isn't supported by the clk infrastructure.
If anything this duplicates clockevents. The main reason for not using
clockevents was that nobody else does! Currently clockevents are
used strictly for time keeping within the kernel, and most architectures
only register those which are intended to be used for this purpose.
We felt a bit nervous about adding code to register all the device's timers
as clockevents, and having the network device driver pick up one of those
for its own use.
>> Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
>> Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> ---
>> drivers/clocksource/Makefile | 1 +
>> drivers/clocksource/generictimer.c | 60 ++++++++++++++++++++++++++++++++++++
>> include/linux/generictimer.h | 41 ++++++++++++++++++++++++
>> 3 files changed, 102 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/clocksource/generictimer.c
>> create mode 100644 include/linux/generictimer.h
>
> I don't think it fits that well into the drivers/clocksource directory,
> because you don't actually register a struct clock_event_device or
> struct clocksource.
True. The intent was that this would be a third interface onto timer
hardware, alongside clocksources and clockevents.
> I don't know if this could also be merged with the clocksource infrastructure,
> but your code currently doesn't do that.
It would probably be clockevent rather than clocksource, but it could be if
people felt that was a better way to go.
>> +struct generic_timer *generic_timer_claim(void (*handler) (void *), void *data)
>> +{
>> + struct generic_timer *gt = NULL;
>> +
>> + if (!handler) {
>> + pr_err("%s: invalid handler\n", __func__);
>> + return NULL;
>> + }
>> +
>> + mutex_lock(>_mutex);
>> + if (!list_empty(>_list)) {
>> + struct list_head *list = gt_list.next;
>> + list_del(list);
>> + gt = container_of(list, struct generic_timer, list);
>> + }
>> + mutex_unlock(>_mutex);
>> +
>> + if (!gt)
>> + return NULL;
>> +
>> + /* Prepare the new handler */
>> + gt->priv_handler = handler;
>> + gt->data = data;
>> +
>> + return gt;
>> +}
>
> This does not seem very generic. You put timers into the list and take
> them out again, but don't have any way to deal with timers that match
> specific purposes. It obviously works for your specific use case where
> you register exactly one timer, and use that in exactly one driver.
>
> If more drivers were converted to generic_timer, which is obviously
> the intention, then you might have a situation with very different
> timers (fixed/variable tick, high/low frequencies, accurate/inaccurate),
> or you might have fewer timers than users.
Agreed, this was a first 'keep it simple' pass, maybe its too simple.
^ permalink raw reply
* [PATCH net-next-2.6] benet: use GFP_KERNEL allocations when possible
From: Eric Dumazet @ 2011-03-01 15:48 UTC (permalink / raw)
To: Ajit Khaparde; +Cc: netdev
Extend be_alloc_pages() with a gfp parameter, so that we use GFP_KERNEL
allocations instead of GFP_ATOMIC when not running in softirq context.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
drivers/net/benet/be_main.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 0bdccb1..ef66dc6 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -1169,20 +1169,20 @@ static inline void be_rx_compl_reset(struct be_eth_rx_compl *rxcp)
rxcp->dw[offsetof(struct amap_eth_rx_compl, valid) / 32] = 0;
}
-static inline struct page *be_alloc_pages(u32 size)
+static inline struct page *be_alloc_pages(u32 size, gfp_t gfp)
{
- gfp_t alloc_flags = GFP_ATOMIC;
u32 order = get_order(size);
+
if (order > 0)
- alloc_flags |= __GFP_COMP;
- return alloc_pages(alloc_flags, order);
+ gfp |= __GFP_COMP;
+ return alloc_pages(gfp, order);
}
/*
* Allocate a page, split it to fragments of size rx_frag_size and post as
* receive buffers to BE
*/
-static void be_post_rx_frags(struct be_rx_obj *rxo)
+static void be_post_rx_frags(struct be_rx_obj *rxo, gfp_t gfp)
{
struct be_adapter *adapter = rxo->adapter;
struct be_rx_page_info *page_info_tbl = rxo->page_info_tbl;
@@ -1196,7 +1196,7 @@ static void be_post_rx_frags(struct be_rx_obj *rxo)
page_info = &rxo->page_info_tbl[rxq->head];
for (posted = 0; posted < MAX_RX_POST && !page_info->page; posted++) {
if (!pagep) {
- pagep = be_alloc_pages(adapter->big_page_size);
+ pagep = be_alloc_pages(adapter->big_page_size, gfp);
if (unlikely(!pagep)) {
rxo->stats.rx_post_fail++;
break;
@@ -1753,7 +1753,7 @@ static int be_poll_rx(struct napi_struct *napi, int budget)
/* Refill the queue */
if (atomic_read(&rxo->q.used) < RX_FRAGS_REFILL_WM)
- be_post_rx_frags(rxo);
+ be_post_rx_frags(rxo, GFP_ATOMIC);
/* All consumed */
if (work_done < budget) {
@@ -1890,7 +1890,7 @@ static void be_worker(struct work_struct *work)
if (rxo->rx_post_starved) {
rxo->rx_post_starved = false;
- be_post_rx_frags(rxo);
+ be_post_rx_frags(rxo, GFP_KERNEL);
}
}
if (!adapter->ue_detected && !lancer_chip(adapter))
@@ -2138,7 +2138,7 @@ static int be_open(struct net_device *netdev)
u16 link_speed;
for_all_rx_queues(adapter, rxo, i) {
- be_post_rx_frags(rxo);
+ be_post_rx_frags(rxo, GFP_KERNEL);
napi_enable(&rxo->rx_eq.napi);
}
napi_enable(&tx_eq->napi);
^ permalink raw reply related
* Re: [PATCH 2/3] [RFC] Changes for MQ virtio-net
From: Krishna Kumar2 @ 2011-03-01 16:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: anthony, arnd, avi, davem, eric.dumazet, horms, kvm, netdev,
rusty
In-Reply-To: <20110228094320.GB28006@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> wrote on 02/28/2011 03:13:20 PM:
Thank you once again for your feedback on both these patches.
I will send the qemu patch tomorrow. I will also send the next
version incorporating these suggestions once we finalize some
minor points.
> Overall looks good.
> The numtxqs meaning the number of rx queues needs some cleanup.
> init/cleanup routines need more symmetry.
> Error handling on setup also seems slightly buggy or at least
asymmetrical.
> Finally, this will use up a large number of MSI vectors,
> while TX interrupts mostly stay unused.
>
> Some comments below.
>
> > +/* Maximum number of individual RX/TX queues supported */
> > +#define VIRTIO_MAX_TXQS 16
> > +
>
> This also does not seem to belong in the header.
Both virtio-net and vhost need some check to make sure very
high values are not passed by userspace. Is this not required?
> > +#define VIRTIO_NET_F_NUMTXQS 21 /* Device supports multiple
TX queue */
>
> VIRTIO_NET_F_MULTIQUEUE ?
Yes, that's a better name.
> > @@ -34,6 +38,8 @@ struct virtio_net_config {
> > __u8 mac[6];
> > /* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
> > __u16 status;
> > + /* number of RX/TX queues */
> > + __u16 numtxqs;
>
> The interface here is a bit ugly:
> - this is really both # of tx and rx queues but called numtxqs
> - there's a hardcoded max value
> - 0 is assumed to be same as 1
> - assumptions above are undocumented.
>
> One way to address this could be num_queue_pairs, and something like
> /* The actual number of TX and RX queues is num_queue_pairs +
1 each. */
> __u16 num_queue_pairs;
> (and tweak code to match).
>
> Alternatively, have separate registers for the number of tx and rx
queues.
OK, so virtio_net_config has num_queue_pairs, and this gets converted to
numtxqs in virtnet_info?
> > +struct virtnet_info {
> > + struct send_queue **sq;
> > + struct receive_queue **rq;
> > +
> > + /* read-mostly variables */
> > + int numtxqs ____cacheline_aligned_in_smp;
>
> Why do you think this alignment is a win?
Actually this code was from the earlier patchset (MQ TX only) where
the layout was different. Now rq and sq are allocated as follows:
vi->sq = kzalloc(numtxqs * sizeof(*vi->sq), GFP_KERNEL);
for (i = 0; i < numtxqs; i++) {
vi->sq[i] = kzalloc(sizeof(*vi->sq[i]), GFP_KERNEL);
Since the two pointers becomes read-only during use, there is no cache
line dirty'ing. I will remove this directive.
> > +/*
> > + * Note for 'qnum' below:
> > + * first 'numtxqs' vqs are RX, next 'numtxqs' vqs are TX.
> > + */
>
> Another option to consider is to have them RX,TX,RX,TX:
> this way vq->queue_index / 2 gives you the
> queue pair number, no need to read numtxqs. On the other hand, it makes
the
> #RX==#TX assumption even more entrenched.
OK. I was following how many drivers were allocating RX and TX's
together - eg ixgbe_adapter has tx_ring and rx_ring arrays; bnx2
has rx_buf_ring and tx_buf_ring arrays, etc. Also, vhost has some
code that processes tx first before rx (e.g. vhost_net_stop/flush),
so this approach seemed helpful. I am OK either way, what do you
suggest?
> > + err = vi->vdev->config->find_vqs(vi->vdev, totalvqs, vqs,
callbacks,
> > + (const char
**)names);
> > + if (err)
> > + goto free_params;
> > +
>
> This would use up quite a lot of vectors. However,
> tx interrupt is, in fact, slow path. So, assuming we don't have
> enough vectors to use per vq, I think it's a good idea to
> support reducing MSI vector usage by mapping all TX VQs to the same
vector
> and separate vectors for RX.
> The hypervisor actually allows this, but we don't have an API at the
virtio
> level to pass that info to virtio pci ATM.
> Any idea what a good API to use would be?
Yes, it is a waste to have these vectors for tx ints. I initially
thought of adding a flag to virtio_device to pass to vp_find_vqs,
but it won't work, so a new API is needed. I can work with you on
this in the background if you like.
> > + for (i = 0; i < numtxqs; i++) {
> > + vi->rq[i]->rvq = vqs[i];
> > + vi->sq[i]->svq = vqs[i + numtxqs];
>
> This logic is spread all over. We need some kind of macro to
> get queue number of vq number and back.
Will add this.
> > + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) {
> > + vi->cvq = vqs[i + numtxqs];
> > +
> > + if (virtio_has_feature(vi->vdev,
VIRTIO_NET_F_CTRL_VLAN))
> > + vi->dev->features |=
NETIF_F_HW_VLAN_FILTER;
>
> This bit does not seem to belong in initialize_vqs.
I will move it back to probe.
> > + err = virtio_config_val(vdev, VIRTIO_NET_F_NUMTXQS,
> > + offsetof(struct
virtio_net_config, numtxqs),
> > + &numtxqs);
> > +
> > + /* We need atleast one txq */
> > + if (err || !numtxqs)
> > + numtxqs = 1;
>
> err is okay, but should we just fail on illegal values?
> Or change the semantics:
> n = 0;
> err = virtio_config_val(vdev, VIRTIO_NET_F_NUMTXQS,
> offsetof(struct virtio_net_config, numtxqs),
> &n);
> numtxq = n + 1;
Will this be better:
int num_queue_pairs = 2;
int numtxqs;
err = virtio_config_val(vdev, VIRTIO_NET_F_MULTIQUEUE,
offsetof(struct virtio_net_config,
num_queue_pairs), &num_queue_pairs);
<ignore error, if any>
numtxqs = num_queue_pairs / 2;
> > + if (numtxqs > VIRTIO_MAX_TXQS)
> > + return -EINVAL;
>
> Do we strictly need this?
> I think we should just use whatever hardware has,
> or alternatively somehow ignore the unused queues
> (easy for tx, not sure about rx).
vq's are matched between qemu, virtio-net and vhost. Isn't some check
required that userspace has not passed a bad value?
> > + if (vi->rq[i]->num == 0) {
> > + err = -ENOMEM;
> > + goto free_recv_bufs;
> > + }
> > }
> If this fails for vq > 0, you have to detach bufs.
Right, will fix this.
> > free_vqs:
> > + for (i = 0; i < numtxqs; i++)
> > + cancel_delayed_work_sync(&vi->rq[i]->refill);
> > vdev->config->del_vqs(vdev);
> > -free:
> > + free_rq_sq(vi);
>
> If we have a wrapper to init all vqs, pls add a wrapper to clean up
> all vqs as well.
Will add that.
> > + for (i = 0; i < vi->numtxqs; i++) {
> > + struct virtqueue *rvq = vi->rq[i]->rvq;
> > +
> > + while (1) {
> > + buf = virtqueue_detach_unused_buf
(rvq);
> > + if (!buf)
> > + break;
> > + if (vi->mergeable_rx_bufs || vi->
big_packets)
> > + give_pages(vi->rq[i],
buf);
> > + else
> > + dev_kfree_skb(buf);
> > + --vi->rq[i]->num;
> > + }
> > + BUG_ON(vi->rq[i]->num != 0);
> > }
> > - BUG_ON(vi->num != 0);
> > +
> > + free_rq_sq(vi);
>
>
> This looks wrong here. This function should detach
> and free all bufs, not internal malloc stuff.
That is being done by free_receive_buf after free_unused_bufs()
returns. I hope this addresses your point.
> I think we should have free_unused_bufs that handles
> a single queue, and call it in a loop.
OK, so define free_unused_bufs() as:
static void free_unused_bufs(struct virtnet_info *vi, struct virtqueue
*svq,
struct virtqueue *rvq)
{
/* Use svq and rvq with the remaining code unchanged */
}
Thanks,
- KK
^ permalink raw reply
* Re: [PATCH 3/3] [RFC] Changes for MQ vhost
From: Krishna Kumar2 @ 2011-03-01 16:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: anthony, arnd, avi, davem, eric.dumazet, horms, kvm, netdev,
rusty
In-Reply-To: <20110228100423.GC28006@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> wrote on 02/28/2011 03:34:23 PM:
> > The number of vhost threads is <= #txqs. Threads handle more
> > than one txq when #txqs is more than MAX_VHOST_THREADS (4).
>
> It is this sharing that prevents us from just reusing multiple vhost
> descriptors?
Sorry, I didn't understand this question.
> 4 seems a bit arbitrary - do you have an explanation
> on why this is a good number?
I was not sure what is the best way - a sysctl parameter? Or should the
maximum depend on number of host cpus? But that results in too many
threads, e.g. if I have 16 cpus and 16 txqs.
> > + struct task_struct *worker; /* worker for this vq */
> > + spinlock_t *work_lock; /* points to a dev->work_lock[] entry
*/
> > + struct list_head *work_list; /* points to a dev->work_list[]
entry */
> > + int qnum; /* 0 for RX, 1 -> n-1 for TX */
>
> Is this right?
Will fix this.
> > @@ -122,12 +128,33 @@ struct vhost_dev {
> > int nvqs;
> > struct file *log_file;
> > struct eventfd_ctx *log_ctx;
> > - spinlock_t work_lock;
> > - struct list_head work_list;
> > - struct task_struct *worker;
> > + spinlock_t *work_lock[MAX_VHOST_THREADS];
> > + struct list_head *work_list[MAX_VHOST_THREADS];
>
> This looks a bit strange. Won't sticking everything in a single
> array of structures rather than multiple arrays be better for cache
> utilization?
Correct. In that context, which is better:
struct {
spinlock_t *work_lock;
struct list_head *work_list;
} work[MAX_VHOST_THREADS];
or, to make sure work_lock/work_list is cache-aligned:
struct work_lock_list {
spinlock_t work_lock;
struct list_head work_list;
} ____cacheline_aligned_in_smp;
and define:
struct vhost_dev {
...
struct work_lock_list work[MAX_VHOST_THREADS];
};
Second method uses a little more space but each vhost needs only
one (read-only) cache line. I tested with this and can confirm it
aligns each element on a cache-line. BW improved slightly (upto
3%), remote SD improves by upto -4% or so.
> > +static inline int get_nvhosts(int nvqs)
>
> nvhosts -> nthreads?
Yes.
> > +static inline int vhost_get_thread_index(int index, int numtxqs, int
nvhosts)
> > +{
> > + return (index % numtxqs) % nvhosts;
> > +}
> > +
>
> As the only caller passes MAX_VHOST_THREADS,
> just use that?
Yes, nice catch.
> > struct vhost_net {
> > struct vhost_dev dev;
> > - struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
> > - struct vhost_poll poll[VHOST_NET_VQ_MAX];
> > + struct vhost_virtqueue *vqs;
> > + struct vhost_poll *poll;
> > + struct socket **socks;
> > /* Tells us whether we are polling a socket for TX.
> > * We only do this when socket buffer fills up.
> > * Protected by tx vq lock. */
> > - enum vhost_net_poll_state tx_poll_state;
> > + enum vhost_net_poll_state *tx_poll_state;
>
> another array?
Yes... I am also allocating twice the space than what is required
to make it's usage simple. Please let me know what you feel about
this.
Thanks,
- KK
^ permalink raw reply
* Re: SO_REUSEPORT - can it be done in kernel?
From: Eric Dumazet @ 2011-03-01 16:31 UTC (permalink / raw)
To: Herbert Xu
Cc: Thomas Graf, David Miller, rick.jones2, therbert, wsommerfeld,
daniel.baluta, netdev
In-Reply-To: <20110301131823.GB8028@gondor.apana.org.au>
Le mardi 01 mars 2011 à 21:18 +0800, Herbert Xu a écrit :
> On Tue, Mar 01, 2011 at 02:03:29PM +0100, Eric Dumazet wrote:
> >
> > I believe its now done properly (in net-next-2.6) with commit
> > 4f57c087de9b46182 (net: implement mechanism for HW based QOS)
>
> Nope, that has nothing to do with this.
Right, I was thinking of commit 1d24eb4815d1e0e8 (xps: Transmit Packet
Steering)
Now you say all this stuff should be replaced by "use this cpu number
nly", just because you have a multi threaded process sending UDP frames
trough one socket...
This wont work for tcp streams, you could imagine a multi-threaded
application using a shared tcp socket as well. Too many OOO packets.
^ permalink raw reply
* Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure
From: Arnd Bergmann @ 2011-03-01 16:43 UTC (permalink / raw)
To: Stuart Menefy
Cc: Peppe CAVALLARO, linux-sh@vger.kernel.org, netdev@vger.kernel.org,
John Stultz, Thomas Gleixner, linux-kernel@vger.kernel.org,
Russell King - ARM Linux
In-Reply-To: <4D6D0EA3.9000504@st.com>
On Tuesday 01 March 2011, Stuart Menefy wrote:
> On 24/02/11 17:20, Arnd Bergmann wrote:
>
> > Why is this code useful to you? In the scenarios I've seen, the
> > board can always assign a timer to a specific device in a fixed
> > way that can be describe in a board file or device tree.
>
> What we were trying to do was separate the code which actually manipulates
> the timer hardware from the code which wants that timer service. In this
> case it was a network device driver which is used on multiple SoC devices,
> while the timer hardware tends to differ from device to device.
Right. It certainly makes sense to have an well-defined interface between
the user and the provider of a timer interrupt.
> The other user of this code which we have is an OProfile driver, which
> with this change can now be independent of the hardware it is running on,
> while the previous version manipulated the timer hardware directly.
Ok.
> > Also, what is the difference between this and clkdev?
>
> clkdev can be used to find a struct clk, which is fine if you just want to
> read the time. In this instance we want to get interrupts from the timer
> hardware, which isn't supported by the clk infrastructure.
(adding Russell to Cc)
Is this something that could sensibly be added to clk/clkdev?
> If anything this duplicates clockevents. The main reason for not using
> clockevents was that nobody else does! Currently clockevents are
> used strictly for time keeping within the kernel, and most architectures
> only register those which are intended to be used for this purpose.
> We felt a bit nervous about adding code to register all the device's timers
> as clockevents, and having the network device driver pick up one of those
> for its own use.
I see. Using a clock_event_device for anything but the system timer tick
is currently not supported, so it certainly would not be straightforward.
I think you need a bit of both, clkdev and clockevent. I think the
options you have are:
1. copy the clkdev code to make it possible to associate a device with
a periodic timer.
2. extend the clkdev/clk code to handle periodic interrupts and reuse
the infrastructure there.
3. extend the clockevent code to make it possible for regular device
drivers to use a clockevent source.
I have no idea which makes the most sense (or if there are other ideas).
Maybe Russell, Thomas or John can comment.
Arnd
^ permalink raw reply
* Re: SO_REUSEPORT - can it be done in kernel?
From: Eric Dumazet @ 2011-03-01 16:43 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, rick.jones2, therbert, wsommerfeld, daniel.baluta,
netdev
In-Reply-To: <20110301123519.GA7453@gondor.apana.org.au>
Le mardi 01 mars 2011 à 20:35 +0800, Herbert Xu a écrit :
> On Mon, Feb 28, 2011 at 07:36:59PM +0800, Herbert Xu wrote:
> > Here are the patches I used. Please don't them yet as I intend
> > to clean them up quite a bit.
>
> OK here is the version ready for merging (please retest them
> though as I have changed things substantially).
>
> The main change is that the legacy UDP code path is now gone
> so we use the same UDP header generation whether corking is on
> or off.
>
> I will add IPv6 support in a later patch set.
>
> Thanks!
For the whole patchset :
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Tests were fine on my dev machine.
Thanks
^ permalink raw reply
* Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure
From: Thomas Gleixner @ 2011-03-01 16:48 UTC (permalink / raw)
To: Stuart Menefy
Cc: Arnd Bergmann, Peppe CAVALLARO, linux-sh@vger.kernel.org,
netdev@vger.kernel.org, John Stultz, linux-kernel@vger.kernel.org
In-Reply-To: <4D6D0EA3.9000504@st.com>
On Tue, 1 Mar 2011, Stuart Menefy wrote:
> On 24/02/11 17:20, Arnd Bergmann wrote:
> > On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
> >> From: Stuart Menefy <stuart.menefy@st.com>
> >>
> >> Many devices targeted at the embedded market provide a number of
> >> generic timers which are capable of generating interrupts at a
> >> requested rate. These can then be used in the implementation of drivers
> >> for other peripherals which require a timer interrupt, without having
> >> to provide an additional timer as part of that peripheral.
Why can't you just use an hrtimer and be done with it? Just because
there is some extra hardware in the chip?
> If anything this duplicates clockevents. The main reason for not using
> clockevents was that nobody else does! Currently clockevents are
> used strictly for time keeping within the kernel, and most architectures
> only register those which are intended to be used for this purpose.
> We felt a bit nervous about adding code to register all the device's timers
> as clockevents, and having the network device driver pick up one of those
> for its own use.
We had this discussion before and there was never an real outcome as
it turned out that hrtimers provide enough abstraction for this kind
of problems.
> True. The intent was that this would be a third interface onto timer
> hardware, alongside clocksources and clockevents.
>
> > I don't know if this could also be merged with the clocksource infrastructure,
> > but your code currently doesn't do that.
>
> It would probably be clockevent rather than clocksource, but it could be if
> people felt that was a better way to go.
If we get some reasonable explanation why an extra timer interrupt is
solving a specific problem better than hrtimers we can do that, but
that needs more thought than picking the first available clockevent
from a list. If we come to the conclusion, that we want/need this kind
of functionality then I really prefer not to create yet another piece
of infrastructure which deals with timer devices.
Thanks,
tglx
^ permalink raw reply
* Re: [Lxc-users] Bad checksums and lost packets with macvlan on dummy
From: Patrick McHardy @ 2011-03-01 16:51 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: Eric Dumazet, Andrian Nord, lxc-users, Linux Netdev List
In-Reply-To: <4D6CF4A8.6000205@free.fr>
On 01.03.2011 14:29, Daniel Lezcano wrote:
> On 02/28/2011 08:45 AM, Eric Dumazet wrote:
>>> In the normal case, dummy0 is supposed to drop the packets. But with
>>> macvlan these packets are broadcasted to the other macvlan ports, so no
>>> checksum is computed when the packets are transmitted between macvlan1
>>> and macvlan2.
>> So where frames get bad checksums ?
>>
>> In this "bridge" mode, I suspect the broadcast is done _before_ sending
>> frame to dummy, so maybe macvlan should not inherit from lowerdev in
>> this particular case ?
>
> Hi Eric,
>
> yes, you are right, the packets are sent before.
>
> In the 'macvlan_queue_xmit', the code checks the dev is in 'bridge'
> mode. If so, it looks if there is a destination port for the packet and
> then calls the 'forward' callback which is 'dev_forward_skb'.
>
> I was able to reproduce the same problem with qemu and an emulated
> 'e1000' card instead of dummy0. The packets are dropped too.
>
> Patrick, do you have any suggestions to fix this ?
Since the frames are only looped back locally, I suppose the easiest
fix would be to mark them with CHECKSUM_UNNECESSARY. Alternatively
we need to complete the checksum manually, similar to what
dev_hard_start_xmit() does.
^ permalink raw reply
* Networking debug - kernel tools
From: Daniel Baluta @ 2011-03-01 16:55 UTC (permalink / raw)
To: netdev
Hello all,
Having the right tools and knowledge about how to use them
can make our life easier in finding bugs / improving performance.
Would you care to share with us what tools do you use to
analyze networking subsystem's performance on high load
machines?
Pointers to documentation are highly welcomed :).
thanks,
Daniel.
^ permalink raw reply
* [PATCH 1/2] tlan: Remove changelog
From: Joe Perches @ 2011-03-01 16:56 UTC (permalink / raw)
To: Samuel Chessman; +Cc: Sakari Ailus, netdev, linux-kernel
As it isn't necessary nor really useful any longer.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/tlan.c | 145 ----------------------------------------------------
1 files changed, 0 insertions(+), 145 deletions(-)
diff --git a/drivers/net/tlan.c b/drivers/net/tlan.c
index e48a808..7721e6c 100644
--- a/drivers/net/tlan.c
+++ b/drivers/net/tlan.c
@@ -25,151 +25,6 @@
* Microchip Technology, 24C01A/02A/04A Data Sheet
* available in PDF format from www.microchip.com
*
- * Change History
- *
- * Tigran Aivazian <tigran@sco.com>: TLan_PciProbe() now uses
- * new PCI BIOS interface.
- * Alan Cox <alan@lxorguk.ukuu.org.uk>:
- * Fixed the out of memory
- * handling.
- *
- * Torben Mathiasen <torben.mathiasen@compaq.com> New Maintainer!
- *
- * v1.1 Dec 20, 1999 - Removed linux version checking
- * Patch from Tigran Aivazian.
- * - v1.1 includes Alan's SMP updates.
- * - We still have problems on SMP though,
- * but I'm looking into that.
- *
- * v1.2 Jan 02, 2000 - Hopefully fixed the SMP deadlock.
- * - Removed dependency of HZ being 100.
- * - We now allow higher priority timers to
- * overwrite timers like TLAN_TIMER_ACTIVITY
- * Patch from John Cagle <john.cagle@compaq.com>.
- * - Fixed a few compiler warnings.
- *
- * v1.3 Feb 04, 2000 - Fixed the remaining HZ issues.
- * - Removed call to pci_present().
- * - Removed SA_INTERRUPT flag from irq handler.
- * - Added __init and __initdata to reduce resisdent
- * code size.
- * - Driver now uses module_init/module_exit.
- * - Rewrote init_module and tlan_probe to
- * share a lot more code. We now use tlan_probe
- * with builtin and module driver.
- * - Driver ported to new net API.
- * - tlan.txt has been reworked to reflect current
- * driver (almost)
- * - Other minor stuff
- *
- * v1.4 Feb 10, 2000 - Updated with more changes required after Dave's
- * network cleanup in 2.3.43pre7 (Tigran & myself)
- * - Minor stuff.
- *
- * v1.5 March 22, 2000 - Fixed another timer bug that would hang the
- * driver if no cable/link were present.
- * - Cosmetic changes.
- * - TODO: Port completely to new PCI/DMA API
- * Auto-Neg fallback.
- *
- * v1.6 April 04, 2000 - Fixed driver support for kernel-parameters.
- * Haven't tested it though, as the kernel support
- * is currently broken (2.3.99p4p3).
- * - Updated tlan.txt accordingly.
- * - Adjusted minimum/maximum frame length.
- * - There is now a TLAN website up at
- * http://hp.sourceforge.net/
- *
- * v1.7 April 07, 2000 - Started to implement custom ioctls. Driver now
- * reports PHY information when used with Donald
- * Beckers userspace MII diagnostics utility.
- *
- * v1.8 April 23, 2000 - Fixed support for forced speed/duplex settings.
- * - Added link information to Auto-Neg and forced
- * modes. When NIC operates with auto-neg the driver
- * will report Link speed & duplex modes as well as
- * link partner abilities. When forced link is used,
- * the driver will report status of the established
- * link.
- * Please read tlan.txt for additional information.
- * - Removed call to check_region(), and used
- * return value of request_region() instead.
- *
- * v1.8a May 28, 2000 - Minor updates.
- *
- * v1.9 July 25, 2000 - Fixed a few remaining Full-Duplex issues.
- * - Updated with timer fixes from Andrew Morton.
- * - Fixed module race in TLan_Open.
- * - Added routine to monitor PHY status.
- * - Added activity led support for Proliant devices.
- *
- * v1.10 Aug 30, 2000 - Added support for EISA based tlan controllers
- * like the Compaq NetFlex3/E.
- * - Rewrote tlan_probe to better handle multiple
- * bus probes. Probing and device setup is now
- * done through TLan_Probe and TLan_init_one. Actual
- * hardware probe is done with kernel API and
- * TLan_EisaProbe.
- * - Adjusted debug information for probing.
- * - Fixed bug that would cause general debug
- * information to be printed after driver removal.
- * - Added transmit timeout handling.
- * - Fixed OOM return values in tlan_probe.
- * - Fixed possible mem leak in tlan_exit
- * (now tlan_remove_one).
- * - Fixed timer bug in TLan_phyMonitor.
- * - This driver version is alpha quality, please
- * send me any bug issues you may encounter.
- *
- * v1.11 Aug 31, 2000 - Do not try to register irq 0 if no irq line was
- * set for EISA cards.
- * - Added support for NetFlex3/E with nibble-rate
- * 10Base-T PHY. This is untestet as I haven't got
- * one of these cards.
- * - Fixed timer being added twice.
- * - Disabled PhyMonitoring by default as this is
- * work in progress. Define MONITOR to enable it.
- * - Now we don't display link info with PHYs that
- * doesn't support it (level1).
- * - Incresed tx_timeout beacuse of auto-neg.
- * - Adjusted timers for forced speeds.
- *
- * v1.12 Oct 12, 2000 - Minor fixes (memleak, init, etc.)
- *
- * v1.13 Nov 28, 2000 - Stop flooding console with auto-neg issues
- * when link can't be established.
- * - Added the bbuf option as a kernel parameter.
- * - Fixed ioaddr probe bug.
- * - Fixed stupid deadlock with MII interrupts.
- * - Added support for speed/duplex selection with
- * multiple nics.
- * - Added partly fix for TX Channel lockup with
- * TLAN v1.0 silicon. This needs to be investigated
- * further.
- *
- * v1.14 Dec 16, 2000 - Added support for servicing multiple frames per.
- * interrupt. Thanks goes to
- * Adam Keys <adam@ti.com>
- * Denis Beaudoin <dbeaudoin@ti.com>
- * for providing the patch.
- * - Fixed auto-neg output when using multiple
- * adapters.
- * - Converted to use new taskq interface.
- *
- * v1.14a Jan 6, 2001 - Minor adjustments (spinlocks, etc.)
- *
- * Samuel Chessman <chessman@tux.org> New Maintainer!
- *
- * v1.15 Apr 4, 2002 - Correct operation when aui=1 to be
- * 10T half duplex no loopback
- * Thanks to Gunnar Eikman
- *
- * Sakari Ailus <sakari.ailus@iki.fi>:
- *
- * v1.15a Dec 15 2008 - Remove bbuf support, it doesn't work anyway.
- * v1.16 Jan 6 2011 - Make checkpatch.pl happy.
- * v1.17 Jan 6 2011 - Add suspend/resume support.
- *
******************************************************************************/
#include <linux/module.h>
--
1.7.4.2.g597a6.dirty
^ permalink raw reply related
* [PATCH 2/2] tlan: Use pr_fmt, pr_<level> and netdev_<level>
From: Joe Perches @ 2011-03-01 16:56 UTC (permalink / raw)
To: Samuel Chessman; +Cc: Sakari Ailus, netdev, linux-kernel
In-Reply-To: <901ce76108a5ab7327159b57a1b8c9283673532b.1298998514.git.joe@perches.com>
Neatening and standardization to the current logging mechanisms.
Miscellaneous speen/speed typo correction.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/tlan.c | 164 ++++++++++++++++++++++++---------------------------
1 files changed, 77 insertions(+), 87 deletions(-)
diff --git a/drivers/net/tlan.c b/drivers/net/tlan.c
index 7721e6c..ace6404 100644
--- a/drivers/net/tlan.c
+++ b/drivers/net/tlan.c
@@ -27,6 +27,8 @@
*
******************************************************************************/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/module.h>
#include <linux/init.h>
#include <linux/ioport.h>
@@ -59,7 +61,7 @@ module_param_array(speed, int, NULL, 0);
MODULE_PARM_DESC(aui, "ThunderLAN use AUI port(s) (0-1)");
MODULE_PARM_DESC(duplex,
"ThunderLAN duplex setting(s) (0-default, 1-half, 2-full)");
-MODULE_PARM_DESC(speed, "ThunderLAN port speen setting(s) (0,10,100)");
+MODULE_PARM_DESC(speed, "ThunderLAN port speed setting(s) (0,10,100)");
MODULE_AUTHOR("Maintainer: Samuel Chessman <chessman@tux.org>");
MODULE_DESCRIPTION("Driver for TI ThunderLAN based ethernet PCI adapters");
@@ -397,7 +399,7 @@ static int __init tlan_probe(void)
{
int rc = -ENODEV;
- printk(KERN_INFO "%s", tlan_banner);
+ pr_info("%s", tlan_banner);
TLAN_DBG(TLAN_DEBUG_PROBE, "Starting PCI Probe....\n");
@@ -406,16 +408,16 @@ static int __init tlan_probe(void)
rc = pci_register_driver(&tlan_driver);
if (rc != 0) {
- printk(KERN_ERR "TLAN: Could not register pci driver.\n");
+ pr_err("Could not register pci driver\n");
goto err_out_pci_free;
}
TLAN_DBG(TLAN_DEBUG_PROBE, "Starting EISA Probe....\n");
tlan_eisa_probe();
- printk(KERN_INFO "TLAN: %d device%s installed, PCI: %d EISA: %d\n",
- tlan_devices_installed, tlan_devices_installed == 1 ? "" : "s",
- tlan_have_pci, tlan_have_eisa);
+ pr_info("%d device%s installed, PCI: %d EISA: %d\n",
+ tlan_devices_installed, tlan_devices_installed == 1 ? "" : "s",
+ tlan_have_pci, tlan_have_eisa);
if (tlan_devices_installed == 0) {
rc = -ENODEV;
@@ -474,7 +476,7 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
rc = pci_request_regions(pdev, tlan_signature);
if (rc) {
- printk(KERN_ERR "TLAN: Could not reserve IO regions\n");
+ pr_err("Could not reserve IO regions\n");
goto err_out;
}
}
@@ -482,7 +484,7 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
dev = alloc_etherdev(sizeof(struct tlan_priv));
if (dev == NULL) {
- printk(KERN_ERR "TLAN: Could not allocate memory for device.\n");
+ pr_err("Could not allocate memory for device\n");
rc = -ENOMEM;
goto err_out_regions;
}
@@ -501,8 +503,7 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
if (rc) {
- printk(KERN_ERR
- "TLAN: No suitable PCI mapping available.\n");
+ pr_err("No suitable PCI mapping available\n");
goto err_out_free_dev;
}
@@ -516,7 +517,7 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
}
}
if (!pci_io_base) {
- printk(KERN_ERR "TLAN: No IO mappings available\n");
+ pr_err("No IO mappings available\n");
rc = -EIO;
goto err_out_free_dev;
}
@@ -572,13 +573,13 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
rc = tlan_init(dev);
if (rc) {
- printk(KERN_ERR "TLAN: Could not set up device.\n");
+ pr_err("Could not set up device\n");
goto err_out_free_dev;
}
rc = register_netdev(dev);
if (rc) {
- printk(KERN_ERR "TLAN: Could not register device.\n");
+ pr_err("Could not register device\n");
goto err_out_uninit;
}
@@ -595,12 +596,11 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
tlan_have_eisa++;
}
- printk(KERN_INFO "TLAN: %s irq=%2d, io=%04x, %s, Rev. %d\n",
- dev->name,
- (int) dev->irq,
- (int) dev->base_addr,
- priv->adapter->device_label,
- priv->adapter_rev);
+ netdev_info(dev, "irq=%2d, io=%04x, %s, Rev. %d\n",
+ (int)dev->irq,
+ (int)dev->base_addr,
+ priv->adapter->device_label,
+ priv->adapter_rev);
return 0;
err_out_uninit:
@@ -716,7 +716,7 @@ static void __init tlan_eisa_probe(void)
}
if (debug == 0x10)
- printk(KERN_INFO "Found one\n");
+ pr_info("Found one\n");
/* Get irq from board */
@@ -745,12 +745,12 @@ static void __init tlan_eisa_probe(void)
out:
if (debug == 0x10)
- printk(KERN_INFO "None found\n");
+ pr_info("None found\n");
continue;
out2:
if (debug == 0x10)
- printk(KERN_INFO "Card found but it is not enabled, skipping\n");
+ pr_info("Card found but it is not enabled, skipping\n");
continue;
}
@@ -818,8 +818,7 @@ static int tlan_init(struct net_device *dev)
priv->dma_size = dma_size;
if (priv->dma_storage == NULL) {
- printk(KERN_ERR
- "TLAN: Could not allocate lists and buffers for %s.\n",
+ pr_err("Could not allocate lists and buffers for %s\n",
dev->name);
return -ENOMEM;
}
@@ -837,9 +836,8 @@ static int tlan_init(struct net_device *dev)
(u8) priv->adapter->addr_ofs + i,
(u8 *) &dev->dev_addr[i]);
if (err) {
- printk(KERN_ERR "TLAN: %s: Error reading MAC from eeprom: %d\n",
- dev->name,
- err);
+ pr_err("%s: Error reading MAC from eeprom: %d\n",
+ dev->name, err);
}
dev->addr_len = 6;
@@ -883,8 +881,8 @@ static int tlan_open(struct net_device *dev)
dev->name, dev);
if (err) {
- pr_err("TLAN: Cannot open %s because IRQ %d is already in use.\n",
- dev->name, dev->irq);
+ netdev_err(dev, "Cannot open because IRQ %d is already in use\n",
+ dev->irq);
return err;
}
@@ -1367,8 +1365,8 @@ static u32 tlan_handle_tx_eof(struct net_device *dev, u16 host_int)
}
if (!ack)
- printk(KERN_INFO
- "TLAN: Received interrupt for uncompleted TX frame.\n");
+ netdev_info(dev,
+ "Received interrupt for uncompleted TX frame\n");
if (eoc) {
TLAN_DBG(TLAN_DEBUG_TX,
@@ -1522,8 +1520,8 @@ drop_and_reuse:
}
if (!ack)
- printk(KERN_INFO
- "TLAN: Received interrupt for uncompleted RX frame.\n");
+ netdev_info(dev,
+ "Received interrupt for uncompleted RX frame\n");
if (eoc) {
@@ -1579,7 +1577,7 @@ drop_and_reuse:
static u32 tlan_handle_dummy(struct net_device *dev, u16 host_int)
{
- pr_info("TLAN: Test interrupt on %s.\n", dev->name);
+ netdev_info(dev, "Test interrupt\n");
return 1;
}
@@ -1673,7 +1671,7 @@ static u32 tlan_handle_status_check(struct net_device *dev, u16 host_int)
if (host_int & TLAN_HI_IV_MASK) {
netif_stop_queue(dev);
error = inl(dev->base_addr + TLAN_CH_PARM);
- pr_info("TLAN: %s: Adaptor Error = 0x%x\n", dev->name, error);
+ netdev_info(dev, "Adaptor Error = 0x%x\n", error);
tlan_read_and_clear_stats(dev, TLAN_RECORD);
outl(TLAN_HC_AD_RST, dev->base_addr + TLAN_HOST_CMD);
@@ -1914,7 +1912,7 @@ static void tlan_reset_lists(struct net_device *dev)
list->buffer[0].count = TLAN_MAX_FRAME_SIZE | TLAN_LAST_BUFFER;
skb = netdev_alloc_skb_ip_align(dev, TLAN_MAX_FRAME_SIZE + 5);
if (!skb) {
- pr_err("TLAN: out of memory for received data.\n");
+ netdev_err(dev, "Out of memory for received data\n");
break;
}
@@ -1998,13 +1996,13 @@ static void tlan_print_dio(u16 io_base)
u32 data0, data1;
int i;
- pr_info("TLAN: Contents of internal registers for io base 0x%04hx.\n",
- io_base);
- pr_info("TLAN: Off. +0 +4\n");
+ pr_info("Contents of internal registers for io base 0x%04hx\n",
+ io_base);
+ pr_info("Off. +0 +4\n");
for (i = 0; i < 0x4C; i += 8) {
data0 = tlan_dio_read32(io_base, i);
data1 = tlan_dio_read32(io_base, i + 0x4);
- pr_info("TLAN: 0x%02x 0x%08x 0x%08x\n", i, data0, data1);
+ pr_info("0x%02x 0x%08x 0x%08x\n", i, data0, data1);
}
}
@@ -2033,14 +2031,14 @@ static void tlan_print_list(struct tlan_list *list, char *type, int num)
{
int i;
- pr_info("TLAN: %s List %d at %p\n", type, num, list);
- pr_info("TLAN: Forward = 0x%08x\n", list->forward);
- pr_info("TLAN: CSTAT = 0x%04hx\n", list->c_stat);
- pr_info("TLAN: Frame Size = 0x%04hx\n", list->frame_size);
+ pr_info("%s List %d at %p\n", type, num, list);
+ pr_info(" Forward = 0x%08x\n", list->forward);
+ pr_info(" CSTAT = 0x%04hx\n", list->c_stat);
+ pr_info(" Frame Size = 0x%04hx\n", list->frame_size);
/* for (i = 0; i < 10; i++) { */
for (i = 0; i < 2; i++) {
- pr_info("TLAN: Buffer[%d].count, addr = 0x%08x, 0x%08x\n",
- i, list->buffer[i].count, list->buffer[i].address);
+ pr_info(" Buffer[%d].count, addr = 0x%08x, 0x%08x\n",
+ i, list->buffer[i].count, list->buffer[i].address);
}
}
@@ -2255,7 +2253,7 @@ tlan_finish_reset(struct net_device *dev)
if ((priv->adapter->flags & TLAN_ADAPTER_UNMANAGED_PHY) ||
(priv->aui)) {
status = MII_GS_LINK;
- pr_info("TLAN: %s: Link forced.\n", dev->name);
+ netdev_info(dev, "Link forced\n");
} else {
tlan_mii_read_reg(dev, phy, MII_GEN_STS, &status);
udelay(1000);
@@ -2267,24 +2265,21 @@ tlan_finish_reset(struct net_device *dev)
tlan_mii_read_reg(dev, phy, MII_AN_LPA, &partner);
tlan_mii_read_reg(dev, phy, TLAN_TLPHY_PAR, &tlphy_par);
- pr_info("TLAN: %s: Link active with ", dev->name);
- if (!(tlphy_par & TLAN_PHY_AN_EN_STAT)) {
- pr_info("forced 10%sMbps %s-Duplex\n",
- tlphy_par & TLAN_PHY_SPEED_100
- ? "" : "0",
- tlphy_par & TLAN_PHY_DUPLEX_FULL
- ? "Full" : "Half");
- } else {
- pr_info("Autonegotiation enabled, at 10%sMbps %s-Duplex\n",
- tlphy_par & TLAN_PHY_SPEED_100
- ? "" : "0",
- tlphy_par & TLAN_PHY_DUPLEX_FULL
- ? "Full" : "half");
- pr_info("TLAN: Partner capability: ");
- for (i = 5; i <= 10; i++)
- if (partner & (1<<i))
- printk("%s", media[i-5]);
- printk("\n");
+ netdev_info(dev,
+ "Link active with %s %uMbps %s-Duplex\n",
+ !(tlphy_par & TLAN_PHY_AN_EN_STAT)
+ ? "forced" : "Autonegotiation enabled,",
+ tlphy_par & TLAN_PHY_SPEED_100
+ ? 100 : 10,
+ tlphy_par & TLAN_PHY_DUPLEX_FULL
+ ? "Full" : "Half");
+
+ if (tlphy_par & TLAN_PHY_AN_EN_STAT) {
+ netdev_info(dev, "Partner capability:");
+ for (i = 5; i < 10; i++)
+ if (partner & (1 << i))
+ pr_cont(" %s", media[i-5]);
+ pr_cont("\n");
}
tlan_dio_write8(dev->base_addr, TLAN_LED_REG,
@@ -2296,7 +2291,7 @@ tlan_finish_reset(struct net_device *dev)
tlan_set_timer(dev, (10*HZ), TLAN_TIMER_LINK_BEAT);
#endif
} else if (status & MII_GS_LINK) {
- pr_info("TLAN: %s: Link active\n", dev->name);
+ netdev_info(dev, "Link active\n");
tlan_dio_write8(dev->base_addr, TLAN_LED_REG,
TLAN_LED_LINK);
}
@@ -2322,8 +2317,7 @@ tlan_finish_reset(struct net_device *dev)
outl(TLAN_HC_GO | TLAN_HC_RT, dev->base_addr + TLAN_HOST_CMD);
netif_carrier_on(dev);
} else {
- pr_info("TLAN: %s: Link inactive, will retry in 10 secs...\n",
- dev->name);
+ netdev_info(dev, "Link inactive, will retry in 10 secs...\n");
tlan_set_timer(dev, (10*HZ), TLAN_TIMER_FINISH_RESET);
return;
}
@@ -2407,23 +2401,20 @@ static void tlan_phy_print(struct net_device *dev)
phy = priv->phy[priv->phy_num];
if (priv->adapter->flags & TLAN_ADAPTER_UNMANAGED_PHY) {
- pr_info("TLAN: Device %s, Unmanaged PHY.\n", dev->name);
+ netdev_info(dev, "Unmanaged PHY\n");
} else if (phy <= TLAN_PHY_MAX_ADDR) {
- pr_info("TLAN: Device %s, PHY 0x%02x.\n", dev->name, phy);
- pr_info("TLAN: Off. +0 +1 +2 +3\n");
+ netdev_info(dev, "PHY 0x%02x\n", phy);
+ pr_info(" Off. +0 +1 +2 +3\n");
for (i = 0; i < 0x20; i += 4) {
- pr_info("TLAN: 0x%02x", i);
tlan_mii_read_reg(dev, phy, i, &data0);
- printk(" 0x%04hx", data0);
tlan_mii_read_reg(dev, phy, i + 1, &data1);
- printk(" 0x%04hx", data1);
tlan_mii_read_reg(dev, phy, i + 2, &data2);
- printk(" 0x%04hx", data2);
tlan_mii_read_reg(dev, phy, i + 3, &data3);
- printk(" 0x%04hx\n", data3);
+ pr_info(" 0x%02x 0x%04hx 0x%04hx 0x%04hx 0x%04hx\n",
+ i, data0, data1, data2, data3);
}
} else {
- pr_info("TLAN: Device %s, Invalid PHY.\n", dev->name);
+ netdev_info(dev, "Invalid PHY\n");
}
}
@@ -2490,7 +2481,7 @@ static void tlan_phy_detect(struct net_device *dev)
else if (priv->phy[0] != TLAN_PHY_NONE)
priv->phy_num = 0;
else
- pr_info("TLAN: Cannot initialize device, no PHY was found!\n");
+ netdev_info(dev, "Cannot initialize device, no PHY was found!\n");
}
@@ -2618,8 +2609,7 @@ static void tlan_phy_start_link(struct net_device *dev)
* but the card need additional time to start AN.
* .5 sec should be plenty extra.
*/
- pr_info("TLAN: %s: Starting autonegotiation.\n",
- dev->name);
+ netdev_info(dev, "Starting autonegotiation\n");
tlan_set_timer(dev, (2*HZ), TLAN_TIMER_PHY_FINISH_AN);
return;
}
@@ -2682,16 +2672,16 @@ static void tlan_phy_finish_auto_neg(struct net_device *dev)
* more time. Perhaps we should fail after a while.
*/
if (!priv->neg_be_verbose++) {
- pr_info("TLAN: Giving autonegotiation more time.\n");
- pr_info("TLAN: Please check that your adapter has\n");
- pr_info("TLAN: been properly connected to a HUB or Switch.\n");
- pr_info("TLAN: Trying to establish link in the background...\n");
+ pr_info("Giving autonegotiation more time.\n");
+ pr_info("Please check that your adapter has\n");
+ pr_info("been properly connected to a HUB or Switch.\n");
+ pr_info("Trying to establish link in the background...\n");
}
tlan_set_timer(dev, (8*HZ), TLAN_TIMER_PHY_FINISH_AN);
return;
}
- pr_info("TLAN: %s: Autonegotiation complete.\n", dev->name);
+ netdev_info(dev, "Autonegotiation complete\n");
tlan_mii_read_reg(dev, phy, MII_AN_ADV, &an_adv);
tlan_mii_read_reg(dev, phy, MII_AN_LPA, &an_lpa);
mode = an_adv & an_lpa & 0x03E0;
@@ -2716,11 +2706,11 @@ static void tlan_phy_finish_auto_neg(struct net_device *dev)
(an_adv & an_lpa & 0x0040)) {
tlan_mii_write_reg(dev, phy, MII_GEN_CTL,
MII_GC_AUTOENB | MII_GC_DUPLEX);
- pr_info("TLAN: Starting internal PHY with FULL-DUPLEX\n");
+ netdev_info(dev, "Starting internal PHY with FULL-DUPLEX\n");
} else {
tlan_mii_write_reg(dev, phy, MII_GEN_CTL,
MII_GC_AUTOENB);
- pr_info("TLAN: Starting internal PHY with HALF-DUPLEX\n");
+ netdev_info(dev, "Starting internal PHY with HALF-DUPLEX\n");
}
}
--
1.7.4.2.g597a6.dirty
^ permalink raw reply related
* [PATCH 1/5] tg3: use <linux/io.h> and <linux/uaccess.h> instead <asm/io.h> and <asm/uaccess.h>
From: Javier Martinez Canillas @ 2011-03-01 17:04 UTC (permalink / raw)
To: Matt Carlson
Cc: Michael Chan, Grant Likely, netdev, kernelnewbies,
Javier Martinez Canillas
It is proper style to include linux/foo.h instead asm/foo.h if both exist
Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
drivers/net/tg3.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 6be4185..4410d73 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -48,9 +48,9 @@
#include <net/ip.h>
#include <asm/system.h>
-#include <asm/io.h>
+#include <linux/io.h>
#include <asm/byteorder.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
#ifdef CONFIG_SPARC
#include <asm/idprom.h>
--
1.7.2.3
^ permalink raw reply related
* [PATCH 1/5] tg3: use usleep_range not msleep for small sleeps
From: Javier Martinez Canillas @ 2011-03-01 17:04 UTC (permalink / raw)
To: Matt Carlson
Cc: Michael Chan, Grant Likely, netdev, kernelnewbies,
Javier Martinez Canillas
In-Reply-To: <1298999069-12740-1-git-send-email-martinez.javier@gmail.com>
Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
drivers/net/tg3.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 6be4185..3310c7a 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -2410,7 +2410,7 @@ static int tg3_nvram_read_using_eeprom(struct tg3 *tp,
if (tmp & EEPROM_ADDR_COMPLETE)
break;
- msleep(1);
+ usleep_range(1000, 2000);
}
if (!(tmp & EEPROM_ADDR_COMPLETE))
return -EBUSY;
@@ -2688,7 +2688,7 @@ static int tg3_power_down_prepare(struct tg3 *tp)
tg3_read_mem(tp, NIC_SRAM_FW_ASF_STATUS_MBOX, &val);
if (val == ~NIC_SRAM_FIRMWARE_MBOX_MAGIC1)
break;
- msleep(1);
+ usleep_range(1000, 2000);
}
}
if (tp->tg3_flags & TG3_FLAG_WOL_CAP)
@@ -8901,7 +8901,7 @@ static int tg3_test_interrupt(struct tg3 *tp)
break;
}
- msleep(10);
+ usleep_range(10000, 20000);
}
tg3_disable_ints(tp);
@@ -11863,7 +11863,7 @@ static void __devinit tg3_nvram_init(struct tg3 *tp)
(EEPROM_DEFAULT_CLOCK_PERIOD <<
EEPROM_ADDR_CLKPERD_SHIFT)));
- msleep(1);
+ usleep_range(1000, 2000);
/* Enable seeprom accesses. */
tw32_f(GRC_LOCAL_CTRL,
@@ -11956,7 +11956,7 @@ static int tg3_nvram_write_block_using_eeprom(struct tg3 *tp,
if (val & EEPROM_ADDR_COMPLETE)
break;
- msleep(1);
+ usleep_range(1000, 2000);
}
if (!(val & EEPROM_ADDR_COMPLETE)) {
rc = -EBUSY;
@@ -12263,7 +12263,7 @@ static void __devinit tg3_get_eeprom_hw_cfg(struct tg3 *tp)
pci_read_config_word(tp->pdev, tp->pm_cap + PCI_PM_CTRL, &pmcsr);
pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
pci_write_config_word(tp->pdev, tp->pm_cap + PCI_PM_CTRL, pmcsr);
- msleep(1);
+ usleep_range(1000, 2000);
/* Make sure register accesses (indirect or otherwise)
* will function correctly.
--
1.7.2.3
^ permalink raw reply related
* [PATCH 3/5] tg3: Enclose macro with complex values in parenthesis
From: Javier Martinez Canillas @ 2011-03-01 17:04 UTC (permalink / raw)
To: Matt Carlson
Cc: Michael Chan, Grant Likely, netdev, kernelnewbies,
Javier Martinez Canillas
In-Reply-To: <1298999069-12740-1-git-send-email-martinez.javier@gmail.com>
Macros with complex values should be enclosed in parenthesis.
Change accordingly.
Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
drivers/net/tg3.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 6dc7a8a..c67eb19 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -513,16 +513,16 @@ static void tg3_write32_mbox_5906(struct tg3 *tp, u32 off, u32 val)
writel(val, tp->regs + off + GRCMBOX_BASE);
}
-#define tw32_mailbox(reg, val) tp->write32_mbox(tp, reg, val)
+#define tw32_mailbox(reg, val) (tp->write32_mbox(tp, reg, val))
#define tw32_mailbox_f(reg, val) tw32_mailbox_flush(tp, (reg), (val))
-#define tw32_rx_mbox(reg, val) tp->write32_rx_mbox(tp, reg, val)
-#define tw32_tx_mbox(reg, val) tp->write32_tx_mbox(tp, reg, val)
-#define tr32_mailbox(reg) tp->read32_mbox(tp, reg)
+#define tw32_rx_mbox(reg, val) (tp->write32_rx_mbox(tp, reg, val))
+#define tw32_tx_mbox(reg, val) (tp->write32_tx_mbox(tp, reg, val))
+#define tr32_mailbox(reg) (tp->read32_mbox(tp, reg))
-#define tw32(reg, val) tp->write32(tp, reg, val)
+#define tw32(reg, val) (tp->write32(tp, reg, val))
#define tw32_f(reg, val) _tw32_flush(tp, (reg), (val), 0)
#define tw32_wait_f(reg, val, us) _tw32_flush(tp, (reg), (val), (us))
-#define tr32(reg) tp->read32(tp, reg)
+#define tr32(reg) (tp->read32(tp, reg))
static void tg3_write_mem(struct tg3 *tp, u32 off, u32 val)
{
--
1.7.2.3
^ permalink raw reply related
* [PATCH 4/5] tg3: Don't use IRQF_SAMPLE_RANDOM
From: Javier Martinez Canillas @ 2011-03-01 17:04 UTC (permalink / raw)
To: Matt Carlson
Cc: Michael Chan, Grant Likely, netdev, kernelnewbies,
Javier Martinez Canillas
In-Reply-To: <1298999069-12740-1-git-send-email-martinez.javier@gmail.com>
This flag is scheduled for removal so we shouldn't used it.
Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
drivers/net/tg3.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index c67eb19..58c6049 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -8844,12 +8844,11 @@ static int tg3_request_irq(struct tg3 *tp, int irq_num)
fn = tg3_msi;
if (tp->tg3_flags2 & TG3_FLG2_1SHOT_MSI)
fn = tg3_msi_1shot;
- flags = IRQF_SAMPLE_RANDOM;
} else {
fn = tg3_interrupt;
if (tp->tg3_flags & TG3_FLAG_TAGGED_STATUS)
fn = tg3_interrupt_tagged;
- flags = IRQF_SHARED | IRQF_SAMPLE_RANDOM;
+ flags = IRQF_SHARED;
}
return request_irq(tnapi->irq_vec, fn, flags, name, tnapi);
@@ -8880,7 +8879,7 @@ static int tg3_test_interrupt(struct tg3 *tp)
}
err = request_irq(tnapi->irq_vec, tg3_test_isr,
- IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, tnapi);
+ IRQF_SHARED, dev->name, tnapi);
if (err)
return err;
--
1.7.2.3
^ permalink raw reply related
* [PATCH 5/5] tg3: Fix inline keyword usage
From: Javier Martinez Canillas @ 2011-03-01 17:04 UTC (permalink / raw)
To: Matt Carlson
Cc: Michael Chan, Grant Likely, netdev, kernelnewbies,
Javier Martinez Canillas
In-Reply-To: <1298999069-12740-1-git-send-email-martinez.javier@gmail.com>
The correct usage is "static inline void" not "static void inline".
Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
drivers/net/tg3.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 58c6049..d7e564d 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -13061,7 +13061,7 @@ done:
static struct pci_dev * __devinit tg3_find_peer(struct tg3 *);
-static void inline vlan_features_add(struct net_device *dev, unsigned long flags)
+static inline void vlan_features_add(struct net_device *dev, unsigned long flags)
{
dev->vlan_features |= flags;
}
--
1.7.2.3
^ permalink raw reply related
* [net-next PATCH] netdevice: Convert printk to pr_info in netif_tx_stop_queue
From: Joe Perches @ 2011-03-01 17:06 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, LKML
This allows any caller to be prefaced by any specific
pr_fmt to better identify which device driver is using
this function inappropriately.
Add terminating newline.
Signed-off-by: Joe Perches <joe@perches.com>
---
include/linux/netdevice.h | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ffe56c1..c778992 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1765,8 +1765,7 @@ static inline void netif_tx_wake_all_queues(struct net_device *dev)
static inline void netif_tx_stop_queue(struct netdev_queue *dev_queue)
{
if (WARN_ON(!dev_queue)) {
- printk(KERN_INFO "netif_stop_queue() cannot be called before "
- "register_netdev()");
+ pr_info("netif_stop_queue() cannot be called before register_netdev()\n");
return;
}
set_bit(__QUEUE_STATE_XOFF, &dev_queue->state);
^ permalink raw reply related
* [PATCH net-next-2.6] benet: use GFP_KERNEL allocations when possible
From: Ajit Khaparde @ 2011-03-01 17:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
Thanks Eric.
> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Tuesday, March 01, 2011 9:48 AM
> To: Khaparde, Ajit
> Cc: netdev
> Subject: [PATCH net-next-2.6] benet: use GFP_KERNEL allocations when
> possible
>
> Extend be_alloc_pages() with a gfp parameter, so that we use GFP_KERNEL
> allocations instead of GFP_ATOMIC when not running in softirq context.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Ajit Khaparde <ajit.khaparde@emulex.com>
^ permalink raw reply
* Re: [Open-FCoE] [PATCH] fcoe: correct checking for bonding
From: Joe Eykholt @ 2011-03-01 17:49 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Jay Vosburgh, James.Bottomley, netdev, devel, linux-scsi
In-Reply-To: <20110301063710.GD2855@psychotron.redhat.com>
On 2/28/11 10:37 PM, Jiri Pirko wrote:
> Mon, Feb 28, 2011 at 06:54:29PM CET, joe.eykholt@gmail.com wrote:
>> On 2/28/11 9:15 AM, Jay Vosburgh wrote:
>>> Jiri Pirko<jpirko@redhat.com> wrote:
>>>
>>>> Check for IFF_BONDING as this flag is set-up for all bonding devices.
>>>>
>>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>>> ---
>>>> drivers/scsi/fcoe/fcoe.c | 4 +---
>>>> 1 files changed, 1 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
>>>> index 9f9600b..67714a4 100644
>>>> --- a/drivers/scsi/fcoe/fcoe.c
>>>> +++ b/drivers/scsi/fcoe/fcoe.c
>>>> @@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
>>>> }
>>>>
>>>> /* Do not support for bonding device */
>>>> - if ((netdev->priv_flags& IFF_MASTER_ALB) ||
>>>> - (netdev->priv_flags& IFF_SLAVE_INACTIVE) ||
>>>> - (netdev->priv_flags& IFF_MASTER_8023AD)) {
>>>> + if (netdev->priv_flags& IFF_BONDING) {
>>>> FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n");
>>>> return -EOPNOTSUPP;
>>>> }
>>>
>>> Based on past discussions, I believe the intent of the code is
>>> to permit FCOE over bonding only for active-backup mode, and possibly
>>> for -xor/-rr as well.
>>>
>>> I'm not sure if the slave or the master is what's being tested
>>> here, so I'm not sure what the right thing to do is. I suspect it's the
>>> master, as I recall discussion of one configuration involving
>>> active-backup mode balancing FCOE traffic over both the active and
>>> inactive slaves. FCOE uses the "orig_dev" logic in __netif_receive_skb
>>> to have the packets delivered even on the nominally inactive slave.
>>>
>>> -J
>>>
>>> ---
>>> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>>
>> Right. That was the intent. It should work on the physical dev, but probably
>> not on the master of the bond.
>>
>> If you have a master/slave bond for IPv4 between eth1 and eth2, say,
>> and they are going to two different DCE (FCoE) switches, presumably on
>> different VSANs but with ultimate access to the same disks,
>> then you want to split the FCoE traffic in active/active
>> mode using separate FCoE instances on eth1 and eth2 even though IP
>> is using active/standby on bond0. This should work. But, putting fcoe
>> on bond0 isn't going to do what you want.
>>
>> However, it seems like the check above shouldn't be checking
>> IFF_SLAVE_INACTIVE. I can't test this.
>
> OK. So I guess the right check should be for:
> (netdev->priv_flags& IFF_BONDING&& netdev->flags& IFF_MASTER)
I think that's OK. How about just checking for MASTER?
When is MASTER going to be set without BONDING?
Otherwise I'd add some parens or I might code this as:
if ((netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) ==
(IFF_BONDING | IFF_MASTER))
Which is less clear, I know, but used to generate better code.
The compiler might generate the same code these days.
Not that this is performance-critical or anything.
> This would disable adding all bond devices (like bond0 etc) and allows
> to use enslaved physdevs.
>
> Note that checking for mode is irrelevant here. Mode could be easily
> changed later without fcoe knowing that.
>
> Jirka
Cheers,
Joe
^ permalink raw reply
* [PATCH] tile on-chip network driver: sync up with latest fixes
From: Chris Metcalf @ 2011-03-01 17:49 UTC (permalink / raw)
To: linux-kernel, netdev
Combine the "command" and "completion" locks into a single lock,
on each egress queue, to improve efficiency.
Support the use of 4KB pages in the "egress queue".
Delete the unused "duplicate ACK compression" code.
Filter "bad" (i.e. truncated) packets.
Avoid corrupting "dev->napi_list", by sequentializing modifications.
Deregister for incoming packets during stop, to reduce unexpected
interrupts. Also, encourage active NAPI loops to complete before
we disable NAPI, which would otherwise crash.
Free any pending completions after shutting down LEPP.
Use a single, permanently registered, IRQ handler, to avoid situations
in which the IRQ handler was firing after being freed, and ignore any
"unexpected" interrupts.
Drop egress packets, instead of spinning, if the hardware cannot keep
up, or is disconnected.
Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
drivers/net/tile/tilepro.c | 867 +++++++++++++++++++++++---------------------
1 files changed, 457 insertions(+), 410 deletions(-)
diff --git a/drivers/net/tile/tilepro.c b/drivers/net/tile/tilepro.c
index f901299..eb1b825 100644
--- a/drivers/net/tile/tilepro.c
+++ b/drivers/net/tile/tilepro.c
@@ -44,10 +44,6 @@
#include <linux/tcp.h>
-/* There is no singlethread_cpu, so schedule work on the current cpu. */
-#define singlethread_cpu -1
-
-
/*
* First, "tile_net_init_module()" initializes all four "devices" which
* can be used by linux.
@@ -73,15 +69,16 @@
* return, knowing we will be called again later. Otherwise, we
* reenable the ingress interrupt, and call "napi_complete()".
*
+ * HACK: Since disabling the ingress interrupt is not reliable, we
+ * ignore the interrupt if the global "active" flag is false.
+ *
*
* NOTE: The use of "native_driver" ensures that EPP exists, and that
- * "epp_sendv" is legal, and that "LIPP" is being used.
+ * we are using "LIPP" and "LEPP".
*
* NOTE: Failing to free completions for an arbitrarily long time
* (which is defined to be illegal) does in fact cause bizarre
* problems. The "egress_timer" helps prevent this from happening.
- *
- * NOTE: The egress code can be interrupted by the interrupt handler.
*/
@@ -142,6 +139,7 @@
MODULE_AUTHOR("Tilera");
MODULE_LICENSE("GPL");
+
/*
* Queue of incoming packets for a specific cpu and device.
*
@@ -177,7 +175,7 @@ struct tile_net_cpu {
struct tile_netio_queue queue;
/* Statistics. */
struct tile_net_stats_t stats;
- /* ISSUE: Is this needed? */
+ /* True iff NAPI is enabled. */
bool napi_enabled;
/* True if this tile has succcessfully registered with the IPP. */
bool registered;
@@ -200,20 +198,20 @@ struct tile_net_cpu {
struct tile_net_priv {
/* Our network device. */
struct net_device *dev;
- /* The actual egress queue. */
- lepp_queue_t *epp_queue;
- /* Protects "epp_queue->cmd_tail" and "epp_queue->comp_tail" */
- spinlock_t cmd_lock;
- /* Protects "epp_queue->comp_head". */
- spinlock_t comp_lock;
+ /* Pages making up the egress queue. */
+ struct page *eq_pages;
+ /* Address of the actual egress queue. */
+ lepp_queue_t *eq;
+ /* Protects "eq". */
+ spinlock_t eq_lock;
/* The hypervisor handle for this interface. */
int hv_devhdl;
/* The intr bit mask that IDs this device. */
u32 intr_id;
/* True iff "tile_net_open_aux()" has succeeded. */
- int partly_opened;
- /* True iff "tile_net_open_inner()" has succeeded. */
- int fully_opened;
+ bool partly_opened;
+ /* True iff the device is "active". */
+ bool active;
/* Effective network cpus. */
struct cpumask network_cpus_map;
/* Number of network cpus. */
@@ -228,6 +226,10 @@ struct tile_net_priv {
struct tile_net_cpu *cpu[NR_CPUS];
};
+/* Log2 of the number of small pages needed for the egress queue. */
+#define EQ_ORDER get_order(sizeof(lepp_queue_t))
+/* Size of the egress queue's pages. */
+#define EQ_SIZE (1 << (PAGE_SHIFT + EQ_ORDER))
/*
* The actual devices (xgbe0, xgbe1, gbe0, gbe1).
@@ -284,7 +286,11 @@ static void net_printk(char *fmt, ...)
*/
static void dump_packet(unsigned char *data, unsigned long length, char *s)
{
+ int my_cpu = smp_processor_id();
+
unsigned long i;
+ char buf[128];
+
static unsigned int count;
pr_info("dump_packet(data %p, length 0x%lx s %s count 0x%x)\n",
@@ -294,10 +300,12 @@ static void dump_packet(unsigned char *data, unsigned long length, char *s)
for (i = 0; i < length; i++) {
if ((i & 0xf) == 0)
- sprintf(buf, "%8.8lx:", i);
+ sprintf(buf, "[%02d] %8.8lx:", my_cpu, i);
sprintf(buf + strlen(buf), " %2.2x", data[i]);
- if ((i & 0xf) == 0xf || i == length - 1)
- pr_info("%s\n", buf);
+ if ((i & 0xf) == 0xf || i == length - 1) {
+ strcat(buf, "\n");
+ pr_info("%s", buf);
+ }
}
}
#endif
@@ -356,9 +364,11 @@ static bool tile_net_provide_needed_buffer(struct tile_net_cpu *info,
bool small)
{
/* ISSUE: What should we use here? */
+ /* Without "jumbo", 2 + 1536 should be sufficient. */
unsigned int large_size = NET_IP_ALIGN + TILE_NET_MTU + 100;
- /* Round up to ensure to avoid "false sharing" with last cache line. */
+ /* Avoid "false sharing" with last cache line. */
+ /* FIXME: This is already done by "dev_alloc_skb()". */
unsigned int buffer_size =
(((small ? LIPP_SMALL_PACKET_SIZE : large_size) +
CHIP_L2_LINE_SIZE() - 1) & -CHIP_L2_LINE_SIZE());
@@ -367,6 +377,12 @@ static bool tile_net_provide_needed_buffer(struct tile_net_cpu *info,
* ISSUE: Since CPAs are 38 bits, and we can only encode the
* high 31 bits in a "linux_buffer_t", the low 7 bits must be
* zero, and thus, we must align the actual "va" mod 128.
+ *
+ * FIXME: We could change the underlying code to assume that
+ * the low SIX bits are zero, and the 7th bit is one, and then
+ * we would need to align mod 128 plus 64, which would be what
+ * linux gives us anyway, in theory, and then we could make
+ * LIPP_SMALL_PACKET_SIZE be 64, or maybe 196.
*/
const unsigned long align = 128;
@@ -375,11 +391,15 @@ static bool tile_net_provide_needed_buffer(struct tile_net_cpu *info,
struct sk_buff **skb_ptr;
- /* Note that "dev_alloc_skb()" adds NET_SKB_PAD more bytes, */
+ /* Note that "dev_alloc_skb()" adds NET_SKB_PAD (32) more bytes, */
/* and also "reserves" that many bytes. */
- /* ISSUE: Can we "share" the NET_SKB_PAD bytes with "skb_ptr"? */
+ /* ISSUE: We could "share" the NET_SKB_PAD bytes with "skb_ptr", */
+ /* but currently there is no real need, as NET_SKB_PAD is only 32. */
int len = sizeof(*skb_ptr) + align + buffer_size;
+ /* NOTE: Another 192 (256?) bytes may be used by "skb_shared_info" */
+ /* stored after the bytes. This explains many things. */
+
while (1) {
/* Allocate (or fail). */
@@ -399,13 +419,24 @@ static bool tile_net_provide_needed_buffer(struct tile_net_cpu *info,
if (small)
break;
- /* ISSUE: This has never been observed! */
/* Large buffers must not span a huge page. */
if (((((long)va & ~HPAGE_MASK) + 1535) & HPAGE_MASK) == 0)
break;
+ /* ISSUE: This appears to be "impossible". */
pr_err("Leaking unaligned linux buffer at %p.\n", va);
}
+#ifdef TILE_NET_PARANOIA
+#if CHIP_HAS_CBOX_HOME_MAP()
+ if (hash_default) {
+ HV_PTE pte = *virt_to_pte(current->mm, (unsigned long)va);
+ if (hv_pte_get_mode(pte) != HV_PTE_MODE_CACHE_HASH_L3)
+ panic("Non-HFH ingress buffer! VA=%p Mode=%d PTE=%llx",
+ va, hv_pte_get_mode(pte), hv_pte_val(pte));
+ }
+#endif
+#endif
+
/* Skip two bytes to satisfy LIPP assumptions. */
/* Note that this aligns IP on a 16 byte boundary. */
/* ISSUE: Do this when the packet arrives? */
@@ -422,16 +453,6 @@ static bool tile_net_provide_needed_buffer(struct tile_net_cpu *info,
/* Make sure "skb_ptr" has been flushed. */
__insn_mf();
-#ifdef TILE_NET_PARANOIA
-#if CHIP_HAS_CBOX_HOME_MAP()
- if (hash_default) {
- HV_PTE pte = *virt_to_pte(current->mm, (unsigned long)va);
- if (hv_pte_get_mode(pte) != HV_PTE_MODE_CACHE_HASH_L3)
- panic("Non-coherent ingress buffer!");
- }
-#endif
-#endif
-
/* Provide the new buffer. */
tile_net_provide_linux_buffer(info, va, small);
@@ -469,48 +490,64 @@ oops:
* Grab some LEPP completions, and store them in "comps", of size
* "comps_size", and return the number of completions which were
* stored, so the caller can free them.
- *
- * If "pending" is not NULL, it will be set to true if there might
- * still be some pending completions caused by this tile, else false.
*/
-static unsigned int tile_net_lepp_grab_comps(struct net_device *dev,
+static unsigned int tile_net_lepp_grab_comps(lepp_queue_t *eq,
struct sk_buff *comps[],
unsigned int comps_size,
- bool *pending)
+ unsigned int min_size)
{
- struct tile_net_priv *priv = netdev_priv(dev);
-
- lepp_queue_t *eq = priv->epp_queue;
-
unsigned int n = 0;
- unsigned int comp_head;
- unsigned int comp_busy;
- unsigned int comp_tail;
-
- spin_lock(&priv->comp_lock);
-
- comp_head = eq->comp_head;
- comp_busy = eq->comp_busy;
- comp_tail = eq->comp_tail;
+ unsigned int comp_head = eq->comp_head;
+ unsigned int comp_busy = eq->comp_busy;
while (comp_head != comp_busy && n < comps_size) {
comps[n++] = eq->comps[comp_head];
LEPP_QINC(comp_head);
}
- if (pending != NULL)
- *pending = (comp_head != comp_tail);
+ if (n < min_size)
+ return 0;
eq->comp_head = comp_head;
- spin_unlock(&priv->comp_lock);
-
return n;
}
/*
+ * Free some comps, and return true iff there are still some pending.
+ */
+static bool tile_net_lepp_free_comps(struct net_device *dev, bool all)
+{
+ struct tile_net_priv *priv = netdev_priv(dev);
+
+ lepp_queue_t *eq = priv->eq;
+
+ struct sk_buff *olds[64];
+ unsigned int wanted = 64;
+ unsigned int i, n;
+ bool pending;
+
+ spin_lock(&priv->eq_lock);
+
+ if (all)
+ eq->comp_busy = eq->comp_tail;
+
+ n = tile_net_lepp_grab_comps(eq, olds, wanted, 0);
+
+ pending = (eq->comp_head != eq->comp_tail);
+
+ spin_unlock(&priv->eq_lock);
+
+ for (i = 0; i < n; i++)
+ kfree_skb(olds[i]);
+
+ return pending;
+}
+
+
+/*
* Make sure the egress timer is scheduled.
*
* Note that we use "schedule if not scheduled" logic instead of the more
@@ -544,21 +581,11 @@ static void tile_net_handle_egress_timer(unsigned long arg)
struct tile_net_cpu *info = (struct tile_net_cpu *)arg;
struct net_device *dev = info->napi.dev;
- struct sk_buff *olds[32];
- unsigned int wanted = 32;
- unsigned int i, nolds = 0;
- bool pending;
-
/* The timer is no longer scheduled. */
info->egress_timer_scheduled = false;
- nolds = tile_net_lepp_grab_comps(dev, olds, wanted, &pending);
-
- for (i = 0; i < nolds; i++)
- kfree_skb(olds[i]);
-
- /* Reschedule timer if needed. */
- if (pending)
+ /* Free comps, and reschedule timer if more are pending. */
+ if (tile_net_lepp_free_comps(dev, false))
tile_net_schedule_egress_timer(info);
}
@@ -636,8 +663,39 @@ static bool is_dup_ack(char *s1, char *s2, unsigned int len)
+static void tile_net_discard_aux(struct tile_net_cpu *info, int index)
+{
+ struct tile_netio_queue *queue = &info->queue;
+ netio_queue_impl_t *qsp = queue->__system_part;
+ netio_queue_user_impl_t *qup = &queue->__user_part;
+
+ int index2_aux = index + sizeof(netio_pkt_t);
+ int index2 =
+ ((index2_aux ==
+ qsp->__packet_receive_queue.__last_packet_plus_one) ?
+ 0 : index2_aux);
+
+ netio_pkt_t *pkt = (netio_pkt_t *)((unsigned long) &qsp[1] + index);
+
+ /* Extract the "linux_buffer_t". */
+ unsigned int buffer = pkt->__packet.word;
+
+ /* Convert "linux_buffer_t" to "va". */
+ void *va = __va((phys_addr_t)(buffer >> 1) << 7);
+
+ /* Acquire the associated "skb". */
+ struct sk_buff **skb_ptr = va - sizeof(*skb_ptr);
+ struct sk_buff *skb = *skb_ptr;
+
+ kfree_skb(skb);
+
+ /* Consume this packet. */
+ qup->__packet_receive_read = index2;
+}
+
+
/*
- * Like "tile_net_handle_packets()", but just discard packets.
+ * Like "tile_net_poll()", but just discard packets.
*/
static void tile_net_discard_packets(struct net_device *dev)
{
@@ -650,32 +708,8 @@ static void tile_net_discard_packets(struct net_device *dev)
while (qup->__packet_receive_read !=
qsp->__packet_receive_queue.__packet_write) {
-
int index = qup->__packet_receive_read;
-
- int index2_aux = index + sizeof(netio_pkt_t);
- int index2 =
- ((index2_aux ==
- qsp->__packet_receive_queue.__last_packet_plus_one) ?
- 0 : index2_aux);
-
- netio_pkt_t *pkt = (netio_pkt_t *)
- ((unsigned long) &qsp[1] + index);
-
- /* Extract the "linux_buffer_t". */
- unsigned int buffer = pkt->__packet.word;
-
- /* Convert "linux_buffer_t" to "va". */
- void *va = __va((phys_addr_t)(buffer >> 1) << 7);
-
- /* Acquire the associated "skb". */
- struct sk_buff **skb_ptr = va - sizeof(*skb_ptr);
- struct sk_buff *skb = *skb_ptr;
-
- kfree_skb(skb);
-
- /* Consume this packet. */
- qup->__packet_receive_read = index2;
+ tile_net_discard_aux(info, index);
}
}
@@ -704,7 +738,8 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
netio_pkt_metadata_t *metadata = NETIO_PKT_METADATA(pkt);
- /* Extract the packet size. */
+ /* Extract the packet size. FIXME: Shouldn't the second line */
+ /* get subtracted? Mostly moot, since it should be "zero". */
unsigned long len =
(NETIO_PKT_CUSTOM_LENGTH(pkt) +
NET_IP_ALIGN - NETIO_PACKET_PADDING);
@@ -722,15 +757,6 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
/* Compare to "NETIO_PKT_CUSTOM_DATA(pkt)". */
unsigned char *buf = va + NET_IP_ALIGN;
-#ifdef IGNORE_DUP_ACKS
-
- static int other;
- static int final;
- static int keep;
- static int skip;
-
-#endif
-
/* Invalidate the packet buffer. */
if (!hash_default)
__inv_buffer(buf, len);
@@ -745,16 +771,8 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
#ifdef TILE_NET_VERIFY_INGRESS
if (!NETIO_PKT_L4_CSUM_CORRECT_M(metadata, pkt) &&
NETIO_PKT_L4_CSUM_CALCULATED_M(metadata, pkt)) {
- /*
- * FIXME: This complains about UDP packets
- * with a "zero" checksum (bug 6624).
- */
-#ifdef TILE_NET_PANIC_ON_BAD
- dump_packet(buf, len, "rx");
- panic("Bad L4 checksum.");
-#else
+ /* Bug 6624: Includes UDP packets with a "zero" checksum. */
pr_warning("Bad L4 checksum on %d byte packet.\n", len);
-#endif
}
if (!NETIO_PKT_L3_CSUM_CORRECT_M(metadata, pkt) &&
NETIO_PKT_L3_CSUM_CALCULATED_M(metadata, pkt)) {
@@ -769,24 +787,22 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
}
break;
case NETIO_PKT_STATUS_BAD:
-#ifdef TILE_NET_PANIC_ON_BAD
- dump_packet(buf, len, "rx");
- panic("Unexpected BAD packet.");
-#else
- pr_warning("Unexpected BAD %d byte packet.\n", len);
-#endif
+ pr_warning("Unexpected BAD %ld byte packet.\n", len);
}
#endif
filter = 0;
+ /* ISSUE: Filter TCP packets with "bad" checksums? */
+
if (!(dev->flags & IFF_UP)) {
/* Filter packets received before we're up. */
filter = 1;
+ } else if (NETIO_PKT_STATUS_M(metadata, pkt) == NETIO_PKT_STATUS_BAD) {
+ /* Filter "truncated" packets. */
+ filter = 1;
} else if (!(dev->flags & IFF_PROMISC)) {
- /*
- * FIXME: Implement HW multicast filter.
- */
+ /* FIXME: Implement HW multicast filter. */
if (is_unicast_ether_addr(buf)) {
/* Filter packets not for our address. */
const u8 *mine = dev->dev_addr;
@@ -794,65 +810,6 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
}
}
-#ifdef IGNORE_DUP_ACKS
-
- if (len != 66) {
- /* FIXME: Must check "is_tcp_ack(buf, len)" somehow. */
-
- other++;
-
- } else if (index2 ==
- qsp->__packet_receive_queue.__packet_write) {
-
- final++;
-
- } else {
-
- netio_pkt_t *pkt2 = (netio_pkt_t *)
- ((unsigned long) &qsp[1] + index2);
-
- netio_pkt_metadata_t *metadata2 =
- NETIO_PKT_METADATA(pkt2);
-
- /* Extract the packet size. */
- unsigned long len2 =
- (NETIO_PKT_CUSTOM_LENGTH(pkt2) +
- NET_IP_ALIGN - NETIO_PACKET_PADDING);
-
- if (len2 == 66 &&
- NETIO_PKT_FLOW_HASH_M(metadata, pkt) ==
- NETIO_PKT_FLOW_HASH_M(metadata2, pkt2)) {
-
- /* Extract the "linux_buffer_t". */
- unsigned int buffer2 = pkt2->__packet.word;
-
- /* Convert "linux_buffer_t" to "va". */
- void *va2 =
- __va((phys_addr_t)(buffer2 >> 1) << 7);
-
- /* Extract the packet data pointer. */
- /* Compare to "NETIO_PKT_CUSTOM_DATA(pkt)". */
- unsigned char *buf2 = va2 + NET_IP_ALIGN;
-
- /* Invalidate the packet buffer. */
- if (!hash_default)
- __inv_buffer(buf2, len2);
-
- if (is_dup_ack(buf, buf2, len)) {
- skip++;
- filter = 1;
- } else {
- keep++;
- }
- }
- }
-
- if (net_ratelimit())
- pr_info("Other %d Final %d Keep %d Skip %d.\n",
- other, final, keep, skip);
-
-#endif
-
if (filter) {
/* ISSUE: Update "drop" statistics? */
@@ -877,10 +834,7 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
/* NOTE: This call also sets "skb->dev = dev". */
skb->protocol = eth_type_trans(skb, dev);
- /* ISSUE: Discard corrupt packets? */
- /* ISSUE: Discard packets with bad checksums? */
-
- /* Avoid recomputing TCP/UDP checksums. */
+ /* Avoid recomputing "good" TCP/UDP checksums. */
if (NETIO_PKT_L4_CSUM_CORRECT_M(metadata, pkt))
skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -912,9 +866,14 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
/*
* Handle some packets for the given device on the current CPU.
*
- * ISSUE: The "rotting packet" race condition occurs if a packet
- * arrives after the queue appears to be empty, and before the
- * hypervisor interrupt is re-enabled.
+ * If "tile_net_stop()" is called on some other tile while this
+ * function is running, we will return, hopefully before that
+ * other tile asks us to call "napi_disable()".
+ *
+ * The "rotting packet" race condition occurs if a packet arrives
+ * during the extremely narrow window between the queue appearing to
+ * be empty, and the ingress interrupt being re-enabled. This happens
+ * a LOT under heavy network load.
*/
static int tile_net_poll(struct napi_struct *napi, int budget)
{
@@ -928,7 +887,7 @@ static int tile_net_poll(struct napi_struct *napi, int budget)
unsigned int work = 0;
- while (1) {
+ while (priv->active) {
int index = qup->__packet_receive_read;
if (index == qsp->__packet_receive_queue.__packet_write)
break;
@@ -941,19 +900,24 @@ static int tile_net_poll(struct napi_struct *napi, int budget)
napi_complete(&info->napi);
- /* Re-enable hypervisor interrupts. */
+ if (!priv->active)
+ goto done;
+
+ /* Re-enable the ingress interrupt. */
enable_percpu_irq(priv->intr_id);
- /* HACK: Avoid the "rotting packet" problem. */
+ /* HACK: Avoid the "rotting packet" problem (see above). */
if (qup->__packet_receive_read !=
- qsp->__packet_receive_queue.__packet_write)
- napi_schedule(&info->napi);
-
- /* ISSUE: Handle completions? */
+ qsp->__packet_receive_queue.__packet_write) {
+ /* ISSUE: Sometimes this returns zero, presumably */
+ /* because an interrupt was handled for this tile. */
+ (void)napi_reschedule(&info->napi);
+ }
done:
- tile_net_provide_needed_buffers(info);
+ if (priv->active)
+ tile_net_provide_needed_buffers(info);
return work;
}
@@ -961,6 +925,12 @@ done:
/*
* Handle an ingress interrupt for the given device on the current cpu.
+ *
+ * ISSUE: Sometimes this gets called after "disable_percpu_irq()" has
+ * been called! This is probably due to "pending hypervisor downcalls".
+ *
+ * ISSUE: Is there any race condition between the "napi_schedule()" here
+ * and the "napi_complete()" call above?
*/
static irqreturn_t tile_net_handle_ingress_interrupt(int irq, void *dev_ptr)
{
@@ -969,9 +939,15 @@ static irqreturn_t tile_net_handle_ingress_interrupt(int irq, void *dev_ptr)
int my_cpu = smp_processor_id();
struct tile_net_cpu *info = priv->cpu[my_cpu];
- /* Disable hypervisor interrupt. */
+ /* Disable the ingress interrupt. */
disable_percpu_irq(priv->intr_id);
+ /* Ignore unwanted interrupts. */
+ if (!priv->active)
+ return IRQ_HANDLED;
+
+ /* ISSUE: Sometimes "info->napi_enabled" is false here. */
+
napi_schedule(&info->napi);
return IRQ_HANDLED;
@@ -1005,8 +981,7 @@ static int tile_net_open_aux(struct net_device *dev)
*/
{
int epp_home = hv_lotar_to_cpu(epp_lotar);
- struct page *page = virt_to_page(priv->epp_queue);
- homecache_change_page_home(page, 0, epp_home);
+ homecache_change_page_home(priv->eq_pages, EQ_ORDER, epp_home);
}
/*
@@ -1015,9 +990,9 @@ static int tile_net_open_aux(struct net_device *dev)
{
netio_ipp_address_t ea = {
.va = 0,
- .pa = __pa(priv->epp_queue),
+ .pa = __pa(priv->eq),
.pte = hv_pte(0),
- .size = PAGE_SIZE,
+ .size = EQ_SIZE,
};
ea.pte = hv_pte_set_lotar(ea.pte, epp_lotar);
ea.pte = hv_pte_set_mode(ea.pte, HV_PTE_MODE_CACHE_TILE_L3);
@@ -1043,7 +1018,7 @@ static int tile_net_open_aux(struct net_device *dev)
/*
- * Register with hypervisor on each CPU.
+ * Register with hypervisor on the current CPU.
*
* Strangely, this function does important things even if it "fails",
* which is especially common if the link is not up yet. Hopefully
@@ -1092,7 +1067,8 @@ static void tile_net_register(void *dev_ptr)
priv->cpu[my_cpu] = info;
/*
- * Register ourselves with the IPP.
+ * Register ourselves with LIPP. This does a lot of stuff,
+ * including invoking the LIPP registration code.
*/
ret = hv_dev_pwrite(priv->hv_devhdl, 0,
(HV_VirtAddr)&config,
@@ -1101,8 +1077,11 @@ static void tile_net_register(void *dev_ptr)
PDEBUG("hv_dev_pwrite(NETIO_IPP_INPUT_REGISTER_OFF) returned %d\n",
ret);
if (ret < 0) {
- printk(KERN_DEBUG "hv_dev_pwrite NETIO_IPP_INPUT_REGISTER_OFF"
- " failure %d\n", ret);
+ if (ret != NETIO_LINK_DOWN) {
+ printk(KERN_DEBUG "hv_dev_pwrite "
+ "NETIO_IPP_INPUT_REGISTER_OFF failure %d\n",
+ ret);
+ }
info->link_down = (ret == NETIO_LINK_DOWN);
return;
}
@@ -1145,15 +1124,47 @@ static void tile_net_register(void *dev_ptr)
NETIO_IPP_GET_FASTIO_OFF);
PDEBUG("hv_dev_pread(NETIO_IPP_GET_FASTIO_OFF) returned %d\n", ret);
- netif_napi_add(dev, &info->napi, tile_net_poll, 64);
-
/* Now we are registered. */
info->registered = true;
}
/*
- * Unregister with hypervisor on each CPU.
+ * Deregister with hypervisor on the current CPU.
+ *
+ * This simply discards all our credits, so no more packets will be
+ * delivered to this tile. There may still be packets in our queue.
+ *
+ * Also, disable the ingress interrupt.
+ */
+static void tile_net_deregister(void *dev_ptr)
+{
+ struct net_device *dev = (struct net_device *)dev_ptr;
+ struct tile_net_priv *priv = netdev_priv(dev);
+ int my_cpu = smp_processor_id();
+ struct tile_net_cpu *info = priv->cpu[my_cpu];
+
+ /* Disable the ingress interrupt. */
+ disable_percpu_irq(priv->intr_id);
+
+ /* Do nothing else if not registered. */
+ if (info == NULL || !info->registered)
+ return;
+
+ {
+ struct tile_netio_queue *queue = &info->queue;
+ netio_queue_user_impl_t *qup = &queue->__user_part;
+
+ /* Discard all our credits. */
+ __netio_fastio_return_credits(qup->__fastio_index, -1);
+ }
+}
+
+
+/*
+ * Unregister with hypervisor on the current CPU.
+ *
+ * Also, disable the ingress interrupt.
*/
static void tile_net_unregister(void *dev_ptr)
{
@@ -1162,35 +1173,23 @@ static void tile_net_unregister(void *dev_ptr)
int my_cpu = smp_processor_id();
struct tile_net_cpu *info = priv->cpu[my_cpu];
- int ret = 0;
+ int ret;
int dummy = 0;
- /* Do nothing if never registered. */
- if (info == NULL)
- return;
+ /* Disable the ingress interrupt. */
+ disable_percpu_irq(priv->intr_id);
- /* Do nothing if already unregistered. */
- if (!info->registered)
+ /* Do nothing else if not registered. */
+ if (info == NULL || !info->registered)
return;
- /*
- * Unregister ourselves with LIPP.
- */
+ /* Unregister ourselves with LIPP/LEPP. */
ret = hv_dev_pwrite(priv->hv_devhdl, 0, (HV_VirtAddr)&dummy,
sizeof(dummy), NETIO_IPP_INPUT_UNREGISTER_OFF);
- PDEBUG("hv_dev_pwrite(NETIO_IPP_INPUT_UNREGISTER_OFF) returned %d\n",
- ret);
- if (ret < 0) {
- /* FIXME: Just panic? */
- pr_err("hv_dev_pwrite NETIO_IPP_INPUT_UNREGISTER_OFF"
- " failure %d\n", ret);
- }
+ if (ret < 0)
+ panic("Failed to unregister with LIPP/LEPP!\n");
- /*
- * Discard all packets still in our NetIO queue. Hopefully,
- * once the unregister call is complete, there will be no
- * packets still in flight on the IDN.
- */
+ /* Discard all packets still in our NetIO queue. */
tile_net_discard_packets(dev);
/* Reset state. */
@@ -1200,11 +1199,6 @@ static void tile_net_unregister(void *dev_ptr)
/* Cancel egress timer. */
del_timer(&info->egress_timer);
info->egress_timer_scheduled = false;
-
- netif_napi_del(&info->napi);
-
- /* Now we are unregistered. */
- info->registered = false;
}
@@ -1212,18 +1206,28 @@ static void tile_net_unregister(void *dev_ptr)
* Helper function for "tile_net_stop()".
*
* Also used to handle registration failure in "tile_net_open_inner()",
- * when "fully_opened" is known to be false, and the various extra
- * steps in "tile_net_stop()" are not necessary. ISSUE: It might be
- * simpler if we could just call "tile_net_stop()" anyway.
+ * when the various extra steps in "tile_net_stop()" are not necessary.
*/
static void tile_net_stop_aux(struct net_device *dev)
{
struct tile_net_priv *priv = netdev_priv(dev);
+ int i;
int dummy = 0;
- /* Unregister all tiles, so LIPP will stop delivering packets. */
+ /*
+ * Unregister all tiles, so LIPP will stop delivering packets.
+ * Also, delete all the "napi" objects (sequentially, to protect
+ * "dev->napi_list").
+ */
on_each_cpu(tile_net_unregister, (void *)dev, 1);
+ for_each_online_cpu(i) {
+ struct tile_net_cpu *info = priv->cpu[i];
+ if (info != NULL && info->registered) {
+ netif_napi_del(&info->napi);
+ info->registered = false;
+ }
+ }
/* Stop LIPP/LEPP. */
if (hv_dev_pwrite(priv->hv_devhdl, 0, (HV_VirtAddr)&dummy,
@@ -1235,18 +1239,15 @@ static void tile_net_stop_aux(struct net_device *dev)
/*
- * Disable ingress interrupts for the given device on the current cpu.
+ * Disable NAPI for the given device on the current cpu.
*/
-static void tile_net_disable_intr(void *dev_ptr)
+static void tile_net_stop_disable(void *dev_ptr)
{
struct net_device *dev = (struct net_device *)dev_ptr;
struct tile_net_priv *priv = netdev_priv(dev);
int my_cpu = smp_processor_id();
struct tile_net_cpu *info = priv->cpu[my_cpu];
- /* Disable hypervisor interrupt. */
- disable_percpu_irq(priv->intr_id);
-
/* Disable NAPI if needed. */
if (info != NULL && info->napi_enabled) {
napi_disable(&info->napi);
@@ -1256,21 +1257,24 @@ static void tile_net_disable_intr(void *dev_ptr)
/*
- * Enable ingress interrupts for the given device on the current cpu.
+ * Enable NAPI and the ingress interrupt for the given device
+ * on the current cpu.
+ *
+ * ISSUE: Only do this for "network cpus"?
*/
-static void tile_net_enable_intr(void *dev_ptr)
+static void tile_net_open_enable(void *dev_ptr)
{
struct net_device *dev = (struct net_device *)dev_ptr;
struct tile_net_priv *priv = netdev_priv(dev);
int my_cpu = smp_processor_id();
struct tile_net_cpu *info = priv->cpu[my_cpu];
- /* Enable hypervisor interrupt. */
- enable_percpu_irq(priv->intr_id);
-
/* Enable NAPI. */
napi_enable(&info->napi);
info->napi_enabled = true;
+
+ /* Enable the ingress interrupt. */
+ enable_percpu_irq(priv->intr_id);
}
@@ -1288,8 +1292,9 @@ static int tile_net_open_inner(struct net_device *dev)
int my_cpu = smp_processor_id();
struct tile_net_cpu *info;
struct tile_netio_queue *queue;
- unsigned int irq;
+ int result = 0;
int i;
+ int dummy = 0;
/*
* First try to register just on the local CPU, and handle any
@@ -1307,42 +1312,52 @@ static int tile_net_open_inner(struct net_device *dev)
/*
* Now register everywhere else. If any registration fails,
* even for "link down" (which might not be possible), we
- * clean up using "tile_net_stop_aux()".
+ * clean up using "tile_net_stop_aux()". Also, add all the
+ * "napi" objects (sequentially, to protect "dev->napi_list").
+ * ISSUE: Only use "netif_napi_add()" for "network cpus"?
*/
smp_call_function(tile_net_register, (void *)dev, 1);
for_each_online_cpu(i) {
- if (!priv->cpu[i]->registered) {
- tile_net_stop_aux(dev);
- return -EAGAIN;
- }
+ struct tile_net_cpu *info = priv->cpu[i];
+ if (info->registered)
+ netif_napi_add(dev, &info->napi, tile_net_poll, 64);
+ else
+ result = -EAGAIN;
+ }
+ if (result != 0) {
+ tile_net_stop_aux(dev);
+ return result;
}
queue = &info->queue;
- /*
- * Set the device intr bit mask.
- * The tile_net_register above sets per tile __intr_id.
- */
- priv->intr_id = queue->__system_part->__intr_id;
- BUG_ON(!priv->intr_id);
-
- /*
- * Register the device interrupt handler.
- * The __ffs() function returns the index into the interrupt handler
- * table from the interrupt bit mask which should have one bit
- * and one bit only set.
- */
- irq = __ffs(priv->intr_id);
- tile_irq_activate(irq, TILE_IRQ_PERCPU);
- BUG_ON(request_irq(irq, tile_net_handle_ingress_interrupt,
- 0, dev->name, (void *)dev) != 0);
-
- /* ISSUE: How could "priv->fully_opened" ever be "true" here? */
+ if (priv->intr_id == 0) {
+ unsigned int irq;
- if (!priv->fully_opened) {
+ /*
+ * Acquire the irq allocated by the hypervisor. Every
+ * queue gets the same irq. The "__intr_id" field is
+ * "1 << irq", so we use "__ffs()" to extract "irq".
+ */
+ priv->intr_id = queue->__system_part->__intr_id;
+ BUG_ON(priv->intr_id == 0);
+ irq = __ffs(priv->intr_id);
- int dummy = 0;
+ /*
+ * Register the ingress interrupt handler for this
+ * device, permanently.
+ *
+ * We used to call "free_irq()" in "tile_net_stop()",
+ * and then re-register the handler here every time,
+ * but that caused DNP errors in "handle_IRQ_event()"
+ * because "desc->action" was NULL. See bug 9143.
+ */
+ tile_irq_activate(irq, TILE_IRQ_PERCPU);
+ BUG_ON(request_irq(irq, tile_net_handle_ingress_interrupt,
+ 0, dev->name, (void *)dev) != 0);
+ }
+ {
/* Allocate initial buffers. */
int max_buffers =
@@ -1359,18 +1374,21 @@ static int tile_net_open_inner(struct net_device *dev)
if (info->num_needed_small_buffers != 0 ||
info->num_needed_large_buffers != 0)
panic("Insufficient memory for buffer stack!");
+ }
- /* Start LIPP/LEPP and activate "ingress" at the shim. */
- if (hv_dev_pwrite(priv->hv_devhdl, 0, (HV_VirtAddr)&dummy,
- sizeof(dummy), NETIO_IPP_INPUT_INIT_OFF) < 0)
- panic("Failed to activate the LIPP Shim!\n");
+ /* We are about to be active. */
+ priv->active = true;
- priv->fully_opened = 1;
- }
+ /* Make sure "active" is visible to all tiles. */
+ mb();
- /* On each tile, enable the hypervisor to trigger interrupts. */
- /* ISSUE: Do this before starting LIPP/LEPP? */
- on_each_cpu(tile_net_enable_intr, (void *)dev, 1);
+ /* On each tile, enable NAPI and the ingress interrupt. */
+ on_each_cpu(tile_net_open_enable, (void *)dev, 1);
+
+ /* Start LIPP/LEPP and activate "ingress" at the shim. */
+ if (hv_dev_pwrite(priv->hv_devhdl, 0, (HV_VirtAddr)&dummy,
+ sizeof(dummy), NETIO_IPP_INPUT_INIT_OFF) < 0)
+ panic("Failed to activate the LIPP Shim!\n");
/* Start our transmit queue. */
netif_start_queue(dev);
@@ -1396,9 +1414,9 @@ static void tile_net_open_retry(struct work_struct *w)
* ourselves to try again later; otherwise, tell Linux we now have
* a working link. ISSUE: What if the return value is negative?
*/
- if (tile_net_open_inner(priv->dev))
- schedule_delayed_work_on(singlethread_cpu, &priv->retry_work,
- TILE_NET_RETRY_INTERVAL);
+ if (tile_net_open_inner(priv->dev) != 0)
+ schedule_delayed_work(&priv->retry_work,
+ TILE_NET_RETRY_INTERVAL);
else
netif_carrier_on(priv->dev);
}
@@ -1412,8 +1430,8 @@ static void tile_net_open_retry(struct work_struct *w)
* The open entry point is called when a network interface is made
* active by the system (IFF_UP). At this point all resources needed
* for transmit and receive operations are allocated, the interrupt
- * handler is registered with the OS, the watchdog timer is started,
- * and the stack is notified that the interface is ready.
+ * handler is registered with the OS (if needed), the watchdog timer
+ * is started, and the stack is notified that the interface is ready.
*
* If the actual link is not available yet, then we tell Linux that
* we have no carrier, and we keep checking until the link comes up.
@@ -1468,6 +1486,10 @@ static int tile_net_open(struct net_device *dev)
#endif
priv->partly_opened = 1;
+
+ } else {
+ /* FIXME: Is this possible? */
+ /* printk("Already partly opened.\n"); */
}
/*
@@ -1487,57 +1509,17 @@ static int tile_net_open(struct net_device *dev)
* and then remember to try again later.
*/
netif_carrier_off(dev);
- schedule_delayed_work_on(singlethread_cpu, &priv->retry_work,
- TILE_NET_RETRY_INTERVAL);
+ schedule_delayed_work(&priv->retry_work, TILE_NET_RETRY_INTERVAL);
return 0;
}
-/*
- * Disables a network interface.
- *
- * Returns 0, this is not allowed to fail.
- *
- * The close entry point is called when an interface is de-activated
- * by the OS. The hardware is still under the drivers control, but
- * needs to be disabled. A global MAC reset is issued to stop the
- * hardware, and all transmit and receive resources are freed.
- *
- * ISSUE: Can this can be called while "tile_net_poll()" is running?
- */
-static int tile_net_stop(struct net_device *dev)
+static int tile_net_drain_lipp_buffers(struct tile_net_priv *priv)
{
- struct tile_net_priv *priv = netdev_priv(dev);
-
- bool pending = true;
-
- PDEBUG("tile_net_stop()\n");
-
- /* ISSUE: Only needed if not yet fully open. */
- cancel_delayed_work_sync(&priv->retry_work);
-
- /* Can't transmit any more. */
- netif_stop_queue(dev);
-
- /*
- * Disable hypervisor interrupts on each tile.
- */
- on_each_cpu(tile_net_disable_intr, (void *)dev, 1);
-
- /*
- * Unregister the interrupt handler.
- * The __ffs() function returns the index into the interrupt handler
- * table from the interrupt bit mask which should have one bit
- * and one bit only set.
- */
- if (priv->intr_id)
- free_irq(__ffs(priv->intr_id), dev);
-
- /*
- * Drain all the LIPP buffers.
- */
+ int n = 0;
+ /* Drain all the LIPP buffers. */
while (true) {
int buffer;
@@ -1560,43 +1542,105 @@ static int tile_net_stop(struct net_device *dev)
kfree_skb(skb);
}
+
+ n++;
}
- /* Stop LIPP/LEPP. */
- tile_net_stop_aux(dev);
+ return n;
+}
+
+
+/*
+ * Disables a network interface.
+ *
+ * Returns 0, this is not allowed to fail.
+ *
+ * The close entry point is called when an interface is de-activated
+ * by the OS. The hardware is still under the drivers control, but
+ * needs to be disabled. A global MAC reset is issued to stop the
+ * hardware, and all transmit and receive resources are freed.
+ *
+ * ISSUE: How closely does "netif_running(dev)" mirror "priv->active"?
+ *
+ * Before we are called by "__dev_close()", "netif_running()" will
+ * have been cleared, so no NEW calls to "tile_net_poll()" will be
+ * made by "netpoll_poll_dev()".
+ *
+ * Often, this can cause some tiles to still have packets in their
+ * queues, so we must call "tile_net_discard_packets()" later.
+ *
+ * Note that some other tile may still be INSIDE "tile_net_poll()",
+ * and in fact, many will be, if there is heavy network load.
+ *
+ * Calling "on_each_cpu(tile_net_stop_disable, (void *)dev, 1)" when
+ * any tile is still "napi_schedule()"'d will induce a horrible crash
+ * when "msleep()" is called. This includes tiles which are inside
+ * "tile_net_poll()" which have not yet called "napi_complete()".
+ *
+ * So, we must first try to wait long enough for other tiles to finish
+ * with any current "tile_net_poll()" call, and, hopefully, to clear
+ * the "scheduled" flag. ISSUE: It is unclear what happens to tiles
+ * which have called "napi_schedule()" but which had not yet tried to
+ * call "tile_net_poll()", or which exhausted their budget inside
+ * "tile_net_poll()" just before this function was called.
+ */
+static int tile_net_stop(struct net_device *dev)
+{
+ struct tile_net_priv *priv = netdev_priv(dev);
+ PDEBUG("tile_net_stop()\n");
- priv->fully_opened = 0;
+ /* Start discarding packets. */
+ priv->active = false;
+ /* Make sure "active" is visible to all tiles. */
+ mb();
/*
- * XXX: ISSUE: It appears that, in practice anyway, by the
- * time we get here, there are no pending completions.
+ * On each tile, make sure no NEW packets get delivered, and
+ * disable the ingress interrupt.
+ *
+ * Note that the ingress interrupt can fire AFTER this,
+ * presumably due to packets which were recently delivered,
+ * but it will have no effect.
*/
- while (pending) {
+ on_each_cpu(tile_net_deregister, (void *)dev, 1);
- struct sk_buff *olds[32];
- unsigned int wanted = 32;
- unsigned int i, nolds = 0;
+ /* Optimistically drain LIPP buffers. */
+ (void)tile_net_drain_lipp_buffers(priv);
- nolds = tile_net_lepp_grab_comps(dev, olds,
- wanted, &pending);
+ /* ISSUE: Only needed if not yet fully open. */
+ cancel_delayed_work_sync(&priv->retry_work);
- /* ISSUE: We have never actually seen this debug spew. */
- if (nolds != 0)
- pr_info("During tile_net_stop(), grabbed %d comps.\n",
- nolds);
+ /* Can't transmit any more. */
+ netif_stop_queue(dev);
- for (i = 0; i < nolds; i++)
- kfree_skb(olds[i]);
- }
+ /* Disable NAPI on each tile. */
+ on_each_cpu(tile_net_stop_disable, (void *)dev, 1);
+ /*
+ * Drain any remaining LIPP buffers. NOTE: This "printk()"
+ * has never been observed, but in theory it could happen.
+ */
+ if (tile_net_drain_lipp_buffers(priv) != 0)
+ printk("Had to drain some extra LIPP buffers!\n");
+
+ /* Stop LIPP/LEPP. */
+ tile_net_stop_aux(dev);
+
+ /*
+ * ISSUE: It appears that, in practice anyway, by the time we
+ * get here, there are no pending completions, but just in case,
+ * we free (all of) them anyway.
+ */
+ while (tile_net_lepp_free_comps(dev, true))
+ /* loop */;
/* Wipe the EPP queue. */
- memset(priv->epp_queue, 0, sizeof(lepp_queue_t));
+ memset(priv->eq, 0, sizeof(lepp_queue_t));
/* Evict the EPP queue. */
- finv_buffer(priv->epp_queue, PAGE_SIZE);
+ finv_buffer(priv->eq, EQ_SIZE);
return 0;
}
@@ -1742,17 +1786,15 @@ static int tile_net_tx_tso(struct sk_buff *skb, struct net_device *dev)
unsigned long irqflags;
- lepp_queue_t *eq = priv->epp_queue;
+ lepp_queue_t *eq = priv->eq;
- struct sk_buff *olds[4];
- unsigned int wanted = 4;
+ struct sk_buff *olds[8];
+ unsigned int wanted = 8;
unsigned int i, nolds = 0;
unsigned int cmd_head, cmd_tail, cmd_next;
unsigned int comp_tail;
- unsigned int free_slots;
-
/* Paranoia. */
BUG_ON(skb->protocol != htons(ETH_P_IP));
@@ -1780,34 +1822,32 @@ static int tile_net_tx_tso(struct sk_buff *skb, struct net_device *dev)
/* Enqueue the command. */
- spin_lock_irqsave(&priv->cmd_lock, irqflags);
+ spin_lock_irqsave(&priv->eq_lock, irqflags);
/*
* Handle completions if needed to make room.
* HACK: Spin until there is sufficient room.
*/
- free_slots = lepp_num_free_comp_slots(eq);
- if (free_slots < 1) {
-spin:
- nolds += tile_net_lepp_grab_comps(dev, olds + nolds,
- wanted - nolds, NULL);
- if (lepp_num_free_comp_slots(eq) < 1)
- goto spin;
+ if (lepp_num_free_comp_slots(eq) == 0) {
+ nolds = tile_net_lepp_grab_comps(eq, olds, wanted, 0);
+ if (nolds == 0) {
+busy:
+ spin_unlock_irqrestore(&priv->eq_lock, irqflags);
+ return NETDEV_TX_BUSY;
+ }
}
cmd_head = eq->cmd_head;
cmd_tail = eq->cmd_tail;
- /* NOTE: The "gotos" below are untested. */
-
/* Prepare to advance, detecting full queue. */
cmd_next = cmd_tail + cmd_size;
if (cmd_tail < cmd_head && cmd_next >= cmd_head)
- goto spin;
+ goto busy;
if (cmd_next > LEPP_CMD_LIMIT) {
cmd_next = 0;
if (cmd_next == cmd_head)
- goto spin;
+ goto busy;
}
/* Copy the command. */
@@ -1823,14 +1863,18 @@ spin:
eq->comp_tail = comp_tail;
/* Flush before allowing LEPP to handle the command. */
+ /* ISSUE: Is this the optimal location for the flush? */
__insn_mf();
eq->cmd_tail = cmd_tail;
- spin_unlock_irqrestore(&priv->cmd_lock, irqflags);
-
+ /* NOTE: Using "4" here is more efficient than "0" or "2", */
+ /* and, strangely, more efficient than pre-checking the number */
+ /* of available completions, and comparing it to 4. */
if (nolds == 0)
- nolds = tile_net_lepp_grab_comps(dev, olds, wanted, NULL);
+ nolds = tile_net_lepp_grab_comps(eq, olds, wanted, 4);
+
+ spin_unlock_irqrestore(&priv->eq_lock, irqflags);
/* Handle completions. */
for (i = 0; i < nolds; i++)
@@ -1870,10 +1914,10 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
unsigned int num_frags;
- lepp_queue_t *eq = priv->epp_queue;
+ lepp_queue_t *eq = priv->eq;
- struct sk_buff *olds[4];
- unsigned int wanted = 4;
+ struct sk_buff *olds[8];
+ unsigned int wanted = 8;
unsigned int i, nolds = 0;
unsigned int cmd_size = sizeof(lepp_cmd_t);
@@ -1883,8 +1927,6 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
lepp_cmd_t cmds[LEPP_MAX_FRAGS];
- unsigned int free_slots;
-
/*
* This is paranoia, since we think that if the link doesn't come
@@ -1905,7 +1947,8 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
if (hash_default) {
HV_PTE pte = *virt_to_pte(current->mm, (unsigned long)data);
if (hv_pte_get_mode(pte) != HV_PTE_MODE_CACHE_HASH_L3)
- panic("Non-coherent egress buffer!");
+ panic("Non-HFH egress buffer! VA=%p Mode=%d PTE=%llx",
+ data, hv_pte_get_mode(pte), hv_pte_val(pte));
}
#endif
#endif
@@ -1958,37 +2001,35 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
/* Enqueue the commands. */
- spin_lock_irqsave(&priv->cmd_lock, irqflags);
+ spin_lock_irqsave(&priv->eq_lock, irqflags);
/*
* Handle completions if needed to make room.
* HACK: Spin until there is sufficient room.
*/
- free_slots = lepp_num_free_comp_slots(eq);
- if (free_slots < 1) {
-spin:
- nolds += tile_net_lepp_grab_comps(dev, olds + nolds,
- wanted - nolds, NULL);
- if (lepp_num_free_comp_slots(eq) < 1)
- goto spin;
+ if (lepp_num_free_comp_slots(eq) == 0) {
+ nolds = tile_net_lepp_grab_comps(eq, olds, wanted, 0);
+ if (nolds == 0) {
+busy:
+ spin_unlock_irqrestore(&priv->eq_lock, irqflags);
+ return NETDEV_TX_BUSY;
+ }
}
cmd_head = eq->cmd_head;
cmd_tail = eq->cmd_tail;
- /* NOTE: The "gotos" below are untested. */
-
/* Copy the commands, or fail. */
for (i = 0; i < num_frags; i++) {
/* Prepare to advance, detecting full queue. */
cmd_next = cmd_tail + cmd_size;
if (cmd_tail < cmd_head && cmd_next >= cmd_head)
- goto spin;
+ goto busy;
if (cmd_next > LEPP_CMD_LIMIT) {
cmd_next = 0;
if (cmd_next == cmd_head)
- goto spin;
+ goto busy;
}
/* Copy the command. */
@@ -2005,14 +2046,18 @@ spin:
eq->comp_tail = comp_tail;
/* Flush before allowing LEPP to handle the command. */
+ /* ISSUE: Is this the optimal location for the flush? */
__insn_mf();
eq->cmd_tail = cmd_tail;
- spin_unlock_irqrestore(&priv->cmd_lock, irqflags);
-
+ /* NOTE: Using "4" here is more efficient than "0" or "2", */
+ /* and, strangely, more efficient than pre-checking the number */
+ /* of available completions, and comparing it to 4. */
if (nolds == 0)
- nolds = tile_net_lepp_grab_comps(dev, olds, wanted, NULL);
+ nolds = tile_net_lepp_grab_comps(eq, olds, wanted, 4);
+
+ spin_unlock_irqrestore(&priv->eq_lock, irqflags);
/* Handle completions. */
for (i = 0; i < nolds; i++)
@@ -2261,7 +2306,6 @@ static struct net_device *tile_net_dev_init(const char *name)
int ret;
struct net_device *dev;
struct tile_net_priv *priv;
- struct page *page;
/*
* Allocate the device structure. This allocates "priv", calls
@@ -2285,23 +2329,21 @@ static struct net_device *tile_net_dev_init(const char *name)
INIT_DELAYED_WORK(&priv->retry_work, tile_net_open_retry);
- spin_lock_init(&priv->cmd_lock);
- spin_lock_init(&priv->comp_lock);
+ spin_lock_init(&priv->eq_lock);
- /* Allocate "epp_queue". */
- BUG_ON(get_order(sizeof(lepp_queue_t)) != 0);
- page = alloc_pages(GFP_KERNEL | __GFP_ZERO, 0);
- if (!page) {
+ /* Allocate "eq". */
+ priv->eq_pages = alloc_pages(GFP_KERNEL | __GFP_ZERO, EQ_ORDER);
+ if (!priv->eq_pages) {
free_netdev(dev);
return NULL;
}
- priv->epp_queue = page_address(page);
+ priv->eq = page_address(priv->eq_pages);
/* Register the network device. */
ret = register_netdev(dev);
if (ret) {
pr_err("register_netdev %s failed %d\n", dev->name, ret);
- free_page((unsigned long)priv->epp_queue);
+ __free_pages(priv->eq_pages, EQ_ORDER);
free_netdev(dev);
return NULL;
}
@@ -2310,7 +2352,7 @@ static struct net_device *tile_net_dev_init(const char *name)
ret = tile_net_get_mac(dev);
if (ret < 0) {
unregister_netdev(dev);
- free_page((unsigned long)priv->epp_queue);
+ __free_pages(priv->eq_pages, EQ_ORDER);
free_netdev(dev);
return NULL;
}
@@ -2321,6 +2363,9 @@ static struct net_device *tile_net_dev_init(const char *name)
/*
* Module cleanup.
+ *
+ * FIXME: If compiled as a module, this module cannot be "unloaded",
+ * because the "ingress interrupt handler" is registered permanently.
*/
static void tile_net_cleanup(void)
{
@@ -2331,8 +2376,8 @@ static void tile_net_cleanup(void)
struct net_device *dev = tile_net_devs[i];
struct tile_net_priv *priv = netdev_priv(dev);
unregister_netdev(dev);
- finv_buffer(priv->epp_queue, PAGE_SIZE);
- free_page((unsigned long)priv->epp_queue);
+ finv_buffer(priv->eq, EQ_SIZE);
+ __free_pages(priv->eq_pages, EQ_ORDER);
free_netdev(dev);
}
}
@@ -2355,7 +2400,12 @@ static int tile_net_init_module(void)
}
+module_init(tile_net_init_module);
+module_exit(tile_net_cleanup);
+
+
#ifndef MODULE
+
/*
* The "network_cpus" boot argument specifies the cpus that are dedicated
* to handle ingress packets.
@@ -2391,8 +2441,5 @@ static int __init network_cpus_setup(char *str)
return 0;
}
__setup("network_cpus=", network_cpus_setup);
-#endif
-
-module_init(tile_net_init_module);
-module_exit(tile_net_cleanup);
+#endif
--
1.6.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