Netdev List
 help / color / mirror / Atom feed
* 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

* Re: Best way to set kernel thread affinity for handling a socket?
From: Ben Hutchings @ 2012-08-22 16:21 UTC (permalink / raw)
  To: Roland Dreier; +Cc: netdev
In-Reply-To: <CAG4TOxNMRR_-bYJV1a_je2HKrpmVuGubH9ks4GdG8Shwa92K0w@mail.gmail.com>

On Wed, 2012-08-22 at 09:10 -0700, Roland Dreier wrote:
> 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?

With RFS we try to do the reverse: move the packets to match the socket
user.   But it's not (yet) turned on by default.  See
Documentation/networking/scaling.txt

> 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()).

Why set the affinity at all?

> 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.

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: Ben Hutchings @ 2012-08-22 16:44 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: David Laight, Benjamin LaHaise, Linus Torvalds, David Miller,
	tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <50350098.6030100@zytor.com>

On Wed, 2012-08-22 at 08:54 -0700, H. Peter Anvin wrote:
> 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.

Well, when the issue of 64-bit MMIO was discussed earlier this year, you
said nothing about this.  I thought the conclusion was that any
definitions provided by <asm/io.h> *must* be atomic and drivers can use
<asm-generic/io-64-nonatomic-hi-lo.h> or
<asm-generic/io-64-nonatomic-lo-hi.h> as a fallback.

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 1/1] tcp: Wrong timeout for SYN segments
From: H.K. Jerry Chu @ 2012-08-22 16:44 UTC (permalink / raw)
  To: Alex Bergmann; +Cc: davem, netdev, linux-kernel
In-Reply-To: <503419D3.1080700@linlab.net>

On Tue, Aug 21, 2012 at 4:29 PM, Alex Bergmann <alex@linlab.net> wrote:
> Hi David,
>
> I'm not 100% sure, but it looks like I found an RFC mismatch with the
> current default values of the TCP implementation.
>
> Alex
>
> From 8b854a525eb45f64ad29dfab16f9d9f681e84495 Mon Sep 17 00:00:00 2001
> From: Alexander Bergmann <alex@linlab.net>
> Date: Wed, 22 Aug 2012 00:29:08 +0200
> Subject: [PATCH 1/1] tcp: Wrong timeout for SYN segments
>
> Commit 9ad7c049 changed the initRTO from 3secs to 1sec in accordance to
> RFC6298 (former RFC2988bis). This introduced a gap with RFC1122 that
> defines a minimum retransmission window for SYN segments of at least
> 180secs.
>
> Prior to 9ad7c049 the timeout was defined with 189secs. Now we have only
> a timeout of 63secs.
>
>         ((2 << 5) - 1) * 3 secs = 189 secs
>         ((2 << 5) - 1) * 1 secs = 63 secs
>
> To fulfill the MUST constraint in RFC1122 section 4.2.3.5 about R2 for
> SYN segments, the values of TCP_SYN_RETRIES and TCP_SYNACK_RETRIES must
> be changed to 7 reties.
>
>         ((2 << 7) - 1) * 1 secs = 255 secs
>
> This would result in an ETIMEDOUT of 4 minutes 15 seconds.

This issue occurred to me right after I submitted the patch for RFC6298.
I did not commit any more change because RFC compliance aside, 180secs
just seem like eternity in the Internet age.

(See my past post on this at
http://marc.info/?l=linux-netdev&m=130759078118866&w=2)

Jerry

>
> Signed-off-by: Alexander Bergmann <alex@linlab.net>
> ---
>  include/net/tcp.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 1f000ff..7eaae19 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -98,10 +98,10 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
>                                  * 15 is ~13-30min depending on RTO.
>                                  */
>
> -#define TCP_SYN_RETRIES         5      /* number of times to retry active opening a
> +#define TCP_SYN_RETRIES         7      /* number of times to retry active opening a
>                                  * connection: ~180sec is RFC minimum   */
>
> -#define TCP_SYNACK_RETRIES 5   /* number of times to retry passive opening a
> +#define TCP_SYNACK_RETRIES 7   /* number of times to retry passive opening a
>                                  * connection: ~180sec is RFC minimum   */
>
>  #define TCP_TIMEWAIT_LEN (60*HZ) /* how long to wait to destroy TIME-WAIT
> --
> 1.7.8.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.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: H. Peter Anvin @ 2012-08-22 16:49 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: <1345653844.2709.51.camel@bwh-desktop.uk.solarflarecom.com>

On 08/22/2012 09:44 AM, Ben Hutchings wrote:
>>
>> 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.
>
> Well, when the issue of 64-bit MMIO was discussed earlier this year, you
> said nothing about this.  I thought the conclusion was that any
> definitions provided by <asm/io.h> *must* be atomic and drivers can use
> <asm-generic/io-64-nonatomic-hi-lo.h> or
> <asm-generic/io-64-nonatomic-lo-hi.h> as a fallback.
>

That is true at the exit interface from the CPU core.  Beyond that 
drivers have to keep in mind the possible limitations of the 
communications fabric between the CPU and the device.

	-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 16:51 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Ben Hutchings, David Laight, Benjamin LaHaise, David Miller, tglx,
	mingo, netdev, linux-net-drivers, x86
In-Reply-To: <50350098.6030100@zytor.com>

On Wed, Aug 22, 2012 at 8:54 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>
> 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.

Indeed.

It's unlikely to be an issue with a PCIe driver, though. I'm pretty
sure you can rely on 64-bit transfers there, especially with a CPU
that is modern enough to run 64-bit mode.

That said, even with PCIe, I wonder if older CPU's (think Intel with a
front-side bus, rather than PCIe on die) necessarily always do 128-bit
writes. The FSB is just 64 bits wide, and I could *imagine* that a
PCIe chipset behind the FSB might end up just always generating at
most 64-bit PCIe transactions for host accesses just because that
would be "natural".

Sounds unlikely, but hey, hardware sometimes does odd things.

             Linus

^ permalink raw reply

* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 16:55 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: H. Peter Anvin, David Laight, Benjamin LaHaise, David Miller,
	tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345653844.2709.51.camel@bwh-desktop.uk.solarflarecom.com>

On Wed, Aug 22, 2012 at 9:44 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> Well, when the issue of 64-bit MMIO was discussed earlier this year, you
> said nothing about this.  I thought the conclusion was that any
> definitions provided by <asm/io.h> *must* be atomic and drivers can use
> <asm-generic/io-64-nonatomic-hi-lo.h> or
> <asm-generic/io-64-nonatomic-lo-hi.h> as a fallback.

Think 32-bit PCI with a 64-bit CPU.

The CPU itself does the 64-bit access no problem. The bus? Not so
much. Even if it's a burst transaction with a single packet, the
actual device on the other side will see the 64-bit value as two
separate parts. Sometimes that matters, sometimes it doesn't (ask
yourself: "What's the atomicity guarantee at the device end? Burst
transaction or individual word of a transaction?").

Again, being limited to PCIe, you are unlikely to hit these issues,
but system bridges can do odd things sometimes, and in the *general*
case it's definitely true that "writeq()" can generate multiple
accesses at the device end even if the *CPU* only generated a single
one.

            Linus

^ permalink raw reply

* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: H. Peter Anvin @ 2012-08-22 16:59 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Ben Hutchings, David Laight, Benjamin LaHaise, David Miller, tglx,
	mingo, netdev, linux-net-drivers, x86
In-Reply-To: <CA+55aFwTyT8v9r7jW2=jp29_yMMacWjMwCka+_YktOOMzBHWBw@mail.gmail.com>

On 08/22/2012 09:51 AM, Linus Torvalds wrote:
> On Wed, Aug 22, 2012 at 8:54 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> 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.
>
> Indeed.
>
> It's unlikely to be an issue with a PCIe driver, though. I'm pretty
> sure you can rely on 64-bit transfers there, especially with a CPU
> that is modern enough to run 64-bit mode.
>
> That said, even with PCIe, I wonder if older CPU's (think Intel with a
> front-side bus, rather than PCIe on die) necessarily always do 128-bit
> writes. The FSB is just 64 bits wide, and I could *imagine* that a
> PCIe chipset behind the FSB might end up just always generating at
> most 64-bit PCIe transactions for host accesses just because that
> would be "natural".
>
> Sounds unlikely, but hey, hardware sometimes does odd things.
>

I'm wondering how e.g. a K8 would work (CPU -> HT -> PCIe) on UC memory 
there.  I know for a fact that some CPU cores break up SSE transactions 
into 64-bit transactions.

	-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 17:09 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: H. Peter Anvin, David Laight, Benjamin LaHaise, David Miller,
	tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <CA+55aFxMD-Jn3fPgum7a+3EhNiWB-Mjv+1yamR7ELHwroA_+7Q@mail.gmail.com>

On Wed, 2012-08-22 at 09:55 -0700, Linus Torvalds wrote:
> On Wed, Aug 22, 2012 at 9:44 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> >
> > Well, when the issue of 64-bit MMIO was discussed earlier this year, you
> > said nothing about this.  I thought the conclusion was that any
> > definitions provided by <asm/io.h> *must* be atomic and drivers can use
> > <asm-generic/io-64-nonatomic-hi-lo.h> or
> > <asm-generic/io-64-nonatomic-lo-hi.h> as a fallback.
> 
> Think 32-bit PCI with a 64-bit CPU.
[...]

Well, sure, I'm assuming that the driver is responsible for checking
that the device and its bus interface support an MMIO of the requested
width.

But the architecture code must be responsible for reporting whether the
host supports it, right?

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 17:12 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Linus Torvalds, David Laight, Benjamin LaHaise, David Miller,
	tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345655343.2709.56.camel@bwh-desktop.uk.solarflarecom.com>

On 08/22/2012 10:09 AM, Ben Hutchings wrote:
>
> Well, sure, I'm assuming that the driver is responsible for checking
> that the device and its bus interface support an MMIO of the requested
> width.
>
> But the architecture code must be responsible for reporting whether the
> host supports it, right?
>

No, the architecture code *can't*.

	-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 17:26 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: H. Peter Anvin, David Laight, Benjamin LaHaise, David Miller,
	tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345655343.2709.56.camel@bwh-desktop.uk.solarflarecom.com>

On Wed, Aug 22, 2012 at 10:09 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> But the architecture code must be responsible for reporting whether the
> host supports it, right?

How? It's impossible. As far as the CPU is concerned, the writes
happen atomically. Look how you use it: you don't even query
dynamically about whether the stuff outside the CPU can handle atomic
128-bit writes. You just assume it at compile-time with an #ifdef. How
the hell do you expect that to be able to then say "oh, on this
machine the device you are doing the access to is behind an odd
PCI->PCIe bridge that will split the access"?

Not that we even tend to *know* those kinds of things. It's really
esoteric chipset knowledge. I wouldn't even expect it to be
necessarily documented in the chipset docs, it *might* be in some
NDA'd BIOS writer's guide thing.

You don't even seem to realize that things like the Intel FSB was
patented and wasn't fully documented by Intel at all? And that's for a
bus interface that was used for over a decade from the dominant CPU
manufacturer. What do you think happens with odd random chipsets? Who
do you expects to know?

I *suspect* that 128-bit writes would generally make it intact over
PCIe in real life, but I absolutely wouldn't guarantee it on all
machines. Exactly because of issues like "what happens with a nVidia
host bridge and the old FSB model on older Intel chips?" or "What does
the AMD memory pipeline do?".

Many CPU cores have 64-bit buses even *internally*, much less
externally. Yes, they have atomicity guarantees in their architecture
manual, but go look at it: those talk about memory accesses, and they
are based on cache coherency (these days - they *used* to be based on
certain bus guarantees). The MMIO side is a completely different
animal, and is still based on the bus - and nobody documents that,
afaik.

                 Linus

^ permalink raw reply

* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Ben Hutchings @ 2012-08-22 17:27 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Linus Torvalds, David Laight, Benjamin LaHaise, David Miller,
	tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <50351304.20608@zytor.com>

On Wed, 2012-08-22 at 10:12 -0700, H. Peter Anvin wrote:
> On 08/22/2012 10:09 AM, Ben Hutchings wrote:
> >
> > Well, sure, I'm assuming that the driver is responsible for checking
> > that the device and its bus interface support an MMIO of the requested
> > width.
> >
> > But the architecture code must be responsible for reporting whether the
> > host supports it, right?
> >
> 
> No, the architecture code *can't*.

So, let me check that I understand this right:
- To support 32-bit architectures, a driver should include one of two
different definitions of readq/writeq depending on which order the
device needs to receive 32-bit operations.
- On 64-bit architectures (or at least x86_64), the system might split
up readq/writeq into 32-bit operations in unspecified order, and the
driver can't control this.

If this is right, how can it be safe to use readq/writeq at all?

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 1/1] tcp: Wrong timeout for SYN segments
From: H.K. Jerry Chu @ 2012-08-22 17:29 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Alex Bergmann, davem, netdev, linux-kernel, Jerry Chu,
	Neal Cardwell, Nandita Dukkipati
In-Reply-To: <1345629806.5158.933.camel@edumazet-glaptop>

On Wed, Aug 22, 2012 at 3:03 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2012-08-22 at 12:00 +0200, Eric Dumazet wrote:
>> On Wed, 2012-08-22 at 11:29 +0200, Alex Bergmann wrote:
>>
>> > Actual 6 SYN frames are sent. The initial one and 5 retries.
>> >
>>
>> first one had a t0 + 0 delay. How can it count ???
>>
>> > The kernel is waiting another 32 seconds for a SYN+ACK and then gives
>> > the ETIMEDOUT back to userspace.
>> >
>> > Do you mean that we have to send another SYN packet after the 3 minutes?
>> >
>>
>> First SYN is not a retransmit
>>
>> R2 = time_of_last_SYN - time_of_initial_SYN (t0) = 31
>>
>> If you read RFC it states :
>>
>> "In particular, R2 for a SYN segment MUST
>>  be set large enough to provide retransmission of the segment
>>  for at least 3 minutes."
>>
>>
>> That means that the last _retransmit_ MUST happen after 180 seconds.
>>
>> And not :
>>
>> Send all the restransmits at t0 + 1, then wait 180 seconds before giving
>> connect() a timeout indication.
>>
>>
>
> Therefore, the minimal connect() timeout should be : 180 + 100 seconds
>
> (allowing 100 seconds for the SYNACKs sent in answer of the very last
> retransmit to come back)
>
> (100 seconds is the R2 for non SYN frames)
>
> RFC quote : The value of R2 SHOULD
>             correspond to at least 100 seconds.

I agree if you take RFC1122 literally the last retransmission must
happen no less than 3 minutes from the 1st SYN... Oh actually it'd be
3 minutes plus initRTO because the 3 minutes applies only to
"retransmission" as in

"R2 for a SYN segment MUST be set large enough to provide retransmission
of the segment for at least 3 minutes.:

But IMHO 6 retries providing 1+2+4+8+16+32 = 63 secs retransmission plus
64 secs wait time totaling 127 secs is really plenty enough.

You have a good point on SYN-ACK.

Jerry

>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 13/17] lockd: use new hashtable implementation
From: Sasha Levin @ 2012-08-22 17:32 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
	fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA, J. Bruce Fields,
	paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
	aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
	ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
	dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
	josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	lw-BthXqXjhjHXQFUHtdCDX3A, teigland-H+wXaHxf7aLQT0dZR+AlfA,
	axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20120822132243.GA2844@Krystal>

On 08/22/2012 03:22 PM, Mathieu Desnoyers wrote:
> * 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.

Ah yes, it was on =y and I didn't notice :/

I'll fix that.

> Thanks,
> 
> Mathieu
> 

^ permalink raw reply

* bonding: time limits too tight in bond_ab_arp_inspect
From: Jiri Bohac @ 2012-08-22 17:45 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek, netdev; +Cc: Petr Tesarik

Hi,

a customer reported that a bonding slave did not come back up
after setting their link down and then up again. ARP monitoring +
arp_validate were used.

Petr has tracked the problem down to the time comaprisons in
bond_ab_arp_inspect().

                if (slave->link != BOND_LINK_UP) {
                        if (time_in_range(jiffies,
                                slave_last_rx(bond, slave) - delta_in_ticks,
                                slave_last_rx(bond, slave) + delta_in_ticks)) {

                                slave->new_link = BOND_LINK_UP;
                                commit++;
                        }

                        continue;
                }

This code is run from bond_activebackup_arp_mon() about
delta_in_ticks jiffies after the previous ARP probe has been
sent. If the delayed work gets executed exactly in delta_in_ticks
jiffies, there is a chance the slave will be brought up.  If the
delayed work runs one jiffy later, the slave will stay down.

