* Re: TCP performance regression
From: Eric Dumazet @ 2013-11-12 14:16 UTC (permalink / raw)
To: Willy Tarreau; +Cc: David Laight, Sujith Manoharan, netdev, Dave Taht
In-Reply-To: <20131112074243.GA10318@1wt.eu>
On Tue, 2013-11-12 at 08:42 +0100, Willy Tarreau wrote:
> Well, it's not *that* large, 532 descriptors is 800 kB or 6.4 ms with
> 1500-bytes packets, and 273 microseconds for 64-byte packets. In fact
> it's not a slow interface, it's the systems it runs on which are
> generally not that fast. For example it is possible to saturate two
> gig ports at once on a single-core Armada370. But you need buffers
> large enough to compensate for the context switch time if you use
> multiple threads to send.
With GSO, each 1500-bytes packet might need 2 descriptors anyway (one
for the headers, one for the payload), so 532 descriptors only hold 400
kB or 3.2 ms ;)
If the NIC was supporting TSO, this would be another story, as a 64KB
packet could use only 3 descriptors.
^ permalink raw reply
* Re: [PATCH RFC 07/10] PM / wakeup : Introduce device_child_may_wakeup
From: Rafael J. Wysocki @ 2013-11-12 14:20 UTC (permalink / raw)
To: srinivas.kandagatla
Cc: linux-arm-kernel, netdev, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Stuart Menefy, Pavel Machek, Len Brown, stephen.gallimore,
Greg Kroah-Hartman, Giuseppe Cavallaro, Grant Likely, devicetree,
linux-doc, linux-kernel, kernel, linux-pm
In-Reply-To: <1384264377-7609-1-git-send-email-srinivas.kandagatla@st.com>
On Tuesday, November 12, 2013 01:52:57 PM srinivas.kandagatla@st.com wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>
> This patch introduces device_child_may_wakeup function, which will be
> useful for wrapper or SoC level driver power management code.
> Without this patch each driver has to write this same code to get the
> functionality.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> ---
> drivers/base/power/wakeup.c | 23 +++++++++++++++++++++++
> include/linux/pm_wakeup.h | 1 +
> 2 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 2d56f41..270f000 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -342,6 +342,29 @@ int device_set_wakeup_enable(struct device *dev, bool enable)
> }
> EXPORT_SYMBOL_GPL(device_set_wakeup_enable);
>
> +/* callback for device_child_may_wakeup */
> +static int __device_child_may_wakeup(struct device *dev, void *c)
> +{
> + return device_may_wakeup(dev);
> +}
This doesn't have anything to do with children in principle, so please call
it differently. Something like device_may_wakeup_cb() would work for me (and
then you may not need the comment even).
Thanks!
> +
> +/**
> + * device_child_may_wakeup - Check if any of the child devices are wakeup
> + * sources.
> + * @dev: parent device to handle.
> + *
> + * Function to check if any of the children of a given parent are wakeup
> + * sources.
> + *
> + * This function will return true if any one of the children of given parent
> + * are wakeup sources, else it returns false.
> + */
> +bool device_child_may_wakeup(struct device *parent)
> +{
> + return device_for_each_child(parent, NULL, __device_child_may_wakeup);
> +}
> +EXPORT_SYMBOL_GPL(device_child_may_wakeup);
> +
> /*
> * The functions below use the observation that each wakeup event starts a
> * period in which the system should not be suspended. The moment this period
> diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
> index a0f7080..b376584 100644
> --- a/include/linux/pm_wakeup.h
> +++ b/include/linux/pm_wakeup.h
> @@ -93,6 +93,7 @@ extern int device_wakeup_disable(struct device *dev);
> extern void device_set_wakeup_capable(struct device *dev, bool capable);
> extern int device_init_wakeup(struct device *dev, bool val);
> extern int device_set_wakeup_enable(struct device *dev, bool enable);
> +bool device_child_may_wakeup(struct device *parent);
> extern void __pm_stay_awake(struct wakeup_source *ws);
> extern void pm_stay_awake(struct device *dev);
> extern void __pm_relax(struct wakeup_source *ws);
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH net 1/2] tuntap: limit head length of skb allocated
From: Jason Wang @ 2013-11-12 14:21 UTC (permalink / raw)
To: davem, netdev, linux-kernel, mst, stefanha
In-Reply-To: <1384250577-20330-1-git-send-email-jasowang@redhat.com>
----- 原始邮件 -----
> We currently use hdr_len as a hint of head length which is advertised by
> guest. But when guest advertise a very big value, it can lead to an 64K+
> allocating of kmalloc() which has a very high possibility of failure when
> host
> memory is fragmented or under heavy stress. The huge hdr_len also reduce the
> effect of zerocopy or even disable if a gso skb is linearized in guest.
>
> To solves those issues, this patch introduces an upper limit (PAGE_SIZE) of
> the
> head, which guarantees an order 0 allocation each time.
>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> The patch was needed for stable.
> ---
> drivers/net/tun.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 7cb105c..5537b65 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -954,6 +954,11 @@ static struct sk_buff *tun_alloc_skb(struct tun_file
> *tfile,
> struct sock *sk = tfile->socket.sk;
> struct sk_buff *skb;
> int err;
> + int good_linear = SKB_MAX_HEAD(prepad);
> +
> + /* Don't use huge linear part */
> + if (linear > good_linear)
> + linear = good_linear;
>
> /* Under a page? Don't bother with paged skb. */
> if (prepad + len < PAGE_SIZE || !linear)
> --
> 1.8.3.2
>
Looks like reduce the linear here will cause callers' iov_pages() under estimation.
Will send V2.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply
* RE: [PATCH] ipvs: Remove unused variable ret from sync_thread_master()
From: David Laight @ 2013-11-12 14:21 UTC (permalink / raw)
To: Peter Zijlstra, Geert Uytterhoeven
Cc: Oleg Nesterov, Ingo Molnar, netdev, linux-kernel
In-Reply-To: <20131112141314.GQ5056@laptop.programming.kicks-ass.net>
> > @@ -1637,7 +1637,7 @@ static int sync_thread_master(void *data)
> > continue;
> > }
> > while (ip_vs_send_sync_msg(tinfo->sock, sb->mesg) < 0) {
> > - int ret = __wait_event_interruptible(*sk_sleep(sk),
>
> So ideally there's be a comment here why we're using interruptible but
> then ignore interruptions.
>
> Julian said (
> http://lkml.kernel.org/r/alpine.LFD.2.00.1310012245020.1782@ja.ssi.bg ):
>
> " Yes, your patch looks ok to me. In the past
> we used ssleep() but IPVS users were confused why
> IPVS threads increase the load average. So, we
> switched to _interruptible calls and later the socket
> polling was added. "
I've done this in the past so that the code sleeps interruptibly
unless there is a signal pending - which would cause it to return
early.
/* Tell scheduler we are going to sleep... */
if (signal_pending(current))
/* We don't want waking immediately (again) */
sleep_state = TASK_UNINTERRUPTIBLE;
else
sleep_state = TASK_INTERRUPTIBLE;
set_current_state(sleep_state);
Shame there isn't a process flag to indicate that the process
will sleep uninterruptibly and that it doesn't matter.
So don't count to the load average and don't emit a warning
if it has been sleeping for a long time.
David
^ permalink raw reply
* Re: [PATCH] ipvs: Remove unused variable ret from sync_thread_master()
From: Peter Zijlstra @ 2013-11-12 14:31 UTC (permalink / raw)
To: David Laight
Cc: Geert Uytterhoeven, Oleg Nesterov, Ingo Molnar, netdev,
linux-kernel
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7405@saturn3.aculab.com>
On Tue, Nov 12, 2013 at 02:21:39PM -0000, David Laight wrote:
> > > @@ -1637,7 +1637,7 @@ static int sync_thread_master(void *data)
> > > continue;
> > > }
> > > while (ip_vs_send_sync_msg(tinfo->sock, sb->mesg) < 0) {
> > > - int ret = __wait_event_interruptible(*sk_sleep(sk),
> >
> > So ideally there's be a comment here why we're using interruptible but
> > then ignore interruptions.
> >
> > Julian said (
> > http://lkml.kernel.org/r/alpine.LFD.2.00.1310012245020.1782@ja.ssi.bg ):
> >
> > " Yes, your patch looks ok to me. In the past
> > we used ssleep() but IPVS users were confused why
> > IPVS threads increase the load average. So, we
> > switched to _interruptible calls and later the socket
> > polling was added. "
>
> I've done this in the past so that the code sleeps interruptibly
> unless there is a signal pending - which would cause it to return
> early.
>
> /* Tell scheduler we are going to sleep... */
> if (signal_pending(current))
> /* We don't want waking immediately (again) */
> sleep_state = TASK_UNINTERRUPTIBLE;
> else
> sleep_state = TASK_INTERRUPTIBLE;
> set_current_state(sleep_state);
If this is for kernel threads, I think you can wipe the pending state;
not entirely sure how signals interact with kthreads; Oleg will know.
^ permalink raw reply
* [PATCH -v2] ipvs: Remove unused variable ret from sync_thread_master()
From: Geert Uytterhoeven @ 2013-11-12 14:34 UTC (permalink / raw)
To: Peter Zijlstra, Oleg Nesterov, Ingo Molnar, Julian Anastasov
Cc: netdev, linux-kernel, Geert Uytterhoeven
net/netfilter/ipvs/ip_vs_sync.c: In function 'sync_thread_master':
net/netfilter/ipvs/ip_vs_sync.c:1640:8: warning: unused variable 'ret' [-Wunused-variable]
Commit 35a2af94c7ce7130ca292c68b1d27fcfdb648f6b ("sched/wait: Make the
__wait_event*() interface more friendly") changed how the interruption
state is returned. However, sync_thread_master() ignores this state,
now causing a compile warning.
According to Julian Anastasov <ja@ssi.bg>, this behavior is OK:
"Yes, your patch looks ok to me. In the past we used ssleep() but IPVS
users were confused why IPVS threads increase the load average. So, we
switched to _interruptible calls and later the socket polling was
added."
Document this, as requested by Peter Zijlstra, to avoid precious developers
disappearing in this pitfall in the future.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2: Document that sync_thread_master() ignores the interruption state,
net/netfilter/ipvs/ip_vs_sync.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index f63c2388f38d..db801263ee9f 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1637,7 +1637,10 @@ static int sync_thread_master(void *data)
continue;
}
while (ip_vs_send_sync_msg(tinfo->sock, sb->mesg) < 0) {
- int ret = __wait_event_interruptible(*sk_sleep(sk),
+ /* (Ab)use interruptible sleep to avoid increasing
+ * the load avg.
+ */
+ __wait_event_interruptible(*sk_sleep(sk),
sock_writeable(sk) ||
kthread_should_stop());
if (unlikely(kthread_should_stop()))
--
1.7.9.5
^ permalink raw reply related
* [PATCH net] bonding: don't permit to use ARP monitoring in 802.3ad mode
From: Veaceslav Falico @ 2013-11-12 14:37 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
Currently the ARP monitoring is not supported with 802.3ad, and it's
prohibited to use it via the module params.
However we still can set it afterwards via sysfs, cause we only check for
*LB modes there.
To fix this - add a check for 802.3ad mode in bonding_store_arp_interval.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_sysfs.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index c29b836..e386d6b 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -587,8 +587,9 @@ static ssize_t bonding_store_arp_interval(struct device *d,
goto out;
}
if (bond->params.mode == BOND_MODE_ALB ||
- bond->params.mode == BOND_MODE_TLB) {
- pr_info("%s: ARP monitoring cannot be used with ALB/TLB. Only MII monitoring is supported on %s.\n",
+ bond->params.mode == BOND_MODE_TLB ||
+ bond->params.mode == BOND_MODE_8023AD) {
+ pr_info("%s: ARP monitoring cannot be used with ALB/TLB/802.3ad. Only MII monitoring is supported on %s.\n",
bond->dev->name, bond->dev->name);
ret = -EINVAL;
goto out;
--
1.8.4
^ permalink raw reply related
* [PATCH] tcp: tsq: restore minimal amount of queueing
From: Eric Dumazet @ 2013-11-12 14:39 UTC (permalink / raw)
To: Arnaud Ebalard, David Miller, Sujith Manoharan
Cc: Cong Wang, netdev, Felix Fietkau
In-Reply-To: <87y54u59zq.fsf@natisbad.org>
From: Eric Dumazet <edumazet@google.com>
After commit c9eeec26e32e ("tcp: TSQ can use a dynamic limit"), several
users reported throughput regressions, notably on mvneta and wifi
adapters.
802.11 AMPDU requires a fair amount of queueing to be effective.
This patch partially reverts the change done in tcp_write_xmit()
so that the minimal amount is sysctl_tcp_limit_output_bytes.
It also remove the use of this sysctl while building skb stored
in write queue, as TSO autosizing does the right thing anyway.
Users with well behaving NICS and correct qdisc (like sch_fq),
can then lower the default sysctl_tcp_limit_output_bytes value from
128KB to 8KB.
The new usage of sysctl_tcp_limit_output_bytes permits each driver
author to check how driver performs when/if the value is set
to a minimum of 4KB :
Normally, line rate for a single TCP flow should be possible,
but some drivers rely on timers to perform TX completion and
too long delays prevent reaching full throughput.
Fixes: c9eeec26e32e ("tcp: TSQ can use a dynamic limit")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Sujith Manoharan <sujith@msujith.org>
Reported-by: Arnaud Ebalard <arno@natisbad.org>
Cc: Felix Fietkau <nbd@openwrt.org>
---
Documentation/networking/ip-sysctl.txt | 3 ---
net/ipv4/tcp.c | 6 ------
net/ipv4/tcp_output.c | 6 +++++-
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index a46d785..7d8dc93 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -588,9 +588,6 @@ tcp_limit_output_bytes - INTEGER
typical pfifo_fast qdiscs.
tcp_limit_output_bytes limits the number of bytes on qdisc
or device to reduce artificial RTT/cwnd and reduce bufferbloat.
- Note: For GSO/TSO enabled flows, we try to have at least two
- packets in flight. Reducing tcp_limit_output_bytes might also
- reduce the size of individual GSO packet (64KB being the max)
Default: 131072
tcp_challenge_ack_limit - INTEGER
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 6e5617b..be5246e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -806,12 +806,6 @@ static unsigned int tcp_xmit_size_goal(struct sock *sk, u32 mss_now,
xmit_size_goal = min_t(u32, gso_size,
sk->sk_gso_max_size - 1 - hlen);
- /* TSQ : try to have at least two segments in flight
- * (one in NIC TX ring, another in Qdisc)
- */
- xmit_size_goal = min_t(u32, xmit_size_goal,
- sysctl_tcp_limit_output_bytes >> 1);
-
xmit_size_goal = tcp_bound_to_half_wnd(tp, xmit_size_goal);
/* We try hard to avoid divides here */
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index d46f214..9f0b338 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1875,8 +1875,12 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
* - better RTT estimation and ACK scheduling
* - faster recovery
* - high rates
+ * Alas, some drivers / subsystems require a fair amount
+ * of queued bytes to ensure line rate.
+ * One example is wifi aggregation (802.11 AMPDU)
*/
- limit = max(skb->truesize, sk->sk_pacing_rate >> 10);
+ limit = max(sysctl_tcp_limit_output_bytes,
+ sk->sk_pacing_rate >> 10);
if (atomic_read(&sk->sk_wmem_alloc) > limit) {
set_bit(TSQ_THROTTLED, &tp->tsq_flags);
^ permalink raw reply related
* RE: [PATCH] ipvs: Remove unused variable ret from sync_thread_master()
From: David Laight @ 2013-11-12 14:38 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Geert Uytterhoeven, Oleg Nesterov, Ingo Molnar, netdev,
linux-kernel
In-Reply-To: <20131112143152.GS5056@laptop.programming.kicks-ass.net>
> > I've done this in the past so that the code sleeps interruptibly
> > unless there is a signal pending - which would cause it to return
> > early.
> >
> > /* Tell scheduler we are going to sleep... */
> > if (signal_pending(current))
> > /* We don't want waking immediately (again) */
> > sleep_state = TASK_UNINTERRUPTIBLE;
> > else
> > sleep_state = TASK_INTERRUPTIBLE;
> > set_current_state(sleep_state);
>
> If this is for kernel threads, I think you can wipe the pending state;
> not entirely sure how signals interact with kthreads; Oleg will know.
I did take me a while to manage to use kthreads, and we probably still
have customers who insist on using pre 2.6.18 kernels!
(In which case the processes are children of a daemon that only
return to userspace in order to exit.)
We also need to send signals (to get the process out of blocking
socket calls), so I have to use force_sig() to get the signal noticed.
The other issue was tidyup - I had to find module_put_and_exit().
David
^ permalink raw reply
* Re:Answer back
From: lee hyuk @ 2013-11-12 10:52 UTC (permalink / raw)
I would like to discuss a very important crude oil project withyou,kindly
revert back to me if this is your valid email address forfurther
information.
Regards,
Lee
^ permalink raw reply
* Re: [PATCH] ipvs: Remove unused variable ret from sync_thread_master()
From: Peter Zijlstra @ 2013-11-12 14:52 UTC (permalink / raw)
To: David Laight
Cc: Geert Uytterhoeven, Oleg Nesterov, Ingo Molnar, netdev,
linux-kernel
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7405@saturn3.aculab.com>
On Tue, Nov 12, 2013 at 02:21:39PM -0000, David Laight wrote:
> Shame there isn't a process flag to indicate that the process
> will sleep uninterruptibly and that it doesn't matter.
> So don't count to the load average and don't emit a warning
> if it has been sleeping for a long time.
A process flag wouldn't work, because the task could block waiting for
actual work to complete in other sleeps.
However, we could do something like the below; which would allow us
writing things like:
(void)___wait_event(*sk_sleep(sk),
sock_writeable(sk) || kthread_should_stop(),
TASK_UNINTERRUPTIBLE | TASK_IDLE, 0, 0,
schedule());
Marking the one wait-for-more-work as TASK_IDLE such that it doesn't
contribute to the load avg.
---
fs/proc/array.c | 23 ++++++++++++-----------
include/linux/sched.h | 8 +++++---
include/trace/events/sched.h | 3 ++-
3 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 1bd2077187fd..da9a9c8b5bba 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -133,17 +133,18 @@ static inline void task_name(struct seq_file *m, struct task_struct *p)
* simple bit tests.
*/
static const char * const task_state_array[] = {
- "R (running)", /* 0 */
- "S (sleeping)", /* 1 */
- "D (disk sleep)", /* 2 */
- "T (stopped)", /* 4 */
- "t (tracing stop)", /* 8 */
- "Z (zombie)", /* 16 */
- "X (dead)", /* 32 */
- "x (dead)", /* 64 */
- "K (wakekill)", /* 128 */
- "W (waking)", /* 256 */
- "P (parked)", /* 512 */
+ "R (running)", /* 0 */
+ "S (sleeping)", /* 1 */
+ "D (disk sleep)", /* 2 */
+ "T (stopped)", /* 4 */
+ "t (tracing stop)", /* 8 */
+ "Z (zombie)", /* 16 */
+ "X (dead)", /* 32 */
+ "x (dead)", /* 64 */
+ "K (wakekill)", /* 128 */
+ "W (waking)", /* 256 */
+ "P (parked)", /* 512 */
+ "I (idle)", /* 1024 */
};
static inline const char *get_task_state(struct task_struct *tsk)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 045b0d227846..a4a9e80688d8 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -145,9 +145,10 @@ print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq);
#define TASK_WAKEKILL 128
#define TASK_WAKING 256
#define TASK_PARKED 512
-#define TASK_STATE_MAX 1024
+#define TASK_IDLE 1024
+#define TASK_STATE_MAX 2048
-#define TASK_STATE_TO_CHAR_STR "RSDTtZXxKWP"
+#define TASK_STATE_TO_CHAR_STR "RSDTtZXxKWPI"
extern char ___assert_task_state[1 - 2*!!(
sizeof(TASK_STATE_TO_CHAR_STR)-1 != ilog2(TASK_STATE_MAX)+1)];
@@ -173,7 +174,8 @@ extern char ___assert_task_state[1 - 2*!!(
((task->state & (__TASK_STOPPED | __TASK_TRACED)) != 0)
#define task_contributes_to_load(task) \
((task->state & TASK_UNINTERRUPTIBLE) != 0 && \
- (task->flags & PF_FROZEN) == 0)
+ (task->flags & PF_FROZEN) == 0 && \
+ (task->state & TASK_IDLE) == 0)
#define __set_task_state(tsk, state_value) \
do { (tsk)->state = (state_value); } while (0)
diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
index 04c308413a5d..4553a2495c25 100644
--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -144,7 +144,8 @@ TRACE_EVENT(sched_switch,
__print_flags(__entry->prev_state & (TASK_STATE_MAX-1), "|",
{ 1, "S"} , { 2, "D" }, { 4, "T" }, { 8, "t" },
{ 16, "Z" }, { 32, "X" }, { 64, "x" },
- { 128, "K" }, { 256, "W" }, { 512, "P" }) : "R",
+ { 128, "K" }, { 256, "W" }, { 512, "P" },
+ { 1024, "I" }) : "R",
__entry->prev_state & TASK_STATE_MAX ? "+" : "",
__entry->next_comm, __entry->next_pid, __entry->next_prio)
);
^ permalink raw reply related
* Re: [PATCH] tcp: tsq: restore minimal amount of queueing
From: Sujith Manoharan @ 2013-11-12 15:24 UTC (permalink / raw)
To: Eric Dumazet
Cc: Arnaud Ebalard, David Miller, Cong Wang, netdev, Felix Fietkau
In-Reply-To: <1384267141.28458.24.camel@edumazet-glaptop2.roam.corp.google.com>
Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> After commit c9eeec26e32e ("tcp: TSQ can use a dynamic limit"), several
> users reported throughput regressions, notably on mvneta and wifi
> adapters.
>
> 802.11 AMPDU requires a fair amount of queueing to be effective.
>
> This patch partially reverts the change done in tcp_write_xmit()
> so that the minimal amount is sysctl_tcp_limit_output_bytes.
>
> It also remove the use of this sysctl while building skb stored
> in write queue, as TSO autosizing does the right thing anyway.
>
> Users with well behaving NICS and correct qdisc (like sch_fq),
> can then lower the default sysctl_tcp_limit_output_bytes value from
> 128KB to 8KB.
>
> The new usage of sysctl_tcp_limit_output_bytes permits each driver
> author to check how driver performs when/if the value is set
> to a minimum of 4KB :
>
> Normally, line rate for a single TCP flow should be possible,
> but some drivers rely on timers to perform TX completion and
> too long delays prevent reaching full throughput.
I tested the patch with ath9k and performance with a 2-stream card is normal
again, about 195 Mbps in open air.
Thanks for the fix !
Also, I think this needs to be marked as a stable candidate, since 3.12 needs
this fix.
Sujith
^ permalink raw reply
* Re: [PATCH net] netlink: fix netlink_ack with large messages
From: Jiri Benc @ 2013-11-12 15:29 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Jamal Hadi Salim, David Miller, tgraf, netdev
In-Reply-To: <20131109180353.GA3715@localhost>
On Sat, 9 Nov 2013 19:03:53 +0100, Pablo Neira Ayuso wrote:
> On Sat, Nov 09, 2013 at 08:43:51AM -0500, Jamal Hadi Salim wrote:
> > for errors, we need to give the user something back. This has been the
> > behavior for 80 years now. Giving them a HUGE message
> > back is rediculuos(tm). Ive had enough of SCTP doing that.
> > We need to cap it - sort of what ICMP does.
> > ICMP caps at 64B; something like 128B is reasonable.
>
> Personally, I have only used the sequence number to correlate the
> original request with the ack reply, so I agree in that trimming it to
> some reasonable amount of bytes like ICMP is the way to go. I prefer
> if we select a large enough amount of bytes to avoid breaking backward
> compatibility, eg. 128KB, since I'm not sure what kind of handling
> others may have done of this.
Do you think capping at NLMSG_GOODSIZE would be too low? The allocation
won't fit into one page with NLMSG_GOODSIZE but I doubt we can go lower
than that. Alternatively, we can do some math to fully use the two
pages, like
NLMSG_GOODSIZE + min(PAGE_SIZE, 8192UL) - NLMSG_HDRLEN - NLMSG_ALIGN(sizeof(struct nlmsgerr))
(which I'm not sure is worth it).
Jiri
--
Jiri Benc
^ permalink raw reply
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Arnaud Ebalard @ 2013-11-12 15:34 UTC (permalink / raw)
To: Willy Tarreau; +Cc: Cong Wang, edumazet, stable, linux-arm-kernel, netdev
In-Reply-To: <20131112100126.GB23981@1wt.eu>
Hi,
Willy Tarreau <w@1wt.eu> writes:
> On Tue, Nov 12, 2013 at 10:14:34AM +0100, Arnaud Ebalard wrote:
>> Tests for the rgression were done w/ scp, and were hence limited by the
>> crypto (16MB/s using arcfour128). But I also did some tests w/ a simple
>> wget for a file served by Apache *before* the regression and I never got
>> more than 60MB/s from what I recall. Can you beat that?
>
> Yes, I finally picked my mirabox out of my bag for a quick test. It boots
> off 3.10.0-rc7 and I totally saturate one port (stable 988 Mbps) with even
> a single TCP stream.
Thanks for the feedback. That's interesting. What are you using for your tests
(wget, ...)?
> With two systems, one directly connected (dockstar) and the other one via
> a switch, I get 2*650 Mbps (a single TCP stream is enough on each).
>
> I'll have to re-run some tests using a more up to date kernel, but that
> will probably not be today though.
Can you give a pre-3.11.7 kernel a try if you find the time? I started
working on RN102 during 3.10-rc cycle but do not remember if I did the
first preformance tests on 3.10 or 3.11. And if you find more time,
3.11.7 would be nice too ;-)
Cheers,
a+
^ permalink raw reply
* [PATCH] usbnet: fix status interrupt urb handling
From: Felix Fietkau @ 2013-11-12 15:34 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, dcbw-H+wXaHxf7aLQT0dZR+AlfA
Since commit 7b0c5f21f348a66de495868b8df0284e8dfd6bbf
"sierra_net: keep status interrupt URB active", sierra_net triggers
status interrupt polling before the net_device is opened (in order to
properly receive the sync message response).
To be able to receive further interrupts, the interrupt urb needs to be
re-submitted, so this patch removes the bogus check for netif_running().
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Felix Fietkau <nbd-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
---
drivers/net/usb/usbnet.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 90a429b..8494bb5 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -204,9 +204,6 @@ static void intr_complete (struct urb *urb)
break;
}
- if (!netif_running (dev->net))
- return;
^ permalink raw reply related
* Re: [PATCH net] bonding: add ip checks when store ip target
From: Ding Tianhong @ 2013-11-12 15:46 UTC (permalink / raw)
To: Veaceslav Falico
Cc: Ding Tianhong, Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Netdev
In-Reply-To: <20131112113610.GB19702@redhat.com>
于 2013/11/12 19:36, Veaceslav Falico 写道:
> On Tue, Nov 12, 2013 at 07:25:24PM +0800, Ding Tianhong wrote:
>> I met a Bug when I add ip target with the wrong ip address:
>>
>> echo +500.500.500.500 > /sys/class/net/bond0/bonding/arp_ip_target
>>
>> the wrong ip address will transfor to 245.245.245.244 and add
>> to the ip target success, it is uncorrect, so I add checks to avoid
>> adding wrong address.
>>
>> The in4_pton() will set wrong ip address to 0.0.0.0, it will return by
>> the next check and will not add to ip target.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_sysfs.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Your mail client, apparently, transformed tabs into spaces, so the patch
> doesn't apply.
>
yes, the patch was made in net-next, I forget to make it in net, I'll
send it by office mail in office later.
Thanks.
Regards
Ding
>>
>> diff --git a/drivers/net/bonding/bond_sysfs.c
>> b/drivers/net/bonding/bond_sysfs.c
>> index 4838a97..5b7bf37 100644
>> --- a/drivers/net/bonding/bond_sysfs.c
>> +++ b/drivers/net/bonding/bond_sysfs.c
>> @@ -612,7 +612,7 @@ static ssize_t bonding_store_arp_targets(struct
>> device *d,
>> return restart_syscall();
>>
>> targets = bond->params.arp_targets;
>> - newtarget = in_aton(buf + 1);
>> + in4_pton(buf + 1, strlen(buf) - 1, (u8 *)&newtarget, -1, NULL);
>> /* look for adds */
>> if (buf[0] == '+') {
>> if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) {
>> --
>> 1.8.2.1
>>
>>
> --
> 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] usb: xhci: Link TRB must not occur with a USB payload burst.
From: Alan Stern @ 2013-11-12 16:08 UTC (permalink / raw)
To: David Laight
Cc: Sarah Sharp, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73FF-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
On Tue, 12 Nov 2013, David Laight wrote:
> > You're right. I do wish the spec had been written more clearly.
>
> I've read a lot of hardware specs in my time ...
>
> > > Reading it all again makes me think that a LINK trb is only
> > > allowed on the burst boundary (which might be 16k bytes).
> > > The only real way to implement that is to ensure that TD never
> > > contain LINK TRB.
> >
> > That's one way to do it. Or you could allow a Link TRB at an
> > intermediate MBP boundary.
>
> If all the fragments are larger than the MBP (assume 16k) then
> that would be relatively easy. However that is very dependant
> on the source of the data. It might be true for disk data, but
> is unlikely to be true for ethernet data.
I don't quite understand your point. Are you saying that if all the
TRBs are very short, you might need more than 64 TRBs to reach a 16-KB
boundary?
> For bulk data the link TRB can be forced at a packet boundary
> by splitting the TD up - the receiving end won't know the difference.
That won't work. What happens if you split a TD up into two pieces and
the first piece receives a short packet? The host controller will
automatically move to the start of the second piece. That's not what
we want.
> > It comes down to a question of how often you want the controller to
> > issue an interrupt. If a ring segment is 4 KB (one page), then it can
> > hold 256 TRBs. With scatter-gather transfers, each SG element
> > typically refers to something like a 2-page buffer (depends on how
> > fragmented the memory is). Therefore a ring segment will describe
> > somewhere around 512 pages of data, i.e., something like 2 MB. Since
> > SuperSpeed is 500 MB/s, you'd end up getting in the vicinity of 250
> > interrupts every second just because of ring segment crossings.
>
> 250 interrupts/sec is noise. Send/receive 13000 ethernet packets/sec
> and then look at the interrupt rate!
>
> There is no necessity for taking an interrupt from every link segment.
Yes, there is. The HCD needs to know when the dequeue pointer has
moved beyond the end of the ring segment, so that it can start reusing
the TRB slots in that segment.
Suppose you have queued a bulk URB because there weren't enough free
TRB slots. How else would you know when the occupied slots became
available?
> The current ring segments contain 64 entries, a strange choice
> since they are created with 2 segments.
> (The ring expansion code soon doubles that for my ethernet traffic.)
>
> I would change the code to use a single segment (for coding simplicity)
> and queue bulk URB when there isn't enough ring space.
> URB with too many fragments could either be rejected, sent in sections,
> or partially linearised (and probably still sent in sections).
Rejecting an URB is not feasible. Splitting it up into multiple TDs is
not acceptable, as explained above. Sending it in sections (i.e.,
queueing only some of the TRBs at any time) would work, provided you
got at least two interrupts every time the queue wrapped around (which
suggests you might want at least two ring segments).
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] ipvs: Remove unused variable ret from sync_thread_master()
From: Oleg Nesterov @ 2013-11-12 16:21 UTC (permalink / raw)
To: Peter Zijlstra, Tejun Heo
Cc: David Laight, Geert Uytterhoeven, Ingo Molnar, netdev,
linux-kernel
In-Reply-To: <20131112145243.GU5056@laptop.programming.kicks-ass.net>
On 11/12, Peter Zijlstra wrote:
>
> On Tue, Nov 12, 2013 at 02:21:39PM -0000, David Laight wrote:
> > Shame there isn't a process flag to indicate that the process
> > will sleep uninterruptibly and that it doesn't matter.
> > So don't count to the load average and don't emit a warning
> > if it has been sleeping for a long time.
>
> A process flag wouldn't work, because the task could block waiting for
> actual work to complete in other sleeps.
>
> However, we could do something like the below; which would allow us
> writing things like:
>
> (void)___wait_event(*sk_sleep(sk),
> sock_writeable(sk) || kthread_should_stop(),
> TASK_UNINTERRUPTIBLE | TASK_IDLE, 0, 0,
> schedule());
>
> Marking the one wait-for-more-work as TASK_IDLE such that it doesn't
> contribute to the load avg.
Agreed, I thought about additional bit too.
> static const char * const task_state_array[] = {
> - "R (running)", /* 0 */
> - "S (sleeping)", /* 1 */
> - "D (disk sleep)", /* 2 */
> - "T (stopped)", /* 4 */
> - "t (tracing stop)", /* 8 */
> - "Z (zombie)", /* 16 */
> - "X (dead)", /* 32 */
> - "x (dead)", /* 64 */
> - "K (wakekill)", /* 128 */
> - "W (waking)", /* 256 */
> - "P (parked)", /* 512 */
> + "R (running)", /* 0 */
> + "S (sleeping)", /* 1 */
> + "D (disk sleep)", /* 2 */
> + "T (stopped)", /* 4 */
> + "t (tracing stop)", /* 8 */
> + "Z (zombie)", /* 16 */
> + "X (dead)", /* 32 */
> + "x (dead)", /* 64 */
> + "K (wakekill)", /* 128 */
> + "W (waking)", /* 256 */
> + "P (parked)", /* 512 */
> + "I (idle)", /* 1024 */
> };
but I am not sure about what /proc/ should report in this case...
> #define task_contributes_to_load(task) \
> ((task->state & TASK_UNINTERRUPTIBLE) != 0 && \
> - (task->flags & PF_FROZEN) == 0)
> + (task->flags & PF_FROZEN) == 0 && \
> + (task->state & TASK_IDLE) == 0)
perhaps
(task->state & (TASK_UNINTERRUPTIBLE | TASK_IDLE)) == TASK_UNINTERRUPTIBLE
can save an insn.
I am also wondering if it makes any sense to turn PF_FROZEN into
TASK_FROZEN, something like (incomplete, probably racy) patch below.
Note that it actually adds the new state, not the the qualifier.
Oleg.
--- x/include/linux/freezer.h
+++ x/include/linux/freezer.h
@@ -23,7 +23,7 @@ extern unsigned int freeze_timeout_msecs
*/
static inline bool frozen(struct task_struct *p)
{
- return p->flags & PF_FROZEN;
+ return p->state & TASK_FROZEN;
}
extern bool freezing_slow_path(struct task_struct *p);
--- x/kernel/freezer.c
+++ x/kernel/freezer.c
@@ -57,16 +57,13 @@ bool __refrigerator(bool check_kthr_stop
pr_debug("%s entered refrigerator\n", current->comm);
for (;;) {
- set_current_state(TASK_UNINTERRUPTIBLE);
-
spin_lock_irq(&freezer_lock);
- current->flags |= PF_FROZEN;
- if (!freezing(current) ||
- (check_kthr_stop && kthread_should_stop()))
- current->flags &= ~PF_FROZEN;
+ if (freezing(current) &&
+ !(check_kthr_stop && kthread_should_stop()))
+ set_current_state(TASK_FROZEN);
spin_unlock_irq(&freezer_lock);
- if (!(current->flags & PF_FROZEN))
+ if (!(current->state & TASK_FROZEN))
break;
was_frozen = true;
schedule();
@@ -148,8 +145,7 @@ void __thaw_task(struct task_struct *p)
* refrigerator.
*/
spin_lock_irqsave(&freezer_lock, flags);
- if (frozen(p))
- wake_up_process(p);
+ try_to_wake_up(p, TASK_FROZEN, 0);
spin_unlock_irqrestore(&freezer_lock, flags);
}
^ permalink raw reply
* Re: [RFC] Revert "sierra_net: keep status interrupt URB active"
From: Dan Williams @ 2013-11-12 16:25 UTC (permalink / raw)
To: Bjørn Mork
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
John Henderson
In-Reply-To: <1383946173.29096.12.camel-wKZy7rqYPVb5EHUCmHmTqw@public.gmane.org>
On Fri, 2013-11-08 at 15:29 -0600, Dan Williams wrote:
> On Fri, 2013-11-08 at 21:44 +0100, Bjørn Mork wrote:
> > Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:
> > > On Mon, 2013-11-04 at 14:27 -0600, Dan Williams wrote:
> > >> On Fri, 2013-11-01 at 13:53 +0100, Bjørn Mork wrote:
> > >> > This reverts commit 7b0c5f21f348a66de495868b8df0284e8dfd6bbf.
> > >> >
> > >> > It's not easy to create a driver for all the various firmware
> > >> > bugs out there.
> > >> >
> > >> > This change caused regressions for a number of devices, which
> > >> > started to fail link detection and therefore became completely
> > >> > non-functional. The exact reason is yet unknown, it looks like
> > >> > the affected firmwares might actually need all or some of the
> > >> > additional SYNC messages the patch got rid of.
> > >> >
> > >> > Reverting is not optimal, as it will re-introduce the original
> > >> > problem, but it is currently the only alternative known to fix
> > >> > this issue.
> > >>
> > >> Instead, how does the following patch work for you?
> > >
> > > Bjorn, did you have a chance to try this patch out on your devices?
> >
> > The only DirectIP device I have is the MC7710, which never had any of
> > the firmware issues you are trying to fix. I only tried to forward Johns
> > issue.
> >
> > When this patch worked for John, then I am pretty confident that you
> > have solved the problem here.
>
> Well, "solved", since I still have no idea why the original patch would
> cause the device behavior based on what I know and have read about the
> expected firmware/host handshaking sequence. But the patch there
> appears to fix the problem *and* not blindly send tons of SYNCs forever.
>
> So I'll go ahead and submit a proper version of it.
Actually, is "[PATCH] usbnet: fix status interrupt urb handling" the
real fix for this problem?
John, any chance you could revert my RFC patch and try Felix's patch in
that mail?
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] ipvs: Remove unused variable ret from sync_thread_master()
From: Oleg Nesterov @ 2013-11-12 16:26 UTC (permalink / raw)
To: Peter Zijlstra
Cc: David Laight, Geert Uytterhoeven, Ingo Molnar, netdev,
linux-kernel
In-Reply-To: <20131112143152.GS5056@laptop.programming.kicks-ass.net>
On 11/12, Peter Zijlstra wrote:
>
> On Tue, Nov 12, 2013 at 02:21:39PM -0000, David Laight wrote:
> >
> > /* Tell scheduler we are going to sleep... */
> > if (signal_pending(current))
> > /* We don't want waking immediately (again) */
> > sleep_state = TASK_UNINTERRUPTIBLE;
> > else
> > sleep_state = TASK_INTERRUPTIBLE;
> > set_current_state(sleep_state);
>
> If this is for kernel threads, I think you can wipe the pending state;
Yes, unless this kthread does allow_signal() signal_pending() can't be
true.
Oleg.
^ permalink raw reply
* Re: [RFC PATCHv2 3/4] of: provide a binding for fixed link PHYs
From: Mark Rutland @ 2013-11-12 16:29 UTC (permalink / raw)
To: Grant Likely, Florian Fainelli
Cc: Thomas Petazzoni, David S. Miller, netdev@vger.kernel.org,
devicetree@vger.kernel.org, Lior Amsalem, Sascha Hauer,
Christian Gmeiner, Ezequiel Garcia, Gregory Clement,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20131112123746.EAEF0C42283@trevor.secretlab.ca>
Hi Florian, Grant,
On Tue, Nov 12, 2013 at 12:37:46PM +0000, Grant Likely wrote:
> On Fri, 25 Oct 2013 05:40:57 +0100, Florian Fainelli <florian@openwrt.org> wrote:
> > Le mercredi 18 septembre 2013, 18:11:12 Thomas Petazzoni a écrit :
> > > Dear Grant Likely,
> > >
> > > On Tue, 17 Sep 2013 23:29:23 -0500, Grant Likely wrote:
> > > > I understand what you're trying to do here, but it causes a troublesome
> > > > leakage of implementation detail into the binding, making the whole
> > > > thing look very odd. This binding tries to make a fixed link look
> > > > exactly like a real PHY even to the point of including a phandle to the
> > > > phy. But having a phandle to a node which is *always* a direct child of
> > > > the MAC node is redundant and a rather looney. Yes, doing it that way
> > > > makes it easy for of_phy_find_device() to be transparent for fixed link,
> > > > but that should *not* drive bindings, especially when that makes the
> > > > binding really rather weird.
> > > >
> > > > Second, this new binding doesn't provide anything over and above the
> > > > existing fixed-link binding. It may not be pretty, but it is
> > > > estabilshed.
> > >
> > > Have you followed the past discussions about this patch set? Basically
> > > the *only* feedback I received on RFCv1 is that the fixed-link property
> > > sucks, and everybody (including the known Device Tree binding
> > > maintainer Mark Rutland) suggested to not use the fixed-link mechanism.
> > > See http://article.gmane.org/gmane.linux.network/276932, where Mark
> > > said:
> > >
> > > ""
> > > I'm not sure grouping these values together is the best way of handling
> > > this. It's rather opaque, and inflexible for future extension.
> > > ""
> > >
> > > So, please DT maintainers, tell me what you want. I honestly don't care
> > > whether fixed-link or a separate node is chosen. However, I do care
> > > about being dragged around between two solutions just because the
> > > former DT maintainer and the new DT maintainers do not agree.
>
> I've been sleepy on this issue, which limits how much I can push. I'll
> say one more thing on the issue (below) and then leave the decision up
> to Mark. I trust him and he knows what he is doing.
>
> > Since I would like to move forward so I can one day use that binding in a
> > real-life product, I am going to go for Mark's side which happens to be how I
> > want the binding to look like.
> >
> > Do we all agree that the new binding is just way better than the old one? In
> > light of the recent unstable DT ABI discussion, we can still continue parsing
> > the old one for the sake of compatibility.
>
> Regardless of what you want it to look like, does the old binding work
> for your purposes? If yes then use it. The only valid reason for
> creating a new binding is if the old one doesn't work for a specific
> (not theoretical) use case.
I think the issue here was that I am not versed in the history of all of
the existing bindings. While I'm not keen on the existing fixed-link
property and I think it should be done differently were it being created
from scratch today, as Grant has pointed out we're already supporting it
today, and adding a new binding is going to make the code handling it
more complex.
If fixed-link works for your use case today, then use fixed-link.
If we have a valid reason to create a new binding, we should. At the
moment I think the only egregious portion of the binding is the globally
unique fake PHY id, and if that causes issues we should be able to
assign IDs within Linux ignoring the values in the DT, or reorganise
things such that the arbitrary ID doesn't matter.
If there are configurations we need to support that the fixed-link
property cannot encode, then I think we should go ahead with the binding
style that you are proposing today. However, we don't need to go with it
right away, and we can continue to support fixed-link regardless.
I apologise for the lack of consistency here, and I'm sorry that I've
delayed this series for so long.
Thanks,
Mark.
^ permalink raw reply
* RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
From: David Laight @ 2013-11-12 16:50 UTC (permalink / raw)
To: Alan Stern
Cc: Sarah Sharp, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <Pine.LNX.4.44L0.1311121051090.1200-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
> > If all the fragments are larger than the MBP (assume 16k) then
> > that would be relatively easy. However that is very dependant
> > on the source of the data. It might be true for disk data, but
> > is unlikely to be true for ethernet data.
>
> I don't quite understand your point. Are you saying that if all the
> TRBs are very short, you might need more than 64 TRBs to reach a 16-KB
> boundary?
Since you don't really want to do all the work twice, the sensible
way is to add each input fragment to the ring one a time.
If you need to cross a link TRB and the last MBP boundary is within
the previous data TRB then you can split the previous data TRB at the
MBP boundary and continue.
If the previous TRB is short then you'd need to go back through the
earlier TRB until you found the one that contains a TRB boundary,
split it, and write a link TRB in the following slot.
If you are within the first MBP then you'd need to replace the first
TRB of the message with a link TRB.
And yes, if the data is really fragmented you might need a lot of
TRB for even 1k of data.
> > For bulk data the link TRB can be forced at a packet boundary
> > by splitting the TD up - the receiving end won't know the difference.
>
> That won't work. What happens if you split a TD up into two pieces and
> the first piece receives a short packet? The host controller will
> automatically move to the start of the second piece. That's not what
> we want.
You can split a bulk TD on a 1k boundary and the target won't know the
difference.
> > There is no necessity for taking an interrupt from every link segment.
>
> Yes, there is. The HCD needs to know when the dequeue pointer has
> moved beyond the end of the ring segment, so that it can start reusing
> the TRB slots in that segment.
You already know that because of the interrupts for the data packets
themselves.
> > I would change the code to use a single segment (for coding simplicity)
> > and queue bulk URB when there isn't enough ring space.
> > URB with too many fragments could either be rejected, sent in sections,
> > or partially linearised (and probably still sent in sections).
>
> Rejecting an URB is not feasible. Splitting it up into multiple TDs is
> not acceptable, as explained above. Sending it in sections (i.e.,
> queueing only some of the TRBs at any time) would work, provided you
> got at least two interrupts every time the queue wrapped around (which
> suggests you might want at least two ring segments).
Rejecting badly fragmented URB is almost certainly ok. You really don't
want the hardware overhead of processing a TRB every few bytes.
This would be especially bad on iommu systems.
Before the ring expansion code was added there was an implicit
limit of (probably) 125 fragments for a URB. Exceeding this limit
wasn't the reason for adding the ring expansion code.
And, as I've pointed out, both bulk and isoc URB can be split unless
they are using too many fragments for a short piece of data.
The current code refuses to write into a TRB segment until it is
empty - I think that restriction is only there so that it can
add another segment when the space runs out.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* oom-kill && frozen()
From: Oleg Nesterov @ 2013-11-12 16:56 UTC (permalink / raw)
To: Peter Zijlstra, Tejun Heo, David Rientjes
Cc: David Laight, Geert Uytterhoeven, Ingo Molnar, netdev,
linux-kernel
In-Reply-To: <20131112162136.GA29065@redhat.com>
On 11/12, Oleg Nesterov wrote:
>
> I am also wondering if it makes any sense to turn PF_FROZEN into
> TASK_FROZEN, something like (incomplete, probably racy) patch below.
> Note that it actually adds the new state, not the the qualifier.
As for the current usage of PF_FROZEN... David, it seems that
oom_scan_process_thread()->__thaw_task() is dead? Probably this
was fine before, when __thaw_task() cleared the "need to freeze"
condition, iirc it was PF_FROZEN.
But today __thaw_task() can't help, no? the task will simply
schedule() in D state again.
Oleg.
^ permalink raw reply
* Re: [PATCH] ipvs: Remove unused variable ret from sync_thread_master()
From: Peter Zijlstra @ 2013-11-12 17:00 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Tejun Heo, David Laight, Geert Uytterhoeven, Ingo Molnar, netdev,
linux-kernel
In-Reply-To: <20131112162136.GA29065@redhat.com>
On Tue, Nov 12, 2013 at 05:21:36PM +0100, Oleg Nesterov wrote:
> On 11/12, Peter Zijlstra wrote:
> >
> > On Tue, Nov 12, 2013 at 02:21:39PM -0000, David Laight wrote:
> > > Shame there isn't a process flag to indicate that the process
> > > will sleep uninterruptibly and that it doesn't matter.
> > > So don't count to the load average and don't emit a warning
> > > if it has been sleeping for a long time.
> >
> > A process flag wouldn't work, because the task could block waiting for
> > actual work to complete in other sleeps.
> >
> > However, we could do something like the below; which would allow us
> > writing things like:
> >
> > (void)___wait_event(*sk_sleep(sk),
> > sock_writeable(sk) || kthread_should_stop(),
> > TASK_UNINTERRUPTIBLE | TASK_IDLE, 0, 0,
> > schedule());
> >
> > Marking the one wait-for-more-work as TASK_IDLE such that it doesn't
> > contribute to the load avg.
>
> Agreed, I thought about additional bit too.
>
> > static const char * const task_state_array[] = {
> > - "R (running)", /* 0 */
> > - "S (sleeping)", /* 1 */
> > - "D (disk sleep)", /* 2 */
> > - "T (stopped)", /* 4 */
> > - "t (tracing stop)", /* 8 */
> > - "Z (zombie)", /* 16 */
> > - "X (dead)", /* 32 */
> > - "x (dead)", /* 64 */
> > - "K (wakekill)", /* 128 */
> > - "W (waking)", /* 256 */
> > - "P (parked)", /* 512 */
> > + "R (running)", /* 0 */
> > + "S (sleeping)", /* 1 */
> > + "D (disk sleep)", /* 2 */
> > + "T (stopped)", /* 4 */
> > + "t (tracing stop)", /* 8 */
> > + "Z (zombie)", /* 16 */
> > + "X (dead)", /* 32 */
> > + "x (dead)", /* 64 */
> > + "K (wakekill)", /* 128 */
> > + "W (waking)", /* 256 */
> > + "P (parked)", /* 512 */
> > + "I (idle)", /* 1024 */
> > };
>
> but I am not sure about what /proc/ should report in this case...
We have to put in something...
BUILD_BUG_ON(1 + ilog2(TASK_STATE_MAX) != ARRAY_SIZE(task_state_array));
However, since we always set it together with TASK_UNINTERUPTIBLE
userspace shouldn't actually ever see the I thing.
> > #define task_contributes_to_load(task) \
> > ((task->state & TASK_UNINTERRUPTIBLE) != 0 && \
> > - (task->flags & PF_FROZEN) == 0)
> > + (task->flags & PF_FROZEN) == 0 && \
> > + (task->state & TASK_IDLE) == 0)
>
> perhaps
>
> (task->state & (TASK_UNINTERRUPTIBLE | TASK_IDLE)) == TASK_UNINTERRUPTIBLE
>
> can save an insn.
Fair enough.
> I am also wondering if it makes any sense to turn PF_FROZEN into
> TASK_FROZEN, something like (incomplete, probably racy) patch below.
> Note that it actually adds the new state, not the the qualifier.
>
> --- x/include/linux/freezer.h
> +++ x/include/linux/freezer.h
> @@ -23,7 +23,7 @@ extern unsigned int freeze_timeout_msecs
> */
> static inline bool frozen(struct task_struct *p)
> {
> - return p->flags & PF_FROZEN;
> + return p->state & TASK_FROZEN;
do we want == there? Does it make sense to allow it be set with other
state flags?
> }
>
> extern bool freezing_slow_path(struct task_struct *p);
> --- x/kernel/freezer.c
> +++ x/kernel/freezer.c
> @@ -57,16 +57,13 @@ bool __refrigerator(bool check_kthr_stop
> pr_debug("%s entered refrigerator\n", current->comm);
>
> for (;;) {
> - set_current_state(TASK_UNINTERRUPTIBLE);
> -
> spin_lock_irq(&freezer_lock);
> - current->flags |= PF_FROZEN;
> - if (!freezing(current) ||
> - (check_kthr_stop && kthread_should_stop()))
> - current->flags &= ~PF_FROZEN;
> + if (freezing(current) &&
> + !(check_kthr_stop && kthread_should_stop()))
> + set_current_state(TASK_FROZEN);
> spin_unlock_irq(&freezer_lock);
>
> - if (!(current->flags & PF_FROZEN))
> + if (!(current->state & TASK_FROZEN))
> break;
> was_frozen = true;
> schedule();
> @@ -148,8 +145,7 @@ void __thaw_task(struct task_struct *p)
> * refrigerator.
> */
> spin_lock_irqsave(&freezer_lock, flags);
> - if (frozen(p))
> - wake_up_process(p);
> + try_to_wake_up(p, TASK_FROZEN, 0);
> spin_unlock_irqrestore(&freezer_lock, flags);
> }
Should work I suppose... I'm not entirely sure why that's a PF to begin
with.
^ permalink raw reply
* [PATCH] rtlwifi: Remove unused calls to rtl_is_special_data()
From: Larry Finger @ 2013-11-12 17:02 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Larry Finger, netdev, mark.cave-ayland
When routine rtl_is_special_data() is called with false as its last argument,
and the returned value is not tested, the call is essentially an extended
no-op. Accordingly, these calls may be removed.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
drivers/net/wireless/rtlwifi/pci.c | 2 --
drivers/net/wireless/rtlwifi/usb.c | 4 ----
2 files changed, 6 deletions(-)
diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
index 0f49444..8707d1a 100644
--- a/drivers/net/wireless/rtlwifi/pci.c
+++ b/drivers/net/wireless/rtlwifi/pci.c
@@ -688,8 +688,6 @@ static void _rtl_receive_one(struct ieee80211_hw *hw, struct sk_buff *skb,
rtlpriv->stats.rxbytesunicast += skb->len;
}
- rtl_is_special_data(hw, skb, false);
-
if (ieee80211_is_data(fc)) {
rtlpriv->cfg->ops->led_control(hw, LED_CTL_RX);
diff --git a/drivers/net/wireless/rtlwifi/usb.c b/drivers/net/wireless/rtlwifi/usb.c
index 6e2b5c5..ec385e4 100644
--- a/drivers/net/wireless/rtlwifi/usb.c
+++ b/drivers/net/wireless/rtlwifi/usb.c
@@ -475,8 +475,6 @@ static void _rtl_usb_rx_process_agg(struct ieee80211_hw *hw,
rtlpriv->stats.rxbytesunicast += skb->len;
}
- rtl_is_special_data(hw, skb, false);
-
if (ieee80211_is_data(fc)) {
rtlpriv->cfg->ops->led_control(hw, LED_CTL_RX);
@@ -517,8 +515,6 @@ static void _rtl_usb_rx_process_noagg(struct ieee80211_hw *hw,
rtlpriv->stats.rxbytesunicast += skb->len;
}
- rtl_is_special_data(hw, skb, false);
-
if (ieee80211_is_data(fc)) {
rtlpriv->cfg->ops->led_control(hw, LED_CTL_RX);
--
1.8.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox