* Libertas related kernel crash
From: Daniel Mack @ 2009-11-05 12:05 UTC (permalink / raw)
To: netdev, libertas-dev; +Cc: linux-kernel, Michael Hirsch
On an ARM (PXA300) embdedded platform with a libertas chip connected via
SDIO, we happen to see the kernel Ooops below once in a while.
Any pointer on where to dig?
Daniel
[ 2659.715112] Unable to handle kernel NULL pointer dereference at virtual address 00000001
[ 2659.723164] pgd = c5ca8000
[ 2659.725846] [00000001] *pgd=a5cbc031, *pte=00000000, *ppte=00000000
[ 2659.732062] Internal error: Oops: 13 [#4]
[ 2659.736041] last sysfs file: /sys/devices/platform/pxa2xx-mci.0/mmc_host/mmc0/mmc0:0001/mmc0:0001:1/net/wlan0/address
[ 2659.746573] Modules linked in: eeti_ts libertas_sdio pxamci ds2760_battery w1_ds2760 wire
[ 2659.754698] CPU: 0 Tainted: G D (2.6.32-rc6 #1)
[ 2659.760255] PC is at kmem_cache_alloc+0x30/0x90
[ 2659.764774] LR is at inet_bind_bucket_create+0x18/0x5c
[ 2659.769876] pc : [<c00a28d8>] lr : [<c02a4b24>] psr: 20000093
[ 2659.769887] sp : c5c27d88 ip : c78ae4a0 fp : 00006e49
[ 2659.781279] r10: 00008928 r9 : c04fcfac r8 : c0532148
[ 2659.786466] r7 : 00000020 r6 : 00000020 r5 : 60000013 r4 : 00000001
[ 2659.792945] r3 : 00000000 r2 : c78ae4a0 r1 : 00000020 r0 : c04d42e4
[ 2659.799427] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user
[ 2659.806602] Control: 0000397f Table: a5ca8018 DAC: 00000015
[ 2659.812304] Process p0-renderer (pid: 1527, stack limit = 0xc5c26278)
[ 2659.818697] Stack: (0xc5c27d88 to 0xc5c28000)
[ 2659.823029] 7d80: c5cc5300 c5cb1000 c5c18800 c78ae4a0 00008928 00008928
[ 2659.831155] 7da0: 00000001 c02a4b24 c2388c2e 00000000 c7eb6200 c02a4c8c c02a4200 c2388c2d
[ 2659.839287] 7dc0: c04fcfac 00000000 0000ee48 00008000 fb0ca8c0 c7eb6200 c04fcfac 00000000
[ 2659.847412] 7de0: 000038e5 fb0ca8c0 fb0ca8c0 c7eb6200 00000000 c02a4e28 c02a40e4 00000000
[ 2659.855543] 7e00: 00000000 00000000 00000000 c02b99d4 00000001 00000000 c5c27f08 00000000
[ 2659.863669] 7e20: c5c7b500 000005a8 00000000 00000000 000200da c5c7b500 c0069a74 c5c27e3c
[ 2659.871802] 7e40: c5c27e3c c0049fdc be96a8fc c0275d70 c5c27e88 00000006 00000005 00000001
[ 2659.879933] 7e60: 00000000 00000000 00000000 fb0ca8c0 1d0ca8c0 00000000 00000000 00000000
[ 2659.888057] 7e80: 00000000 00000000 00000000 00000000 00000006 38e50000 00000000 c7ff6e00
[ 2659.896183] 7ea0: 000063f8 c5c27f08 00000010 c7eb6200 c5c27f08 c00440c4 c5c26000 c76cd2c0
[ 2659.904314] 7ec0: 00000802 c02c4f70 0000c1ff 00000001 00000000 00000000 00000000 00045108
[ 2659.912441] 7ee0: 00000010 c76cd2c0 00045108 00000010 c5c27f08 c00440c4 c5c26000 40343800
[ 2659.920573] 7f00: 00044440 c0276888 38e50002 fb0ca8c0 00000000 00000000 000063f8 00000000
[ 2659.928698] 7f20: 00000000 be96a858 c5c27f48 000450c0 000000c5 c00440c4 c5c26000 40343800
[ 2659.936829] 7f40: 00044440 c00a9864 000063f8 00000000 00000005 c047c1ff 00000001 00000000
[ 2659.944952] 7f60: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000003
[ 2659.953078] 7f80: 00000000 c7b5aa00 00000000 00000000 00000000 00046cb8 00045108 40370ef8
[ 2659.961203] 7fa0: 0000011b c0043f40 00046cb8 00045108 0000000a 00045108 00000010 0000001c
[ 2659.969328] 7fc0: 00046cb8 00045108 40370ef8 0000011b 00000000 00043178 40343800 00044440
[ 2659.977451] 7fe0: 40371050 be96a968 4035b3c8 40790638 60000010 0000000a 00000000 00000000
[ 2659.985599] [<c00a28d8>] (kmem_cache_alloc+0x30/0x90) from [<c02a4b24>] (inet_bind_bucket_create+0x18/0x5c)
[ 2659.995296] [<c02a4b24>] (inet_bind_bucket_create+0x18/0x5c) from [<c02a4c8c>] (__inet_hash_connect+0x124/0x280)
[ 2660.005415] [<c02a4c8c>] (__inet_hash_connect+0x124/0x280) from [<c02a4e28>] (inet_hash_connect+0x40/0x50)
[ 2660.015034] [<c02a4e28>] (inet_hash_connect+0x40/0x50) from [<c02b99d4>] (tcp_v4_connect+0x2a8/0x420)
[ 2660.024211] [<c02b99d4>] (tcp_v4_connect+0x2a8/0x420) from [<c02c4f70>] (inet_stream_connect+0xac/0x26c)
[ 2660.033653] [<c02c4f70>] (inet_stream_connect+0xac/0x26c) from [<c0276888>] (sys_connect+0x6c/0x90)
[ 2660.042662] [<c0276888>] (sys_connect+0x6c/0x90) from [<c0043f40>] (ret_fast_syscall+0x0/0x28)
[ 2660.051228] Code: e5904080 e5907090 e3540000 1590308c (17943103) [ 2660.057440] ---[ end trace 416b23b4578ffa42 ]---
[ 2660.062021] Kernel panic - not syncing: Fatal exception in interrupt
[ 2660.068400] [<c00486cc>] (unwind_backtrace+0x0/0xdc) from [<c0351758>] (panic+0x34/0x120)
[ 2660.076555] [<c0351758>] (panic+0x34/0x120) from [<c004758c>] (die+0x14c/0x178)
[ 2660.083823] [<c004758c>] (die+0x14c/0x178) from [<c0049900>] (__do_kernel_fault+0x68/0x80)
[ 2660.092060] [<c0049900>] (__do_kernel_fault+0x68/0x80) from [<c004b678>] (do_alignment+0x59c/0x700)
[ 2660.101067] [<c004b678>] (do_alignment+0x59c/0x700) from [<c00432c8>] (do_DataAbort+0x34/0x94)
[ 2660.109649] [<c00432c8>] (do_DataAbort+0x34/0x94) from [<c0043acc>] (__dabt_svc+0x4c/0x60)
[ 2660.117874] Exception stack(0xc5c27d40 to 0xc5c27d88)
[ 2660.122898] 7d40: c04d42e4 00000020 c78ae4a0 00000000 00000001 60000013 00000020 00000020
[ 2660.131046] 7d60: c0532148 c04fcfac 00008928 00006e49 c78ae4a0 c5c27d88 c02a4b24 c00a28d8
[ 2660.139189] 7d80: 20000093 ffffffff
[ 2660.142673] [<c0043acc>] (__dabt_svc+0x4c/0x60) from [<c00a28d8>] (kmem_cache_alloc+0x30/0x90)
[ 2660.151265] [<c00a28d8>] (kmem_cache_alloc+0x30/0x90) from [<c02a4b24>] (inet_bind_bucket_create+0x18/0x5c)
[ 2660.160998] [<c02a4b24>] (inet_bind_bucket_create+0x18/0x5c) from [<c02a4c8c>] (__inet_hash_connect+0x124/0x280)
[ 2660.171148] [<c02a4c8c>] (__inet_hash_connect+0x124/0x280) from [<c02a4e28>] (inet_hash_connect+0x40/0x50)
[ 2660.180780] [<c02a4e28>] (inet_hash_connect+0x40/0x50) from [<c02b99d4>] (tcp_v4_connect+0x2a8/0x420)
[ 2660.189971] [<c02b99d4>] (tcp_v4_connect+0x2a8/0x420) from [<c02c4f70>] (inet_stream_connect+0xac/0x26c)
[ 2660.199424] [<c02c4f70>] (inet_stream_connect+0xac/0x26c) from [<c0276888>] (sys_connect+0x6c/0x90)
[ 2660.208443] [<c0276888>] (sys_connect+0x6c/0x90) from [<c0043f40>] (ret_fast_syscall+0x0/0x28)
^ permalink raw reply
* Re: [rfc 0/4] igb: bandwidth allocation
From: Andi Kleen @ 2009-11-05 12:09 UTC (permalink / raw)
To: Simon Horman; +Cc: e1000-devel, netdev, Arnd Bergmann, Jeff Kirsher
In-Reply-To: <20091105005847.941190065@vergenet.net>
Simon Horman <horms@verge.net.au> writes:
> Hi,
>
> this series of patches exposes the bandwidth allocation
> hardware support of the Intel 82576. It does so through
> a rather hackish sysfs entry. That interface is just intended
> for testing so that the exposed hardware feature can
> be exercised. I would like to find a generic way to expose
> this feature to user-space.
It would be cool if you had a interface that did a software
fallback for NICs that don't support this.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* Re: [RFC] [PATCH] udp: optimize lookup of UDP sockets to by including destination address in the hash key
From: Andi Kleen @ 2009-11-05 12:12 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Octavian Purdila, Lucian Adrian Grijincu, netdev
In-Reply-To: <4AF20F02.7000601@gmail.com>
Eric Dumazet <eric.dumazet@gmail.com> writes:
>
> I have struct reorderings in progress to reduce number of cache lines read
> per socket from two to one. So this would reduce by 50% time to find
> a particular socket in the chain.
Assuming that each access takes equal time seems like a rather dubious
assumption. Consider caches.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: William Allen Simpson @ 2009-11-05 12:17 UTC (permalink / raw)
To: paulmck
Cc: Eric Dumazet, Linux Kernel Developers,
Linux Kernel Network Developers
In-Reply-To: <20091104214844.GA6714@linux.vnet.ibm.com>
Paul E. McKenney wrote:
> On Tue, Nov 03, 2009 at 05:38:10PM -0500, William Allen Simpson wrote:
>> Documentation/RCU/checklist.txt #7 says:
>>
>> One exception to this rule: rcu_read_lock() and rcu_read_unlock()
>> may be substituted for rcu_read_lock_bh() and rcu_read_unlock_bh()
>> in cases where local bottom halves are already known to be
>> disabled, for example, in irq or softirq context. Commenting
>> such cases is a must, of course! And the jury is still out on
>> whether the increased speed is worth it.
>
> I strongly suggest using the matching primitives unless you have a
> really strong reason not to.
>
Eric gave contrary advice. But he also suggested (in an earlier message)
clearing the secrets with a timer, which could be a separate context --
although much later in time.
As you suggest, I'll use the _bh suffix everywhere until every i is dotted
and t is crossed. Then, check for efficiency later after thorough
analysis by experts such as yourself.
This code will be hit on every SYN and SYNACK that has a cookie option.
But it's just prior to a CPU intensive sha_transform -- in comparison,
it's trivial.
>> + rcu_assign_pointer(tcp_secret_generating,
>> + tcp_secret_secondary);
>> + rcu_assign_pointer(tcp_secret_retiring,
>> + tcp_secret_primary);
>> + spin_unlock_bh(&tcp_secret_locker);
>> + /* call_rcu() or synchronize_rcu() not needed. */
>
> Would you be willing to say why? Are you relying on a time delay for a
> given item to pass through tcp_secret_secondary and tcp_secret_retiring
> or some such? If so, how do you know that this time delay will always
> be long enough?
>
> Or are you just shuffling the data structures around, without ever
> freeing them? If so, is it really OK for a given reader to keep a
> reference to a given item through the full range of shuffling, especially
> given that it might be accesssing this concurrently with the ->expires
> assignments above?
>
> Either way, could you please expand the comment to give at least some
> hint to the poor guy reading your code? ;-)
>
Yes. Just shuffling the pointers without ever freeing anything. So,
there's nothing for call_rcu() to do, and nothing else to synchronize
(only the pointers). This assumes that after _unlock_ any CPU cache
with an old pointer->expires will hit the _lock_ code, and that will
update *both* ->expires and the other array elements concurrently?
One of the advantages of this scheme is the new secret is initialized
while the old secret is still used, and the old secret can continue to
be verified as old packets arrive. (I originally designed this for
Photuris [RFC-2522] circa 1995.)
As described in the long header given, each array element goes through
four (4) states. This is handling the first state transition. It will
hit at least 2 more locks, pointer updates, and unlocks before reuse.
Also, a great deal of time passes. After being retired (and expired), it
will be unused for approximately 5 minutes.
All that's a bit long for a comment.
+ /*
+ * The retiring data is never freed. Instead, it is
+ * replaced after later pointer updates and a quiet
+ * time of approximately 5 minutes. There is nothing
+ * for call_rcu() or synchronize_rcu() to handle.
+ */
Clear enough?
^ permalink raw reply
* Re: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)
From: Anton Vorontsov @ 2009-11-05 12:38 UTC (permalink / raw)
To: Kumar Gopalpet-B05799
Cc: Fleming Andy-AFLEMING, netdev, linuxppc-dev, David Miller
In-Reply-To: <9F4C7D19E8361D4C94921B95BE08B81B95069E@zin33exm22.fsl.freescale.net>
On Thu, Nov 05, 2009 at 11:11:56AM +0530, Kumar Gopalpet-B05799 wrote:
[...]
> >HI Anton, thanks for the changes. I have only one concern, has
> >this code been tried for ucc_geth ? I remember I had some
> >issues with getting the ucc_geth mdio also working. I will
> >take in these changes and try at my end for ucc_geth.
>
> Sorry, if my earlier statement was confusing. What I meant was with
> similar changes as Anton's changes
> I had some issues with ucc_geth mdio ( may be Anton's changes don't have
> that issue).
Nope, I see no issues on MPC8360E-MDS using ucc_geth and fsl_pq_mdio
drivers.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: William Allen Simpson @ 2009-11-05 12:45 UTC (permalink / raw)
To: paulmck
Cc: Eric Dumazet, Linux Kernel Developers,
Linux Kernel Network Developers
In-Reply-To: <4AF2C266.1010603@gmail.com>
William Allen Simpson wrote:
> Yes. Just shuffling the pointers without ever freeing anything. So,
> there's nothing for call_rcu() to do, and nothing else to synchronize
> (only the pointers). This assumes that after _unlock_ any CPU cache
> with an old pointer->expires will hit the _lock_ code, and that will
> update *both* ->expires and the other array elements concurrently?
>
Reiterating, I've not found Documentation showing that this code works:
+ unsigned long jiffy = jiffies;
+
+ if (unlikely(time_after(jiffy, tcp_secret_generating->expires))) {
+ spin_lock_bh(&tcp_secret_locker);
+ if (!time_after(jiffy, tcp_secret_generating->expires)) {
+ /* refreshed by another */
+ spin_unlock_bh(&tcp_secret_locker);
+ memcpy(&xvp->cookie_bakery[0],
+ &tcp_secret_generating->secrets[0],
+ sizeof(tcp_secret_generating->secrets));
+ } else {
How is it ensured that an old tcp_secret_generating or an old ->expires,
followed by a spin_lock, has updated both?
And even when both are updated, then every word of the ->secrets array has
also been updated in the local cache?
Is this a property of spin_lock()? Or spin_unlock()?
^ permalink raw reply
* Re: [RFC] [PATCH] udp: optimize lookup of UDP sockets to by including destination address in the hash key
From: Eric Dumazet @ 2009-11-05 13:02 UTC (permalink / raw)
To: Andi Kleen; +Cc: Octavian Purdila, Lucian Adrian Grijincu, netdev
In-Reply-To: <877hu5892g.fsf@basil.nowhere.org>
Andi Kleen a écrit :
> Eric Dumazet <eric.dumazet@gmail.com> writes:
>> I have struct reorderings in progress to reduce number of cache lines read
>> per socket from two to one. So this would reduce by 50% time to find
>> a particular socket in the chain.
>
> Assuming that each access takes equal time seems like a rather dubious
> assumption. Consider caches.
Yes, and it depends on SMP affinities too.
I assume cache is cold or even on other cpu (worst case), dealing with
100.000+ sockets or so...
If workload fits in one CPU cache/registers, we dont mind taking one
or two cache lines per object, obviously.
^ permalink raw reply
* RE: [PATCH 03/25] mlx4_core: add multi-function communicationchannel
From: Liran Liss @ 2009-11-05 13:06 UTC (permalink / raw)
To: Roland Dreier, Yevgeny Petrilin
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Tziporet Koren
In-Reply-To: <adaiqdq17zo.fsf-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
S.B.
--Liran
> --- a/drivers/net/mlx4/cmd.c
> +++ b/drivers/net/mlx4/cmd.c
> @@ -41,6 +41,7 @@
> #include <asm/io.h>
>
> #include "mlx4.h"
> +#include "en_port.h"
Why does core mlx4 command handling end up depending on stuff from
en_port.h?
LL: some of the FW commands (e.g., MLX4_CMD_SET_VLAN_FLTR) are defined
here and are used in cmd.h. We will move these definitions to cmd.h.
> + __be32 status = readl(&priv->mfunc.comm->slave_read);
This can't be endian-clean, can it? What does sparse with
-D__CHECK_ENDIAN__ say?
LL: will fix.
> + queue_delayed_work(priv->mfunc.comm_wq, &priv->mfunc.comm_work,
> + polled ? HZ / 1000 : HZ
/ 10);
So this is always running at least 10 times a second? That's a lot of
wakeups on an idle system. Is there no way to make this event-driven?
LL: events are not implemented yet - this is the next step.
And HZ/1000 is going to be 0 if HZ is less than 1000 ... so this is just
going to run continuously in the polling case.
LL: This is what we want as long as there are more pending commands.
> + /* Write command */
> + if (cmd == MLX4_COMM_CMD_RESET)
> + priv->cmd.comm_toggle = 0;
> + else if (++priv->cmd.comm_toggle > 2)
> + priv->cmd.comm_toggle = 1;
Is this right? comm_toggle goes 0, 1, 2, 1, 2, ...?
LL: Yes - we need to distinguish the reset state from all other states
to support asych reset (e.g., FLR). The only way to continue from this
state
is a new boot sequence.
> +static struct mlx4_cmd_info {
> + u8 opcode;
> + u8 has_inbox;
> + u8 has_outbox;
> + u8 out_is_imm;
> + int (*verify)(struct mlx4_dev *dev, int slave, struct mlx4_vhcr
*vhcr,
> + struct mlx4_cmd_mailbox
*inbox);
> + int (*wrapper)(struct mlx4_dev *dev, int slave, struct mlx4_vhcr
*vhcr,
> + struct mlx4_cmd_mailbox
*inbox,
> + struct mlx4_cmd_mailbox
*outbox);
> +} cmd_info[] = {
> + {MLX4_CMD_QUERY_FW, 0, 1, 0, NULL, NULL},
> + {MLX4_CMD_QUERY_ADAPTER, 0, 1, 0, NULL, NULL},
This big structure would be better with designated initializers. Also
instead of u8 for the flags bool would be better probably. Then it
becomes more self documenting, ie
{ .opcode = MLX4_CMD_QUERY_FW, .has_outbox = true }, ...
LL: OK. 10x.
> +struct mlx4_vhcr {
> + u64 in_param;
> + u64 out_param;
> + u32 in_modifier;
> + u32 timeout;
> + u16 op;
> + u16 token;
> + u8 op_modifier;
> + int errno;
> +};
trivial but can you use tabs to line up the structure field names the
way the rest of the mlx4 declarations do?
LL: Ack.
- R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Shared i2c adapter locking
From: Jean Delvare @ 2009-11-05 13:11 UTC (permalink / raw)
To: Ben Hutchings
Cc: Stephen Rothwell, David Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-next-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mika Kuoppala, Linux I2C
In-Reply-To: <1256828976.2827.27.camel@achroite>
Hi Ben,
On Thu, 29 Oct 2009 15:09:36 +0000, Ben Hutchings wrote:
> On Thu, 2009-10-29 at 15:43 +0100, Jean Delvare wrote:
> > Hi Stephen,
> >
> > On Mon, 26 Oct 2009 13:37:57 +1100, Stephen Rothwell wrote:
> > > Today's linux-next merge of the net tree got a conflict in
> > > drivers/net/sfc/sfe4001.c between commit
> > > 3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
> > > inversion on top of bus lock") from the i2c tree and commit
> > > c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
> > > falcon_boards.c") from the net tree.
> > >
> > > I have applied the following merge fixup patch (after removing
> > > drivers/net/sfc/sfe4001.c) and can carry it as necessary.
> >
> > Thanks for fixing it. The core problem here IMHO is that the sfc
> > network driver touches i2c internals which it would rather leave alone.
>
> I'm just a little proud of having the idea that we could avoid using an
> I/O-expander on this board, but yes, the software side of this
> multiplexing is a hack.
>
> > This is the only driver I know of which does this.
> >
> > I can think of 3 different ways to address the issue.
> >
> > Method #1: add a public API to grab/release an I2C segment.
> >
> > void i2c_adapter_lock(struct i2c_adapter *adapter)
> > {
> > rt_mutex_lock(&adapter->bus_lock);
> > }
> >
> > void i2c_adapter_unlock(struct i2c_adapter *adapter)
> > {
> > rt_mutex_unlock(&adapter->bus_lock);
> > }
> [...]
> > I'm not really sure if I have a preference yet, so please speak up if
> > you do.
>
> Indirect lock operations are a recipe for deadlock, and there doesn't
> seem to be any other user for this, so method 1 seems best.
Well, all 3 methods rely on indirect lock operations to some degree.
But I am fine with method #1 for now. We can always move to something
more complex if the need ever arises.
What about the following patch?
From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Subject: i2c: Add an interface to lock/unlock I2C bus segment
Some drivers need to be able to prevent access to an I2C bus segment
for a specific period of time. Add an interface for them to do so
without twiddling with i2c-core internals.
Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
---
drivers/net/sfc/sfe4001.c | 4 ++--
include/linux/i2c.h | 18 ++++++++++++++++++
2 files changed, 20 insertions(+), 2 deletions(-)
--- linux-2.6.32-rc6.orig/drivers/net/sfc/sfe4001.c 2009-11-05 10:51:56.000000000 +0100
+++ linux-2.6.32-rc6/drivers/net/sfc/sfe4001.c 2009-11-05 13:40:17.000000000 +0100
@@ -188,7 +188,7 @@ static int sfn4111t_reset(struct efx_nic
efx_oword_t reg;
/* GPIO 3 and the GPIO register are shared with I2C, so block that */
- mutex_lock(&efx->i2c_adap.bus_lock);
+ i2c_lock_adapter(&efx->i2c_adap);
/* Pull RST_N (GPIO 2) low then let it up again, setting the
* FLASH_CFG_1 strap (GPIO 3) appropriately. Only change the
@@ -204,7 +204,7 @@ static int sfn4111t_reset(struct efx_nic
falcon_write(efx, ®, GPIO_CTL_REG_KER);
msleep(1);
- mutex_unlock(&efx->i2c_adap.bus_lock);
+ i2c_unlock_adapter(&efx->i2c_adap);
ssleep(1);
return 0;
--- linux-2.6.32-rc6.orig/include/linux/i2c.h 2009-11-05 10:51:56.000000000 +0100
+++ linux-2.6.32-rc6/include/linux/i2c.h 2009-11-05 14:03:53.000000000 +0100
@@ -361,6 +361,24 @@ static inline void i2c_set_adapdata(stru
dev_set_drvdata(&dev->dev, data);
}
+/**
+ * i2c_lock_adapter - Prevent access to an I2C bus segment
+ * @adapter: Target I2C bus segment
+ */
+static inline void i2c_lock_adapter(struct i2c_adapter *adapter)
+{
+ mutex_lock(&adapter->bus_lock);
+}
+
+/**
+ * i2c_unlock_adapter - Reauthorize access to an I2C bus segment
+ * @adapter: Target I2C bus segment
+ */
+static inline void i2c_unlock_adapter(struct i2c_adapter *adapter)
+{
+ mutex_unlock(&adapter->bus_lock);
+}
+
/*flags for the client struct: */
#define I2C_CLIENT_PEC 0x04 /* Use Packet Error Checking */
#define I2C_CLIENT_TEN 0x10 /* we have a ten bit chip address */
--
Jean Delvare
^ permalink raw reply
* RE: [PATCH 05/25] mlx4_core: add slave resource allocation
From: Liran Liss @ 2009-11-05 13:14 UTC (permalink / raw)
To: Roland Dreier, Yevgeny Petrilin
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Tziporet Koren
In-Reply-To: <ada7hu617aw.fsf-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> + u32 param1 = *((u32 *) &vhcr->in_param);
> + u32 param2 = *(((u32 *) &vhcr->in_param) + 1);
Is this endian clean?
LL: Yes - the master and slave always run on the same arch...
- R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: Eric Dumazet @ 2009-11-05 13:19 UTC (permalink / raw)
To: William Allen Simpson
Cc: paulmck, Linux Kernel Developers, Linux Kernel Network Developers
In-Reply-To: <4AF2C266.1010603@gmail.com>
William Allen Simpson a écrit :
> Paul E. McKenney wrote:
>> On Tue, Nov 03, 2009 at 05:38:10PM -0500, William Allen Simpson wrote:
>>> Documentation/RCU/checklist.txt #7 says:
>>>
>>> One exception to this rule: rcu_read_lock() and rcu_read_unlock()
>>> may be substituted for rcu_read_lock_bh() and rcu_read_unlock_bh()
>>> in cases where local bottom halves are already known to be
>>> disabled, for example, in irq or softirq context. Commenting
>>> such cases is a must, of course! And the jury is still out on
>>> whether the increased speed is worth it.
>>
>> I strongly suggest using the matching primitives unless you have a
>> really strong reason not to.
>>
> Eric gave contrary advice. But he also suggested (in an earlier message)
> clearing the secrets with a timer, which could be a separate context --
> although much later in time.
>
> As you suggest, I'll use the _bh suffix everywhere until every i is dotted
> and t is crossed. Then, check for efficiency later after thorough
> analysis by experts such as yourself.
>
> This code will be hit on every SYN and SYNACK that has a cookie option.
> But it's just prior to a CPU intensive sha_transform -- in comparison,
> it's trivial.
>
I think you misunderstood my advice ;)
In the same function, you *cannot* use both variants like your last patch did :
spin_lock(&tcp_secret_locker);
...
rcu_read_lock_bh();
memcpy(&xvp->cookie_bakery[0],
&rcu_dereference(tcp_secret_generating)->secrets[0],
sizeof(tcp_secret_generating->secrets));
rcu_read_unlock_bh();
Reasoning is :
If you need _bh() for the rcu_read_lock_bh(), thats because you know
soft irq can happen anytime (they are not masked).
Then you also need _bh for the spin_lock() call, or risk deadlock.
-> tcp_cookie_generator();
spin_lock();
-> interrupt -> softirq -> SYN frame received -> tcp_cookie_generator() -> spin_lock(); hang
Your choices are :
------------------
1) Caller took care of disabling softirqs (or is only called from softirq handler),
then _bh suffixes are not necessary in tcp_cookie_generator().
-> spin_lock() & rcu_read_lock();
2) You dont know what called you (process context or softirq context)
-> you MUST use _bh prefixes on spin_lock_bh() & rcu_read_lock_bh();
^ permalink raw reply
* Re: [PATCH] tcp: set SPLICE_F_NONBLOCK after first buffer has been spliced
From: Max Kellermann @ 2009-11-05 13:23 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, jens.axboe, Linux Netdev List
In-Reply-To: <4AF2B551.6010302@gmail.com>
On 2009/11/05 12:21, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Max Kellermann a écrit :
> > Do you think that a splice() should block if the socket is readable
> > and the pipe is writable according to select()?
> >
>
> Yes, this is perfectly legal
>
> select() can return "OK to write on fd",
> and still, write(fd, buffer, 10000000) is supposer/allowed to block if fd is not O_NDELAY
From the select() manpage: "those in writefds will be watched to see
if a write will not block"
From the poll() manpage: "Writing now will not block."
This looks unambiguous to me, and contradicts with your thesis. Can
you provide sources?
What is your interpretation of the guarantees provided by select() and
poll()? Which byte count is "ok" to write after POLLOUT, and how much
is "too much"? How does the application know?
> Please read recent commit on this area and why I think your patch
> conflicts with this commit.
I understand your patch, but I don't understand the conflict with my
patch. Can you describe a breakage caused by my patch?
^ permalink raw reply
* RE: [PATCH 16/25] mlx4_core: boot sriov
From: Liran Liss @ 2009-11-05 13:26 UTC (permalink / raw)
To: Roland Dreier, Yevgeny Petrilin; +Cc: linux-rdma, netdev, Tziporet Koren
In-Reply-To: <aday6mmyshm.fsf@cisco.com>
S.B.
--Liran
-----Original Message-----
From: Roland Dreier [mailto:rdreier@cisco.com]
Sent: Wednesday, November 04, 2009 9:56 PM
To: Yevgeny Petrilin
Cc: linux-rdma@vger.kernel.org; netdev@vger.kernel.org; Liran Liss;
Tziporet Koren
Subject: Re: [PATCH 16/25] mlx4_core: boot sriov
> + /* Detect if this device is a virtual function */
> + switch (id->device) {
> + case 0x6341:
> + case 0x634b:
> + case 0x6733:
> + case 0x673d:
> + case 0x6369:
> + case 0x6751:
> + case 0x6765:
This isn't be maintainable or sane. How about using driver_data in the
PCI device table?
LL: good idea; 10x.
> +#ifdef CONFIG_PCI_IOV
> + if (sr_iov) {
Can we avoid a lot of these ifdefs by just doing
#else
#define sr_iov 0
#endif /* CONFIG_PCI_IOV */
at the beginning and letting the IOV code be optimized away?
LL: I think that this won't pass -Wall when compiling against a kernel
with sriov compiled out.
- R.
^ permalink raw reply
* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: Eric Dumazet @ 2009-11-05 13:34 UTC (permalink / raw)
To: William Allen Simpson
Cc: paulmck, Linux Kernel Developers, Linux Kernel Network Developers
In-Reply-To: <4AF2C8E4.9020202@gmail.com>
William Allen Simpson a écrit :
> William Allen Simpson wrote:
>> Yes. Just shuffling the pointers without ever freeing anything. So,
>> there's nothing for call_rcu() to do, and nothing else to synchronize
>> (only the pointers). This assumes that after _unlock_ any CPU cache
>> with an old pointer->expires will hit the _lock_ code, and that will
>> update *both* ->expires and the other array elements concurrently?
>>
> Reiterating, I've not found Documentation showing that this code works:
>
> + unsigned long jiffy = jiffies;
> +
> + if (unlikely(time_after(jiffy, tcp_secret_generating->expires))) {
> + spin_lock_bh(&tcp_secret_locker);
> + if (!time_after(jiffy, tcp_secret_generating->expires)) {
> + /* refreshed by another */
> + spin_unlock_bh(&tcp_secret_locker);
> + memcpy(&xvp->cookie_bakery[0],
> + &tcp_secret_generating->secrets[0],
> + sizeof(tcp_secret_generating->secrets));
> + } else {
>
> How is it ensured that an old tcp_secret_generating or an old ->expires,
> followed by a spin_lock, has updated both?
>
> And even when both are updated, then every word of the ->secrets array has
> also been updated in the local cache?
>
> Is this a property of spin_lock()? Or spin_unlock()?
Yes,
$ vi +1121 Documentation/memory-barriers.txt
(1) LOCK operation implication:
Memory operations issued after the LOCK will be completed after the LOCK
operation has completed.
Memory operations issued before the LOCK may be completed after the LOCK
operation has completed.
(2) UNLOCK operation implication:
Memory operations issued before the UNLOCK will be completed before the
UNLOCK operation has completed.
Memory operations issued after the UNLOCK may be completed before the
UNLOCK operation has completed.
^ permalink raw reply
* Re: [PATCH 1/3] net: TCP thin-stream detection
From: Andreas Petlund @ 2009-11-05 13:34 UTC (permalink / raw)
To: Arnd Hannemann
Cc: William Allen Simpson, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, shemminger@vyatta.com,
ilpo.jarvinen@helsinki.fi, davem@davemloft.net
In-Reply-To: <4AEB0512.4010804@nets.rwth-aachen.de>
Arnd Hannemann wrote:
> Both mechanism prevent retransmission timeouts, thereby reducing latency.
> Who cares, that they were motivated by performance?
The essence of motivation is that there exist an incentive for performing an
action. If the motivation for fast retransmitting earlier is to keep the cwnd
open for a greedy application with small time-dependency, the question may be
posed whether it is worth the effort of the proposed changes. With the
thin-stream applications, we have confirmed that this is very often an
indication of time-dependent/interactive applications (like SSH-text sessions,
RDP, sensor networks, stock trading systems, interactive games etc). We have
further shown that such applications are prone to lag upon retransmissions due
to the inadequacies of TCP to deal with thin streams. We have also shown that
by performing the proposed adjustments, we can drastically improve the
situation.
Since we now know that the modifications can drastically improve the user
experience, the motivation/incentive for implementing the modifications is
increased.
> I agree, that you are more aggressive, and that your scheme may have
> latency advantages, at least for the Limited Transmit case. And there are
> probably good reasons for your proposal. But I really think you should
> bring your proposal up in IETF TCPM WG. I have the feeling that there are
> a lot of corner cases we didn't think of.
>
> One example: Consider standard NewReno non-SACK enabled flow:
> For some reasons two data packets get reordered.
> The TCP sender will produce a dupACK and an ACK.
> The dupACK will trigger (because of your logic) a spurious retransmit.
> The spurious retransmit will trigger a dupACK.
> This dupACK will again trigger a spurious retransmit.
> And this game will continue, unless a packet is dropped by coincidence.
Such an effect will be extremely rare. It will depend on the application
producing an extremely even flow of packets with just the right
interarrival time, and also on reordering of data (which also will
happen very seldom when the number of packets in flight are so low).
Even though it can happen, the data flow will progress (with spurious
retransmissions). The effect will stop as soon as the application sends
more than 4 segments in an RTT (which will disable the thin-stream
modifications) or less than 1 (which will cause all segments to be
successfully ACKed), or if, as you say, a packet is dropped.
I will be thankful for more input on eventual corner cases and also on
test cases that we may perform to evaluate the modifications for
scenarios that are of concern.
Best regards,
Andreas
^ permalink raw reply
* Re: [PATCH 1/3] net: TCP thin-stream detection
From: Andreas Petlund @ 2009-11-05 13:36 UTC (permalink / raw)
To: William Allen Simpson
Cc: Linux Kernel Network Developers, Ilpo Järvinen,
Arnd Hannemann, linux-kernel@vger.kernel.org,
shemminger@vyatta.com, davem@davemloft.net, Christian Samsel
In-Reply-To: <4AEB109A.7090506@gmail.com>
William Allen Simpson wrote:
> I'm finding it hard to follow 3 threads, for the 3 parts of the patch.
>
> As I mentioned in one of these threads, I've plenty of experience with
> designing and implementing protocols for gaming. And it seems to me that
> you're making changes to the entire TCP stack to make up for shortcomings
> in the implementor's design. Yet, these changes require application
> implementors to set a sockopt that's only available in Linux. Unlikely,
> as they probably don't even keep track of such things....
>
The target is not only games, but for instance SSH sessions, RDP or VNC,
stock trading services, sensor networks and so forth. There are a lot of
time-dependent applications that shows thin stream properties. Many of
these use TCP, and will continue to use it. Some of these applications
use UDP as default, but fall back to TCP if there is a problem with the
UDP connection (for instance Skype). By providing better latency for thin
streams, we can increase the service level for all these applications.
Our experience is that at least some designers of interactive/time-dependent
applications are skilled enough and concerned enough to investigate whether
options exist that may improve the applications they are designing. Of
course there are exceptions, but for open-sourced software, there will be
people who can provide this input. If the argument is that there is no need
for customised options because developers are stupid, we could strip away a
lot of the existing network code.
> I've already suggested the end-to-end interest list, where you'll find many
> of us with a strong interest in this topic.
I've been reading end-to-end for several years, and I think I will take this
discussion to that list eventually. We have discussed whether we should take
this to end-to-end first, and netdev after, but decided to go here for the
following reasons: 1) We have working patches that we wanted to contribute.
2) The modifications are implemented as optional. 3) When active, the
modifications handle a special case of TCP streams that we have shown to
have minimal impact on general TCP behaviour.
Also, in my experience, the end-to-end list discussions tend to digress,
making it difficult to keep the discussion to the special case that we
address. Since we wanted technical and practical feedback that would help us
to refine the modifications in the patches in addition to the discussion on
transport protocols, we chose to go to netdev first.
> The IETF has two related working groups:
> tcpm -- tcp modifications
> tsvwg -- general transport, including sctp modifications
There are plenty of examples of TCP mechanisms present in the Linux
kernel that has not been standardised, for instance TCP CUBIC, the
default congestion control for many Linux distributions at this time.
We have a set of patches, and a large body of experiments that shows them to
be effective for the thin-stream scenario without any significant disadvantages.
Please consider this before discarding the proposition based on a general
principle of standardisation. We believe that the thin-stream modifications
will provide extra value to Linux networking.
Best regards,
Andreas
^ permalink raw reply
* Re: [PATCH 2/3] net: TCP thin linear timeouts
From: Andreas Petlund @ 2009-11-05 13:37 UTC (permalink / raw)
To: William Allen Simpson
Cc: Rick Jones, Ilpo Järvinen, Arnd Hannemann, Eric Dumazet,
Netdev, LKML, shemminger, David Miller
In-Reply-To: <4AEB2C55.7040208@gmail.com>
William Allen Simpson wrote:
>> Further blue-skying...
>>
>> If SACK were also enabled, it would seem that only loss of the last
>> segment in the "thin train" would be an issue? Presumably, the thin
>> stream receiver would be in a position to detect this, perhaps with an
>> application-level timeout. Whether then it would suffice to allow the
>> receiving app to make a setsockopt() call to force an extra ACK or two
>> I'm not sure. Perhaps if the thin-stream had a semi-aggressive
>> "heartbeat" going...
>>
> Heartbeats are the usual solution for gaming. Handles a host of
> issues, including detection of clients that have become unreachable.
>
> (No, these are not the same as TCP keep-alives.)
>
> Beside my code in the field and widespread discussion, I know that Paul
> Francis had several related papers a decade or so ago. My memory is that
> younger game coders weren't particularly avid readers....
>
>> But it does seem that it should be possible to deal with this sort of
>> thing without having to make wholesale changes to TCP's RTO policies
>> and whatnot?
>>
> Yep.
We recognise the possibility of increasing the aggressiveness of application
send rate in order to counteract the effect of thin streams on retransmission
latency. Applications are by nature uninformed about the state of the layers
below. To work around the fast-retransmit latency problems, an application
would have to keep a very aggressive heartbeat rate even though there is no
data to send, thus spamming the network with unneeded traffic.
To exemplify this, let's choose an SSH session from this set of statistics:
http://folk.uio.no/apetlund/lktmp/thin_apps_table.pdf. This thin stream has
an averge packet interarrival time of 323ms. The application developer would
have to consider how many "duds" to send in order to ensure a low
retransmission latency. Let's say he considers RTTs lower than 60ms harmless,
he would need to send more than 4 packets per 60ms. This would mean a
heartbeat rate of one packet each 15ms. Considering this, the aggressively
heartbeated application would send 67 packets per second compared to 3 in
the original stream.
By including thin-stream semantics into the TCP code, informed decisions
can be made to minimise the overhead while still reducing the retransmission
latency.
Best regards,
Andreas
^ permalink raw reply
* RE: [PATCH 19/25] mlx4: Randomizing mac addresses for slaves
From: Liran Liss @ 2009-11-05 13:38 UTC (permalink / raw)
To: Or Gerlitz, Roland Dreier
Cc: Yevgeny Petrilin, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Tziporet Koren
In-Reply-To: <15ddcffd0911041333l165ee274mfae3508a3db755e7-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
This approach seems to be common practice now (e.g., drivers/net/igb/igb_main.c:1332).
In any case, the user can change the randomized mac.
--Liran
-----Original Message-----
From: Or Gerlitz [mailto:or.gerlitz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
Sent: Wednesday, November 04, 2009 11:33 PM
To: Roland Dreier
Cc: Yevgeny Petrilin; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TaqPxH82wqD4g@public.gmane.orgg; Liran Liss; Tziporet Koren
Subject: Re: [PATCH 19/25] mlx4: Randomizing mac addresses for slaves
On Wed, Nov 4, 2009 at 10:04 PM, Roland Dreier <rdreier-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> wrote:
>> +#define MLX4_MAC_HEAD 0x2c9000000ULL
> Is this a good idea? You're basically choosing 24 random bits within your OUI...
> seems the chance of collision with another MAC used on the same
> network is high enough that it could easily happen in practice on a moderately big network.
yes, this has been brought by Stephen and others on this last back on September 11th, this year @
http://marc.info/?l=linux-netdev&m=125263488409128
> Can you pick a reserved range or something?
Using different OUI for the VF device wouldn't help either I think, since the #VF becomes fairly big even on a modest side cluster with
(say) a VM consuming VF per 1-2 cores.
Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] net: TCP thin-stream detection
From: Ilpo Järvinen @ 2009-11-05 13:45 UTC (permalink / raw)
To: Andreas Petlund
Cc: Arnd Hannemann, William Allen Simpson, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, shemminger@vyatta.com,
davem@davemloft.net
In-Reply-To: <4AF2D47F.4030701@simula.no>
On Thu, 5 Nov 2009, Andreas Petlund wrote:
> Arnd Hannemann wrote:
> >
> > One example: Consider standard NewReno non-SACK enabled flow:
> > For some reasons two data packets get reordered.
> > The TCP sender will produce a dupACK and an ACK.
> > The dupACK will trigger (because of your logic) a spurious retransmit.
> > The spurious retransmit will trigger a dupACK.
> > This dupACK will again trigger a spurious retransmit.
> > And this game will continue, unless a packet is dropped by coincidence.
>
> Such an effect will be extremely rare. It will depend on the application
> producing an extremely even flow of packets with just the right
> interarrival time, and also on reordering of data (which also will
> happen very seldom when the number of packets in flight are so low).
> Even though it can happen, the data flow will progress (with spurious
> retransmissions). The effect will stop as soon as the application sends
> more than 4 segments in an RTT (which will disable the thin-stream
> modifications) or less than 1 (which will cause all segments to be
> successfully ACKed), or if, as you say, a packet is dropped.
I'd simply workaround this problem by requiring SACK to be enabled for
such a connection. This is reinforced by the fact that small windowed
transfers want it certainly to be on anyway to get the best out of ACK
flow even if there were some ACK losses.
--
i.
^ permalink raw reply
* Re: Shared i2c adapter locking
From: Ben Hutchings @ 2009-11-05 13:57 UTC (permalink / raw)
To: Jean Delvare
Cc: Stephen Rothwell, David Miller, netdev, linux-next, linux-kernel,
Mika Kuoppala, Linux I2C
In-Reply-To: <20091105141122.56b6b4f8@hyperion.delvare>
On Thu, 2009-11-05 at 14:11 +0100, Jean Delvare wrote:
[...]
> What about the following patch?
>
> From: Jean Delvare <khali@linux-fr.org>
> Subject: i2c: Add an interface to lock/unlock I2C bus segment
>
> Some drivers need to be able to prevent access to an I2C bus segment
> for a specific period of time. Add an interface for them to do so
> without twiddling with i2c-core internals.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Ben Hutchings <bhutchings@solarflare.com>
Acked-by: Ben Hutchings <bhutchings@solarflare.com>
Presumably this is meant for net-next-2.6, and you'll implement
i2c_{lock,unlock}_adapter() using rt_mutex in your i2c tree?
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: Shared i2c adapter locking
From: Jean Delvare @ 2009-11-05 14:07 UTC (permalink / raw)
To: Ben Hutchings
Cc: Stephen Rothwell, David Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-next-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mika Kuoppala, Linux I2C
In-Reply-To: <1257429444.2793.2.camel-xQnnTUlwzDrdvaEqJLTMTA9jg9n5Vt1AMm0uRHvK7Nw@public.gmane.org>
On Thu, 05 Nov 2009 13:57:24 +0000, Ben Hutchings wrote:
> On Thu, 2009-11-05 at 14:11 +0100, Jean Delvare wrote:
> [...]
> > What about the following patch?
> >
> > From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Subject: i2c: Add an interface to lock/unlock I2C bus segment
> >
> > Some drivers need to be able to prevent access to an I2C bus segment
> > for a specific period of time. Add an interface for them to do so
> > without twiddling with i2c-core internals.
> >
> > Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Cc: Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
> Acked-by: Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
>
> Presumably this is meant for net-next-2.6, and you'll implement
Actually I meant to push this to Linus immediately, through my i2c
tree. This is essentially a no-op: the binary code will be the same as
before the patch, so guaranteed to be safe, and this will solve
conflicts in linux-next.
> i2c_{lock,unlock}_adapter() using rt_mutex in your i2c tree?
Correct.
--
Jean Delvare
^ permalink raw reply
* Re: [PATCH] tcp: set SPLICE_F_NONBLOCK after first buffer has been spliced
From: Eric Dumazet @ 2009-11-05 14:11 UTC (permalink / raw)
To: Max Kellermann; +Cc: linux-kernel, jens.axboe, Linux Netdev List
In-Reply-To: <20091105132352.GA14453@rabbit.intern.cm-ag>
Max Kellermann a écrit :
> On 2009/11/05 12:21, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> Max Kellermann a écrit :
>>> Do you think that a splice() should block if the socket is readable
>>> and the pipe is writable according to select()?
>>>
>> Yes, this is perfectly legal
>>
>> select() can return "OK to write on fd",
>> and still, write(fd, buffer, 10000000) is supposer/allowed to block if fd is not O_NDELAY
>
>>From the select() manpage: "those in writefds will be watched to see
> if a write will not block"
>
>>From the poll() manpage: "Writing now will not block."
>
> This looks unambiguous to me, and contradicts with your thesis. Can
> you provide sources?
>
> What is your interpretation of the guarantees provided by select() and
> poll()? Which byte count is "ok" to write after POLLOUT, and how much
> is "too much"? How does the application know?
It cannot, therefore an application uses O_NDELAY to avoid blocking.
Try following program if you are not convinced
#include <unistd.h>
#include <sys/poll.h>
#include <stdio.h>
char buffer[1000000];
int main(int argc, char *argv[])
{
int fds[2];
struct pollfd pfd;
int res;
pipe(fds);
pfd.fd = fds[1];
pfd.events = POLLOUT;
res = poll(&pfd, 1, -1);
if (res > 0 && pfd.revents & POLLOUT)
printf("OK to write on pipe\n");
write(fds[1], buffer, sizeof(buffer)); // why it blocks, did poll() lied ???
return 0;
}
> I understand your patch, but I don't understand the conflict with my
> patch. Can you describe a breakage caused by my patch?
I only pointed out that using splice(tcp -> pipe) and blocking on pipe
_can_ block, even on _first_ frame received from tcp, as you discovered.
Your only choices to avoid a deadlock are :
1) to use SPLICE_F_NONBLOCK.
2) Using a second thread to read the pipe and empty it. First thread will
happily transfert 1000000 bytes in one syscall...
3) or limit your splice(... len, flags) length to 16 (16 buffers of one byte
in pathological cases)
Your patch basically makes SPLICE_F_NONBLOCK option always set (choice 1) above)
So users wanting option 3) are stuck. You force them to use a poll()/select()
thing while they dont want to poll : They have a producer thread(s), and a consumer
thread(s).
producer()
{
while (1)
splice(tcp, &offset, pfds[1], NULL, 10000000,
SPLICE_F_MORE | SPLICE_F_MOVE);
}
Why in the first place have an option if it is always set ?
^ permalink raw reply
* Re: [PATCH RFC] gianfar: Make polling safe with IRQs disabled
From: Anton Vorontsov @ 2009-11-05 14:20 UTC (permalink / raw)
To: Jon Loeliger
Cc: David Miller, linuxppc-dev, netdev, Andy Fleming, Jason Wessel
In-Reply-To: <E1N62ti-0003iS-K2@jdl.com>
On Thu, Nov 05, 2009 at 08:01:10AM -0600, Jon Loeliger wrote:
> > When using KGDBoE, gianfar driver spits 'Interrupt problem' messages,
> > which appears to be a legitimate warning, i.e. we may end up calling
> > netif_receive_skb() or vlan_hwaccel_receive_skb() with IRQs disabled.
> >
> > This patch reworks the RX path so that if netpoll is enabled (the
> > only case when the driver don't know from what context the polling
> > may be called), we check whether IRQs are disabled, and if so we
> > fall back to safe variants of skb receiving functions.
> >
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > ---
> >
> > I'm not sure if this is suitable for mainline since it doesn't
> > have KGDBoE support. Jason, if the patch is OK, would you like
> > to merge it into KGDB tree?
>
> It's a legitimate problem with or without KGDBoE. I see it
> occasionally when conn_track is enabled as well, for example.
Hm, then I'd better remove the #ifdef CONFIG_NETPOLL.
Interestingly though, why conn_track does the polling with irqs
disabled, could be a bug in the conn_track? Because pretty much
drivers assume that polling is called with IRQs enabled.
If it's easily reproducible, could you replace the printk() with
WARN_ON(1) and post the backtrace? Or I can try to reproduce the
issue if you tell me how.
Thanks!
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH] tcp: set SPLICE_F_NONBLOCK after first buffer has been spliced
From: Max Kellermann @ 2009-11-05 14:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, jens.axboe, Linux Netdev List
In-Reply-To: <4AF2DD21.8060604@gmail.com>
On 2009/11/05 15:11, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> It cannot, therefore an application uses O_NDELAY to avoid blocking.
>
> Try following program if you are not convinced
Indeed, I'm surprised by the result rendered by the Linux kernel.
That however still contradicts with the poll() documentation.
So this boils down to the question: kernel bug or documentation bug?
http://www.opengroup.org/onlinepubs/9699919799/functions/pselect.html
"A descriptor shall be considered ready for writing when a call to an
output function with O_NONBLOCK clear would not block, whether or not
the function would transfer data successfully."
There is no size limit mentioned here. Your program reveals that the
kernel violates this definition.
> Your patch basically makes SPLICE_F_NONBLOCK option always set
> (choice 1) above)
[...]
> Why in the first place have an option if it is always set ?
It is not, you misunderstood my patch. If there's no room in the pipe
buffer, then the first iteration of the "while" loop will block (as
usual). *After* the first iteration has finished (and at least one
buffer has been moved already), the flag is set, and further
iterations will not block.
^ permalink raw reply
* [PATCH 1/5] net/appletalk: push down BKL into a atalk_dgram_ops
From: Arnd Bergmann @ 2009-11-05 14:37 UTC (permalink / raw)
To: linux-kernel
Cc: David Miller, John Kacur, Thomas Gleixner, Frederic Weisbecker,
Arnd Bergmann, Arnaldo Carvalho de Melo, Stephen Hemminger,
netdev
In-Reply-To: <1257431850-20874-1-git-send-email-arnd@arndb.de>
Making the BKL usage explicit in appletalk makes it more
obvious where it is used, reduces code size and helps
getting rid of the BKL in common code.
I did not analyse how to kill lock_kernel from appletalk
entirely, this will involve either proving that it's not
needed, or replacing with a proper mutex or spinlock,
after finding out which data structures are protected
by the lock.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/appletalk/ddp.c | 105 +++++++++++++++++++++++++++++++++++++-------------
1 files changed, 77 insertions(+), 28 deletions(-)
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index b1a4290..3b831c0 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1054,11 +1054,13 @@ static int atalk_release(struct socket *sock)
{
struct sock *sk = sock->sk;
+ lock_kernel();
if (sk) {
sock_orphan(sk);
sock->sk = NULL;
atalk_destroy_socket(sk);
}
+ unlock_kernel();
return 0;
}
@@ -1134,6 +1136,7 @@ static int atalk_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
struct sockaddr_at *addr = (struct sockaddr_at *)uaddr;
struct sock *sk = sock->sk;
struct atalk_sock *at = at_sk(sk);
+ int err;
if (!sock_flag(sk, SOCK_ZAPPED) ||
addr_len != sizeof(struct sockaddr_at))
@@ -1142,37 +1145,44 @@ static int atalk_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
if (addr->sat_family != AF_APPLETALK)
return -EAFNOSUPPORT;
+ lock_kernel();
if (addr->sat_addr.s_net == htons(ATADDR_ANYNET)) {
struct atalk_addr *ap = atalk_find_primary();
+ err = -EADDRNOTAVAIL;
if (!ap)
- return -EADDRNOTAVAIL;
+ goto out;
at->src_net = addr->sat_addr.s_net = ap->s_net;
at->src_node = addr->sat_addr.s_node= ap->s_node;
} else {
+ err = -EADDRNOTAVAIL;
if (!atalk_find_interface(addr->sat_addr.s_net,
addr->sat_addr.s_node))
- return -EADDRNOTAVAIL;
+ goto out;
at->src_net = addr->sat_addr.s_net;
at->src_node = addr->sat_addr.s_node;
}
if (addr->sat_port == ATADDR_ANYPORT) {
- int n = atalk_pick_and_bind_port(sk, addr);
+ err = atalk_pick_and_bind_port(sk, addr);
- if (n < 0)
- return n;
+ if (err < 0)
+ goto out;
} else {
at->src_port = addr->sat_port;
+ err = -EADDRINUSE;
if (atalk_find_or_insert_socket(sk, addr))
- return -EADDRINUSE;
+ goto out;
}
sock_reset_flag(sk, SOCK_ZAPPED);
- return 0;
+ err = 0;
+out:
+ unlock_kernel();
+ return err;
}
/* Set the address we talk to */
@@ -1182,6 +1192,7 @@ static int atalk_connect(struct socket *sock, struct sockaddr *uaddr,
struct sock *sk = sock->sk;
struct atalk_sock *at = at_sk(sk);
struct sockaddr_at *addr;
+ int err;
sk->sk_state = TCP_CLOSE;
sock->state = SS_UNCONNECTED;
@@ -1206,12 +1217,15 @@ static int atalk_connect(struct socket *sock, struct sockaddr *uaddr,
#endif
}
+ lock_kernel();
+ err = -EBUSY;
if (sock_flag(sk, SOCK_ZAPPED))
if (atalk_autobind(sk) < 0)
- return -EBUSY;
+ goto out;
+ err = -ENETUNREACH;
if (!atrtr_get_dev(&addr->sat_addr))
- return -ENETUNREACH;
+ goto out;
at->dest_port = addr->sat_port;
at->dest_net = addr->sat_addr.s_net;
@@ -1219,7 +1233,10 @@ static int atalk_connect(struct socket *sock, struct sockaddr *uaddr,
sock->state = SS_CONNECTED;
sk->sk_state = TCP_ESTABLISHED;
- return 0;
+ err = 0;
+out:
+ unlock_kernel();
+ return err;
}
/*
@@ -1232,17 +1249,21 @@ static int atalk_getname(struct socket *sock, struct sockaddr *uaddr,
struct sockaddr_at sat;
struct sock *sk = sock->sk;
struct atalk_sock *at = at_sk(sk);
+ int err;
+ lock_kernel();
+ err = -ENOBUFS;
if (sock_flag(sk, SOCK_ZAPPED))
if (atalk_autobind(sk) < 0)
- return -ENOBUFS;
+ goto out;
*uaddr_len = sizeof(struct sockaddr_at);
memset(&sat.sat_zero, 0, sizeof(sat.sat_zero));
if (peer) {
+ err = -ENOTCONN;
if (sk->sk_state != TCP_ESTABLISHED)
- return -ENOTCONN;
+ goto out;
sat.sat_addr.s_net = at->dest_net;
sat.sat_addr.s_node = at->dest_node;
@@ -1253,9 +1274,23 @@ static int atalk_getname(struct socket *sock, struct sockaddr *uaddr,
sat.sat_port = at->src_port;
}
+ err = 0;
sat.sat_family = AF_APPLETALK;
memcpy(uaddr, &sat, sizeof(sat));
- return 0;
+
+out:
+ unlock_kernel();
+ return err;
+}
+
+static unsigned int atalk_poll(struct file *file, struct socket *sock,
+ poll_table *wait)
+{
+ int err;
+ lock_kernel();
+ err = datagram_poll(file, sock, wait);
+ unlock_kernel();
+ return err;
}
#if defined(CONFIG_IPDDP) || defined(CONFIG_IPDDP_MODULE)
@@ -1563,23 +1598,28 @@ static int atalk_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
if (len > DDP_MAXSZ)
return -EMSGSIZE;
+ lock_kernel();
if (usat) {
+ err = -EBUSY;
if (sock_flag(sk, SOCK_ZAPPED))
if (atalk_autobind(sk) < 0)
- return -EBUSY;
+ goto out;
+ err = -EINVAL;
if (msg->msg_namelen < sizeof(*usat) ||
usat->sat_family != AF_APPLETALK)
- return -EINVAL;
+ goto out;
+ err = -EPERM;
/* netatalk didn't implement this check */
if (usat->sat_addr.s_node == ATADDR_BCAST &&
!sock_flag(sk, SOCK_BROADCAST)) {
- return -EPERM;
+ goto out;
}
} else {
+ err = -ENOTCONN;
if (sk->sk_state != TCP_ESTABLISHED)
- return -ENOTCONN;
+ goto out;
usat = &local_satalk;
usat->sat_family = AF_APPLETALK;
usat->sat_port = at->dest_port;
@@ -1603,8 +1643,9 @@ static int atalk_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
rt = atrtr_find(&at_hint);
}
+ err = ENETUNREACH;
if (!rt)
- return -ENETUNREACH;
+ goto out;
dev = rt->dev;
@@ -1614,7 +1655,7 @@ static int atalk_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
size += dev->hard_header_len;
skb = sock_alloc_send_skb(sk, size, (flags & MSG_DONTWAIT), &err);
if (!skb)
- return err;
+ goto out;
skb->sk = sk;
skb_reserve(skb, ddp_dl->header_length);
@@ -1637,7 +1678,8 @@ static int atalk_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len);
if (err) {
kfree_skb(skb);
- return -EFAULT;
+ err = -EFAULT;
+ goto out;
}
if (sk->sk_no_check == 1)
@@ -1676,7 +1718,8 @@ static int atalk_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
rt = atrtr_find(&at_lo);
if (!rt) {
kfree_skb(skb);
- return -ENETUNREACH;
+ err = -ENETUNREACH;
+ goto out;
}
dev = rt->dev;
skb->dev = dev;
@@ -1696,7 +1739,9 @@ static int atalk_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
}
SOCK_DEBUG(sk, "SK %p: Done write (%Zd).\n", sk, len);
- return len;
+out:
+ unlock_kernel();
+ return err ? : len;
}
static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
@@ -1708,10 +1753,13 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
int copied = 0;
int offset = 0;
int err = 0;
- struct sk_buff *skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
+ struct sk_buff *skb;
+
+ lock_kernel();
+ skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
flags & MSG_DONTWAIT, &err);
if (!skb)
- return err;
+ goto out;
/* FIXME: use skb->cb to be able to use shared skbs */
ddp = ddp_hdr(skb);
@@ -1739,6 +1787,9 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
}
skb_free_datagram(sk, skb); /* Free the datagram. */
+
+out:
+ unlock_kernel();
return err ? : copied;
}
@@ -1827,7 +1878,7 @@ static struct net_proto_family atalk_family_ops = {
.owner = THIS_MODULE,
};
-static const struct proto_ops SOCKOPS_WRAPPED(atalk_dgram_ops) = {
+static const struct proto_ops atalk_dgram_ops = {
.family = PF_APPLETALK,
.owner = THIS_MODULE,
.release = atalk_release,
@@ -1836,7 +1887,7 @@ static const struct proto_ops SOCKOPS_WRAPPED(atalk_dgram_ops) = {
.socketpair = sock_no_socketpair,
.accept = sock_no_accept,
.getname = atalk_getname,
- .poll = datagram_poll,
+ .poll = atalk_poll,
.ioctl = atalk_ioctl,
#ifdef CONFIG_COMPAT
.compat_ioctl = atalk_compat_ioctl,
@@ -1851,8 +1902,6 @@ static const struct proto_ops SOCKOPS_WRAPPED(atalk_dgram_ops) = {
.sendpage = sock_no_sendpage,
};
-SOCKOPS_WRAP(atalk_dgram, PF_APPLETALK);
-
static struct notifier_block ddp_notifier = {
.notifier_call = ddp_device_event,
};
--
1.6.3.3
^ 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