With arp_validate this is more noticeable, since traffic other than the
bonding-generated ARP probes does not update the slave_last_rx timestamp.

A simple patch will fix this case.

--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3001,7 +3001,7 @@ static int bond_ab_arp_inspect(struct bo
 		if (slave->link != BOND_LINK_UP) {
 			if (time_in_range(jiffies,
 				slave_last_rx(bond, slave) - delta_in_ticks,
-				slave_last_rx(bond, slave) + delta_in_ticks)) {
+				slave_last_rx(bond, slave) + 2 * delta_in_ticks)) {
 
 				slave->new_link = BOND_LINK_UP;
 				commit++;


The remaining time comparisons inside bond_ab_arp_inspect() have larger
tolerances (3*delta_in_ticks or 2*delta_in_ticks), but it still seems strange
that the precision of delayed work scheduling should steal a full
arp_interval from the time limits.

What is the intention of e.g. the "3*delta since last receive" limit? 
Was this really meant to be "as little as 2*delta + 1 jiffy"?

Should they perhaps all be increased by, say, delta_in_ticks/2, to make this
less dependent on the current scheduling latencies?

Thoughts?

-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ

^ permalink raw reply

* Re: bonding: time limits too tight in bond_ab_arp_inspect
From: Chris Friesen @ 2012-08-22 17:54 UTC (permalink / raw)
  To: Jiri Bohac; +Cc: Jay Vosburgh, Andy Gospodarek, netdev, Petr Tesarik
In-Reply-To: <20120822174534.GA20260@midget.suse.cz>

On 08/22/2012 11:45 AM, Jiri Bohac wrote:

> This code is run from bond_activebackup_arp_mon() about
> delta_in_ticks jiffies after the previous ARP probe has been
> sent. If the delayed work gets executed exactly in delta_in_ticks
> jiffies, there is a chance the slave will be brought up.  If the
> delayed work runs one jiffy later, the slave will stay down.

<snip>

> Should they perhaps all be increased by, say, delta_in_ticks/2, to make this
> less dependent on the current scheduling latencies?

We have been using a patch that tracks the arpmon requested sleep time 
vs the actual sleep time and adds any scheduling latency to the allowed 
delta.  That way if we sleep too long due to scheduling latency it 
doesn't affect the calculation.

Chris

^ permalink raw reply

* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 17:54 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: H. Peter Anvin, David Laight, Benjamin LaHaise, David Miller,
	tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345656446.2709.65.camel@bwh-desktop.uk.solarflarecom.com>

On Wed, Aug 22, 2012 at 10:27 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> If this is right, how can it be safe to use readq/writeq at all?

Pray.

Or don't care about ordering: use hardware that is well-designed and
doesn't have crap interfaces that are fragile.

If you care about ordering, you need to do them as two separate
accesses, and have a fence in between. Which, quite frankly, sounds
like the right model for you *anyway*, since then you could use
write-combining memory and you might even go faster, despite an
explicit fence and thus a minimum of 2 transactions.

Seriously. If you care that deeply about the ordering of the bytes you
write out, MAKE THAT ORDERING VERY EXPLICIT IN THE SOURCE CODE. Don't
say "oh, with this hack, I win 100ns". You need to ask yourself: what
do you care about more? Going really fast on some machine that you can
test, or being safe?

With PCIe, it's *probably* fine to just say "we expect 64-bit accesses
to make it through unmolested".

The 128-bit case I really don't know about. It probably works too. But
while I'd call the 64-bit case almost certain (in the absence of truly
crap hardware), the 128-bit case I have a hard time judging how
certain it is going to be.

                   Linus

^ permalink raw reply

* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Tejun Heo @ 2012-08-22 18:01 UTC (permalink / raw)
  To: Sasha Levin
  Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
	fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	bfields-uC3wQj2KruNg9hUCZPvPmw,
	paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
	aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
	ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
	dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
	josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w,
	axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, lw-BthXqXjhjHXQFUHtdCDX3A,
	teigland-H+wXaHxf7aLQT0dZR+AlfA,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1345602432-27673-2-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hello, Sasha.

On Wed, Aug 22, 2012 at 04:26:56AM +0200, Sasha Levin wrote:
> +#define DEFINE_HASHTABLE(name, bits)					\
> +	struct hlist_head name[HASH_SIZE(bits)];

Shouldn't this be something like the following?

#define DEFINE_HASHTABLE(name, bits)					\
	struct hlist_head name[HASH_SIZE(bits)] =			\
		{ [0 ... HASH_SIZE(bits) - 1] = HLIST_HEAD_INIT };

Also, given that the declaration isn't non-trivial, you'll probably
want a matching DECLARE_HASHTABLE() macro too.

> +/* Use hash_32 when possible to allow for fast 32bit hashing in 64bit kernels. */
> +#define hash_min(val, bits) ((sizeof(val)==4) ? hash_32((val), (bits)) : hash_long((val), (bits)))

Why is the branching condition sizeof(val) == 4 instead of <= 4?
Also, no biggie but why isn't this macro in caps?

> +/**
> + * hash_add_size - add an object to a hashtable
> + * @hashtable: hashtable to add to
> + * @bits: bit count used for hashing
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add_size(hashtable, bits, node, key)				\
> +	hlist_add_head(node, &hashtable[hash_min(key, bits)]);
> +
> +/**
> + * hash_add - add an object to a hashtable
> + * @hashtable: hashtable to add to
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add(hashtable, node, key)						\
> +	hash_add_size(hashtable, HASH_BITS(hashtable), node, key)

It would be nice if the comments actually say something which can
differentiate the two.  Ditto for rcu variants.

> +/**
> + * hash_add_rcu_size - add an object to a rcu enabled hashtable
> + * @hashtable: hashtable to add to
> + * @bits: bit count used for hashing
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add_rcu_size(hashtable, bits, node, key)				\
> +	hlist_add_head_rcu(node, &hashtable[hash_min(key, bits)]);
> +
> +/**
> + * hash_add_rcu - add an object to a rcu enabled hashtable
> + * @hashtable: hashtable to add to
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add_rcu(hashtable, node, key)					\
> +	hash_add_rcu_size(hashtable, HASH_BITS(hashtable), node, key)

Or maybe we're better off with hash_head_size() and hash_head()?  I'll
expand on it later.  Please bear with me.

> +/**
> + * hash_hashed - check whether an object is in any hashtable
> + * @node: the &struct hlist_node of the object to be checked
> + */
> +#define hash_hashed(node) (!hlist_unhashed(node))

As the 'h' in hlist* stand for hash anyway and I think this type of
thin wrappers tend to obfuscate more than anything else.

> +/**
> + * hash_del - remove an object from a hashtable
> + * @node: &struct hlist_node of the object to remove
> + */
> +static inline void hash_del(struct hlist_node *node)
> +{
> +	hlist_del_init(node);
> +}
> +
> +/**
> + * hash_del_rcu - remove an object from a rcu enabled hashtable
> + * @node: &struct hlist_node of the object to remove
> + */
> +static inline void hash_del_rcu(struct hlist_node *node)
> +{
> +	hlist_del_init_rcu(node);
> +}

If we do that, we can remove all these thin wrappers.

> +#define hash_for_each_size(name, bits, bkt, node, obj, member)			\
> +	for (bkt = 0; bkt < HASH_SIZE(bits); bkt++)				\
> +		hlist_for_each_entry(obj, node, &name[bkt], member)
..
> +#define hash_for_each(name, bkt, node, obj, member)				\
> +	hash_for_each_size(name, HASH_BITS(name), bkt, node, obj, member)
...
> +#define hash_for_each_rcu_size(name, bits, bkt, node, obj, member)		\
> +	for (bkt = 0; bkt < HASH_SIZE(bits); bkt++)				\
> +		hlist_for_each_entry_rcu(obj, node, &name[bkt], member)
...
> +#define hash_for_each_rcu(name, bkt, node, obj, member)				\
> +	hash_for_each_rcu_size(name, HASH_BITS(name), bkt, node, obj, member)
...
> +#define hash_for_each_safe_size(name, bits, bkt, node, tmp, obj, member)	\
> +	for (bkt = 0; bkt < HASH_SIZE(bits); bkt++)                     	\
> +		hlist_for_each_entry_safe(obj, node, tmp, &name[bkt], member)
...
> +#define hash_for_each_safe(name, bkt, node, tmp, obj, member)			\
> +	hash_for_each_safe_size(name, HASH_BITS(name), bkt, node,		\
> +				tmp, obj, member)
...
> +#define hash_for_each_possible_size(name, obj, bits, node, member, key)		\
> +	hlist_for_each_entry(obj, node,	&name[hash_min(key, bits)], member)
...
> +#define hash_for_each_possible(name, obj, node, member, key)			\
> +	hash_for_each_possible_size(name, obj, HASH_BITS(name), node, member, key)
...
> +#define hash_for_each_possible_rcu_size(name, obj, bits, node, member, key)	\
> +	hlist_for_each_entry_rcu(obj, node, &name[hash_min(key, bits)], member)
...
> +#define hash_for_each_possible_rcu(name, obj, node, member, key)		\
> +	hash_for_each_possible_rcu_size(name, obj, HASH_BITS(name),		\
...
> +#define hash_for_each_possible_safe_size(name, obj, bits, node, tmp, member, key)\
> +	hlist_for_each_entry_safe(obj, node, tmp,				\
> +		&name[hash_min(key, bits)], member)
...
> +#define hash_for_each_possible_safe(name, obj, node, tmp, member, key)		\
> +	hash_for_each_possible_safe_size(name, obj, HASH_BITS(name),		\

And also all these.  We'd only need hash_for_each_head() and
hash_head().  hash_for_each_possible*() could be nice for convenience,
I suppose.

I think the almost trivial nature of hlist hashtables makes this a bit
tricky and I'm not very sure but having this combinatory explosion is
a bit dazzling when the same functionality can be achieved by simply
combining operations which are already defined and named considering
hashtable.  I'm not feeling too strong about this tho.  What do others
think?

Also, can you please audit the comments on top of each macro?  They
have wrong names and don't differentiate the different variants very
well.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v3 04/17] workqueue: use new hashtable implementation
From: Tejun Heo @ 2012-08-22 18:05 UTC (permalink / raw)
  To: Sasha Levin
  Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
	fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
	Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	bfields-uC3wQj2KruNg9hUCZPvPmw,
	paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
	aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
	ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
	dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
	josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w,
	axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, lw-BthXqXjhjHXQFUHtdCDX3A,
	teigland-H+wXaHxf7aLQT0dZR+AlfA,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1345602432-27673-5-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Wed, Aug 22, 2012 at 04:26:59AM +0200, Sasha Levin wrote:
> Switch workqueues to use the new hashtable implementation. This reduces the amount of
> generic unrelated code in the workqueues.
> 
> Signed-off-by: Sasha Levin <levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Acked-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Ben Hutchings @ 2012-08-22 18:11 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: H. Peter Anvin, David Laight, Benjamin LaHaise, David Miller,
	tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <CA+55aFzcqhHU_+uR33=q+49jK97e=EofFF7g7Z0K1RU_PGwMTw@mail.gmail.com>

On Wed, 2012-08-22 at 10:54 -0700, Linus Torvalds wrote:
> On Wed, Aug 22, 2012 at 10:27 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> >
> > If this is right, how can it be safe to use readq/writeq at all?
> 
> Pray.
> 
> Or don't care about ordering: use hardware that is well-designed and
> doesn't have crap interfaces that are fragile.

Well the whole point of having the two 32-bit generic implementations is
that hardware may care about the order!  How can it be right that a
64-bit implementation assumes it doesn't?

> If you care about ordering, you need to do them as two separate
> accesses, and have a fence in between. Which, quite frankly, sounds
> like the right model for you *anyway*, since then you could use
> write-combining memory and you might even go faster, despite an
> explicit fence and thus a minimum of 2 transactions.

Yes, which unfortunately is no better than we have at the moment.

> Seriously. If you care that deeply about the ordering of the bytes you
> write out, MAKE THAT ORDERING VERY EXPLICIT IN THE SOURCE CODE. Don't
> say "oh, with this hack, I win 100ns". You need to ask yourself: what
> do you care about more? Going really fast on some machine that you can
> test, or being safe?

I have to care quite a lot about both. :-)  But yes, safety first.

> With PCIe, it's *probably* fine to just say "we expect 64-bit accesses
> to make it through unmolested".

I have to hope so.

> The 128-bit case I really don't know about. It probably works too. But
> while I'd call the 64-bit case almost certain (in the absence of truly
> crap hardware), the 128-bit case I have a hard time judging how
> certain it is going to be.

Right, I think it's been made pretty clear that it's going to be
dependent on more than just architecture.

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 18:18 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Linus Torvalds, David Laight, Benjamin LaHaise, David Miller,
	tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345659074.2709.80.camel@bwh-desktop.uk.solarflarecom.com>

On 08/22/2012 11:11 AM, Ben Hutchings wrote:
> 
> Well the whole point of having the two 32-bit generic implementations is
> that hardware may care about the order!  How can it be right that a
> 64-bit implementation assumes it doesn't?
> 

On x86 platforms it is pretty much universal (as in: I have never seen
an exception) that transactions that are broken up are broken up in
littleendian order.  That was the original readq/writeq implementation
on x86-32, but it screwed up some other architectures.

The other reason to not have readq/writeq by default where we *know* it
can't be supported is that some drivers (e.g. the CNIC/OPA-2 driver I
mentioned) can do better, performance-wise, if it knows that.

	-hpa

^ permalink raw reply

* Unusual page allocation failure (network softirq related)
From: David @ 2012-08-22 18:24 UTC (permalink / raw)
  To: linux-kernel, netdev

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

Hi

Just for information, my home server produced the following allocation
failure this morning. First time I've seen it on this box. All seems
fine though, so no symptoms.

config & dmesg attached.

Cheers
David


Aug 22 08:52:29 server kernel: [302505.069788] swapper/0: page
allocation failure: order:0, mode:0x120
Aug 22 08:52:29 server kernel: [302505.069794] Pid: 0, comm: swapper/0
Tainted: G           O 3.5.2 #2
Aug 22 08:52:29 server kernel: [302505.069796] Call Trace:
Aug 22 08:52:29 server kernel: [302505.069798]  <IRQ> 
[<ffffffff810e15f1>] warn_alloc_failed+0xf1/0x140
Aug 22 08:52:29 server kernel: [302505.069809]  [<ffffffff814c82b0>] ?
ip_rcv+0x2c0/0x2c0
Aug 22 08:52:29 server kernel: [302505.069813]  [<ffffffff810e28a1>]
__alloc_pages_nodemask+0x591/0x840
Aug 22 08:52:29 server kernel: [302505.069817]  [<ffffffff814e7e25>] ?
tcp4_gro_receive+0x55/0xc0
Aug 22 08:52:29 server kernel: [302505.069821]  [<ffffffff8148ca1d>]
netdev_alloc_frag+0xbd/0xf0
Aug 22 08:52:29 server kernel: [302505.069823]  [<ffffffff8148cad0>]
__netdev_alloc_skb+0x80/0xc0
Aug 22 08:52:29 server kernel: [302505.069842]  [<ffffffffa01a9c7d>]
rtl8169_poll+0x3fd/0x670 [r8169]
Aug 22 08:52:29 server kernel: [302505.069845]  [<ffffffff81496d3f>]
net_rx_action+0xef/0x230
Aug 22 08:52:29 server kernel: [302505.069849]  [<ffffffff8103997f>]
__do_softirq+0xaf/0x1e0
Aug 22 08:52:29 server kernel: [302505.069854]  [<ffffffff815b4c0c>]
call_softirq+0x1c/0x30
Aug 22 08:52:29 server kernel: [302505.069858]  [<ffffffff81003fad>]
do_softirq+0x4d/0x80
Aug 22 08:52:29 server kernel: [302505.069860]  [<ffffffff810397a5>]
irq_exit+0xb5/0xd0
Aug 22 08:52:29 server kernel: [302505.069863]  [<ffffffff81003911>]
do_IRQ+0x61/0xe0
Aug 22 08:52:29 server kernel: [302505.069867]  [<ffffffff815ac4a7>]
common_interrupt+0x67/0x67
Aug 22 08:52:29 server kernel: [302505.069868]  <EOI> 
[<ffffffff8100aa75>] ? default_idle+0x55/0x190
Aug 22 08:52:29 server kernel: [302505.069875]  [<ffffffff8100ac33>]
amd_e400_idle+0x83/0x100
Aug 22 08:52:29 server kernel: [302505.069877]  [<ffffffff8100a1e6>]
cpu_idle+0x86/0xd0
Aug 22 08:52:29 server kernel: [302505.069881]  [<ffffffff81595338>]
rest_init+0x68/0x70
Aug 22 08:52:29 server kernel: [302505.069885]  [<ffffffff81ab2cc3>]
start_kernel+0x344/0x351
Aug 22 08:52:29 server kernel: [302505.069888]  [<ffffffff81ab279f>] ?
kernel_init+0x1ca/0x1ca
Aug 22 08:52:29 server kernel: [302505.069891]  [<ffffffff81ab232d>]
x86_64_start_reservations+0x131/0x136
Aug 22 08:52:29 server kernel: [302505.069894]  [<ffffffff81ab241f>]
x86_64_start_kernel+0xed/0xf4
Aug 22 08:52:29 server kernel: [302505.069895] Mem-Info:
Aug 22 08:52:29 server kernel: [302505.069897] DMA per-cpu:
Aug 22 08:52:29 server kernel: [302505.069899] CPU    0: hi:    0,
btch:   1 usd:   0
Aug 22 08:52:29 server kernel: [302505.069900] CPU    1: hi:    0,
btch:   1 usd:   0
Aug 22 08:52:29 server kernel: [302505.069902] DMA32 per-cpu:
Aug 22 08:52:29 server kernel: [302505.069903] CPU    0: hi:  186,
btch:  31 usd: 175
Aug 22 08:52:29 server kernel: [302505.069905] CPU    1: hi:  186,
btch:  31 usd:  31
Aug 22 08:52:29 server kernel: [302505.069909] active_anon:50844
inactive_anon:54976 isolated_anon:0
Aug 22 08:52:29 server kernel: [302505.069909]  active_file:57409
inactive_file:235120 isolated_file:0
Aug 22 08:52:29 server kernel: [302505.069909]  unevictable:0
dirty:74711 writeback:8193 unstable:0
Aug 22 08:52:29 server kernel: [302505.069909]  free:2248
slab_reclaimable:16276 slab_unreclaimable:7333
Aug 22 08:52:29 server kernel: [302505.069909]  mapped:10831 shmem:8286
pagetables:3710 bounce:0
Aug 22 08:52:29 server kernel: [302505.069917] DMA free:7004kB min:44kB
low:52kB high:64kB active_anon:0kB inactive_anon:44kB active_file:36kB
inactive_file:2216kB unevictable:0kB isolated(anon):0kB
isolated(file):0kB present:15632kB mlocked:0kB dirty:248kB writeback:0kB
mapped:4kB shmem:4kB slab_reclaimable:6528kB slab_unreclaimable:16kB
kernel_stack:8kB pagetables:0kB unstable:0kB bounce:0kB
writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
Aug 22 08:52:29 server kernel: [302505.069919] lowmem_reserve[]: 0 1747
1747 1747
Aug 22 08:52:29 server kernel: [302505.069926] DMA32 free:1988kB
min:5324kB low:6652kB high:7984kB active_anon:203376kB
inactive_anon:219860kB active_file:229600kB inactive_file:938264kB
unevictable:0kB isolated(anon):0kB isolated(file):0kB present:1789764kB
mlocked:0kB dirty:298596kB writeback:32772kB mapped:43320kB
shmem:33140kB slab_reclaimable:58576kB slab_unreclaimable:29316kB
kernel_stack:3240kB pagetables:14840kB unstable:0kB bounce:0kB
writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
Aug 22 08:52:29 server kernel: [302505.069928] lowmem_reserve[]: 0 0 0 0
Aug 22 08:52:29 server kernel: [302505.069931] DMA: 8*4kB 6*8kB 9*16kB
10*32kB 7*64kB 9*128kB 5*256kB 3*512kB 0*1024kB 1*2048kB 0*4096kB = 7008kB
Aug 22 08:52:29 server kernel: [302505.069938] DMA32: 14*4kB 11*8kB
10*16kB 11*32kB 2*64kB 1*128kB 0*256kB 2*512kB 0*1024kB 0*2048kB
0*4096kB = 1936kB
Aug 22 08:52:29 server kernel: [302505.069944] 312175 total pagecache pages
Aug 22 08:52:29 server kernel: [302505.069946] 11352 pages in swap cache
Aug 22 08:52:29 server kernel: [302505.069947] Swap cache stats: add
92803, delete 81451, find 4745898/4749655
Aug 22 08:52:29 server kernel: [302505.069949] Free swap  = 3652688kB
Aug 22 08:52:29 server kernel: [302505.069950] Total swap = 3903676kB
Aug 22 08:52:29 server kernel: [302505.077625] 458624 pages RAM
Aug 22 08:52:29 server kernel: [302505.077626] 10730 pages reserved
Aug 22 08:52:29 server kernel: [302505.077627] 241182 pages shared
Aug 22 08:52:29 server kernel: [302505.077628] 233322 pages non-shared


