* Re: [patch 1/4] net: percpufy frequently used vars -- add percpu_counter_mod_bh
From: Nick Piggin @ 2006-03-09 8:14 UTC (permalink / raw)
To: Ravikiran G Thirumalai
Cc: Andrew Morton, bcrl, linux-kernel, davem, netdev, shai,
Andi Kleen
In-Reply-To: <20060309080651.GA3599@localhost.localdomain>
Ravikiran G Thirumalai wrote:
> Here's a patch making x86_64 local_t to 64 bits like other 64 bit arches.
> This keeps local_t unsigned long. (We can change it to signed value
> along with other arches later in one go I guess)
>
Why not just keep naming and structure of interfaces consistent with
atomic_t?
That would be signed and 32-bit. You then also have a local64_t.
--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com
^ permalink raw reply
* Re: [patch 1/4] net: percpufy frequently used vars -- add percpu_counter_mod_bh
From: Ravikiran G Thirumalai @ 2006-03-09 8:22 UTC (permalink / raw)
To: Nick Piggin
Cc: Andrew Morton, bcrl, linux-kernel, davem, netdev, shai,
Andi Kleen
In-Reply-To: <440FE3E2.1060307@yahoo.com.au>
On Thu, Mar 09, 2006 at 07:14:26PM +1100, Nick Piggin wrote:
> Ravikiran G Thirumalai wrote:
>
> >Here's a patch making x86_64 local_t to 64 bits like other 64 bit arches.
> >This keeps local_t unsigned long. (We can change it to signed value
> >along with other arches later in one go I guess)
> >
>
> Why not just keep naming and structure of interfaces consistent with
> atomic_t?
>
> That would be signed and 32-bit. You then also have a local64_t.
No, local_t is supposed to be 64-bits on 64bits arches and 32 bit on 32 bit
arches. x86_64 was the only exception, so this patch fixes that.
^ permalink raw reply
* Re: [patch 1/4] net: percpufy frequently used vars -- add percpu_counter_mod_bh
From: Nick Piggin @ 2006-03-09 8:41 UTC (permalink / raw)
To: Ravikiran G Thirumalai
Cc: Andrew Morton, bcrl, linux-kernel, davem, netdev, shai,
Andi Kleen
In-Reply-To: <20060309082251.GB3599@localhost.localdomain>
Ravikiran G Thirumalai wrote:
> On Thu, Mar 09, 2006 at 07:14:26PM +1100, Nick Piggin wrote:
>
>>Ravikiran G Thirumalai wrote:
>>
>>
>>>Here's a patch making x86_64 local_t to 64 bits like other 64 bit arches.
>>>This keeps local_t unsigned long. (We can change it to signed value
>>>along with other arches later in one go I guess)
>>>
>>
>>Why not just keep naming and structure of interfaces consistent with
>>atomic_t?
>>
>>That would be signed and 32-bit. You then also have a local64_t.
>
>
> No, local_t is supposed to be 64-bits on 64bits arches and 32 bit on 32 bit
> arches. x86_64 was the only exception, so this patch fixes that.
>
>
Right. If it wasn't I wouldn't have proposed the change.
Considering that local_t has been broken so that basically nobody
is using it, now is a great time to rethink the types before it
gets fixed and people start using it.
And modelling the type on the atomic types would make the most
sense because everyone already knows them.
--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com
^ permalink raw reply
* Re: [PATCH, RESEND] Add MWI workaround for Tulip DC21143
From: Geert Uytterhoeven @ 2006-03-09 9:37 UTC (permalink / raw)
To: Francois Romieu
Cc: Ralf Baechle, Martin Michlmayr, netdev, Linux/MIPS Development,
P. Horton
In-Reply-To: <20060308224139.GA7536@electric-eye.fr.zoreil.com>
On Wed, 8 Mar 2006, Francois Romieu wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> :
> > On Tue, 7 Mar 2006, Ralf Baechle wrote:
> [...]
> > > I'm just not convinced of having such a workaround as a build option.
> > > The average person building a a kernel will probably not know if the
> > > option needs to be enabled or not.
> >
> > Indeed, if it's mentioned in the errata of the chip, the driver should take
> > care of it.
>
> Something like the patch below (+Mr Horton Signed-off-by: and description):
>
> diff --git a/drivers/net/tulip/tulip.h b/drivers/net/tulip/tulip.h
> index 05d2d96..d109540 100644
> --- a/drivers/net/tulip/tulip.h
> +++ b/drivers/net/tulip/tulip.h
> @@ -262,7 +262,14 @@ enum t21143_csr6_bits {
> #define RX_RING_SIZE 128
> #define MEDIA_MASK 31
>
> +/* MWI can fail on 21143 rev 65 if the receive buffer ends
> + on a cache line boundary. Ensure it doesn't ... */
> +
> +#ifdef CONFIG_MIPS_COBALT
> +#define PKT_BUF_SZ (1536 + 4)
> +#else
> #define PKT_BUF_SZ 1536 /* Size of each temporary Rx buffer. */
> +#endif
>
> #define TULIP_MIN_CACHE_LINE 8 /* in units of 32-bit words */
>
> diff --git a/drivers/net/tulip/tulip_core.c b/drivers/net/tulip/tulip_core.c
> index c67c912..ca6eeda 100644
> --- a/drivers/net/tulip/tulip_core.c
> +++ b/drivers/net/tulip/tulip_core.c
> @@ -294,6 +294,8 @@ static void tulip_up(struct net_device *
> if (tp->mii_cnt || (tp->mtable && tp->mtable->has_mii))
> iowrite32(0x00040000, ioaddr + CSR6);
>
> + printk(KERN_DEBUG "%s: CSR0 %08x\n", dev->name, tp->csr0);
> +
> /* Reset the chip, holding bit 0 set at least 50 PCI cycles. */
> iowrite32(0x00000001, ioaddr + CSR0);
> udelay(100);
> @@ -1155,8 +1157,10 @@ static void __devinit tulip_mwi_config (
> /* if we have any cache line size at all, we can do MRM */
> csr0 |= MRM;
>
> +#ifndef CONFIG_MIPS_COBALT
> /* ...and barring hardware bugs, MWI */
> if (!(tp->chip_id == DC21143 && tp->revision == 65))
> +#endif
> csr0 |= MWI;
So when compiling for Cobalt, we work around the hardware bug, while for other
platforms, we just disable MWI?
Wouldn't it be possible to always (I mean, when a rev 65 chip is detected) work
around the bug?
> /* set or disable MWI in the standard PCI command bit.
> @@ -1182,7 +1186,7 @@ static void __devinit tulip_mwi_config (
> */
> switch (cache) {
> case 8:
> - csr0 |= MRL | (1 << CALShift) | (16 << BurstLenShift);
> + csr0 |= MRL | (1 << CALShift) | (8 << BurstLenShift);
> break;
> case 16:
> csr0 |= MRL | (2 << CALShift) | (16 << BurstLenShift);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [UPDATED PATCH] Re: Re: [Patch 7/7] Generic netlink interface (delay accounting)
From: Balbir Singh @ 2006-03-09 14:37 UTC (permalink / raw)
To: Shailabh Nagar; +Cc: hadi, netdev, linux-kernel, lse-tech
In-Reply-To: <440F52FF.30908@watson.ibm.com>
> Thanks for the clarification of the usage model. While our needs are
> certainly much less complex,
> it is useful to know the range of options.
>
> >There are no hard rules on what you need to be multicasting and as an
> >example you could send periodic(aka time based) samples from the kernel
> >on a multicast channel and that would be received by all. It did seem
> >odd that you want to have a semi-promiscous mode where a response to a
> >GET is multicast. If that is still what you want to achieve, then you
> >should.
> >
> >>>Also if you can provide feedback whether the doc i sent was any use
> >>>and what wasnt clear etc.
> >also take a look at the excellent documentation Thomas Graf has put in
> >the kernel for all the utilities for manipulating netlink messages and
> >tell me if that should also be put in this doc (It is listed as a TODO).
Hello, Jamal,
Please find the latest version of the patch for review. The genetlink
code has been updated as per your review comments. The changelog is provided
below
1. Eliminated TASKSTATS_CMD_LISTEN and TASKSTATS_CMD_IGNORE
2. Provide generic functions called genlmsg_data() and genlmsg_len()
in linux/net/genetlink.h
3. Do not multicast all replies, multicast only events generated due
to task exit.
4. The taskstats and taskstats_reply structures are now 64 bit aligned.
5. Family id is dynamically generated.
Please let us know if we missed something out.
Thanks,
Balbir
Signed-off-by: Shailabh Nagar <nagar@watson.ibm.com>
Signed-off-by: Balbir Singh <balbir@in.ibm.com>
---
include/linux/delayacct.h | 2
include/linux/taskstats.h | 128 ++++++++++++++++++++++++
include/net/genetlink.h | 20 +++
init/Kconfig | 16 ++-
kernel/Makefile | 1
kernel/delayacct.c | 56 ++++++++++
kernel/taskstats.c | 244 ++++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 464 insertions(+), 3 deletions(-)
diff -puN include/linux/delayacct.h~delayacct-genetlink include/linux/delayacct.h
--- linux-2.6.16-rc5/include/linux/delayacct.h~delayacct-genetlink 2006-03-09 17:15:31.000000000 +0530
+++ linux-2.6.16-rc5-balbir/include/linux/delayacct.h 2006-03-09 17:15:31.000000000 +0530
@@ -15,6 +15,7 @@
#define _LINUX_TASKDELAYS_H
#include <linux/sched.h>
+#include <linux/taskstats.h>
#ifdef CONFIG_TASK_DELAY_ACCT
extern int delayacct_on; /* Delay accounting turned on/off */
@@ -24,6 +25,7 @@ extern void __delayacct_tsk_init(struct
extern void __delayacct_tsk_exit(struct task_struct *);
extern void __delayacct_blkio(void);
extern void __delayacct_swapin(void);
+extern int delayacct_add_tsk(struct taskstats_reply *, struct task_struct *);
static inline void delayacct_tsk_init(struct task_struct *tsk)
{
diff -puN /dev/null include/linux/taskstats.h
--- /dev/null 2004-06-24 23:34:38.000000000 +0530
+++ linux-2.6.16-rc5-balbir/include/linux/taskstats.h 2006-03-09 19:28:54.000000000 +0530
@@ -0,0 +1,128 @@
+/* taskstats.h - exporting per-task statistics
+ *
+ * Copyright (C) Shailabh Nagar, IBM Corp. 2006
+ * (C) Balbir Singh, IBM Corp. 2006
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2.1 of the GNU Lesser General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ */
+
+#ifndef _LINUX_TASKSTATS_H
+#define _LINUX_TASKSTATS_H
+
+/* Format for per-task data returned to userland when
+ * - a task exits
+ * - listener requests stats for a task
+ *
+ * The struct is versioned. Newer versions should only add fields to
+ * the bottom of the struct to maintain backward compatibility.
+ *
+ * To create the next version, bump up the taskstats_version variable
+ * and delineate the start of newly added fields with a comment indicating
+ * the version number.
+ */
+
+#define TASKSTATS_VERSION 1
+
+struct taskstats {
+ /* Maintain 64-bit alignment while extending */
+
+ /* Version 1 */
+#define TASKSTATS_NOPID -1
+ __s64 pid;
+ __s64 tgid;
+
+ /* XXX_count is number of delay values recorded.
+ * XXX_total is corresponding cumulative delay in nanoseconds
+ */
+
+#define TASKSTATS_NOCPUSTATS 1
+ __u64 cpu_count;
+ __u64 cpu_delay_total; /* wait, while runnable, for cpu */
+ __u64 blkio_count;
+ __u64 blkio_delay_total; /* sync,block io completion wait*/
+ __u64 swapin_count;
+ __u64 swapin_delay_total; /* swapin page fault wait*/
+
+ __u64 cpu_run_total; /* cpu running time
+ * no count available/provided */
+};
+
+
+#define TASKSTATS_LISTEN_GROUP 0x1
+
+/*
+ * Commands sent from userspace
+ * Not versioned. New commands should only be inserted at the enum's end
+ */
+
+enum {
+ TASKSTATS_CMD_UNSPEC, /* Reserved */
+ TASKSTATS_CMD_NONE, /* Not a valid cmd to send
+ * Marks data sent on task/tgid exit */
+ TASKSTATS_CMD_LISTEN, /* Start listening */
+ TASKSTATS_CMD_IGNORE, /* Stop listening */
+ TASKSTATS_CMD_PID, /* Send stats for a pid */
+ TASKSTATS_CMD_TGID, /* Send stats for a tgid */
+};
+
+/* Parameters for commands
+ * New parameters should only be inserted at the struct's end
+ */
+
+struct taskstats_cmd_param {
+ /* Maintain 64-bit alignment while extending */
+ union {
+ __s64 pid;
+ __s64 tgid;
+ } id;
+};
+
+enum outtype {
+ TASKSTATS_REPLY_NONE = 1, /* Control cmd response */
+ TASKSTATS_REPLY_PID, /* per-pid data cmd response*/
+ TASKSTATS_REPLY_TGID, /* per-tgid data cmd response*/
+ TASKSTATS_REPLY_EXIT_PID, /* Exiting task's stats */
+ TASKSTATS_REPLY_EXIT_TGID, /* Exiting tgid's stats
+ * (sent on each tid's exit) */
+};
+
+/*
+ * Reply sent from kernel
+ * Version number affects size/format of struct taskstats only
+ */
+
+struct taskstats_reply {
+ /* Maintain 64-bit alignment while extending */
+ __u16 outtype; /* Must be one of enum outtype */
+ __u16 version;
+ __u32 err;
+ struct taskstats stats; /* Invalid if err != 0 */
+};
+
+/* NETLINK_GENERIC related info */
+
+#define TASKSTATS_GENL_NAME "TASKSTATS"
+#define TASKSTATS_GENL_VERSION 0x1
+
+#define TASKSTATS_HDRLEN (NLMSG_SPACE(GENL_HDRLEN))
+#define TASKSTATS_BODYLEN (sizeof(struct taskstats_reply))
+
+#ifdef __KERNEL__
+
+#include <linux/sched.h>
+
+#ifdef CONFIG_TASKSTATS
+extern void taskstats_exit_pid(struct task_struct *);
+#else
+static inline void taskstats_exit_pid(struct task_struct *tsk)
+{}
+#endif
+
+#endif /* __KERNEL__ */
+#endif /* _LINUX_TASKSTATS_H */
diff -puN init/Kconfig~delayacct-genetlink init/Kconfig
--- linux-2.6.16-rc5/init/Kconfig~delayacct-genetlink 2006-03-09 17:15:31.000000000 +0530
+++ linux-2.6.16-rc5-balbir/init/Kconfig 2006-03-09 17:15:31.000000000 +0530
@@ -158,11 +158,21 @@ config TASK_DELAY_ACCT
in pages. Such statistics can help in setting a task's priorities
relative to other tasks for cpu, io, rss limits etc.
- Unlike BSD process accounting, this information is available
- continuously during the lifetime of a task.
-
Say N if unsure.
+config TASKSTATS
+ bool "Export task/process statistics through netlink (EXPERIMENTAL)"
+ depends on TASK_DELAY_ACCT
+ default y
+ help
+ Export selected statistics for tasks/processes through the
+ generic netlink interface. Unlike BSD process accounting, the
+ statistics are available during the lifetime of tasks/processes as
+ responses to commands. Like BSD accounting, they are sent to user
+ space on task exit.
+
+ Say Y if unsure.
+
config SYSCTL
bool "Sysctl support"
---help---
diff -puN kernel/delayacct.c~delayacct-genetlink kernel/delayacct.c
--- linux-2.6.16-rc5/kernel/delayacct.c~delayacct-genetlink 2006-03-09 17:15:31.000000000 +0530
+++ linux-2.6.16-rc5-balbir/kernel/delayacct.c 2006-03-09 17:15:31.000000000 +0530
@@ -16,9 +16,12 @@
#include <linux/time.h>
#include <linux/sysctl.h>
#include <linux/delayacct.h>
+#include <linux/taskstats.h>
+#include <linux/mutex.h>
int delayacct_on = 0; /* Delay accounting turned on/off */
kmem_cache_t *delayacct_cache;
+static DEFINE_MUTEX(delayacct_exit_mutex);
static int __init delayacct_setup_enable(char *str)
{
@@ -51,8 +54,14 @@ void __delayacct_tsk_init(struct task_st
void __delayacct_tsk_exit(struct task_struct *tsk)
{
+ /*
+ * Protect against racing thread group exits
+ */
+ mutex_lock(&delayacct_exit_mutex);
+ taskstats_exit_pid(tsk);
kmem_cache_free(delayacct_cache, tsk->delays);
tsk->delays = NULL;
+ mutex_unlock(&delayacct_exit_mutex);
}
static inline nsec_t delayacct_measure(void)
@@ -97,3 +106,50 @@ void __delayacct_swapin(void)
current->delays->swapin_count++;
spin_unlock(¤t->delays->lock);
}
+
+#ifdef CONFIG_TASKSTATS
+
+int delayacct_add_tsk(struct taskstats_reply *reply, struct task_struct *tsk)
+{
+ struct taskstats *d = &reply->stats;
+ nsec_t tmp;
+ struct timespec ts;
+ unsigned long t1,t2;
+
+ if (!tsk->delays || !delayacct_on)
+ return -EINVAL;
+
+ /* zero XXX_total,non-zero XXX_count implies XXX stat overflowed */
+#ifdef CONFIG_SCHEDSTATS
+
+ tmp = (nsec_t)d->cpu_run_total ;
+ tmp += (u64)(tsk->utime+tsk->stime)*TICK_NSEC;
+ d->cpu_run_total = (tmp < (nsec_t)d->cpu_run_total)? 0:tmp;
+
+ /* No locking available for sched_info. Take snapshot first. */
+ t1 = tsk->sched_info.pcnt;
+ t2 = tsk->sched_info.run_delay;
+
+ d->cpu_count += t1;
+
+ jiffies_to_timespec(t2, &ts);
+ tmp = (nsec_t)d->cpu_delay_total + timespec_to_ns(&ts);
+ d->cpu_delay_total = (tmp < (nsec_t)d->cpu_delay_total)? 0:tmp;
+#else
+ /* Non-zero XXX_total,zero XXX_count implies XXX stat unavailable */
+ d->cpu_count = 0;
+ d->cpu_run_total = d->cpu_delay_total = TASKSTATS_NOCPUSTATS;
+#endif
+ spin_lock(&tsk->delays->lock);
+ tmp = d->blkio_delay_total + tsk->delays->blkio_delay;
+ d->blkio_delay_total = (tmp < d->blkio_delay_total)? 0:tmp;
+ tmp = d->swapin_delay_total + tsk->delays->swapin_delay;
+ d->swapin_delay_total = (tmp < d->swapin_delay_total)? 0:tmp;
+ d->blkio_count += tsk->delays->blkio_count;
+ d->swapin_count += tsk->delays->swapin_count;
+ spin_unlock(&tsk->delays->lock);
+
+ return 0;
+}
+
+#endif /* CONFIG_TASKSTATS */
diff -puN kernel/Makefile~delayacct-genetlink kernel/Makefile
--- linux-2.6.16-rc5/kernel/Makefile~delayacct-genetlink 2006-03-09 17:15:31.000000000 +0530
+++ linux-2.6.16-rc5-balbir/kernel/Makefile 2006-03-09 17:15:31.000000000 +0530
@@ -35,6 +35,7 @@ obj-$(CONFIG_GENERIC_HARDIRQS) += irq/
obj-$(CONFIG_SECCOMP) += seccomp.o
obj-$(CONFIG_RCU_TORTURE_TEST) += rcutorture.o
obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o
+obj-$(CONFIG_TASKSTATS) += taskstats.o
ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
# According to Alan Modra <alan@linuxcare.com.au>, the -fno-omit-frame-pointer is
diff -puN /dev/null kernel/taskstats.c
--- /dev/null 2004-06-24 23:34:38.000000000 +0530
+++ linux-2.6.16-rc5-balbir/kernel/taskstats.c 2006-03-09 18:52:47.000000000 +0530
@@ -0,0 +1,244 @@
+/*
+ * taskstats.c - Export per-task statistics to userland
+ *
+ * Copyright (C) Shailabh Nagar, IBM Corp. 2006
+ * (C) Balbir Singh, IBM Corp. 2006
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/taskstats.h>
+#include <linux/delayacct.h>
+#include <net/genetlink.h>
+#include <asm/atomic.h>
+
+const int taskstats_version = TASKSTATS_VERSION;
+static DEFINE_PER_CPU(__u32, taskstats_seqnum) = { 0 };
+static int family_registered = 0;
+
+
+static struct genl_family family = {
+ .id = GENL_ID_GENERATE,
+ .name = TASKSTATS_GENL_NAME,
+ .version = TASKSTATS_GENL_VERSION,
+ .hdrsize = 0,
+ .maxattr = 0,
+};
+
+/* Taskstat specific functions */
+static int prepare_reply(struct genl_info *info, u8 cmd,
+ struct sk_buff **skbp, struct taskstats_reply **replyp)
+{
+ struct sk_buff *skb;
+ struct taskstats_reply *reply;
+
+ skb = nlmsg_new(TASKSTATS_HDRLEN + TASKSTATS_BODYLEN);
+ if (!skb)
+ return -ENOMEM;
+
+ if (!info) {
+ int seq = get_cpu_var(taskstats_seqnum)++;
+ put_cpu_var(taskstats_seqnum);
+
+ reply = genlmsg_put(skb, 0, seq,
+ family.id, 0, NLM_F_REQUEST,
+ cmd, family.version);
+ } else
+ reply = genlmsg_put(skb, info->snd_pid, info->snd_seq,
+ family.id, 0, info->nlhdr->nlmsg_flags,
+ info->genlhdr->cmd, family.version);
+ if (reply == NULL) {
+ nlmsg_free(skb);
+ return -EINVAL;
+ }
+ skb_put(skb, TASKSTATS_BODYLEN);
+
+ memset(reply, 0, sizeof(*reply));
+ reply->version = taskstats_version;
+ reply->err = 0;
+
+ *skbp = skb;
+ *replyp = reply;
+ return 0;
+}
+
+static int send_reply(struct sk_buff *skb, int replytype, pid_t pid, int event)
+{
+ struct genlmsghdr *genlhdr = nlmsg_data((struct nlmsghdr *)skb->data);
+ struct taskstats_reply *reply;
+ int rc;
+
+ reply = (struct taskstats_reply *)genlmsg_data(genlhdr);
+ reply->outtype = replytype;
+
+ rc = genlmsg_end(skb, reply);
+ if (rc < 0) {
+ nlmsg_free(skb);
+ return rc;
+ }
+
+ if (event)
+ return genlmsg_multicast(skb, pid, TASKSTATS_LISTEN_GROUP);
+ else
+ return genlmsg_unicast(skb, pid);
+}
+
+static inline void fill_pid(struct taskstats_reply *reply, pid_t pid,
+ struct task_struct *pidtsk)
+{
+ int rc;
+ struct task_struct *tsk = pidtsk;
+
+ if (!pidtsk) {
+ read_lock(&tasklist_lock);
+ tsk = find_task_by_pid(pid);
+ if (!tsk) {
+ read_unlock(&tasklist_lock);
+ reply->err = EINVAL;
+ return;
+ }
+ get_task_struct(tsk);
+ read_unlock(&tasklist_lock);
+ } else
+ get_task_struct(tsk);
+
+ rc = delayacct_add_tsk(reply, tsk);
+ if (!rc) {
+ reply->stats.pid = (s64)tsk->pid;
+ reply->stats.tgid = (s64)tsk->tgid;
+ } else
+ reply->err = (rc < 0) ? -rc : rc ;
+
+ put_task_struct(tsk);
+}
+
+static int taskstats_send_pid(struct sk_buff *skb, struct genl_info *info)
+{
+ int rc;
+ struct sk_buff *rep_skb;
+ struct taskstats_reply *reply;
+ struct taskstats_cmd_param *param= info->userhdr;
+
+ rc = prepare_reply(info, info->genlhdr->cmd, &rep_skb, &reply);
+ if (rc)
+ return rc;
+ fill_pid(reply, param->id.pid, NULL);
+ return send_reply(rep_skb, TASKSTATS_REPLY_PID, info->snd_pid, 0);
+}
+
+static inline void fill_tgid(struct taskstats_reply *reply, pid_t tgid,
+ struct task_struct *tgidtsk)
+{
+ int rc;
+ struct task_struct *tsk, *first;
+
+ first = tgidtsk;
+ read_lock(&tasklist_lock);
+ if (!first) {
+ first = find_task_by_pid(tgid);
+ if (!first) {
+ read_unlock(&tasklist_lock);
+ reply->err = EINVAL;
+ return;
+ }
+ }
+ tsk = first;
+ do {
+ rc = delayacct_add_tsk(reply, tsk);
+ if (rc)
+ break;
+ } while_each_thread(first, tsk);
+ read_unlock(&tasklist_lock);
+
+ if (!rc) {
+ reply->stats.pid = (s64)TASKSTATS_NOPID;
+ reply->stats.tgid = (s64)tgid;
+ } else
+ reply->err = (rc < 0) ? -rc : rc ;
+}
+
+static int taskstats_send_tgid(struct sk_buff *skb, struct genl_info *info)
+{
+ int rc;
+ struct sk_buff *rep_skb;
+ struct taskstats_reply *reply;
+ struct taskstats_cmd_param *param= info->userhdr;
+
+ rc = prepare_reply(info, info->genlhdr->cmd, &rep_skb, &reply);
+ if (rc)
+ return rc;
+ fill_tgid(reply, param->id.tgid, NULL);
+ return send_reply(rep_skb, TASKSTATS_REPLY_TGID, info->snd_pid, 0);
+}
+
+/* Send pid data out on exit */
+void taskstats_exit_pid(struct task_struct *tsk)
+{
+ int rc;
+ struct sk_buff *rep_skb;
+ struct taskstats_reply *reply;
+
+ /*
+ * tasks can start to exit very early. Ensure that the family
+ * is registered before notifications are sent out
+ */
+ if (!family_registered)
+ return;
+
+ rc = prepare_reply(NULL, TASKSTATS_CMD_NONE, &rep_skb, &reply);
+ if (rc)
+ return;
+ fill_pid(reply, tsk->pid, tsk);
+ rc = send_reply(rep_skb, TASKSTATS_REPLY_EXIT_PID, 0, 1);
+
+ if (rc || thread_group_empty(tsk))
+ return;
+
+ /* Send tgid data too */
+ rc = prepare_reply(NULL, TASKSTATS_CMD_NONE, &rep_skb, &reply);
+ if (rc)
+ return;
+ fill_tgid(reply, tsk->tgid, tsk);
+ send_reply(rep_skb, TASKSTATS_REPLY_EXIT_TGID, 0, 1);
+}
+
+static struct genl_ops pid_ops = {
+ .cmd = TASKSTATS_CMD_PID,
+ .doit = taskstats_send_pid,
+};
+
+static struct genl_ops tgid_ops = {
+ .cmd = TASKSTATS_CMD_TGID,
+ .doit = taskstats_send_tgid,
+};
+
+static int __init taskstats_init(void)
+{
+ if (genl_register_family(&family))
+ return -EFAULT;
+ family_registered = 1;
+
+ if (genl_register_ops(&family, &pid_ops))
+ goto err;
+ if (genl_register_ops(&family, &tgid_ops))
+ goto err;
+
+ return 0;
+err:
+ genl_unregister_family(&family);
+ family_registered = 0;
+ return -EFAULT;
+}
+
+late_initcall(taskstats_init);
+
diff -puN include/net/genetlink.h~delayacct-genetlink include/net/genetlink.h
--- linux-2.6.16-rc5/include/net/genetlink.h~delayacct-genetlink 2006-03-09 17:15:31.000000000 +0530
+++ linux-2.6.16-rc5-balbir/include/net/genetlink.h 2006-03-09 17:48:39.000000000 +0530
@@ -150,4 +150,24 @@ static inline int genlmsg_unicast(struct
return nlmsg_unicast(genl_sock, skb, pid);
}
+/**
+ * gennlmsg_data - head of message payload
+ * @gnlh: genetlink messsage header
+ */
+static inline void *genlmsg_data(const struct genlmsghdr *gnlh)
+{
+ return ((unsigned char *) gnlh + GENL_HDRLEN);
+}
+
+/**
+ * genlmsg_len - length of message payload
+ * @gnlh: genetlink message header
+ */
+static inline int genlmsg_len(const struct genlmsghdr *gnlh)
+{
+ struct nlmsghdr *nlh = (struct nlmsghdr *)((unsigned char *)gnlh -
+ NLMSG_HDRLEN);
+ return (nlh->nlmsg_len - GENL_HDRLEN - NLMSG_HDRLEN);
+}
+
#endif /* __NET_GENERIC_NETLINK_H */
_
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [UPDATED PATCH] Re: Re: [Patch 7/7] Generic netlink interface (delay accounting)
From: Shailabh Nagar @ 2006-03-09 16:06 UTC (permalink / raw)
To: balbir; +Cc: hadi, netdev, linux-kernel, lse-tech
In-Reply-To: <20060309143759.GA4653@in.ibm.com>
Balbir Singh wrote:
<snip>
>Hello, Jamal,
>
>Please find the latest version of the patch for review. The genetlink
>code has been updated as per your review comments. The changelog is provided
>below
>
>1. Eliminated TASKSTATS_CMD_LISTEN and TASKSTATS_CMD_IGNORE
>2. Provide generic functions called genlmsg_data() and genlmsg_len()
> in linux/net/genetlink.h
>
>
Balbir,
it might be a good idea to split 2. out separately, since it has generic
value beyond the
delay accounting patches (just like we did for the timespec_diff_ns change)
Thanks,
Shailabh
>3. Do not multicast all replies, multicast only events generated due
> to task exit.
>4. The taskstats and taskstats_reply structures are now 64 bit aligned.
>5. Family id is dynamically generated.
>
>Please let us know if we missed something out.
>
>Thanks,
>Balbir
>
>
>Signed-off-by: Shailabh Nagar <nagar@watson.ibm.com>
>Signed-off-by: Balbir Singh <balbir@in.ibm.com>
>
>---
>
> include/linux/delayacct.h | 2
> include/linux/taskstats.h | 128 ++++++++++++++++++++++++
> include/net/genetlink.h | 20 +++
> init/Kconfig | 16 ++-
> kernel/Makefile | 1
> kernel/delayacct.c | 56 ++++++++++
> kernel/taskstats.c | 244 ++++++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 464 insertions(+), 3 deletions(-)
>
>
>
<snip>
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [patch 1/4] net: percpufy frequently used vars -- add percpu_counter_mod_bh
From: Benjamin LaHaise @ 2006-03-09 18:39 UTC (permalink / raw)
To: Nick Piggin
Cc: Ravikiran G Thirumalai, Andrew Morton, linux-kernel, davem,
netdev, shai, Andi Kleen
In-Reply-To: <440FEA24.3060307@yahoo.com.au>
On Thu, Mar 09, 2006 at 07:41:08PM +1100, Nick Piggin wrote:
> Considering that local_t has been broken so that basically nobody
> is using it, now is a great time to rethink the types before it
> gets fixed and people start using it.
I'm starting to get more concerned as the per-cpu changes that keep adding
more overhead is getting out of hand.
> And modelling the type on the atomic types would make the most
> sense because everyone already knows them.
Except that the usage models are different; local_t is most likely to be
used for cpu local statistics or for sequences, where making them signed
is a bit backwards.
-ben
--
"Time is of no importance, Mr. President, only life is important."
Don't Email: <dont@kvack.org>.
^ permalink raw reply
* Re: [RFC PATCH] softmac: (v2) send WEXT assoc/disassoc events to userspace
From: Larry Finger @ 2006-03-09 20:36 UTC (permalink / raw)
To: Dan Williams
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, bcm43xx-dev-0fE9KPoRgkgATYTw5x5z8w,
softmac-dev-cdvu00un1VgdHxzADdlk8Q, Denis Vlasenko,
David Woodhouse
In-Reply-To: <1141935893.28038.2.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
Dan Williams wrote:
> Completely untested, not entirely sure it compiles. For whatever
> reason, softmac is sending custom events to userspace already, but it
> should _really_ be sending the right WEXT events instead. Comments? If
> this looks good, please apply it.
>
> Signed-off-by: Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
V2 compiles cleanly. It still doesn't authenticate with my WRT54G V5, but that must be another problem.
Larry
^ permalink raw reply
* Re: [PATCH, RESEND] Add MWI workaround for Tulip DC21143
From: Francois Romieu @ 2006-03-09 22:44 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Ralf Baechle, Martin Michlmayr, netdev, Linux/MIPS Development,
P. Horton
In-Reply-To: <Pine.LNX.4.62.0603091032490.9741@pademelon.sonytel.be>
Geert Uytterhoeven <geert@linux-m68k.org> :
[...]
> So when compiling for Cobalt, we work around the hardware bug, while for other
> platforms, we just disable MWI?
>
> Wouldn't it be possible to always (I mean, when a rev 65 chip is detected)
> work around the bug?
Of course it is possible but it is not the same semantic as the initial
patch (not that I know if it is right or not).
So:
- does the issue exist beyond Cobalt hosts ?
- is the fix Cobalt-only ?
--
Ueimor
^ permalink raw reply
* [RFC: 2.6 patch] hostap_hw.c:hfa384x_set_rid(): fix error handling
From: Adrian Bunk @ 2006-03-09 23:06 UTC (permalink / raw)
To: jkmaline; +Cc: hostap, linux-kernel, netdev, linville
The Coverity checker noted that the call to prism2_hw_reset() was dead
code.
Does this patch change the code to what was intended?
Signed-off-by: Adrian Bunk <bunk@stusta.de>
--- linux-2.6.16-rc5-mm3-full/drivers/net/wireless/hostap/hostap_hw.c.old 2006-03-09 23:28:30.000000000 +0100
+++ linux-2.6.16-rc5-mm3-full/drivers/net/wireless/hostap/hostap_hw.c 2006-03-09 23:30:19.000000000 +0100
@@ -928,16 +928,16 @@ static int hfa384x_set_rid(struct net_de
res = hfa384x_cmd(dev, HFA384X_CMDCODE_ACCESS_WRITE, rid, NULL, NULL);
up(&local->rid_bap_sem);
+
if (res) {
+ if (res == -ETIMEDOUT)
+ prism2_hw_reset(dev);
+
printk(KERN_DEBUG "%s: hfa384x_set_rid: CMDCODE_ACCESS_WRITE "
"failed (res=%d, rid=%04x, len=%d)\n",
dev->name, res, rid, len);
- return res;
}
- if (res == -ETIMEDOUT)
- prism2_hw_reset(dev);
-
return res;
}
^ permalink raw reply
* [2.6 patch] tg3.c:tg3_bus_string(): remove dead code
From: Adrian Bunk @ 2006-03-09 23:06 UTC (permalink / raw)
To: jgarzik; +Cc: netdev, linux-kernel
The Coverity checker spotted this dead code (note that (clock_ctrl == 7)
is already handled above).
Signed-off-by: Adrian Bunk <bunk@stusta.de>
--- linux-2.6.16-rc5-mm3-full/drivers/net/tg3.c.old 2006-03-09 23:25:25.000000000 +0100
+++ linux-2.6.16-rc5-mm3-full/drivers/net/tg3.c 2006-03-09 23:25:39.000000000 +0100
@@ -10596,26 +10596,24 @@ static char * __devinit tg3_bus_string(s
if ((clock_ctrl == 7) ||
((tr32(GRC_MISC_CFG) & GRC_MISC_CFG_BOARD_ID_MASK) ==
GRC_MISC_CFG_BOARD_ID_5704CIOBE))
strcat(str, "133MHz");
else if (clock_ctrl == 0)
strcat(str, "33MHz");
else if (clock_ctrl == 2)
strcat(str, "50MHz");
else if (clock_ctrl == 4)
strcat(str, "66MHz");
else if (clock_ctrl == 6)
strcat(str, "100MHz");
- else if (clock_ctrl == 7)
- strcat(str, "133MHz");
} else {
strcpy(str, "PCI:");
if (tp->tg3_flags & TG3_FLAG_PCI_HIGH_SPEED)
strcat(str, "66MHz");
else
strcat(str, "33MHz");
}
if (tp->tg3_flags & TG3_FLAG_PCI_32BIT)
strcat(str, ":32-bit");
else
strcat(str, ":64-bit");
return str;
^ permalink raw reply
* [RFC: 2.6 patch] chelsio/espi.c:tricn_init(): remove dead code
From: Adrian Bunk @ 2006-03-09 23:06 UTC (permalink / raw)
To: maintainers; +Cc: jgarzik, netdev, linux-kernel
The Coverity checker spotted these two unused variables.
Please check whether this patch is correct or whether they should be
used.
Signed-off-by: Adrian Bunk <bunk@stusta.de>
---
drivers/net/chelsio/espi.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
--- linux-2.6.16-rc5-mm3-full/drivers/net/chelsio/espi.c.old 2006-03-09 23:19:54.000000000 +0100
+++ linux-2.6.16-rc5-mm3-full/drivers/net/chelsio/espi.c 2006-03-09 23:20:35.000000000 +0100
@@ -87,15 +87,9 @@
static int tricn_init(adapter_t *adapter)
{
int i = 0;
- int sme = 1;
int stat = 0;
int timeout = 0;
int is_ready = 0;
- int dynamic_deskew = 0;
-
- if (dynamic_deskew)
- sme = 0;
-
/* 1 */
timeout=1000;
@@ -113,11 +107,9 @@
}
/* 2 */
- if (sme) {
- tricn_write(adapter, 0, 0, 0, TRICN_CNFG, 0x81);
- tricn_write(adapter, 0, 1, 0, TRICN_CNFG, 0x81);
- tricn_write(adapter, 0, 2, 0, TRICN_CNFG, 0x81);
- }
+ tricn_write(adapter, 0, 0, 0, TRICN_CNFG, 0x81);
+ tricn_write(adapter, 0, 1, 0, TRICN_CNFG, 0x81);
+ tricn_write(adapter, 0, 2, 0, TRICN_CNFG, 0x81);
for (i=1; i<= 8; i++) tricn_write(adapter, 0, 0, i, TRICN_CNFG, 0xf1);
for (i=1; i<= 2; i++) tricn_write(adapter, 0, 1, i, TRICN_CNFG, 0xf1);
for (i=1; i<= 3; i++) tricn_write(adapter, 0, 2, i, TRICN_CNFG, 0xe1);
^ permalink raw reply
* Re: [2.6 patch] tg3.c:tg3_bus_string(): remove dead code
From: David S. Miller @ 2006-03-09 23:09 UTC (permalink / raw)
To: bunk; +Cc: jgarzik, netdev, linux-kernel
In-Reply-To: <20060309230650.GJ21864@stusta.de>
From: Adrian Bunk <bunk@stusta.de>
Date: Fri, 10 Mar 2006 00:06:50 +0100
> The Coverity checker spotted this dead code (note that (clock_ctrl == 7)
> is already handled above).
>
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
Applied, thanks Adrian.
^ permalink raw reply
* Re: [RFC: 2.6 patch] chelsio/espi.c:tricn_init(): remove dead code
From: Scott Bardone @ 2006-03-09 23:34 UTC (permalink / raw)
To: Adrian Bunk; +Cc: maintainers, jgarzik, netdev, linux-kernel
In-Reply-To: <20060309230653.GK21864@stusta.de>
This patch is correct, these two variables are unused in this driver. Thanks for
catching this!
Signed-off-by: Scott Bardone <sbardone@chelsio.com>
Adrian Bunk wrote:
> The Coverity checker spotted these two unused variables.
>
> Please check whether this patch is correct or whether they should be
> used.
>
>
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
>
> ---
>
> drivers/net/chelsio/espi.c | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> --- linux-2.6.16-rc5-mm3-full/drivers/net/chelsio/espi.c.old 2006-03-09 23:19:54.000000000 +0100
> +++ linux-2.6.16-rc5-mm3-full/drivers/net/chelsio/espi.c 2006-03-09 23:20:35.000000000 +0100
> @@ -87,15 +87,9 @@
> static int tricn_init(adapter_t *adapter)
> {
> int i = 0;
> - int sme = 1;
> int stat = 0;
> int timeout = 0;
> int is_ready = 0;
> - int dynamic_deskew = 0;
> -
> - if (dynamic_deskew)
> - sme = 0;
> -
>
> /* 1 */
> timeout=1000;
> @@ -113,11 +107,9 @@
> }
>
> /* 2 */
> - if (sme) {
> - tricn_write(adapter, 0, 0, 0, TRICN_CNFG, 0x81);
> - tricn_write(adapter, 0, 1, 0, TRICN_CNFG, 0x81);
> - tricn_write(adapter, 0, 2, 0, TRICN_CNFG, 0x81);
> - }
> + tricn_write(adapter, 0, 0, 0, TRICN_CNFG, 0x81);
> + tricn_write(adapter, 0, 1, 0, TRICN_CNFG, 0x81);
> + tricn_write(adapter, 0, 2, 0, TRICN_CNFG, 0x81);
> for (i=1; i<= 8; i++) tricn_write(adapter, 0, 0, i, TRICN_CNFG, 0xf1);
> for (i=1; i<= 2; i++) tricn_write(adapter, 0, 1, i, TRICN_CNFG, 0xf1);
> for (i=1; i<= 3; i++) tricn_write(adapter, 0, 2, i, TRICN_CNFG, 0xe1);
>
^ permalink raw reply
* Re: TSO and IPoIB performance degradation
From: David S. Miller @ 2006-03-09 23:48 UTC (permalink / raw)
To: mst; +Cc: netdev, rdreier, linux-kernel, openib-general, shemminger
In-Reply-To: <20060308125311.GE17618@mellanox.co.il>
From: "Michael S. Tsirkin" <mst@mellanox.co.il>
Date: Wed, 8 Mar 2006 14:53:11 +0200
> What I was trying to figure out was, how can we re-enable the trick
> without hurting TSO? Could a solution be to simply look at the frame
> size, and call tcp_send_delayed_ack if the frame size is small?
The change is really not related to TSO.
By reverting it, you are reducing the number of ACKs on the wire, and
the number of context switches at the sender to push out new data.
That's why it can make things go faster, but it also leads to bursty
TCP sender behavior, which is bad for congestion on the internet.
When the receiver has a strong cpu and can keep up with the incoming
packet rate very well and we are in an environment with no congestion,
the old code helps a lot. But if the receiver is cpu limited or we
have congestion of any kind, it does exactly the wrong thing. It will
delay ACKs a very long time to the point where the pipe is depleted
and this kills performance in that case. For congested environments,
due to the decreased ACK feedback, packet loss recovery will be
extremely poor. This is the first reason behind my change.
The behavior is also specifically frowned upon in the TCP implementor
community. It is specifically mentioned in the Known TCP
Implementation Problems RFC2525, in section 2.13 "Stretch ACK
violation".
The entry, quoted below for reference, is very clear on the reasons
why stretch ACKs are bad. And although it may help performance for
your case, in congested environments and also with cpu limited
receivers it will have a negative impact on performance. So, this was
the second reason why I made this change.
So reverting the change isn't really an option.
Name of Problem
Stretch ACK violation
Classification
Congestion Control/Performance
Description
To improve efficiency (both computer and network) a data receiver
may refrain from sending an ACK for each incoming segment,
according to [RFC1122]. However, an ACK should not be delayed an
inordinate amount of time. Specifically, ACKs SHOULD be sent for
every second full-sized segment that arrives. If a second full-
sized segment does not arrive within a given timeout (of no more
than 0.5 seconds), an ACK should be transmitted, according to
[RFC1122]. A TCP receiver which does not generate an ACK for
every second full-sized segment exhibits a "Stretch ACK
Violation".
Significance
TCP receivers exhibiting this behavior will cause TCP senders to
generate burstier traffic, which can degrade performance in
congested environments. In addition, generating fewer ACKs
increases the amount of time needed by the slow start algorithm to
open the congestion window to an appropriate point, which
diminishes performance in environments with large bandwidth-delay
products. Finally, generating fewer ACKs may cause needless
retransmission timeouts in lossy environments, as it increases the
possibility that an entire window of ACKs is lost, forcing a
retransmission timeout.
Implications
When not in loss recovery, every ACK received by a TCP sender
triggers the transmission of new data segments. The burst size is
determined by the number of previously unacknowledged segments
each ACK covers. Therefore, a TCP receiver ack'ing more than 2
segments at a time causes the sending TCP to generate a larger
burst of traffic upon receipt of the ACK. This large burst of
traffic can overwhelm an intervening gateway, leading to higher
drop rates for both the connection and other connections passing
through the congested gateway.
In addition, the TCP slow start algorithm increases the congestion
window by 1 segment for each ACK received. Therefore, increasing
the ACK interval (thus decreasing the rate at which ACKs are
transmitted) increases the amount of time it takes slow start to
increase the congestion window to an appropriate operating point,
and the connection consequently suffers from reduced performance.
This is especially true for connections using large windows.
Relevant RFCs
RFC 1122 outlines delayed ACKs as a recommended mechanism.
Trace file demonstrating it
Trace file taken using tcpdump at host B, the data receiver (and
ACK originator). The advertised window (which never changed) and
timestamp options have been omitted for clarity, except for the
first packet sent by A:
12:09:24.820187 A.1174 > B.3999: . 2049:3497(1448) ack 1
win 33580 <nop,nop,timestamp 2249877 2249914> [tos 0x8]
12:09:24.824147 A.1174 > B.3999: . 3497:4945(1448) ack 1
12:09:24.832034 A.1174 > B.3999: . 4945:6393(1448) ack 1
12:09:24.832222 B.3999 > A.1174: . ack 6393
12:09:24.934837 A.1174 > B.3999: . 6393:7841(1448) ack 1
12:09:24.942721 A.1174 > B.3999: . 7841:9289(1448) ack 1
12:09:24.950605 A.1174 > B.3999: . 9289:10737(1448) ack 1
12:09:24.950797 B.3999 > A.1174: . ack 10737
12:09:24.958488 A.1174 > B.3999: . 10737:12185(1448) ack 1
12:09:25.052330 A.1174 > B.3999: . 12185:13633(1448) ack 1
12:09:25.060216 A.1174 > B.3999: . 13633:15081(1448) ack 1
12:09:25.060405 B.3999 > A.1174: . ack 15081
This portion of the trace clearly shows that the receiver (host B)
sends an ACK for every third full sized packet received. Further
investigation of this implementation found that the cause of the
increased ACK interval was the TCP options being used. The
implementation sent an ACK after it was holding 2*MSS worth of
unacknowledged data. In the above case, the MSS is 1460 bytes so
the receiver transmits an ACK after it is holding at least 2920
bytes of unacknowledged data. However, the length of the TCP
options being used [RFC1323] took 12 bytes away from the data
portion of each packet. This produced packets containing 1448
bytes of data. But the additional bytes used by the options in
the header were not taken into account when determining when to
trigger an ACK. Therefore, it took 3 data segments before the
data receiver was holding enough unacknowledged data (>= 2*MSS, or
2920 bytes in the above example) to transmit an ACK.
Trace file demonstrating correct behavior
Trace file taken using tcpdump at host B, the data receiver (and
ACK originator), again with window and timestamp information
omitted except for the first packet:
12:06:53.627320 A.1172 > B.3999: . 1449:2897(1448) ack 1
win 33580 <nop,nop,timestamp 2249575 2249612> [tos 0x8]
12:06:53.634773 A.1172 > B.3999: . 2897:4345(1448) ack 1
12:06:53.634961 B.3999 > A.1172: . ack 4345
12:06:53.737326 A.1172 > B.3999: . 4345:5793(1448) ack 1
12:06:53.744401 A.1172 > B.3999: . 5793:7241(1448) ack 1
12:06:53.744592 B.3999 > A.1172: . ack 7241
12:06:53.752287 A.1172 > B.3999: . 7241:8689(1448) ack 1
12:06:53.847332 A.1172 > B.3999: . 8689:10137(1448) ack 1
12:06:53.847525 B.3999 > A.1172: . ack 10137
This trace shows the TCP receiver (host B) ack'ing every second
full-sized packet, according to [RFC1122]. This is the same
implementation shown above, with slight modifications that allow
the receiver to take the length of the options into account when
deciding when to transmit an ACK.
References
This problem is documented in [Allman97] and [Paxson97].
How to detect
Stretch ACK violations show up immediately in receiver-side packet
traces of bulk transfers, as shown above. However, packet traces
made on the sender side of the TCP connection may lead to
ambiguities when diagnosing this problem due to the possibility of
lost ACKs.
^ permalink raw reply
* Re: TSO and IPoIB performance degradation
From: Michael S. Tsirkin @ 2006-03-10 0:10 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, rdreier, linux-kernel, openib-general, shemminger
In-Reply-To: <20060309.154819.104282952.davem@davemloft.net>
Quoting David S. Miller <davem@davemloft.net>:
> Description
> To improve efficiency (both computer and network) a data receiver
> may refrain from sending an ACK for each incoming segment,
> according to [RFC1122]. However, an ACK should not be delayed an
> inordinate amount of time. Specifically, ACKs SHOULD be sent for
> every second full-sized segment that arrives. If a second full-
> sized segment does not arrive within a given timeout (of no more
> than 0.5 seconds), an ACK should be transmitted, according to
> [RFC1122]. A TCP receiver which does not generate an ACK for
> every second full-sized segment exhibits a "Stretch ACK
> Violation".
Thanks very much for the info!
So the longest we can delay, according to this spec, is until we have two full
sized segments.
But with the change we are discussing, could an ack now be sent even sooner than
we have at least two full sized segments? Or does __tcp_ack_snd_check delay
until we have at least two full sized segments? David, could you explain please?
--
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies
^ permalink raw reply
* Re: TSO and IPoIB performance degradation
From: Rick Jones @ 2006-03-10 0:21 UTC (permalink / raw)
To: netdev; +Cc: rdreier, linux-kernel, openib-general
In-Reply-To: <20060309.154819.104282952.davem@davemloft.net>
David S. Miller wrote:
> From: "Michael S. Tsirkin" <mst@mellanox.co.il>
> Date: Wed, 8 Mar 2006 14:53:11 +0200
>
>
>>What I was trying to figure out was, how can we re-enable the trick
>>without hurting TSO? Could a solution be to simply look at the frame
>>size, and call tcp_send_delayed_ack if the frame size is small?
>
>
> The change is really not related to TSO.
>
> By reverting it, you are reducing the number of ACKs on the wire, and
> the number of context switches at the sender to push out new data.
> That's why it can make things go faster, but it also leads to bursty
> TCP sender behavior, which is bad for congestion on the internet.
naughty naughty Solaris and HP-UX TCP :)
>
> When the receiver has a strong cpu and can keep up with the incoming
> packet rate very well and we are in an environment with no congestion,
> the old code helps a lot. But if the receiver is cpu limited or we
> have congestion of any kind, it does exactly the wrong thing. It will
> delay ACKs a very long time to the point where the pipe is depleted
> and this kills performance in that case. For congested environments,
> due to the decreased ACK feedback, packet loss recovery will be
> extremely poor. This is the first reason behind my change.
well, there are stacks which do "stretch acks" (after a fashion) that
make sure when they see packet loss to "do the right thing" wrt sending
enough acks to allow cwnds to open again in a timely fashion.
that brings-back all that stuff I posted ages ago about the performance
delta when using an HP-UX receiver and altering the number of segmetns
per ACK. should be in the netdev archive somewhere.
might have been around the time of the discussions about MacOS and its
ack avoidance - which wasn't done very well at the time.
>
> The behavior is also specifically frowned upon in the TCP implementor
> community. It is specifically mentioned in the Known TCP
> Implementation Problems RFC2525, in section 2.13 "Stretch ACK
> violation".
>
> The entry, quoted below for reference, is very clear on the reasons
> why stretch ACKs are bad. And although it may help performance for
> your case, in congested environments and also with cpu limited
> receivers it will have a negative impact on performance. So, this was
> the second reason why I made this change.
I would have thought that a receiver "stretching ACK's" would be helpful
when it was CPU limited since it was spending fewer CPU cycles
generating ACKs?
>
> So reverting the change isn't really an option.
>
> Name of Problem
> Stretch ACK violation
>
> Classification
> Congestion Control/Performance
>
> Description
> To improve efficiency (both computer and network) a data receiver
> may refrain from sending an ACK for each incoming segment,
> according to [RFC1122]. However, an ACK should not be delayed an
> inordinate amount of time. Specifically, ACKs SHOULD be sent for
> every second full-sized segment that arrives. If a second full-
> sized segment does not arrive within a given timeout (of no more
> than 0.5 seconds), an ACK should be transmitted, according to
> [RFC1122]. A TCP receiver which does not generate an ACK for
> every second full-sized segment exhibits a "Stretch ACK
> Violation".
How can it be a "violation" of a SHOULD?-)
>
> Significance
> TCP receivers exhibiting this behavior will cause TCP senders to
> generate burstier traffic, which can degrade performance in
> congested environments. In addition, generating fewer ACKs
> increases the amount of time needed by the slow start algorithm to
> open the congestion window to an appropriate point, which
> diminishes performance in environments with large bandwidth-delay
> products. Finally, generating fewer ACKs may cause needless
> retransmission timeouts in lossy environments, as it increases the
> possibility that an entire window of ACKs is lost, forcing a
> retransmission timeout.
Of those three, I think the most meaningful is the second, which can be
dealt with by smarts in the ACK-stretching receiver.
For the first, it will only degrade performance if it triggers packet loss.
I'm not sure I've ever seen the third item happen.
>
> Implications
> When not in loss recovery, every ACK received by a TCP sender
> triggers the transmission of new data segments. The burst size is
> determined by the number of previously unacknowledged segments
> each ACK covers. Therefore, a TCP receiver ack'ing more than 2
> segments at a time causes the sending TCP to generate a larger
> burst of traffic upon receipt of the ACK. This large burst of
> traffic can overwhelm an intervening gateway, leading to higher
> drop rates for both the connection and other connections passing
> through the congested gateway.
Doesn't RED mean that those other connections are rather less likely to
be affected?
>
> In addition, the TCP slow start algorithm increases the congestion
> window by 1 segment for each ACK received. Therefore, increasing
> the ACK interval (thus decreasing the rate at which ACKs are
> transmitted) increases the amount of time it takes slow start to
> increase the congestion window to an appropriate operating point,
> and the connection consequently suffers from reduced performance.
> This is especially true for connections using large windows.
This one is dealt with by ABC isn't it? At least in part since ABC
appears to cap cwnd increase at 2*SMSS (I only know this because I just
read the RFC mentioned in another thread - seems a bit much to have made
that limit a MUST rather than a SHOULD :)
rick jones
^ permalink raw reply
* Re: TSO and IPoIB performance degradation
From: Michael S. Tsirkin @ 2006-03-10 0:38 UTC (permalink / raw)
To: David S. Miller; +Cc: rdreier, netdev, linux-kernel, openib-general, shemminger
In-Reply-To: <20060310001031.GA19040@mellanox.co.il>
Quoting r. Michael S. Tsirkin <mst@mellanox.co.il>:
> Or does __tcp_ack_snd_check delay until we have at least two full sized
> segments?
What I'm trying to say, since RFC 2525, 2.13 talks about
"every second full-sized segment", so following the code from
__tcp_ack_snd_check, why does it do
/* More than one full frame received... */
if (((tp->rcv_nxt - tp->rcv_wup) > inet_csk(sk)->icsk_ack.rcv_mss
rather than
/* At least two full frames received... */
if (((tp->rcv_nxt - tp->rcv_wup) >= 2 * inet_csk(sk)->icsk_ack.rcv_mss
--
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies
^ permalink raw reply
* Re: TSO and IPoIB performance degradation
From: David S. Miller @ 2006-03-10 7:18 UTC (permalink / raw)
To: mst; +Cc: netdev, rdreier, linux-kernel, openib-general, shemminger
In-Reply-To: <20060310001031.GA19040@mellanox.co.il>
From: "Michael S. Tsirkin" <mst@mellanox.co.il>
Date: Fri, 10 Mar 2006 02:10:31 +0200
> But with the change we are discussing, could an ack now be sent even
> sooner than we have at least two full sized segments? Or does
> __tcp_ack_snd_check delay until we have at least two full sized
> segments? David, could you explain please?
__tcp_ack_snd_check() delays until we have at least two full
sized segments.
^ permalink raw reply
* Re: TSO and IPoIB performance degradation
From: David S. Miller @ 2006-03-10 7:23 UTC (permalink / raw)
To: rick.jones2; +Cc: netdev, rdreier, linux-kernel, openib-general
In-Reply-To: <4410C671.2050300@hp.com>
From: Rick Jones <rick.jones2@hp.com>
Date: Thu, 09 Mar 2006 16:21:05 -0800
> well, there are stacks which do "stretch acks" (after a fashion) that
> make sure when they see packet loss to "do the right thing" wrt sending
> enough acks to allow cwnds to open again in a timely fashion.
Once a loss happens, it's too late to stop doing the stretch ACKs, the
damage is done already. It is going to take you at least one
extra RTT to recover from the loss compared to if you were not doing
stretch ACKs.
You have to keep giving consistent well spaced ACKs back to the
receiver in order to recover from loss optimally.
The ACK every 2 full sized frames behavior of TCP is absolutely
essential.
^ permalink raw reply
* Re: KERNEL: assertion (!sk->sk_forward_alloc) failed
From: David S. Miller @ 2006-03-10 10:59 UTC (permalink / raw)
To: imcdnzl; +Cc: bb, jesse.brandeburg, yoseph.basri, linux-kernel, netdev
In-Reply-To: <cbec11ac0602091137p4ee233bdgdcfbf3d6cb62a62f@mail.gmail.com>
From: Ian McDonald <imcdnzl@gmail.com>
Date: Fri, 10 Feb 2006 08:37:48 +1300
> On 2/10/06, Boris B. Zhmurov <bb@kernelpanic.ru> wrote:
> > Hello, Ian McDonald.
> >
> > On 09.02.2006 22:25 you said the following:
> >
> > > Is it possible for you to download 2.6.16-rc2 or similar and see if it
> > > goes away?
> >
> > It'll be better, if I get only patch fixs that problem, not all 2.6.16-rc2.
>
> Oops I didn't read Jesse's message earlier properly.
>
> That patch which probably fixed it is (from his message):
> I think the commit id that is missing from 2.6.14.X is
> fb5f5e6e0cebd574be737334671d1aa8f170d5f3
This patch is in the linux-2.6.14 stable tree, I just
verified this.
^ permalink raw reply
* Re: KERNEL: assertion (!sk->sk_forward_alloc) failed
From: David S. Miller @ 2006-03-10 10:59 UTC (permalink / raw)
To: jesse.brandeburg; +Cc: yoseph.basri, bb, linux-kernel, netdev
In-Reply-To: <4807377b0602081810l55e4cbdfyeb9102bd50d04641@mail.gmail.com>
From: Jesse Brandeburg <jesse.brandeburg@gmail.com>
Date: Wed, 8 Feb 2006 18:10:21 -0800
> I think the commit id that is missing from 2.6.14.X is
> fb5f5e6e0cebd574be737334671d1aa8f170d5f3
It's in 2.6.14.x I just double checked.
^ permalink raw reply
* Re: [PATCH] IPv6: Cleanups for net/ipv6/addrconf.c (kzalloc, early exit) v2
From: David S. Miller @ 2006-03-10 11:02 UTC (permalink / raw)
To: yoshfuji; +Cc: ioe-lkml, linux-kernel, netdev
In-Reply-To: <20060212.021103.76157181.yoshfuji@linux-ipv6.org>
From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Sun, 12 Feb 2006 02:11:03 +0900 (JST)
> In article <200602111737.20010.ioe-lkml@rameria.de> (at Sat, 11 Feb 2006 17:37:18 +0100), Ingo Oeser <ioe-lkml@rameria.de> says:
>
> > From: Ingo Oeser <ioe-lkml@rameria.de>
> >
> > Here are some possible (and trivial) cleanups.
> > - use kzalloc() where possible
> > - remove unused label
> > - invert allocation failure test like
> :
> > Signed-off-by: Ingo Oeser <ioe-lkml@rameria.de>
> Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
This patch no longer applied cleanly, Ingo can you generate
a fresh version of your patch against my net-2.6.16 tree?
Thanks a lot!
^ permalink raw reply
* Re: [RFC: 2.6 patch] let NET_CLS_ACT no longer depend on EXPERIMENTAL
From: David S. Miller @ 2006-03-10 11:04 UTC (permalink / raw)
To: hadi; +Cc: kaber, bunk, netdev, linux-kernel
In-Reply-To: <1141498341.5185.32.camel@localhost.localdomain>
From: jamal <hadi@cyberus.ca>
Date: Sat, 04 Mar 2006 13:52:21 -0500
> So on Adrian's patch and above reasoning:
>
> ACKed-by: Jamal Hadi Salim <hadi@cyberus.ca>
Applied, thanks Adrian.
^ permalink raw reply
* Re: [UPDATED PATCH] Re: Re: [Patch 7/7] Generic netlink interface (delay accounting)
From: jamal @ 2006-03-10 14:53 UTC (permalink / raw)
To: balbir; +Cc: Shailabh Nagar, netdev, linux-kernel, lse-tech
In-Reply-To: <20060309143759.GA4653@in.ibm.com>
On Thu, 2006-09-03 at 20:07 +0530, Balbir Singh wrote:
> Please find the latest version of the patch for review. The genetlink
> code has been updated as per your review comments. The changelog is provided
> below
>
> 1. Eliminated TASKSTATS_CMD_LISTEN and TASKSTATS_CMD_IGNORE
> 2. Provide generic functions called genlmsg_data() and genlmsg_len()
> in linux/net/genetlink.h
> 3. Do not multicast all replies, multicast only events generated due
> to task exit.
> 4. The taskstats and taskstats_reply structures are now 64 bit aligned.
> 5. Family id is dynamically generated.
>
> Please let us know if we missed something out.
Design still shaky IMO - now that i think i may understand what your end
goal is.
Using the principles i described in earlier email, the problem you are
trying to solve is:
a) shipping of the taskstats from kernel to user-space asynchronously to
all listeners on multicast channel/group TASKSTATS_LISTEN_GRP
at the point when some process exits.
b) responding to queries issued by the user to the kernel for taskstats
of a particular defined tgid and/or pid combination.
Did i summarize your goals correctly?
So lets stat with #b:
i) the message is multicast; there has to be a user space app registered
to the multicast group otherwise nothing goes to user space.
ii) user space issues a GET and it seems to me the appropriate naming
for the response is a NEW.
Lets go to #a:
The issued multicast messages are also NEW and no different from the
ones sent in response to a GET.
Having said that then, you have the following commands:
enum {
TASKSTATS_CMD_UNSPEC, /* Reserved */
TASKSTATS_CMD_GET, /* user -> kernel query*/
TASKSTATS_CMD_NEW, /* kernel -> user update */
};
You also need the following TLVs
enum {
TASKSTATS_TYPE_UNSPEC, /* Reserved */
TASKSTATS_TYPE_TGID, /* The TGID */
TASKSTATS_TYPE_PID, /* The PID */
TASKSTATS_TYPE_STATS, /* carries the taskstats */
TASKSTATS_TYPE_VERS, /* carries the version */
};
Refer to the doc i passed you and provide feedback if how to use the
above is not obvious.
The use of TLVs above implies that any of these can be optionally
appearing.
So when you are going from user->kernel with a GET a in #a above, then
you can specify the PID and/or TGID and you dont need to specify the
STATS and this would be perfectly legal.
On kernel->user (in the case of response to #a or async notifiation as
in #b) you really dont need to specify the TG/PID since they appear in
the STATS etc.
I take it you dont want to configure the values of taskstats from user
space, otherwise user->kernel will send a NEW as well.
I also take it dumping doesnt apply to you, so you dont need a dump
callback in your kernel code.
>From what i described so far, you dont really need a header for yourself
either (maybe you need one to just store the VERSION?)
I didnt understand the point to the err field you had in the reply.
Netlink already does issue errors which can be read via perror. If this
is different from what you need, it may be an excuse to have your own
header.
I hope this helps.
cheers,
jamal
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox