* Re: [PATCH v3 13/17] lockd: use new hashtable implementation
From: Mathieu Desnoyers @ 2012-08-22 13:22 UTC (permalink / raw)
To: Sasha Levin
Cc: J. Bruce Fields, torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
tj-DgEjT+Ai2ygdnm+yROfE0A, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mingo-X9Un+BFzKDI, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, josh-iaAMLnmF4UmaiuxdJuQwMA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w, jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <5034CD02.2010103-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
* Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> On 08/22/2012 01:47 PM, J. Bruce Fields wrote:
> > On Wed, Aug 22, 2012 at 04:27:08AM +0200, Sasha Levin wrote:
> >> +static int __init nlm_init(void)
> >> +{
> >> + hash_init(nlm_files);
> >> + return 0;
> >> +}
> >> +
> >> +module_init(nlm_init);
> >
> > That's giving me:
> >
> > fs/lockd/svcsubs.o: In function `nlm_init':
> > /home/bfields/linux-2.6/fs/lockd/svcsubs.c:454: multiple definition of `init_module'
> > fs/lockd/svc.o:/home/bfields/linux-2.6/fs/lockd/svc.c:606: first defined here
> > make[2]: *** [fs/lockd/lockd.o] Error 1
> > make[1]: *** [fs/lockd] Error 2
> > make[1]: *** Waiting for unfinished jobs....
>
> I tested this entire patch set both with linux-next and Linus' latest master,
> and it worked fine in both places.
>
> Is it possible that lockd has a -next tree which isn't pulled into linux-next?
> (there's nothing listed in MAINTAINERS that I could see).
fs/lockd/Makefile:
obj-$(CONFIG_LOCKD) += lockd.o
lockd-objs-y := clntlock.o clntproc.o clntxdr.o host.o svc.o svclock.o \
svcshare.o svcproc.o svcsubs.o mon.o xdr.o grace.o
your patch adds a module_init to svcsubs.c.
However, there is already one in svc.c, pulled into the same module.
in your test build, is CONFIG_LOCKD defined as "m" or "y" ? You should
always test both.
One solution here is to create a "local" init function in svcsubs.c and
expose it to svc.c, so the latter can call it from its module init
function.
Thanks,
Mathieu
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Ben Hutchings @ 2012-08-22 13:26 UTC (permalink / raw)
To: Linus Torvalds
Cc: H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <CA+55aFy=ohFoZ1yZGdj1DCcNXU9gsndgH9Qod4q8+s=sbGKUzQ@mail.gmail.com>
On Tue, 2012-08-21 at 20:52 -0700, Linus Torvalds wrote:
[...]
> I haven't seen the patch being discussed, or the rationale for it. But
> I doubt it makes sense to do 128-bit MMIO and expect any kind of
> atomicity things anyway, and I very much doubt that using SSE would
> make all that much sense. What's the background, and why would you
> want to do this crap?
It's <1345598804.2659.78.camel@bwh-desktop.uk.solarflarecom.com>.
When updating a TX DMA ring pointer in sfc, we can push the first new
descriptor at the same time, so that for a linear packet only one DMA
read is then required before the packet goes out on the wire. Currently
this requires 2-4 MMIO writes (each a separate PCIe transactions),
depending on the host word size. There is a measurable reduction in
latency if we can reduce it to 1 PCIe transaction. (But as previously
discussed, write-combining isn't suitable for this.)
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 14:06 UTC (permalink / raw)
To: David Miller; +Cc: hpa, bhutchings, tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <20120821.220010.1158630981089834558.davem@davemloft.net>
On Tue, Aug 21, 2012 at 10:00 PM, David Miller <davem@davemloft.net> wrote:
>
> All the x86 crypto code hits this case all the time, easiest example
> is doing a dm-crypt on a block device when an IPSEC packet arrives.
>
> The crypto code has all of this special code and layering that is
> there purely so it can fall back to the slow non-optimized version
> of the crypto operation when it hits this can't-nest-fpu-saving
> situation.
Right. And it needs to be there. The current interface is fine and correct.
We can maybe optimize the current model (as outlined earlier), but no,
there's no way in hell we're doing lazy saving of FPU state from
interrupts etc. So all the "check if I can use FPU state at all", and
the explicit kernel_fpu_begin/end() interfaces are absolutely the
right model.
I realize that the people who write that code think that *their* code
is the most important thing in the world, and everything else should
revolve around them, and we should make everything else slower just to
make them happy. But they are wrong.
Deal with it, or do not. You can do the crypto entirely in software. I
think the current model is absolutely the right one, exactly because
it *allows* you to use the FPU for crypto, but it doesn't force the
rest of the kernel to make sure the FPU is always available to you.
I think your complaining about the interface is entirely bogus, and
comes from not appreciating that other areas and uses have other
concerns.
What I would suggest is trying to do profiling, and seeing if the
"let's try to save only once, and restore only when returning to user
space" approach helps. But that's an *implementation* change, not an
interface change. The interface is doing the right thing, the
implementation is just not perhaps optimal.
Linus
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 14:20 UTC (permalink / raw)
To: Ben Hutchings
Cc: H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <1345642009.15245.0.camel@deadeye.wl.decadent.org.uk>
On Wed, Aug 22, 2012 at 6:26 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> It's <1345598804.2659.78.camel@bwh-desktop.uk.solarflarecom.com>.
That doesn't help me in the least. I don't *have* that email. It was
never sent to me. I don't know what list it went on, and googling the
ID doesn't get me anything. So sending me the mail ID is kind of
pointless.
So I still don't see the actual patch. But everything I heard about it
indirectly makes me go "That's just crazy".
> When updating a TX DMA ring pointer in sfc, we can push the first new
> descriptor at the same time, so that for a linear packet only one DMA
> read is then required before the packet goes out on the wire. Currently
> this requires 2-4 MMIO writes (each a separate PCIe transactions),
> depending on the host word size. There is a measurable reduction in
> latency if we can reduce it to 1 PCIe transaction. (But as previously
> discussed, write-combining isn't suitable for this.)
Quite frankly, this isn't even *remotely* changing my mind about our
FPU model. I'm like "ok, some random driver is trying to be clever,
and it's almost certainly getting things entirely wrong and doing
things that only work on certain machines".
If you really think it's a big deal, do it in *your* driver, and make
it do the whole irq_fpu_usable() check together with
kernel_fpu_begin/end(). And make it very much x86-specific and with a
fallback to non-atomic accesses.
Any patch that exports some "atomic 128-bit MMIO writes" for general
use sounds totally and utterly broken. You can't do it. It's
*fundamentally* an operation that many CPU's cannot even do. 64-bit
buses (or even 32-bit ones) will make the 128-bit write be split up
*anyway*, regardless of any 128-bit register issues.
And nobody else sane cares about this, so it shouldn't even be a
x86-64 specific thing. It should be a driver hack, since that's what
it is. We don't want to support crazy stuff like this in general, that
is not just architecture-, but microarchitecture-specific.
I think you'd be crazy to even want to do this in the first place, but
if you do, keep it internal to your driver, and don't expose the crazy
to anybody else.
Linus
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Ben Hutchings @ 2012-08-22 14:24 UTC (permalink / raw)
To: Linus Torvalds
Cc: H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <CA+55aFzxhxjOeFSUUJ6XQGUmnaZf7QPSz611zmj2VziEDUB_sA@mail.gmail.com>
On Wed, 2012-08-22 at 07:20 -0700, Linus Torvalds wrote:
> On Wed, Aug 22, 2012 at 6:26 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> >
> > It's <1345598804.2659.78.camel@bwh-desktop.uk.solarflarecom.com>.
>
> That doesn't help me in the least. I don't *have* that email. It was
> never sent to me. I don't know what list it went on, and googling the
> ID doesn't get me anything. So sending me the mail ID is kind of
> pointless.
>
> So I still don't see the actual patch. But everything I heard about it
> indirectly makes me go "That's just crazy".
Sorry, I'll paste it below.
> > When updating a TX DMA ring pointer in sfc, we can push the first new
> > descriptor at the same time, so that for a linear packet only one DMA
> > read is then required before the packet goes out on the wire. Currently
> > this requires 2-4 MMIO writes (each a separate PCIe transactions),
> > depending on the host word size. There is a measurable reduction in
> > latency if we can reduce it to 1 PCIe transaction. (But as previously
> > discussed, write-combining isn't suitable for this.)
>
> Quite frankly, this isn't even *remotely* changing my mind about our
> FPU model. I'm like "ok, some random driver is trying to be clever,
> and it's almost certainly getting things entirely wrong and doing
> things that only work on certain machines".
>
> If you really think it's a big deal, do it in *your* driver, and make
> it do the whole irq_fpu_usable() check together with
> kernel_fpu_begin/end(). And make it very much x86-specific and with a
> fallback to non-atomic accesses.
[...]
Which is what I first submitted, but David wanted it to be generic.
Ben.
---
Subject: sfc: Use __raw_writeo() to perform TX descriptor push where possible
Use the new __raw_writeo() function for TX descriptor push where
available. This means we now use only a single PCIe transaction
on x86_64 (vs 2 before), reducing TX latency slightly.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/bitfield.h | 3 +++
drivers/net/ethernet/sfc/io.h | 18 ++++++++++++++++--
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sfc/bitfield.h b/drivers/net/ethernet/sfc/bitfield.h
index b26a954..5feeba2 100644
--- a/drivers/net/ethernet/sfc/bitfield.h
+++ b/drivers/net/ethernet/sfc/bitfield.h
@@ -83,6 +83,9 @@ typedef union efx_qword {
/* An octword (eight-word, i.e. 16 byte) datatype - little-endian in HW */
typedef union efx_oword {
+#ifdef HAVE_INT128
+ __le128 u128;
+#endif
__le64 u64[2];
efx_qword_t qword[2];
__le32 u32[4];
diff --git a/drivers/net/ethernet/sfc/io.h b/drivers/net/ethernet/sfc/io.h
index 751d1ec..fbcdc6d 100644
--- a/drivers/net/ethernet/sfc/io.h
+++ b/drivers/net/ethernet/sfc/io.h
@@ -57,10 +57,22 @@
* current state.
*/
-#if BITS_PER_LONG == 64
+#if defined(writeo)
+#define EFX_USE_OWORD_IO 1
+#endif
+
+#if defined(readq) && defined(writeq)
#define EFX_USE_QWORD_IO 1
#endif
+#ifdef EFX_USE_OWORD_IO
+static inline void _efx_writeo(struct efx_nic *efx, __le128 value,
+ unsigned int reg)
+{
+ __raw_writeo((__force u128)value, efx->membase + reg);
+}
+#endif
+
#ifdef EFX_USE_QWORD_IO
static inline void _efx_writeq(struct efx_nic *efx, __le64 value,
unsigned int reg)
@@ -235,7 +247,9 @@ static inline void _efx_writeo_page(struct efx_nic *efx, efx_oword_t *value,
"writing register %x with " EFX_OWORD_FMT "\n", reg,
EFX_OWORD_VAL(*value));
-#ifdef EFX_USE_QWORD_IO
+#if defined(EFX_USE_OWORD_IO)
+ _efx_writeo(efx, value->u128, reg);
+#elif defined(EFX_USE_QWORD_IO)
_efx_writeq(efx, value->u64[0], reg + 0);
_efx_writeq(efx, value->u64[1], reg + 8);
#else
--
Ben Hutchings, Staff Engineer, Solarflare
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 related
* Re: NULL deref in bnx2 / crashes ? ( was: netconsole leads to stalled CPU task )
From: Sylvain Munaut @ 2012-08-22 14:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1345640757.5158.1321.camel@edumazet-glaptop>
Hi,
> my patch was incomplete, sorry :
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 346b1eb..ddc453b 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -335,8 +335,13 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
> /* don't get messages out of order, and no recursion */
> if (skb_queue_len(&npinfo->txq) == 0 && !netpoll_owner_active(dev)) {
> struct netdev_queue *txq;
> + int queue_index = skb_get_queue_mapping(skb);
>
> - txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
> + if (queue_index >= dev->real_num_tx_queues) {
> + queue_index = 0;
> + skb_set_queue_mapping(skb, 0);
> + }
> + txq = netdev_get_tx_queue(dev, queue_index);
>
> /* try until next clock tick */
> for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
Ok, I tried this.
The machine with the intel card still hard freeze (no output / no nothing ...)
The machine with the bnx2 don't crash anymore and no NULL deref, but
the modprobe still hangs and I get this every 180 sec or so :
[ 775.956926] INFO: rcu_preempt self-detected stall on CPU { 14}
(t=600009 jiffies)
[ 775.956927] Pid: 3154, comm: modprobe Not tainted
3.6.0-rc2-00092-g9040592-dirty #8
[ 775.956928] Call Trace:
[ 775.956931] <IRQ> [<ffffffff81094a58>] ? rcu_pending+0xee/0x490
[ 775.956933] [<ffffffff810571da>] ?
irqtime_account_process_tick.isra.73+0x134/0x23a
[ 775.956934] [<ffffffff81095406>] ? rcu_check_callbacks+0x79/0x85
[ 775.956936] [<ffffffff81041609>] ? update_process_times+0x30/0x62
[ 775.956938] [<ffffffff8106f204>] ? tick_sched_timer+0x75/0x9e
[ 775.956939] [<ffffffff8106f18f>] ? tick_nohz_handler+0xd0/0xd0
[ 775.956941] [<ffffffff810501cf>] ? __run_hrtimer.isra.26+0x75/0xcd
[ 775.956943] [<ffffffff81050873>] ? hrtimer_interrupt+0xe2/0x1f0
[ 775.956948] [<ffffffffa000941d>] ? bnx2_poll_work+0x2d3/0xa52 [bnx2]
[ 775.956950] [<ffffffff8100fa5c>] ? sched_clock+0x5/0x8
[ 775.956952] [<ffffffff81023d93>] ? smp_apic_timer_interrupt+0x6d/0x7f
[ 775.956954] [<ffffffff8133244a>] ? apic_timer_interrupt+0x6a/0x70
[ 775.956956] [<ffffffff8126d407>] ? __napi_complete+0x1c/0x23
[ 775.956958] [<ffffffff81074249>] ? do_raw_spin_lock+0x18/0x1b
[ 775.956960] [<ffffffff8126e596>] ? net_rx_action+0x7f/0x185
[ 775.956962] [<ffffffff8100f80e>] ? __cycles_2_ns+0x9/0x45
[ 775.956964] [<ffffffff8103caae>] ? __do_softirq+0x9c/0x14b
[ 775.956966] [<ffffffff81332b3c>] ? call_softirq+0x1c/0x30
[ 775.956968] <EOI> [<ffffffff8100aea6>] ? do_softirq+0x3c/0x7a
[ 775.956970] [<ffffffff8103c9db>] ? _local_bh_enable_ip.isra.7+0x76/0xa3
[ 775.956972] [<ffffffff812804b7>] ? netpoll_poll_dev+0xfe/0x4bc
[ 775.956974] [<ffffffff81280b02>] ? netpoll_send_skb_on_dev+0x28d/0x33b
[ 775.956978] [<ffffffffa0ff2c4c>] ? bond_dev_queue_xmit+0x62/0x7f [bonding]
[ 775.956982] [<ffffffffa0ff7588>] ? bond_3ad_xmit_xor+0xe7/0x10c [bonding]
[ 775.956984] [<ffffffffa0ff2ffd>] ? bond_start_xmit+0x394/0x3ff [bonding]
[ 775.956987] [<ffffffff81280ac1>] ? netpoll_send_skb_on_dev+0x24c/0x33b
[ 775.956990] [<ffffffffa0079fd5>] ?
vlan_dev_hard_start_xmit+0xab/0xf6 [8021q]
[ 775.956992] [<ffffffff81280ac1>] ? netpoll_send_skb_on_dev+0x24c/0x33b
[ 775.956996] [<ffffffffa01998e8>] ? __br_deliver+0x93/0xbe [bridge]
[ 775.956998] [<ffffffffa019837d>] ? br_dev_xmit+0x14a/0x16b [bridge]
[ 775.957001] [<ffffffff81280ac1>] ? netpoll_send_skb_on_dev+0x24c/0x33b
[ 775.957003] [<ffffffff81280372>] ? find_skb.isra.24+0x31/0x78
[ 775.957005] [<ffffffff81280bdc>] ? netpoll_send_skb+0x2c/0x39
[ 775.957007] [<ffffffffa00c422a>] ? write_msg+0x98/0xf3 [netconsole]
[ 775.957009] [<ffffffff81037db2>] ?
call_console_drivers.constprop.17+0x6e/0x7d
[ 775.957011] [<ffffffff81038248>] ? console_unlock+0x2ab/0x351
[ 775.957012] [<ffffffff81039112>] ? register_console+0x273/0x303
[ 775.957014] [<ffffffffa0103182>] ? init_netconsole+0x182/0x210 [netconsole]
[ 775.957016] [<ffffffffa0103000>] ? 0xffffffffa0102fff
[ 775.957018] [<ffffffff81002085>] ? do_one_initcall+0x75/0x12c
[ 775.957019] [<ffffffff81077b35>] ? sys_init_module+0x80/0x1c5
[ 775.957020] [<ffffffff813319b9>] ? system_call_fastpath+0x16/0x1b
Cheers,
Sylvain
^ permalink raw reply
* RE: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: David Laight @ 2012-08-22 14:42 UTC (permalink / raw)
To: Linus Torvalds, Ben Hutchings
Cc: H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <CA+55aFzxhxjOeFSUUJ6XQGUmnaZf7QPSz611zmj2VziEDUB_sA@mail.gmail.com>
> Any patch that exports some "atomic 128-bit MMIO writes" for general
> use sounds totally and utterly broken. You can't do it. It's
> *fundamentally* an operation that many CPU's cannot even do. 64-bit
> buses (or even 32-bit ones) will make the 128-bit write be split up
> *anyway*, regardless of any 128-bit register issues.
>
> And nobody else sane cares about this, so it shouldn't even be a
> x86-64 specific thing...
There are several other processors that can generate long
PCIe transfers, sometimes by using a DMA engine associated
with the PCIe interface (eg freescale 83xx ppc).
Given the slow speed of PCIe transactions (think ISA bus
speeds - at least with some targets), it is important to
be able to request multi-word transfers in a generic way
on systems that can support it.
This support is a property of the PCIe interface block,
not that of the driver that wishes to use the function.
I don't know if XMM register transfers generate 16byte
TLP on any x86 cpus - they might on some.
Perhaps claiming the function is atomic is the real
problem - otherwise a single TLP could be used on
systems (and in contexts) where it is possible, but
a slower mulit-TLP transfer done (possibly without
guaranteeing the transfer order) done where it is not.
David
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 14:50 UTC (permalink / raw)
To: Ben Hutchings
Cc: H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <1345645499.15245.8.camel@deadeye.wl.decadent.org.uk>
On Wed, Aug 22, 2012 at 7:24 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> Sorry, I'll paste it below.
The thing you pasted isn't actually the thing in the subject line.
It's just you *using* it.
I wanted to see what that "writeo()" looks like for x86-64.
But I got google to find it for me by looking for "__raw_writeo", so I
can see the patch now. It looks like it might work. Does it really
help performance despite always doing those TS games in CR0 for each
access?
Linus
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Benjamin LaHaise @ 2012-08-22 14:30 UTC (permalink / raw)
To: Ben Hutchings
Cc: Linus Torvalds, H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <1345645499.15245.8.camel@deadeye.wl.decadent.org.uk>
On Wed, Aug 22, 2012 at 03:24:59PM +0100, Ben Hutchings wrote:
> -#ifdef EFX_USE_QWORD_IO
> +#if defined(EFX_USE_OWORD_IO)
> + _efx_writeo(efx, value->u128, reg);
> +#elif defined(EFX_USE_QWORD_IO)
> _efx_writeq(efx, value->u64[0], reg + 0);
> _efx_writeq(efx, value->u64[1], reg + 8);
> #else
This looks like a perfect fit for write combining. I have traces showing
that enabling write combining on MMIO does indeed generate a single PCIe
transaction on at least a couple of different current systems. Why is
that not an option?
-ben
--
"Thought is the essence of where you are now."
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 14:56 UTC (permalink / raw)
To: Ben Hutchings
Cc: H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <CA+55aFxoWLad9vtfAzQ7qXWcspL92D5s5+MFJ-HfyH3LVqfpaA@mail.gmail.com>
On Wed, Aug 22, 2012 at 7:50 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> But I got google to find it for me by looking for "__raw_writeo", so I
> can see the patch now. It looks like it might work. Does it really
> help performance despite always doing those TS games in CR0 for each
> access?
Btw, are we even certain that a 128-bit PCIe write is going to remain
atomic across a bus (ie over various PCIe bridges etc)? Do you you
care? Is it just a "one transaction is cheaper than two", and it
doesn't really have any ordering constraints? If the thing gets split
into two 64-bit transactions (in whatever order) by a bridge on the
way, would that be ok?
We've seen buses split accesses before (ie 64-bit writes on 32-bit PCI).
Linus
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Ben Hutchings @ 2012-08-22 14:58 UTC (permalink / raw)
To: Benjamin LaHaise
Cc: Linus Torvalds, H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <20120822143054.GD9803@kvack.org>
On Wed, 2012-08-22 at 10:30 -0400, Benjamin LaHaise wrote:
> On Wed, Aug 22, 2012 at 03:24:59PM +0100, Ben Hutchings wrote:
> > -#ifdef EFX_USE_QWORD_IO
> > +#if defined(EFX_USE_OWORD_IO)
> > + _efx_writeo(efx, value->u128, reg);
> > +#elif defined(EFX_USE_QWORD_IO)
> > _efx_writeq(efx, value->u64[0], reg + 0);
> > _efx_writeq(efx, value->u64[1], reg + 8);
> > #else
>
> This looks like a perfect fit for write combining. I have traces showing
> that enabling write combining on MMIO does indeed generate a single PCIe
> transaction on at least a couple of different current systems. Why is
> that not an option?
Because reordering, and see the comment at the top of this file.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: David Laight @ 2012-08-22 15:05 UTC (permalink / raw)
To: Linus Torvalds, Ben Hutchings
Cc: H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <CA+55aFzQDx78qBgDQyb6LuSyAyi_m=y8XjyRWf1J4kb0399U5w@mail.gmail.com>
> Btw, are we even certain that a 128-bit PCIe write is going to remain
> atomic across a bus (ie over various PCIe bridges etc)? Do you you
> care? Is it just a "one transaction is cheaper than two", and it
> doesn't really have any ordering constraints? If the thing gets split
> into two 64-bit transactions (in whatever order) by a bridge on the
> way, would that be ok?
PCIe transfers are basically hdlc packets containing the address,
command and any associated data. Unless they get bridged
though some strange PCIe<->PCI<->PCIe system they are very
unlikely to get broken up.
Maybe if they are longer than the maximum TLP size for the
target somewhere - but that is probably at least 128 bytes.
The time taken is largely independent of the transfer size.
The systems I've used (ppc accessing an Altera FPGA) have
PCIe cycles types of the order of microseconds.
Even for slow comms it is important to generate long TLP.
David
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: H. Peter Anvin @ 2012-08-22 15:13 UTC (permalink / raw)
To: Ben Hutchings
Cc: Benjamin LaHaise, Linus Torvalds, David Miller, tglx, mingo,
netdev, linux-net-drivers, x86
In-Reply-To: <1345647537.2709.0.camel@bwh-desktop.uk.solarflarecom.com>
On 08/22/2012 07:58 AM, Ben Hutchings wrote:
>
> Because reordering, and see the comment at the top of this file.
>
Your architecture sounds similar to one I once worked on (Orion
Microsystems CNIC/OPA-2). That architecture had a descriptor ring in
device memory, and a single trigger bit would move the head pointer.
We used write combining to write out a set of descriptors, and then used
a non-write-combining write to do the final write which bumps the head
pointer. The UC write flushes the write combiners ahead of it, so it
ends up with two transactions (one for the WC data and one for the UC
trigger) but it could frequently push quite a few descriptors in that
operation.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 15:16 UTC (permalink / raw)
To: David Laight
Cc: Ben Hutchings, H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B6FC0@saturn3.aculab.com>
On Wed, Aug 22, 2012 at 8:05 AM, David Laight <David.Laight@aculab.com> wrote:
>
> PCIe transfers are basically hdlc packets containing the address,
> command and any associated data. Unless they get bridged
> though some strange PCIe<->PCI<->PCIe system they are very
> unlikely to get broken up.
It's exactly the odd kind of "mix in a non-native PCIe bridge" setups
I'd worry about. But I guess that is pretty unlikely in any modern
machine (except for thunderbolt, and I think that's going to pass any
PCIe stuff through unchanged).
Linus
^ permalink raw reply
* RE: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: David Laight @ 2012-08-22 15:27 UTC (permalink / raw)
To: H. Peter Anvin, Ben Hutchings
Cc: Benjamin LaHaise, Linus Torvalds, David Miller, tglx, mingo,
netdev, linux-net-drivers, x86
In-Reply-To: <5034F725.2090802@zytor.com>
> Your architecture sounds similar to one I once worked on (Orion
> Microsystems CNIC/OPA-2). That architecture had a descriptor ring in
> device memory, and a single trigger bit would move the head pointer.
>
> We used write combining to write out a set of descriptors, and then
> used
> a non-write-combining write to do the final write which bumps the head
> pointer. The UC write flushes the write combiners ahead of it, so it
> ends up with two transactions (one for the WC data and one for the UC
> trigger) but it could frequently push quite a few descriptors in that
> operation.
The code actually looks more like a normal ethernet ring interface
with an 'owner' bit in each entry.
So it is important to write the owner bit last.
It might be possibly to set multiple ring entries in two TLPs
by first writing all of them (maybe with write combining)
but without changing the ownership of the first entry.
Then doing a second transfer to update the owner bit it
the first entry.
The order of the writes in the first transfer would then not
matter.
FWIW can you even guarantee to do an atomic 64bit PCIe transfer
on many systems (without resorting to a dma unit).
David
^ permalink raw reply
* TIPC handling of ethernet mac address change
From: Chris Friesen @ 2012-08-22 15:36 UTC (permalink / raw)
To: Allan Stephens, Jon Maloy, netdev
Hi,
I'm pretty new to tipc so pardon me if I use the wrong terminology. :)
I'm using TIPC 1.7.7 and bonding. I'm seeing some strange behaviour
when the bond MAC address changes--the tipc LINK_CONFIG messages still
contain the old MAC address embedded in them. This causes the other end
to send back to us on the wrong MAC and the packets get dropped.
I see recv_notification() has a case for NETDEV_CHANGEADDR so it looks
like MAC address change was intended to be handled, but I'm not entirely
clear on where the MAC address is supposed to be changed.
Any assistance on debugging/patching this would be appreciated.
Chris
--
Chris Friesen
Software Designer
3500 Carling Avenue
Ottawa, Ontario K2H 8E9
www.genband.com
^ permalink raw reply
* Re: NULL deref in bnx2 / crashes ? ( was: netconsole leads to stalled CPU task )
From: Cong Wang @ 2012-08-22 15:40 UTC (permalink / raw)
To: netdev
In-Reply-To: <CAF6-1L7nRoG10P=+QRb=LfL4O8K877zUD6L6d5EoCq-QNt1FWA@mail.gmail.com>
On Wed, 22 Aug 2012 at 14:29 GMT, Sylvain Munaut <s.munaut@whatever-company.com> wrote:
> Hi,
>
>> my patch was incomplete, sorry :
>>
>> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
>> index 346b1eb..ddc453b 100644
>> --- a/net/core/netpoll.c
>> +++ b/net/core/netpoll.c
>> @@ -335,8 +335,13 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
>> /* don't get messages out of order, and no recursion */
>> if (skb_queue_len(&npinfo->txq) == 0 && !netpoll_owner_active(dev)) {
>> struct netdev_queue *txq;
>> + int queue_index = skb_get_queue_mapping(skb);
>>
>> - txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
>> + if (queue_index >= dev->real_num_tx_queues) {
>> + queue_index = 0;
>> + skb_set_queue_mapping(skb, 0);
>> + }
>> + txq = netdev_get_tx_queue(dev, queue_index);
>>
>> /* try until next clock tick */
>> for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
>
> Ok, I tried this.
>
> The machine with the intel card still hard freeze (no output / no nothing ...)
> The machine with the bnx2 don't crash anymore and no NULL deref, but
> the modprobe still hangs and I get this every 180 sec or so :
>
Thanks for reporting this!
I will try to see if I can reproduce it on KVM guest tomorrow.
Need to go to sleep now.
To be honest, I never try to setup netconsole on such a complex NIC,
bridge on vlan tagged bonding.
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Ben Hutchings @ 2012-08-22 15:41 UTC (permalink / raw)
To: Linus Torvalds
Cc: H. Peter Anvin, David Miller, tglx, mingo, netdev,
linux-net-drivers, x86
In-Reply-To: <CA+55aFxoWLad9vtfAzQ7qXWcspL92D5s5+MFJ-HfyH3LVqfpaA@mail.gmail.com>
On Wed, 2012-08-22 at 07:50 -0700, Linus Torvalds wrote:
> On Wed, Aug 22, 2012 at 7:24 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> >
> > Sorry, I'll paste it below.
>
> The thing you pasted isn't actually the thing in the subject line.
> It's just you *using* it.
>
> I wanted to see what that "writeo()" looks like for x86-64.
>
> But I got google to find it for me by looking for "__raw_writeo", so I
> can see the patch now. It looks like it might work. Does it really
> help performance despite always doing those TS games in CR0 for each
> access?
I haven't run the experiment myself, but my colleagues observed a net
reduction of 100s of nanoseconds of latency. That may not sound like
much, but for a small packet traversing an uncongested twinax link it's
around 5-10% of the total latency from the descriptor pointer write to
DMA completion on the peer.
Later, you wrote:
> Btw, are we even certain that a 128-bit PCIe write is going to remain
> atomic across a bus (ie over various PCIe bridges etc)?
I don't think PCIe bridges are allowed to split up TLPs (this is why the
PCI core has to be so careful about programming Max Payload Size). What
happens between the processor core and the host bridge is another
matter, though.
> Do you you
> care? Is it just a "one transaction is cheaper than two", and it
> doesn't really have any ordering constraints? If the thing gets split
> into two 64-bit transactions (in whatever order) by a bridge on the
> way, would that be ok?
We care if the two transactions are not in ascending address order;
that's why we had to abandon write combining.
> We've seen buses split accesses before (ie 64-bit writes on 32-bit
> PCI).
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: H. Peter Anvin @ 2012-08-22 15:49 UTC (permalink / raw)
To: David Laight
Cc: Ben Hutchings, Benjamin LaHaise, Linus Torvalds, David Miller,
tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B6FC1@saturn3.aculab.com>
On 08/22/2012 08:27 AM, David Laight wrote:
>> Your architecture sounds similar to one I once worked on (Orion
>> Microsystems CNIC/OPA-2). That architecture had a descriptor ring in
>> device memory, and a single trigger bit would move the head pointer.
>>
>> We used write combining to write out a set of descriptors, and then
>> used
>> a non-write-combining write to do the final write which bumps the head
>> pointer. The UC write flushes the write combiners ahead of it, so it
>> ends up with two transactions (one for the WC data and one for the UC
>> trigger) but it could frequently push quite a few descriptors in that
>> operation.
>
> The code actually looks more like a normal ethernet ring interface
> with an 'owner' bit in each entry.
> So it is important to write the owner bit last.
>
> It might be possibly to set multiple ring entries in two TLPs
> by first writing all of them (maybe with write combining)
> but without changing the ownership of the first entry.
> Then doing a second transfer to update the owner bit it
> the first entry.
> The order of the writes in the first transfer would then not
> matter.
>
> FWIW can you even guarantee to do an atomic 64bit PCIe transfer
> on many systems (without resorting to a dma unit).
>
On many systems, perhaps, but I suspect that 32 bits is the maximum you
can truly guarantee.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply
* RE: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Ben Hutchings @ 2012-08-22 15:51 UTC (permalink / raw)
To: David Laight
Cc: H. Peter Anvin, Benjamin LaHaise, Linus Torvalds, David Miller,
tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B6FC1@saturn3.aculab.com>
On Wed, 2012-08-22 at 16:27 +0100, David Laight wrote:
> > Your architecture sounds similar to one I once worked on (Orion
> > Microsystems CNIC/OPA-2). That architecture had a descriptor ring in
> > device memory, and a single trigger bit would move the head pointer.
> >
> > We used write combining to write out a set of descriptors, and then
> > used
> > a non-write-combining write to do the final write which bumps the head
> > pointer. The UC write flushes the write combiners ahead of it, so it
> > ends up with two transactions (one for the WC data and one for the UC
> > trigger) but it could frequently push quite a few descriptors in that
> > operation.
>
> The code actually looks more like a normal ethernet ring interface
> with an 'owner' bit in each entry.
> So it is important to write the owner bit last.
You're confused. The 'owner' field in the descriptor pointer is part of
the memory protection mechanism for user-level networking. And we don't
have up to 1024 TX descriptors in a single ring, we have up to 1024
separate rings - in host memory, of course. Which is why we have the
'TX push' feature to reduce latency for a currently empty TX queue.
> It might be possibly to set multiple ring entries in two TLPs
> by first writing all of them (maybe with write combining)
> but without changing the ownership of the first entry.
> Then doing a second transfer to update the owner bit it
> the first entry.
> The order of the writes in the first transfer would then not
> matter.
>
> FWIW can you even guarantee to do an atomic 64bit PCIe transfer
> on many systems (without resorting to a dma unit).
On any architecture that implements readq and writeq these had better be
atomic.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: H. Peter Anvin @ 2012-08-22 15:51 UTC (permalink / raw)
To: David Laight
Cc: Ben Hutchings, Benjamin LaHaise, Linus Torvalds, David Miller,
tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B6FC1@saturn3.aculab.com>
On 08/22/2012 08:27 AM, David Laight wrote:
>> Your architecture sounds similar to one I once worked on (Orion
>> Microsystems CNIC/OPA-2). That architecture had a descriptor ring in
>> device memory, and a single trigger bit would move the head pointer.
>>
>> We used write combining to write out a set of descriptors, and then
>> used
>> a non-write-combining write to do the final write which bumps the head
>> pointer. The UC write flushes the write combiners ahead of it, so it
>> ends up with two transactions (one for the WC data and one for the UC
>> trigger) but it could frequently push quite a few descriptors in that
>> operation.
>
> The code actually looks more like a normal ethernet ring interface
> with an 'owner' bit in each entry.
> So it is important to write the owner bit last.
>
> It might be possibly to set multiple ring entries in two TLPs
> by first writing all of them (maybe with write combining)
> but without changing the ownership of the first entry.
> Then doing a second transfer to update the owner bit it
> the first entry.
> The order of the writes in the first transfer would then not
> matter.
>
The design flaw in that kind of design would be the need to set the
owner bit on every entry. Now, in the case of CNIC/OPA-2 support for
write combining was an explicit design goal, so writes are inert until
the trigger bit is written, at which point the head pointer is moved to
the entry containing the trigger bit. Very effective.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply
* Re: [PATCH] libceph: Fix sparse warning
From: Sage Weil @ 2012-08-22 15:53 UTC (permalink / raw)
To: Daniel Baluta; +Cc: davem, ceph-devel, netdev, Iulius Curt
In-Reply-To: <CAEnQRZBcX7vmaa2rc3-CcCSpayKZca2p11_3eSQaEsj_FAQLNg@mail.gmail.com>
On Wed, 22 Aug 2012, Daniel Baluta wrote:
> On Tue, Aug 14, 2012 at 4:27 PM, Iulius Curt <iulius.curt@gmail.com> wrote:
> > From: Iulius Curt <iulius.curt@gmail.com>
> >
> > Make ceph_monc_do_poolop() static to remove the following sparse warning:
> > * net/ceph/mon_client.c:616:5: warning: symbol 'ceph_monc_do_poolop' was not
> > declared. Should it be static?
> >
> > Signed-off-by: Iulius Curt <icurt@ixiacom.com>
> > ---
> > net/ceph/mon_client.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/ceph/mon_client.c b/net/ceph/mon_client.c
> > index 105d533..3875c60 100644
> > --- a/net/ceph/mon_client.c
> > +++ b/net/ceph/mon_client.c
> > @@ -613,7 +613,7 @@ bad:
> > /*
> > * Do a synchronous pool op.
> > */
> > -int ceph_monc_do_poolop(struct ceph_mon_client *monc, u32 op,
> > +static int ceph_monc_do_poolop(struct ceph_mon_client *monc, u32 op,
> > u32 pool, u64 snapid,
> > char *buf, int len)
> > {
> > --
> > 1.7.9.5
> >
> > --
>
> Hi Sage,
>
> Can you have a look on this? :)
Sorry, this one fell through the cracks. Yes, we can switch it to static,
but while we're doing that let's drop the ceph_monc_ prefix too (since
it's private).
Thanks!
sage
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: H. Peter Anvin @ 2012-08-22 15:54 UTC (permalink / raw)
To: Ben Hutchings
Cc: David Laight, Benjamin LaHaise, Linus Torvalds, David Miller,
tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345650689.2709.32.camel@bwh-desktop.uk.solarflarecom.com>
On 08/22/2012 08:51 AM, Ben Hutchings wrote:
>>
>> FWIW can you even guarantee to do an atomic 64bit PCIe transfer
>> on many systems (without resorting to a dma unit).
>
> On any architecture that implements readq and writeq these had better be
> atomic.
>
Sorry, you fail. There are definitely systems in the field where
readq() and writeq() are implemented, because the CPU supports them,
where the fabric does not guarantee they are intact.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply
* Re: [PATCH 1/2] ipv6: do not hold route table lock when send ndisc probe
From: Banerjee, Debabrata @ 2012-08-22 16:04 UTC (permalink / raw)
To: Cong Wang, Debabrata Banerjee
Cc: netdev@vger.kernel.org, David S. Miller, Hideaki YOSHIFUJI,
Patrick McHardy
In-Reply-To: <1345520643.12468.6.camel@cr0>
Thanks for the patch. We're discussing how to reach to this code properly
in the lab now. Although we will probably have to modify so it's compliant
with the RFC, by checking if a ndisc_send_ns() has already been queued
within rt->rt6i_idev->cnf.rtr_probe_interval, otherwise it could flood the
network with neighbor discoveries.
-Debabrata
On 8/20/12 11:44 PM, "Cong Wang" <amwang@redhat.com> wrote:
>Hi, Debabrata,
>
>Could you help to test the attached patch below?
>
>Thanks!
>
^ permalink raw reply
* Best way to set kernel thread affinity for handling a socket?
From: Roland Dreier @ 2012-08-22 16:10 UTC (permalink / raw)
To: netdev
Hi everyone,
Let's say I have kernel code that's sitting in a loop doing
kernel_accept() on a TCP socket. As each connection comes in, it
forks off a kernel thread to deal with that socket.
If I have a modern NIC with RSS and multiple queues, each TCP flow is
going to be steered to one queue, which is probably bound to one CPU.
So when I fork off that kernel thread, I'd like to bind it to the CPU
where its NIC queues are going to be processed. My question is, how
do I find out which CPU that is? Is there anything in the new socket
structure I get back from kernel_accept() that I can look at to know
which CPU the packets came in on?
I'm thinking about this in the context of the kernel's iSCSI target
code (drivers/target/iscsi), which creates threads to handle each
iSCSI connection and sets their CPU affinity pretty much randomly
(well, based on some "thread id", cf iscsit_thread_get_cpumask()).
And with a modern NIC, this leads to packets being received on one CPU
but the data being consumed on another CPU, all the time, which is
obviously far from optimal.
Thanks!
Roland
^ permalink raw reply
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