[-- Attachment #2: config.bz2 --]
[-- Type: application/x-bzip, Size: 29749 bytes --]

[-- Attachment #3: dmesg.bz2 --]
[-- Type: application/x-bzip, Size: 16819 bytes --]

^ permalink raw reply

* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 18:28 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: H. Peter Anvin, David Laight, Benjamin LaHaise, David Miller,
	tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345659074.2709.80.camel@bwh-desktop.uk.solarflarecom.com>

On Wed, Aug 22, 2012 at 11:11 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> Right, I think it's been made pretty clear that it's going to be
> dependent on more than just architecture.

Well, it's entirely possible that the 128-bit case will work correctly
on all x86-64 hardware out there on PCIe.

I just can't guarantee it, because we certainly have had issues with
hw doing odd things before. But maybe PCIe really is well-specified
enough, and maybe nobody has done a odd PCIe bridges, and maybe every
time some 128-bit access is split, the bus in question still always
remembers the original 128-bit size in the transaction. It's not at
all impossible. I just wouldn't *guarantee* it.

And to some degree, for high-end server-only hardware in particular,
it really *is* acceptable to say "If you have odd hardware, odd things
will happen". So for this particular driver, maybe the right approach
is simply to say "we require that your fabric works right". And see if
anybody ever complains.

The 100ns may be worth those kinds of "you'd better not have old/crap
hardware" decisions. It's not acceptable for some drivers (a driver
for some consumer ATA chip might not want to make that kind of choice,
and say "whatever, we'll be really conservative), but "Quod licet
Jovi, non licet bovi".

The fact that something might not be *guaranteed* to always work
doesn't necessarily mean that it is always the wrong thing to do..

              Linus

^ permalink raw reply

* Re: bonding: time limits too tight in bond_ab_arp_inspect
From: Jay Vosburgh @ 2012-08-22 18:42 UTC (permalink / raw)
  To: Chris Friesen; +Cc: Jiri Bohac, Andy Gospodarek, netdev, Petr Tesarik
In-Reply-To: <50351CC5.3030109@genband.com>

Chris Friesen <chris.friesen@genband.com> wrote:

>On 08/22/2012 11:45 AM, Jiri Bohac wrote:
>
>> This code is run from bond_activebackup_arp_mon() about
>> delta_in_ticks jiffies after the previous ARP probe has been
>> sent. If the delayed work gets executed exactly in delta_in_ticks
>> jiffies, there is a chance the slave will be brought up.  If the
>> delayed work runs one jiffy later, the slave will stay down.

	Presumably the ARP reply is coming back in less than one jiffy,
then, so the slave_last_rx() value is the same jiffy as when the
_inspect was previously called?

><snip>
>
>> Should they perhaps all be increased by, say, delta_in_ticks/2, to make this
>> less dependent on the current scheduling latencies?
>
>We have been using a patch that tracks the arpmon requested sleep time vs
>the actual sleep time and adds any scheduling latency to the allowed
>delta.  That way if we sleep too long due to scheduling latency it doesn't
>affect the calculation.

	How much scheduling latency do you see?

	Is that really better than just permitting a bit more slack in
the timing window?

	As to the 2 * delta and 3 * delta calculations, these values
predate my involvement with bonding, so I'm not entirely sure why those
specific values were chosen (there are no log messages from that era
that I'm aware of).  My presumption has been that this part:

                /*
                 * Active slave is down if:
                 * - more than 2*delta since transmitting OR
                 * - (more than 2*delta since receive AND
                 *    the bond has an IP address)
                 */
                trans_start = dev_trans_start(slave->dev);
                if (bond_is_active_slave(slave) &&
                    (!time_in_range(jiffies,
                        trans_start - delta_in_ticks,
                        trans_start + 2 * delta_in_ticks) ||
                     !time_in_range(jiffies,
                        slave_last_rx(bond, slave) - delta_in_ticks,
                        slave_last_rx(bond, slave) + 2 * delta_in_ticks))) {

                        slave->new_link = BOND_LINK_DOWN;
                        commit++;
                }

	was structured this way (allowing 2 * delta) to permit the loss
of a single ARP on an otherwise idle interface without triggering a link
down.

	My guess, though, is that until relatively recently the timing
window was not too tight, and there was effectively some slack in the
calculation, because the slave_last_rx() would be set to some small
number of jiffies after the last exection of the monitor, and so the
"slave_last_rx() + delta_in_ticks" wasn't as narrow a window as it
appears to be now.

	So, without having tested this myself, based on the above, I
don't see that adding some slack would be a problem.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: bonding: time limits too tight in bond_ab_arp_inspect
From: Chris Friesen @ 2012-08-22 18:58 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Jiri Bohac, Andy Gospodarek, netdev, Petr Tesarik
In-Reply-To: <24655.1345660922@death.nxdomain>

On 08/22/2012 12:42 PM, Jay Vosburgh wrote:
> Chris Friesen<chris.friesen@genband.com>  wrote:
>
>> On 08/22/2012 11:45 AM, Jiri Bohac wrote:
>>
>>> This code is run from bond_activebackup_arp_mon() about
>>> delta_in_ticks jiffies after the previous ARP probe has been
>>> sent. If the delayed work gets executed exactly in delta_in_ticks
>>> jiffies, there is a chance the slave will be brought up.  If the
>>> delayed work runs one jiffy later, the slave will stay down.
>
> 	Presumably the ARP reply is coming back in less than one jiffy,
> then, so the slave_last_rx() value is the same jiffy as when the
> _inspect was previously called?
>
>> <snip>
>>
>>> Should they perhaps all be increased by, say, delta_in_ticks/2, to make this
>>> less dependent on the current scheduling latencies?
>>
>> We have been using a patch that tracks the arpmon requested sleep time vs
>> the actual sleep time and adds any scheduling latency to the allowed
>> delta.  That way if we sleep too long due to scheduling latency it doesn't
>> affect the calculation.
>
> 	How much scheduling latency do you see?
>
> 	Is that really better than just permitting a bit more slack in
> the timing window?

We hit enough latency that it triggered arpmon to falsely mark multiple 
links as lost.  This triggered our system maintenance code to go into a 
"oh no we can't talk to the outside world" secenario, which does fairly 
intrusive things to try and bring connectivity back up.  Basically a bad 
thing to happen just because of a random scheduler latency spike.

I should note that we added this some time back and are still running 
older kernels so I have no idea what latency on modern kernels is like.

Chris

^ permalink raw reply


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