* [RESEND patch 5/6] s390: remove tty support from ctc network device driver [1/2]
From: Frank Pavlic @ 2006-03-24 9:07 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-kernel, netdev
In-Reply-To: <4421FA37.4050703@pobox.com>
[patch 5/6] s390: remove tty support from ctc network device driver [1/2]
From: Peter Tiedemann <ptiedem@de.ibm.com>
[1/2]:
tty support code will be removed from the ctc network device driver.
Today we have a couple of alternatives which are performing much
better. The second thing is that ctc should be a network
device driver only.
We should not mix tty and networking here.
This first patch will remove the tty code from ctcmain.c .
It also removes the build entry from the Makefile as well as TTY
definitions from ctcmain.h.
The second patch will remove two files, ctctty.c and ctctty.h.
Signed-off-by: Frank Pavlic <fpavlic@de.ibm.com>
diffstat:
Makefile | 3 ++-
ctcmain.c | 45 +++++++++------------------------------------
ctcmain.h | 12 +++++-------
3 files changed, 16 insertions(+), 44 deletions(-)
diff -Naupr git-linux/drivers/s390/net/ctcmain.c git-patched/drivers/s390/net/ctcmain.c
--- git-linux/drivers/s390/net/ctcmain.c 2006-03-24 09:21:59.000000000 +0100
+++ git-patched/drivers/s390/net/ctcmain.c 2006-03-24 09:41:17.000000000 +0100
@@ -6,7 +6,7 @@
* Fixes by : Jochen Röhrig (roehrig@de.ibm.com)
* Arnaldo Carvalho de Melo <acme@conectiva.com.br>
Peter Tiedemann (ptiedem@de.ibm.com)
- * Driver Model stuff by : Cornelia Huck <huckc@de.ibm.com>
+ * Driver Model stuff by : Cornelia Huck <cornelia.huck@de.ibm.com>
*
* Documentation used:
* - Principles of Operation (IBM doc#: SA22-7201-06)
@@ -65,7 +65,6 @@
#include <asm/idals.h>
-#include "ctctty.h"
#include "fsm.h"
#include "cu3088.h"
@@ -479,10 +478,7 @@ ctc_unpack_skb(struct channel *ch, struc
skb->dev = pskb->dev;
skb->protocol = pskb->protocol;
pskb->ip_summed = CHECKSUM_UNNECESSARY;
- if (ch->protocol == CTC_PROTO_LINUX_TTY)
- ctc_tty_netif_rx(skb);
- else
- netif_rx_ni(skb);
+ netif_rx_ni(skb);
/**
* Successful rx; reset logflags
*/
@@ -557,8 +553,7 @@ ccw_unit_check(struct channel *ch, unsig
DBF_TEXT(trace, 5, __FUNCTION__);
if (sense & SNS0_INTERVENTION_REQ) {
if (sense & 0x01) {
- if (ch->protocol != CTC_PROTO_LINUX_TTY)
- ctc_pr_debug("%s: Interface disc. or Sel. reset "
+ ctc_pr_debug("%s: Interface disc. or Sel. reset "
"(remote)\n", ch->id);
fsm_event(ch->fsm, CH_EVENT_UC_RCRESET, ch);
} else {
@@ -2034,7 +2029,6 @@ static void
dev_action_chup(fsm_instance * fi, int event, void *arg)
{
struct net_device *dev = (struct net_device *) arg;
- struct ctc_priv *privptr = dev->priv;
DBF_TEXT(trace, 3, __FUNCTION__);
switch (fsm_getstate(fi)) {
@@ -2049,8 +2043,6 @@ dev_action_chup(fsm_instance * fi, int e
fsm_newstate(fi, DEV_STATE_RUNNING);
ctc_pr_info("%s: connected with remote side\n",
dev->name);
- if (privptr->protocol == CTC_PROTO_LINUX_TTY)
- ctc_tty_setcarrier(dev, 1);
ctc_clear_busy(dev);
}
break;
@@ -2059,8 +2051,6 @@ dev_action_chup(fsm_instance * fi, int e
fsm_newstate(fi, DEV_STATE_RUNNING);
ctc_pr_info("%s: connected with remote side\n",
dev->name);
- if (privptr->protocol == CTC_PROTO_LINUX_TTY)
- ctc_tty_setcarrier(dev, 1);
ctc_clear_busy(dev);
}
break;
@@ -2086,14 +2076,10 @@ dev_action_chup(fsm_instance * fi, int e
static void
dev_action_chdown(fsm_instance * fi, int event, void *arg)
{
- struct net_device *dev = (struct net_device *) arg;
- struct ctc_priv *privptr = dev->priv;
DBF_TEXT(trace, 3, __FUNCTION__);
switch (fsm_getstate(fi)) {
case DEV_STATE_RUNNING:
- if (privptr->protocol == CTC_PROTO_LINUX_TTY)
- ctc_tty_setcarrier(dev, 0);
if (event == DEV_EVENT_TXDOWN)
fsm_newstate(fi, DEV_STATE_STARTWAIT_TX);
else
@@ -2397,8 +2383,6 @@ ctc_tx(struct sk_buff *skb, struct net_d
*/
if (fsm_getstate(privptr->fsm) != DEV_STATE_RUNNING) {
fsm_event(privptr->fsm, DEV_EVENT_START, dev);
- if (privptr->protocol == CTC_PROTO_LINUX_TTY)
- return -EBUSY;
dev_kfree_skb(skb);
privptr->stats.tx_dropped++;
privptr->stats.tx_errors++;
@@ -2608,20 +2592,13 @@ ctc_netdev_unregister(struct net_device
if (!dev)
return;
privptr = (struct ctc_priv *) dev->priv;
- if (privptr->protocol != CTC_PROTO_LINUX_TTY)
- unregister_netdev(dev);
- else
- ctc_tty_unregister_netdev(dev);
+ unregister_netdev(dev);
}
static int
ctc_netdev_register(struct net_device * dev)
{
- struct ctc_priv *privptr = (struct ctc_priv *) dev->priv;
- if (privptr->protocol != CTC_PROTO_LINUX_TTY)
- return register_netdev(dev);
- else
- return ctc_tty_register_netdev(dev);
+ return register_netdev(dev);
}
static void
@@ -2667,7 +2644,9 @@ ctc_proto_store(struct device *dev, stru
if (!priv)
return -ENODEV;
sscanf(buf, "%u", &value);
- if ((value < 0) || (value > CTC_PROTO_MAX))
+ if (!((value == CTC_PROTO_S390) ||
+ (value == CTC_PROTO_LINUX) ||
+ (value == CTC_PROTO_OS390)))
return -EINVAL;
priv->protocol = value;
@@ -2897,10 +2876,7 @@ ctc_new_device(struct ccwgroup_device *c
goto out;
}
- if (privptr->protocol == CTC_PROTO_LINUX_TTY)
- strlcpy(dev->name, "ctctty%d", IFNAMSIZ);
- else
- strlcpy(dev->name, "ctc%d", IFNAMSIZ);
+ strlcpy(dev->name, "ctc%d", IFNAMSIZ);
for (direction = READ; direction <= WRITE; direction++) {
privptr->channel[direction] =
@@ -3046,7 +3022,6 @@ ctc_exit(void)
{
DBF_TEXT(setup, 3, __FUNCTION__);
unregister_cu3088_discipline(&ctc_group_driver);
- ctc_tty_cleanup();
ctc_unregister_dbf_views();
ctc_pr_info("CTC driver unloaded\n");
}
@@ -3073,10 +3048,8 @@ ctc_init(void)
ctc_pr_crit("ctc_init failed with ctc_register_dbf_views rc = %d\n", ret);
return ret;
}
- ctc_tty_init();
ret = register_cu3088_discipline(&ctc_group_driver);
if (ret) {
- ctc_tty_cleanup();
ctc_unregister_dbf_views();
}
return ret;
diff -Naupr git-linux/drivers/s390/net/ctcmain.h git-patched/drivers/s390/net/ctcmain.h
--- git-linux/drivers/s390/net/ctcmain.h 2006-03-24 09:21:59.000000000 +0100
+++ git-patched/drivers/s390/net/ctcmain.h 2006-03-24 09:41:17.000000000 +0100
@@ -35,7 +35,9 @@
#include <asm/ccwdev.h>
#include <asm/ccwgroup.h>
-#include "ctctty.h"
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+
#include "fsm.h"
#include "cu3088.h"
@@ -50,9 +52,7 @@
#define CTC_PROTO_S390 0
#define CTC_PROTO_LINUX 1
-#define CTC_PROTO_LINUX_TTY 2
#define CTC_PROTO_OS390 3
-#define CTC_PROTO_MAX 3
#define CTC_BUFSIZE_LIMIT 65535
#define CTC_BUFSIZE_DEFAULT 32768
@@ -257,15 +257,13 @@ static __inline__ void
ctc_clear_busy(struct net_device * dev)
{
clear_bit(0, &(((struct ctc_priv *) dev->priv)->tbusy));
- if (((struct ctc_priv *)dev->priv)->protocol != CTC_PROTO_LINUX_TTY)
- netif_wake_queue(dev);
+ netif_wake_queue(dev);
}
static __inline__ int
ctc_test_and_set_busy(struct net_device * dev)
{
- if (((struct ctc_priv *)dev->priv)->protocol != CTC_PROTO_LINUX_TTY)
- netif_stop_queue(dev);
+ netif_stop_queue(dev);
return test_and_set_bit(0, &((struct ctc_priv *) dev->priv)->tbusy);
}
diff -Naupr git-linux/drivers/s390/net/Makefile git-patched/drivers/s390/net/Makefile
--- git-linux/drivers/s390/net/Makefile 2006-03-24 09:21:59.000000000 +0100
+++ git-patched/drivers/s390/net/Makefile 2006-03-24 09:41:17.000000000 +0100
@@ -2,7 +2,7 @@
# S/390 network devices
#
-ctc-objs := ctcmain.o ctctty.o ctcdbug.o
+ctc-objs := ctcmain.o ctcdbug.o
obj-$(CONFIG_IUCV) += iucv.o
obj-$(CONFIG_NETIUCV) += netiucv.o fsm.o
@@ -10,6 +10,7 @@ obj-$(CONFIG_SMSGIUCV) += smsgiucv.o
obj-$(CONFIG_CTC) += ctc.o fsm.o cu3088.o
obj-$(CONFIG_LCS) += lcs.o cu3088.o
obj-$(CONFIG_CLAW) += claw.o cu3088.o
+obj-$(CONFIG_MPC) += ctcmpc.o fsm.o cu3088.o
qeth-y := qeth_main.o qeth_mpc.o qeth_sys.o qeth_eddp.o
qeth-$(CONFIG_PROC_FS) += qeth_proc.o
obj-$(CONFIG_QETH) += qeth.o
^ permalink raw reply
* Re: [patch 2/6] s390: qeth driver statistics fixes
From: Frank Pavlic @ 2006-03-24 9:06 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-kernel, netdev
In-Reply-To: <4421FA37.4050703@pobox.com>
On Wed, 22 Mar 2006 20:30:31 -0500
Jeff Garzik <jgarzik@pobox.com> wrote:
> Frank Pavlic wrote:
> > [patch 2/6] s390: qeth driver statistics fixes
> >
> > From: Ursula Braun <braunu@de.ibm.com>
> > - display "unsigned int" values in /proc/qeth_perf with %u instead of %i
> > - omit qdio header length when increasing card->stats.tx_bytes
> >
> > Signed-off-by: Frank Pavlic <fpavlic@de.ibm.com>
>
> applied 2-4
>
> I am OK with removing tty from network driver (patches 5-6), but they
> didn't apply
>
>
hmm ok , I have built new ones today ...
Let me know if you still run into problems to apply the new two ctc tty removal patches...
Thanks
Frank
^ permalink raw reply
* Re: 2.6 patch] ip_conntrack_helper_h323.c: make get_h245_addr()static
From: David S. Miller @ 2006-03-24 7:55 UTC (permalink / raw)
To: bunk; +Cc: netdev, netfilter-devel, zhaojignmin, linux-kernel, zhaojingmin
In-Reply-To: <20060324075511.GV22727@stusta.de>
From: Adrian Bunk <bunk@stusta.de>
Date: Fri, 24 Mar 2006 08:55:11 +0100
> The point is:
> There have been many occasions where people have said "I will need this
> soon" in many different places in the kernel, and one year later it was
> still unused.
>
> If it will be needed at some point in the future, reverting my patch
> will be trivial.
Agreed.
^ permalink raw reply
* Re: 2.6 patch] ip_conntrack_helper_h323.c: make get_h245_addr()static
From: Adrian Bunk @ 2006-03-24 7:55 UTC (permalink / raw)
To: Jing Min Zhao; +Cc: netdev, netfilter-devel, Jing Min Zhao, linux-kernel
In-Reply-To: <BAY109-DAV122F44146DB217251703AEB3DF0@phx.gbl>
On Thu, Mar 23, 2006 at 09:37:15PM -0500, Jing Min Zhao wrote:
>
> I'd like to keep it global. In the future we may need it.
The point is:
There have been many occasions where people have said "I will need this
soon" in many different places in the kernel, and one year later it was
still unused.
If it will be needed at some point in the future, reverting my patch
will be trivial.
> Thanks
>
> Jing Min Zhao
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* Re: [2.6 patch] ip_conntrack_helper_h323.c: EXPORT_SYMBOL'ed functions shouldn't be static
From: David S. Miller @ 2006-03-24 6:08 UTC (permalink / raw)
To: kaber; +Cc: netdev, netfilter-devel, zhaojignmin, linux-kernel, bunk
In-Reply-To: <44234B10.5040802@trash.net>
From: Patrick McHardy <kaber@trash.net>
Date: Fri, 24 Mar 2006 02:27:44 +0100
> I guess I should send it to lkml anyway. It boots fine, I couldn't
> figure out how to compare checksums, since the time of compilation
> and a couple other dynamically generated strings end up in the binary.
This looks fine, I'll push it in during my next round of networking
updates.
^ permalink raw reply
* Re: [2.6.16-gitX] heavy performance regression in ipw2200 wireless driver
From: Zhu Yi @ 2006-03-24 3:41 UTC (permalink / raw)
To: Alessandro Suardi; +Cc: Andrew Morton, linux-kernel, James Ketrenos, netdev
In-Reply-To: <5a4c581d0603230602s1a868a4apbfd79ec2bc568011@mail.gmail.com>
On Thu, 2006-03-23 at 15:02 +0100, Alessandro Suardi wrote:
> That scp test shows 50%ish - but that was a quickie. The VNC
> client even reported a 719Kbps throughput down from the more
> usual 11500Kbps it starts off with. The first scp I tried when the
> sluggishness was intolerable was going at 200KB/s - which
> shows the problem can easily get in the neighborhood of an
> order of magnitude.
What kind of wireless encryption do you use? We turned off hardware
encryption by default recently as a workaround for a firmware restart
bug. You might want to load module with "modprobe ipw2200 hwcrypto=1"
and retest.
Thanks,
-yi
^ permalink raw reply
* Re: Two comments on the H.323 conntrack/NAT helper
From: Jing Min Zhao @ 2006-03-24 2:40 UTC (permalink / raw)
To: Patrick McHardy, Adrian Bunk
Cc: netdev, netfilter-devel, zhaojingmin, linux-kernel, Jing Min Zhao
In-Reply-To: <44235324.3080607@trash.net>
----- Original Message -----
From: "Patrick McHardy" <kaber@trash.net>
To: "Adrian Bunk" <bunk@stusta.de>
Cc: <netdev@vger.kernel.org>; <zhaojingmin@users.sourceforge.net>;
<netfilter-devel@lists.netfilter.org>; "Jing Min Zhao"
<zhaojignmin@hotmail.com>; <linux-kernel@vger.kernel.org>
Sent: Thursday, March 23, 2006 9:02 PM
Subject: Re: Two comments on the H.323 conntrack/NAT helper
> [The hotmail address of the author doesn't work, CCed sourceforge-address]
>
> Adrian Bunk wrote:
>> Two comments on the H.323 conntrack/NAT helper:
>> - the function prototypes in ip_nat_helper_h323.c are _ugly_,
>> please move them to a header file
>
> Their ugliness is because of the current API, which cleaned up
> quite a lot of the surrounding code, but requires this ugliness
> from each helper. I would like to keep them visible as a reminder
> that a cleaner solution is wanted, but moving them to header
> files certainly sound like a good idea to eliminate the risk
> of prototype conflicts. But please do this for all helpers
> at once.
>
>> - is there a reason for not using EXPORT_SYMBOL_GPL?
>
> I would prefer that too.
>
>
Sure, I'll do that.
Thanks
Jing Min Zhao
^ permalink raw reply
* Re: 2.6 patch] ip_conntrack_helper_h323.c: make get_h245_addr()static
From: Jing Min Zhao @ 2006-03-24 2:37 UTC (permalink / raw)
To: Adrian Bunk, Jing Min Zhao; +Cc: netdev, netfilter-devel, linux-kernel
In-Reply-To: <20060324000916.GN22727@stusta.de>
----- Original Message -----
From: "Adrian Bunk" <bunk@stusta.de>
To: "Jing Min Zhao" <zhaojignmin@hotmail.com>
Cc: <netdev@vger.kernel.org>; <netfilter-devel@lists.netfilter.org>;
<linux-kernel@vger.kernel.org>
Sent: Thursday, March 23, 2006 7:09 PM
Subject: [RFC: 2.6 patch] ip_conntrack_helper_h323.c: make
get_h245_addr()static
I'd like to keep it global. In the future we may need it.
Thanks
Jing Min Zhao
> This patch makes a needlessly global function static.
>
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
>
> ---
>
> net/ipv4/netfilter/ip_conntrack_helper_h323.c | 5 ++---
> net/ipv4/netfilter/ip_nat_helper_h323.c | 2 --
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> --- linux-2.6.16-mm1-full/net/ipv4/netfilter/ip_nat_helper_h323.c.old
> 2006-03-23 23:13:59.000000000 +0100
> +++ linux-2.6.16-mm1-full/net/ipv4/netfilter/ip_nat_helper_h323.c
> 2006-03-23 23:14:05.000000000 +0100
> @@ -49,8 +49,6 @@
> #define DEBUGP(format, args...)
> #endif
>
> -extern int get_h245_addr(unsigned char *data, H245_TransportAddress *
> addr,
> - u_int32_t * ip, u_int16_t * port);
> extern int get_h225_addr(unsigned char *data, TransportAddress * addr,
> u_int32_t * ip, u_int16_t * port);
> extern void ip_conntrack_h245_expect(struct ip_conntrack *new,
> ---
> linux-2.6.16-mm1-full/net/ipv4/netfilter/ip_conntrack_helper_h323.c.old
> 2006-03-23 23:14:21.000000000 +0100
> +++ linux-2.6.16-mm1-full/net/ipv4/netfilter/ip_conntrack_helper_h323.c
> 2006-03-23 23:14:35.000000000 +0100
> @@ -222,8 +222,8 @@
> }
>
> /****************************************************************************/
> -int get_h245_addr(unsigned char *data, H245_TransportAddress * addr,
> - u_int32_t * ip, u_int16_t * port)
> +static int get_h245_addr(unsigned char *data, H245_TransportAddress *
> addr,
> + u_int32_t * ip, u_int16_t * port)
> {
> unsigned char *p;
>
> @@ -1713,7 +1713,6 @@
> module_init(init);
> module_exit(fini);
>
> -EXPORT_SYMBOL(get_h245_addr);
> EXPORT_SYMBOL(get_h225_addr);
> EXPORT_SYMBOL(ip_conntrack_h245_expect);
> EXPORT_SYMBOL(ip_conntrack_q931_expect);
>
>
>
^ permalink raw reply
* Re: Two comments on the H.323 conntrack/NAT helper
From: Jing Min Zhao @ 2006-03-24 2:34 UTC (permalink / raw)
To: Adrian Bunk, Jing Min Zhao; +Cc: netdev, netfilter-devel, linux-kernel
In-Reply-To: <20060324001307.GO22727@stusta.de>
----- Original Message -----
From: "Adrian Bunk" <bunk@stusta.de>
To: "Jing Min Zhao" <zhaojignmin@hotmail.com>
Cc: <netdev@vger.kernel.org>; <netfilter-devel@lists.netfilter.org>;
<linux-kernel@vger.kernel.org>
Sent: Thursday, March 23, 2006 7:13 PM
Subject: Two comments on the H.323 conntrack/NAT helper
> Two comments on the H.323 conntrack/NAT helper:
Thank you for your advice.
> - the function prototypes in ip_nat_helper_h323.c are _ugly_,
> please move them to a header file
Correct, I'll do that.
> - is there a reason for not using EXPORT_SYMBOL_GPL?
>
No, I just forgot it. I'll change this too.
> cu
Hope I can get more advice from you.
> Adrian
>
> --
>
> "Is there not promise of rain?" Ling Tan asked suddenly out
> of the darkness. There had been need of rain for many days.
> "Only a promise," Lao Er said.
> Pearl S. Buck - Dragon Seed
>
That's Hepburn :-)
>
>
^ permalink raw reply
* Re: Two comments on the H.323 conntrack/NAT helper
From: Patrick McHardy @ 2006-03-24 2:02 UTC (permalink / raw)
To: Adrian Bunk
Cc: netdev, zhaojingmin, netfilter-devel, Jing Min Zhao, linux-kernel
In-Reply-To: <20060324001307.GO22727@stusta.de>
[The hotmail address of the author doesn't work, CCed sourceforge-address]
Adrian Bunk wrote:
> Two comments on the H.323 conntrack/NAT helper:
> - the function prototypes in ip_nat_helper_h323.c are _ugly_,
> please move them to a header file
Their ugliness is because of the current API, which cleaned up
quite a lot of the surrounding code, but requires this ugliness
from each helper. I would like to keep them visible as a reminder
that a cleaner solution is wanted, but moving them to header
files certainly sound like a good idea to eliminate the risk
of prototype conflicts. But please do this for all helpers
at once.
> - is there a reason for not using EXPORT_SYMBOL_GPL?
I would prefer that too.
^ permalink raw reply
* Re: [RFC][UPDATED PATCH 2.6.16] [Patch 9/9] Generic netlink interface for delay accounting
From: Balbir Singh @ 2006-03-24 1:32 UTC (permalink / raw)
To: jamal; +Cc: Matt Helsley, Shailabh Nagar, linux-kernel, netdev
In-Reply-To: <1143122686.5186.27.camel@jzny2>
On Thu, Mar 23, 2006 at 09:04:46AM -0500, jamal wrote:
>
> Hi Balbir,
>
> Looking good.
> This is a quick scan, so i didnt look at little details.
> Some comments embedded.
Hi, Jamal,
I tried addressing your comments in this new version.
Changelog
---------
1. Moved TASKSTATS_MSG_* to under #ifdef __KERNEL__
2. Got rid of .hdrsize = 0 in genl_family family
3. nlmsg_new() now allocates for 2*u32 + sizeof(taskstats)
4. Got rid of NLM_F_REQUEST, all flags passed down to user space are now 0
5. The response to TASKSTATS_CMD_GET is TASKSTATS_CMD_NEW
6. taskstats_send_stats() now validates the command attributes and ensures
that it either gets a PID or a TGID. If it gets both simultaneously
the PID stats are sent.
7. Do not put the PID/TGID into the skb if there are errors in fill_pid() or
fill_tgid().
Thanks,
Balbir
Signed-off-by: Shailabh Nagar <nagar@watson.ibm.com>
Signed-off-by: Balbir Singh <balbir@in.ibm.com>
---
include/linux/delayacct.h | 11 +
include/linux/taskstats.h | 111 ++++++++++++++++++++
init/Kconfig | 16 ++
kernel/Makefile | 1
kernel/delayacct.c | 44 +++++++
kernel/taskstats.c | 255 ++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 435 insertions(+), 3 deletions(-)
diff -puN include/linux/delayacct.h~delayacct-genetlink include/linux/delayacct.h
--- linux-2.6.16/include/linux/delayacct.h~delayacct-genetlink 2006-03-22 11:56:03.000000000 +0530
+++ linux-2.6.16-balbir/include/linux/delayacct.h 2006-03-22 11:56:03.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 */
@@ -25,6 +26,7 @@ extern void __delayacct_tsk_exit(struct
extern void __delayacct_blkio_start(void);
extern void __delayacct_blkio_end(void);
extern unsigned long long __delayacct_blkio_ticks(struct task_struct *);
+extern int __delayacct_add_tsk(struct taskstats *, struct task_struct *);
static inline void delayacct_tsk_init(struct task_struct *tsk)
{
@@ -72,4 +74,13 @@ static inline unsigned long long delayac
return 0;
}
#endif /* CONFIG_TASK_DELAY_ACCT */
+#ifdef CONFIG_TASKSTATS
+static inline int delayacct_add_tsk(struct taskstats *d,
+ struct task_struct *tsk)
+{
+ if (!tsk->delays)
+ return -EINVAL;
+ return __delayacct_add_tsk(d, tsk);
+}
+#endif
#endif /* _LINUX_TASKDELAYS_H */
diff -puN /dev/null include/linux/taskstats.h
--- /dev/null 2004-06-24 23:34:38.000000000 +0530
+++ linux-2.6.16-balbir/include/linux/taskstats.h 2006-03-24 06:49:24.000000000 +0530
@@ -0,0 +1,111 @@
+/* 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
+#define TASKSTATS_NOPID -1
+
+struct taskstats {
+ /* Maintain 64-bit alignment while extending */
+ /* Version 1 */
+
+ /* 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 = 0, /* Reserved */
+ TASKSTATS_CMD_GET, /* user->kernel request */
+ TASKSTATS_CMD_NEW, /* kernel->user event */
+ __TASKSTATS_CMD_MAX,
+};
+
+#define TASKSTATS_CMD_MAX (__TASKSTATS_CMD_MAX - 1)
+
+enum {
+ TASKSTATS_TYPE_UNSPEC = 0, /* Reserved */
+ TASKSTATS_TYPE_TGID, /* Thread group id */
+ TASKSTATS_TYPE_PID, /* Process id */
+ TASKSTATS_TYPE_STATS, /* taskstats structure */
+ __TASKSTATS_TYPE_MAX,
+};
+
+#define TASKSTATS_TYPE_MAX (__TASKSTATS_TYPE_MAX - 1)
+
+enum {
+ TASKSTATS_CMD_ATTR_UNSPEC = 0,
+ TASKSTATS_CMD_ATTR_PID,
+ TASKSTATS_CMD_ATTR_TGID,
+ __TASKSTATS_CMD_ATTR_MAX,
+};
+
+#define TASKSTATS_CMD_ATTR_MAX (__TASKSTATS_CMD_ATTR_MAX - 1)
+
+/* NETLINK_GENERIC related info */
+
+#define TASKSTATS_GENL_NAME "TASKSTATS"
+#define TASKSTATS_GENL_VERSION 0x1
+
+#ifdef __KERNEL__
+
+#include <linux/sched.h>
+
+enum {
+ TASKSTATS_MSG_UNICAST, /* send data only to requester */
+ TASKSTATS_MSG_MULTICAST, /* send data to a group */
+};
+
+#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/init/Kconfig~delayacct-genetlink 2006-03-22 11:56:03.000000000 +0530
+++ linux-2.6.16-balbir/init/Kconfig 2006-03-22 11:56:03.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/kernel/delayacct.c~delayacct-genetlink 2006-03-22 11:56:03.000000000 +0530
+++ linux-2.6.16-balbir/kernel/delayacct.c 2006-03-24 06:49:24.000000000 +0530
@@ -1,6 +1,7 @@
/* delayacct.c - per-task delay accounting
*
* Copyright (C) Shailabh Nagar, IBM Corp. 2006
+ * Copyright (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
@@ -16,9 +17,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,10 +55,16 @@ 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);
if (tsk->delays) {
kmem_cache_free(delayacct_cache, tsk->delays);
tsk->delays = NULL;
}
+ mutex_unlock(&delayacct_exit_mutex);
}
/*
@@ -124,3 +134,37 @@ unsigned long long __delayacct_blkio_tic
spin_unlock(&tsk->delays->lock);
return ret;
}
+#ifdef CONFIG_TASKSTATS
+int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk)
+{
+ nsec_t tmp;
+ struct timespec ts;
+ unsigned long t1,t2;
+
+ /* zero XXX_total,non-zero XXX_count implies XXX stat overflowed */
+
+ 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;
+
+ 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/kernel/Makefile~delayacct-genetlink 2006-03-22 11:56:03.000000000 +0530
+++ linux-2.6.16-balbir/kernel/Makefile 2006-03-22 11:56:03.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-balbir/kernel/taskstats.c 2006-03-24 06:50:03.000000000 +0530
@@ -0,0 +1,255 @@
+/*
+ * 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,
+ .maxattr = TASKSTATS_CMD_ATTR_MAX,
+};
+
+static struct nla_policy taskstats_cmd_get_policy[TASKSTATS_CMD_ATTR_MAX+1] __read_mostly = {
+ [TASKSTATS_CMD_ATTR_PID] = { .type = NLA_U32 },
+ [TASKSTATS_CMD_ATTR_TGID] = { .type = NLA_U32 },
+};
+
+
+static int prepare_reply(struct genl_info *info, u8 cmd, struct sk_buff **skbp,
+ void **replyp)
+{
+ struct sk_buff *skb;
+ void *reply;
+
+ /*
+ * If new attributes are added, please revisit this allocation
+ */
+ skb = nlmsg_new((2 * sizeof(u32)) + sizeof(struct taskstats));
+ 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, 0,
+ cmd, family.version);
+ } else
+ reply = genlmsg_put(skb, info->snd_pid, info->snd_seq,
+ family.id, 0, 0,
+ cmd, family.version);
+ if (reply == NULL) {
+ nlmsg_free(skb);
+ return -EINVAL;
+ }
+
+ *skbp = skb;
+ *replyp = reply;
+ return 0;
+}
+
+static int send_reply(struct sk_buff *skb, pid_t pid, int event)
+{
+ struct genlmsghdr *genlhdr = nlmsg_data((struct nlmsghdr *)skb->data);
+ void *reply;
+ int rc;
+
+ reply = genlmsg_data(genlhdr);
+
+ rc = genlmsg_end(skb, reply);
+ if (rc < 0) {
+ nlmsg_free(skb);
+ return rc;
+ }
+
+ if (event == TASKSTATS_MSG_MULTICAST)
+ return genlmsg_multicast(skb, pid, TASKSTATS_LISTEN_GROUP);
+ return genlmsg_unicast(skb, pid);
+}
+
+static inline int fill_pid(pid_t pid, struct task_struct *pidtsk,
+ struct taskstats *stats)
+{
+ 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);
+ return -ESRCH;
+ }
+ get_task_struct(tsk);
+ read_unlock(&tasklist_lock);
+ } else
+ get_task_struct(tsk);
+
+ rc = delayacct_add_tsk(stats, tsk);
+ put_task_struct(tsk);
+
+ return rc;
+
+}
+
+static inline int fill_tgid(pid_t tgid, struct task_struct *tgidtsk,
+ struct taskstats *stats)
+{
+ 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);
+ return -ESRCH;
+ }
+ }
+ tsk = first;
+ do {
+ rc = delayacct_add_tsk(stats, tsk);
+ if (rc)
+ break;
+ } while_each_thread(first, tsk);
+ read_unlock(&tasklist_lock);
+
+ return rc;
+}
+
+static int taskstats_send_stats(struct sk_buff *skb, struct genl_info *info)
+{
+ int rc;
+ struct sk_buff *rep_skb;
+ struct taskstats stats;
+ void *reply;
+
+ memset(&stats, 0, sizeof(stats));
+ rc = prepare_reply(info, TASKSTATS_CMD_NEW, &rep_skb, &reply);
+ if (rc < 0)
+ return rc;
+
+ if (info->attrs[TASKSTATS_CMD_ATTR_PID]) {
+ u32 pid = nla_get_u32(info->attrs[TASKSTATS_CMD_ATTR_PID]);
+ rc = fill_pid((pid_t)pid, NULL, &stats);
+ if (rc < 0)
+ return rc;
+
+ NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_PID, pid);
+ } else if (info->attrs[TASKSTATS_CMD_ATTR_TGID]) {
+ u32 tgid = nla_get_u32(info->attrs[TASKSTATS_CMD_ATTR_TGID]);
+ rc = fill_tgid((pid_t)tgid, NULL, &stats);
+ if (rc < 0)
+ return rc;
+
+ NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_TGID, tgid);
+ } else {
+ return -EINVAL;
+ }
+
+ NLA_PUT_TYPE(rep_skb, struct taskstats, TASKSTATS_TYPE_STATS, stats);
+ return send_reply(rep_skb, info->snd_pid, TASKSTATS_MSG_UNICAST);
+
+nla_put_failure:
+ return genlmsg_cancel(rep_skb, reply);
+
+}
+
+
+/* Send pid data out on exit */
+void taskstats_exit_pid(struct task_struct *tsk)
+{
+ int rc;
+ struct sk_buff *rep_skb;
+ void *reply;
+ struct taskstats stats;
+
+ /*
+ * tasks can start to exit very early. Ensure that the family
+ * is registered before notifications are sent out
+ */
+ if (!family_registered)
+ return;
+
+ memset(&stats, 0, sizeof(stats));
+ rc = prepare_reply(NULL, TASKSTATS_CMD_NEW, &rep_skb, &reply);
+ if (rc < 0)
+ return;
+
+ rc = fill_pid(tsk->pid, tsk, &stats);
+ if (rc < 0)
+ return;
+
+ NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_PID, (u32)tsk->pid);
+ NLA_PUT_TYPE(rep_skb, struct taskstats, TASKSTATS_TYPE_STATS, stats);
+ rc = send_reply(rep_skb, 0, TASKSTATS_MSG_MULTICAST);
+
+ if (rc || thread_group_empty(tsk))
+ return;
+
+ /* Send tgid data too */
+ rc = prepare_reply(NULL, TASKSTATS_CMD_NEW, &rep_skb, &reply);
+ if (rc < 0)
+ return;
+
+ rc = fill_tgid(tsk->tgid, tsk, &stats);
+ if (rc < 0)
+ return;
+
+ NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_TGID, (u32)tsk->tgid);
+ NLA_PUT_TYPE(rep_skb, struct taskstats, TASKSTATS_TYPE_STATS, stats);
+ send_reply(rep_skb, 0, TASKSTATS_MSG_MULTICAST);
+
+nla_put_failure:
+ genlmsg_cancel(rep_skb, reply);
+}
+
+static struct genl_ops taskstats_ops = {
+ .cmd = TASKSTATS_CMD_GET,
+ .doit = taskstats_send_stats,
+ .policy = taskstats_cmd_get_policy,
+};
+
+static int __init taskstats_init(void)
+{
+ if (genl_register_family(&family))
+ return -EFAULT;
+ family_registered = 1;
+
+ if (genl_register_ops(&family, &taskstats_ops))
+ goto err;
+
+ return 0;
+err:
+ genl_unregister_family(&family);
+ family_registered = 0;
+ return -EFAULT;
+}
+
+late_initcall(taskstats_init);
_
^ permalink raw reply
* Re: [2.6 patch] ip_conntrack_helper_h323.c: EXPORT_SYMBOL'ed functions shouldn't be static
From: Patrick McHardy @ 2006-03-24 1:27 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, netfilter-devel, zhaojignmin, linux-kernel, bunk
In-Reply-To: <20060323.161314.59991770.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 668 bytes --]
David S. Miller wrote:
> From: Adrian Bunk <bunk@stusta.de>
> Date: Fri, 24 Mar 2006 01:08:01 +0100
>
>
>>EXPORT_SYMBOL'ed functions shouldn't be static.
>>
>>Signed-off-by: Adrian Bunk <bunk@stusta.de>
>
>
> Fixed in Linus's tree as of yesterday.
>
> I actually have a patch from Patrick McHardy that will make
> this kind of error a build time failure instead of silently
> working in the modular case. I just need to test it out
> a bit before pushing.
I guess I should send it to lkml anyway. It boots fine, I couldn't
figure out how to compare checksums, since the time of compilation
and a couple other dynamically generated strings end up in the binary.
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1070 bytes --]
[MODULES]: Don't allow statically declared exports
Add an extern declaration for exported symbols to make the compiler warn
on symbols declared statically.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit 8648236083e488ff4fc279b66d63b1187e22e558
tree cba9ee372f1056c8cf63cdc6a37a6a761fa490c9
parent 8b21e6d05d6ac0aeb44f5866ab611e2709c2f08e
author Patrick McHardy <kaber@trash.net> Thu, 23 Mar 2006 05:07:39 +0100
committer Patrick McHardy <kaber@trash.net> Thu, 23 Mar 2006 05:07:39 +0100
include/linux/module.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 70bd843..d956915 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -183,6 +183,7 @@ void *__symbol_get_gpl(const char *symbo
/* For every exported symbol, place a struct in the __ksymtab section */
#define __EXPORT_SYMBOL(sym, sec) \
+ extern typeof(sym) sym; \
__CRC_SYMBOL(sym, sec) \
static const char __kstrtab_##sym[] \
__attribute__((section("__ksymtab_strings"))) \
^ permalink raw reply related
* Re: [2.6 patch] ip_conntrack_helper_h323.c: EXPORT_SYMBOL'ed functions shouldn't be static
From: Adrian Bunk @ 2006-03-24 0:29 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, netfilter-devel, zhaojignmin, linux-kernel
In-Reply-To: <20060323.161314.59991770.davem@davemloft.net>
On Thu, Mar 23, 2006 at 04:13:14PM -0800, David S. Miller wrote:
> From: Adrian Bunk <bunk@stusta.de>
> Date: Fri, 24 Mar 2006 01:08:01 +0100
>
> > EXPORT_SYMBOL'ed functions shouldn't be static.
> >
> > Signed-off-by: Adrian Bunk <bunk@stusta.de>
>
> Fixed in Linus's tree as of yesterday.
Sorry for missing this, this must have been after Andrew pulled Linus'
tree for creating 2.6.16-mm1 (which is where I was looking at).
> I actually have a patch from Patrick McHardy that will make
> this kind of error a build time failure instead of silently
> working in the modular case. I just need to test it out
> a bit before pushing.
Yes, it would be nice if it would also fail in CONFIG_MODULES=y builds.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* Re: [2.6 patch] ip_conntrack_helper_h323.c: EXPORT_SYMBOL'ed functions shouldn't be static
From: David S. Miller @ 2006-03-24 0:13 UTC (permalink / raw)
To: bunk; +Cc: zhaojignmin, linux-kernel, netdev, netfilter-devel
In-Reply-To: <20060324000801.GM22727@stusta.de>
From: Adrian Bunk <bunk@stusta.de>
Date: Fri, 24 Mar 2006 01:08:01 +0100
> EXPORT_SYMBOL'ed functions shouldn't be static.
>
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
Fixed in Linus's tree as of yesterday.
I actually have a patch from Patrick McHardy that will make
this kind of error a build time failure instead of silently
working in the modular case. I just need to test it out
a bit before pushing.
^ permalink raw reply
* Two comments on the H.323 conntrack/NAT helper
From: Adrian Bunk @ 2006-03-24 0:13 UTC (permalink / raw)
To: Jing Min Zhao; +Cc: netdev, netfilter-devel, linux-kernel
Two comments on the H.323 conntrack/NAT helper:
- the function prototypes in ip_nat_helper_h323.c are _ugly_,
please move them to a header file
- is there a reason for not using EXPORT_SYMBOL_GPL?
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* [RFC: 2.6 patch] ip_conntrack_helper_h323.c: make get_h245_addr() static
From: Adrian Bunk @ 2006-03-24 0:09 UTC (permalink / raw)
To: Jing Min Zhao; +Cc: netdev, netfilter-devel, linux-kernel
This patch makes a needlessly global function static.
Signed-off-by: Adrian Bunk <bunk@stusta.de>
---
net/ipv4/netfilter/ip_conntrack_helper_h323.c | 5 ++---
net/ipv4/netfilter/ip_nat_helper_h323.c | 2 --
2 files changed, 2 insertions(+), 5 deletions(-)
--- linux-2.6.16-mm1-full/net/ipv4/netfilter/ip_nat_helper_h323.c.old 2006-03-23 23:13:59.000000000 +0100
+++ linux-2.6.16-mm1-full/net/ipv4/netfilter/ip_nat_helper_h323.c 2006-03-23 23:14:05.000000000 +0100
@@ -49,8 +49,6 @@
#define DEBUGP(format, args...)
#endif
-extern int get_h245_addr(unsigned char *data, H245_TransportAddress * addr,
- u_int32_t * ip, u_int16_t * port);
extern int get_h225_addr(unsigned char *data, TransportAddress * addr,
u_int32_t * ip, u_int16_t * port);
extern void ip_conntrack_h245_expect(struct ip_conntrack *new,
--- linux-2.6.16-mm1-full/net/ipv4/netfilter/ip_conntrack_helper_h323.c.old 2006-03-23 23:14:21.000000000 +0100
+++ linux-2.6.16-mm1-full/net/ipv4/netfilter/ip_conntrack_helper_h323.c 2006-03-23 23:14:35.000000000 +0100
@@ -222,8 +222,8 @@
}
/****************************************************************************/
-int get_h245_addr(unsigned char *data, H245_TransportAddress * addr,
- u_int32_t * ip, u_int16_t * port)
+static int get_h245_addr(unsigned char *data, H245_TransportAddress * addr,
+ u_int32_t * ip, u_int16_t * port)
{
unsigned char *p;
@@ -1713,7 +1713,6 @@
module_init(init);
module_exit(fini);
-EXPORT_SYMBOL(get_h245_addr);
EXPORT_SYMBOL(get_h225_addr);
EXPORT_SYMBOL(ip_conntrack_h245_expect);
EXPORT_SYMBOL(ip_conntrack_q931_expect);
^ permalink raw reply
* [2.6 patch] ip_conntrack_helper_h323.c: EXPORT_SYMBOL'ed functions shouldn't be static
From: Adrian Bunk @ 2006-03-24 0:08 UTC (permalink / raw)
To: Jing Min Zhao; +Cc: netdev, netfilter-devel, linux-kernel
EXPORT_SYMBOL'ed functions shouldn't be static.
Signed-off-by: Adrian Bunk <bunk@stusta.de>
--- linux-2.6.16-mm1-full/net/ipv4/netfilter/ip_conntrack_helper_h323.c.old 2006-03-23 19:29:58.000000000 +0100
+++ linux-2.6.16-mm1-full/net/ipv4/netfilter/ip_conntrack_helper_h323.c 2006-03-23 19:30:39.000000000 +0100
@@ -639,8 +639,8 @@
}
/****************************************************************************/
-static int get_h225_addr(unsigned char *data, TransportAddress * addr,
- u_int32_t * ip, u_int16_t * port)
+int get_h225_addr(unsigned char *data, TransportAddress * addr,
+ u_int32_t * ip, u_int16_t * port)
{
unsigned char *p;
^ permalink raw reply
* [ANNOUNCE] iproute2 2.6.16-060323
From: Stephen Hemminger @ 2006-03-23 22:32 UTC (permalink / raw)
To: netdev, linux-net, LARTC
New version of iproute2 is available. No major changes, mostly just
small bug fixes.
http://developer.osdl.org/dev/iproute2/download/iproute2-2.6.16-060323.tar.gz
James Lentini
Increase size of hw address allowed for ip neigh to allow for IB.
Russell Stuart
Fix missing memset in tc sample
Add sample divisor
Alpt
Add more rt_proto values
Dale Sedivec
Warn when using "handle" instead of "classid" with "tc class"
Jean Tourrilhes
Fix endless loop in netlink error handling
Stephen Hemminger
Change default lnstat count to 1
Update to 2.6.16 headers
Add fake version of include/linux/socket.h to fix warnings
^ permalink raw reply
* [git patches] net driver build fix + update
From: Jeff Garzik @ 2006-03-23 22:27 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
[just sent to andrew/linus; patch omitted due to size]
Notables:
* merges wireless "softmac" layer, enabling support for newer wireless
hardware which requires software management of low-level details.
Not quite winmodem level, in terms of hardware simplicity, but close.
* 'make allmodconfig' build fix, in the sky2 update.
The 'upstream-linus' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
contains the following updates:
drivers/net/skge.c | 105 +-
drivers/net/skge.h | 1
drivers/net/sky2.c | 8
drivers/net/wireless/Kconfig | 9
drivers/net/wireless/airo.c | 455 +++++++----
drivers/net/wireless/hostap/hostap_ap.c | 2
drivers/net/wireless/hostap/hostap_cs.c | 2
drivers/net/wireless/hostap/hostap_hw.c | 8
drivers/net/wireless/hostap/hostap_ioctl.c | 4
drivers/net/wireless/hostap/hostap_pci.c | 4
drivers/net/wireless/hostap/hostap_plx.c | 13
include/linux/wireless.h | 10
include/net/ieee80211softmac.h | 292 +++++++
include/net/ieee80211softmac_wx.h | 94 ++
include/net/iw_handler.h | 12
net/core/rtnetlink.c | 98 ++
net/core/wireless.c | 911 +++++++++++++++++++++---
net/ieee80211/Kconfig | 1
net/ieee80211/Makefile | 1
net/ieee80211/ieee80211_rx.c | 74 +
net/ieee80211/softmac/Kconfig | 10
net/ieee80211/softmac/Makefile | 9
net/ieee80211/softmac/ieee80211softmac_assoc.c | 396 ++++++++++
net/ieee80211/softmac/ieee80211softmac_auth.c | 364 +++++++++
net/ieee80211/softmac/ieee80211softmac_event.c | 159 ++++
net/ieee80211/softmac/ieee80211softmac_io.c | 474 ++++++++++++
net/ieee80211/softmac/ieee80211softmac_module.c | 457 ++++++++++++
net/ieee80211/softmac/ieee80211softmac_priv.h | 230 ++++++
net/ieee80211/softmac/ieee80211softmac_scan.c | 244 ++++++
net/ieee80211/softmac/ieee80211softmac_wx.c | 412 ++++++++++
30 files changed, 4531 insertions(+), 328 deletions(-)
Adrian Bunk:
hostap: Fix hw reset after CMDCODE_ACCESS_WRITE timeout
hostap: Fix ap_add_sta() return value verification
Dan Williams:
wireless/airo: clean up printk usage to print device name
wireless/airo: define default MTU
wireless/airo: cache wireless scans
David Woodhouse:
Restore channel setting after scan.
Denis Vlasenko:
ieee80211_rx_any: filter out packets, call ieee80211_rx or ieee80211_rx_mgt
Eugene Teo:
hostap: Fix double free in prism2_config() error path
Jean Tourrilhes:
WE-20 for kernel 2.6.16
Johannes Berg:
wireless: Add softmac layer to the kernel
make softmac depend on IEEE80211 and EXPERIMENTAL
softmac: fix some sparse warnings
softmac: fix Makefiles
softmac: convert to use global workqueue
softmac: correctly use netif_carrier_{on,off}
softmac: try to reassociate when being disassociated from the AP
softmac: add fixme for disassoc
softmac: select "best" network based on rssi
softmac: check if disassociation is for us before processing it
softmac: scan at least once before selecting a network by essid
softmac: properly check return value of ieee80211softmac_alloc_mgt
softmac: some comment stuff
softmac: add copyright and license headers
softmac: add MODULE_DESCRIPTION and MODULE_AUTHORs
softmac: move EXPORT_SYMBOL_GPL right after functions
update copyright in softmac
trivial fixes to softmac
softmac: update deauth handler to quiet warning
softmac: add reassociation code
softmac: remove dead code
John W. Linville:
softmac: remove function_enter()
Jouni Malinen:
hostap: Fix unlikely read overrun in CIS parsing
hostap: Remove dead code (duplicated idx != 0)
hostap: Fix memory leak on PCI probe error path
Larry Finger:
Fix softmac scan
Stephen Hemminger:
sky2: typo in last stats patch
sky2: Fix RX stats
sky2: dont need to use dev_kfree_skb_any
skge: align receive buffers
skge: dont use dev_alloc_skb for rx buffs
skge: rx_reuse called twice
skge: multicast statistics fix
skge: dont free skb until multi-part transmit complete
skge: compute available ring buffers
skge: version 1.5
^ permalink raw reply
* Re: Re: [iproute2] IPoIB link layer address bug
From: Mark Butler @ 2006-03-23 18:10 UTC (permalink / raw)
To: James Lentini; +Cc: netdev, openib-general, Stephen Hemminger
In-Reply-To: <Pine.LNX.4.61.0603231155401.23670@jlentini-linux.nane.netapp.com>
[-- Attachment #1.1: Type: text/plain, Size: 1460 bytes --]
James Lentini wrote:
>On Tue, 21 Mar 2006, Jason Gunthorpe wrote:
>
>
>
>>On Tue, Mar 21, 2006 at 03:56:17PM -0800, Stephen Hemminger wrote:
>>
>>
>>
>>>Okay, but there are number of other places in iproute2 that call
>>>ll_addr_a2n() with ifr.ifr_hwaddr.sa_data. And that is 14 bytes.
>>>If you want to fix those it will be harder since it would increase
>>>the sizeof(struct sockaddr) and potentially break compatibility.
>>>
>>>
>>Maybe the best thing is to upgrade ip (and or netlink?) to use
>>netlink messages instead of ioctls for the remaining problematic
>>operations. Since netlink already supports an arbitary length hwaddr
>>there should be no compatability problem.
>>
>>Just browsing I see usages of SIOCSIFHWBROADCAST, SIOCSIFHWADDR,
>>SIOCADDMULTI, SIOCDELMULTI and SIOCGIFHWADDR that use a struct
>>ifreq..
>>
>>I know SIOCGIFHWADDR can be done over netlink, but I'm not too
>>familiar with the others..
>>
>>
>
>Making ip neighbor work with IPoIB address is what I'm interested in
>now.
>
>As you and Jason point out there are a lot of places where ifreqs are
>used and hence options that will not support IPoIB addresses.
>
>
>
The sockaddr union is at the end of struct ifreq. Couldn't the union
sockaddr members be changed to sockaddr_storage, and the SIOCxxxx
encoded size bits be changed? dev_ifsioc() would just need to mask out
(or substitute) the size bits before the switch statement.
- Mark
[-- Attachment #1.2: Type: text/html, Size: 1989 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: Re: [iproute2] IPoIB link layer address bug
From: James Lentini @ 2006-03-23 17:12 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, openib-general
In-Reply-To: <20060322013046.GA29127@obsidianresearch.com>
On Tue, 21 Mar 2006, Jason Gunthorpe wrote:
> On Tue, Mar 21, 2006 at 03:56:17PM -0800, Stephen Hemminger wrote:
>
> > Okay, but there are number of other places in iproute2 that call
> > ll_addr_a2n() with ifr.ifr_hwaddr.sa_data. And that is 14 bytes.
> > If you want to fix those it will be harder since it would increase
> > the sizeof(struct sockaddr) and potentially break compatibility.
>
> Maybe the best thing is to upgrade ip (and or netlink?) to use
> netlink messages instead of ioctls for the remaining problematic
> operations. Since netlink already supports an arbitary length hwaddr
> there should be no compatability problem.
>
> Just browsing I see usages of SIOCSIFHWBROADCAST, SIOCSIFHWADDR,
> SIOCADDMULTI, SIOCDELMULTI and SIOCGIFHWADDR that use a struct
> ifreq..
>
> I know SIOCGIFHWADDR can be done over netlink, but I'm not too
> familiar with the others..
Making ip neighbor work with IPoIB address is what I'm interested in
now.
As you and Jason point out there are a lot of places where ifreqs are
used and hence options that will not support IPoIB addresses.
Do you agree with Jason's strategy of moving the ioctls to netlink
messages (if netlink analogs exist)?
^ permalink raw reply
* Re: [RFC][UPDATED PATCH 2.6.16] [Patch 9/9] Generic netlink interface for delay accounting
From: Balbir Singh @ 2006-03-23 15:41 UTC (permalink / raw)
To: jamal; +Cc: Matt Helsley, Shailabh Nagar, linux-kernel, netdev
In-Reply-To: <1143122686.5186.27.camel@jzny2>
On Thu, Mar 23, 2006 at 09:04:46AM -0500, jamal wrote:
>
> Hi Balbir,
>
> Looking good.
> This is a quick scan, so i didnt look at little details.
Thanks for your detailed feedback. Please find my response interspersed
along with your comments.
> Some comments embedded.
>
> On Wed, 2006-22-03 at 13:19 +0530, Balbir Singh wrote:
>
>
>
>
> >
>
> > diff -puN /dev/null include/linux/taskstats.h
>
> > + * 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.
> > + */
> > +
>
>
>
> > +enum {
> > + TASKSTATS_MSG_UNICAST, /* send data only to requester */
> > + TASKSTATS_MSG_MULTICAST, /* send data to a group */
> > +};
> > +
>
> Above will never be used outside of the kernel.
> Should it go under the ifdef kernel below?
>
Will do
>
> > +#ifdef __KERNEL__
> > +
>
> Note: some people will argue that you should probably have
> two header files. One for all kernel things and includes another
> which contains all the stuff above ifdef __KERNEL__
>
> > +
> > +#endif /* __KERNEL__ */
> > +#endif /* _LINUX_TASKSTATS_H */
>
>
>
>
> > diff -puN kernel/Makefile~delayacct-genetlink kernel/Makefile
> > --- linux-2.6.16/kernel/Makefile~delayacct-genetlink 2006-03-22 11:56:03.000000000 +0530
> > +++ linux-2.6.16-balbir/kernel/Makefile 2006-03-22 11:56:03.000000000 +0530
>
> > +
> > +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,
>
> Do you need to specify hdrsize of 0?
I will remove it
>
>
> > +static int prepare_reply(struct genl_info *info, u8 cmd, struct sk_buff **skbp,
> > + void **replyp)
> > +{
> > + struct sk_buff *skb;
> > + void *reply;
> > +
> > + skb = nlmsg_new(NLMSG_GOODSIZE);
>
> Ok, getting a size of NLMSG_GOODSIZE is not a good idea.
> The max size youll ever get it seems to me is 2*32-bit-data TLVs +
> sizeof struct stats. Why dont you allocate that size?
>
Will do
> > + 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);
>
> Double check if you need NLM_F_REQUEST
>
It is not required, I will remove it
> > + } else
> > + reply = genlmsg_put(skb, info->snd_pid, info->snd_seq,
> > + family.id, 0, info->nlhdr->nlmsg_flags,
> > + info->genlhdr->cmd, family.version);
>
> A Response to a GET is a NEW. So i dont think info->genlhdr->cmd is the
> right thing?
>
>
I will change it
>
>
>
> > +static int taskstats_send_stats(struct sk_buff *skb, struct genl_info *info)
> > +{
> > + int rc;
> > + struct sk_buff *rep_skb;
> > + struct taskstats stats;
> > + void *reply;
> > +
> > + memset(&stats, 0, sizeof(stats));
> > + rc = prepare_reply(info, info->genlhdr->cmd, &rep_skb, &reply);
>
> Same comment as before: a response to a GET is a NEW; so
> info->genlhdr->cmd doesnt seem right.
>
I will change it
> > + if (rc < 0)
> > + return rc;
> > +
> > + if (info->attrs[TASKSTATS_CMD_ATTR_PID]) {
> > + u32 pid = nla_get_u32(info->attrs[TASKSTATS_CMD_ATTR_PID]);
> > + NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_PID, pid);
> > + rc = fill_pid((pid_t)pid, NULL, &stats);
> > + if (rc < 0)
> > + return rc;
> > + }
> > +
> > + if (info->attrs[TASKSTATS_CMD_ATTR_TGID]) {
> > + u32 tgid = nla_get_u32(info->attrs[TASKSTATS_CMD_ATTR_TGID]);
> > + NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_TGID, tgid);
> > + rc = fill_tgid((pid_t)tgid, NULL, &stats);
> > + if (rc < 0)
> > + return rc;
> > + }
> > +
>
> Should there be at least either a pid or tgid? If yes, you need to
> validate here...
>
Yes, you are correct. One of my test cases caught it too.. But I did
not want to untidy the code with if-else's which will keep growing if
the attributes change in the future. I just followed the controller
example. I will change it and validate it. Currently if the attribute
is not valid, a stat of all zero's is returned back.
> > + NLA_PUT_TYPE(rep_skb, struct taskstats, TASKSTATS_TYPE_STATS, stats);
> > + return send_reply(rep_skb, info->snd_pid, TASKSTATS_MSG_UNICAST);
> > +
> > +nla_put_failure:
> > + return genlmsg_cancel(rep_skb, reply);
> > +
>
> As a general comment double check your logic for errors; if you already
> have stashed something in the skb, you need to remove it etc.
>
Wouldn't genlmsg_cancel() take care of cleaning all attributes?
> > +}
> > +
> > +
> > +/* Send pid data out on exit */
> > +void taskstats_exit_pid(struct task_struct *tsk)
> > +{
> > + int rc;
> > + struct sk_buff *rep_skb;
> > + void *reply;
> > + struct taskstats stats;
> > +
> > + /*
> > + * tasks can start to exit very early. Ensure that the family
> > + * is registered before notifications are sent out
> > + */
> > + if (!family_registered)
> > + return;
> > +
> > + memset(&stats, 0, sizeof(stats));
> > + rc = prepare_reply(NULL, TASKSTATS_CMD_NEW, &rep_skb, &reply);
> > + if (rc < 0)
> > + return;
> > +
> > + NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_PID, (u32)tsk->pid);
> > + rc = fill_pid(tsk->pid, tsk, &stats);
> > + if (rc < 0)
> > + return;
> > +
> > + NLA_PUT_TYPE(rep_skb, struct taskstats, TASKSTATS_TYPE_STATS, stats);
> > + rc = send_reply(rep_skb, 0, TASKSTATS_MSG_MULTICAST);
> > +
> > + if (rc || thread_group_empty(tsk))
> > + return;
> > +
> > + /* Send tgid data too */
> > + rc = prepare_reply(NULL, TASKSTATS_CMD_NEW, &rep_skb, &reply);
> > + if (rc < 0)
> > + return;
> > +
>
> A single message with PID+TGID sounds reasonable. Why two messages with
> two stats? all you will need to do is get rid of the prepare_reply()
> above and NLA_PUT_U32() below (just like you do in a response to a GET.
>
The reason for two stats is that for TGID, we return accumulated values
(of all threads in the group) and for PID we return the value just
for that pid. The return value is
pid
<stats for pid>
tgid
<accumulated stats for tgid>
> > + NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_TGID, (u32)tsk->tgid);
> > + rc = fill_tgid(tsk->tgid, tsk, &stats);
> > + if (rc < 0)
> > + return;
> > +
> > + NLA_PUT_TYPE(rep_skb, struct taskstats, TASKSTATS_TYPE_STATS, stats);
> > + send_reply(rep_skb, 0, TASKSTATS_MSG_MULTICAST);
> > +
> > +nla_put_failure:
> > + genlmsg_cancel(rep_skb, reply);
> > +}
>
>
> Other than the above comments - I believe you have it right.
>
Thanks, I will get back to you with the updated patch soon.
> cheers,
> jamal
Thanks,
Balbir
^ permalink raw reply
* Re: [RFC][UPDATED PATCH 2.6.16] [Patch 9/9] Generic netlink interface for delay accounting
From: jamal @ 2006-03-23 14:04 UTC (permalink / raw)
To: balbir; +Cc: Matt Helsley, Shailabh Nagar, linux-kernel, netdev
In-Reply-To: <20060322074922.GA1164@in.ibm.com>
Hi Balbir,
Looking good.
This is a quick scan, so i didnt look at little details.
Some comments embedded.
On Wed, 2006-22-03 at 13:19 +0530, Balbir Singh wrote:
>
> diff -puN /dev/null include/linux/taskstats.h
> + * 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.
> + */
> +
> +enum {
> + TASKSTATS_MSG_UNICAST, /* send data only to requester */
> + TASKSTATS_MSG_MULTICAST, /* send data to a group */
> +};
> +
Above will never be used outside of the kernel.
Should it go under the ifdef kernel below?
> +#ifdef __KERNEL__
> +
Note: some people will argue that you should probably have
two header files. One for all kernel things and includes another
which contains all the stuff above ifdef __KERNEL__
> +
> +#endif /* __KERNEL__ */
> +#endif /* _LINUX_TASKSTATS_H */
> diff -puN kernel/Makefile~delayacct-genetlink kernel/Makefile
> --- linux-2.6.16/kernel/Makefile~delayacct-genetlink 2006-03-22 11:56:03.000000000 +0530
> +++ linux-2.6.16-balbir/kernel/Makefile 2006-03-22 11:56:03.000000000 +0530
> +
> +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,
Do you need to specify hdrsize of 0?
> +static int prepare_reply(struct genl_info *info, u8 cmd, struct sk_buff **skbp,
> + void **replyp)
> +{
> + struct sk_buff *skb;
> + void *reply;
> +
> + skb = nlmsg_new(NLMSG_GOODSIZE);
Ok, getting a size of NLMSG_GOODSIZE is not a good idea.
The max size youll ever get it seems to me is 2*32-bit-data TLVs +
sizeof struct stats. Why dont you allocate that size?
> + 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);
Double check if you need NLM_F_REQUEST
> + } else
> + reply = genlmsg_put(skb, info->snd_pid, info->snd_seq,
> + family.id, 0, info->nlhdr->nlmsg_flags,
> + info->genlhdr->cmd, family.version);
A Response to a GET is a NEW. So i dont think info->genlhdr->cmd is the
right thing?
> +static int taskstats_send_stats(struct sk_buff *skb, struct genl_info *info)
> +{
> + int rc;
> + struct sk_buff *rep_skb;
> + struct taskstats stats;
> + void *reply;
> +
> + memset(&stats, 0, sizeof(stats));
> + rc = prepare_reply(info, info->genlhdr->cmd, &rep_skb, &reply);
Same comment as before: a response to a GET is a NEW; so
info->genlhdr->cmd doesnt seem right.
> + if (rc < 0)
> + return rc;
> +
> + if (info->attrs[TASKSTATS_CMD_ATTR_PID]) {
> + u32 pid = nla_get_u32(info->attrs[TASKSTATS_CMD_ATTR_PID]);
> + NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_PID, pid);
> + rc = fill_pid((pid_t)pid, NULL, &stats);
> + if (rc < 0)
> + return rc;
> + }
> +
> + if (info->attrs[TASKSTATS_CMD_ATTR_TGID]) {
> + u32 tgid = nla_get_u32(info->attrs[TASKSTATS_CMD_ATTR_TGID]);
> + NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_TGID, tgid);
> + rc = fill_tgid((pid_t)tgid, NULL, &stats);
> + if (rc < 0)
> + return rc;
> + }
> +
Should there be at least either a pid or tgid? If yes, you need to
validate here...
> + NLA_PUT_TYPE(rep_skb, struct taskstats, TASKSTATS_TYPE_STATS, stats);
> + return send_reply(rep_skb, info->snd_pid, TASKSTATS_MSG_UNICAST);
> +
> +nla_put_failure:
> + return genlmsg_cancel(rep_skb, reply);
> +
As a general comment double check your logic for errors; if you already
have stashed something in the skb, you need to remove it etc.
> +}
> +
> +
> +/* Send pid data out on exit */
> +void taskstats_exit_pid(struct task_struct *tsk)
> +{
> + int rc;
> + struct sk_buff *rep_skb;
> + void *reply;
> + struct taskstats stats;
> +
> + /*
> + * tasks can start to exit very early. Ensure that the family
> + * is registered before notifications are sent out
> + */
> + if (!family_registered)
> + return;
> +
> + memset(&stats, 0, sizeof(stats));
> + rc = prepare_reply(NULL, TASKSTATS_CMD_NEW, &rep_skb, &reply);
> + if (rc < 0)
> + return;
> +
> + NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_PID, (u32)tsk->pid);
> + rc = fill_pid(tsk->pid, tsk, &stats);
> + if (rc < 0)
> + return;
> +
> + NLA_PUT_TYPE(rep_skb, struct taskstats, TASKSTATS_TYPE_STATS, stats);
> + rc = send_reply(rep_skb, 0, TASKSTATS_MSG_MULTICAST);
> +
> + if (rc || thread_group_empty(tsk))
> + return;
> +
> + /* Send tgid data too */
> + rc = prepare_reply(NULL, TASKSTATS_CMD_NEW, &rep_skb, &reply);
> + if (rc < 0)
> + return;
> +
A single message with PID+TGID sounds reasonable. Why two messages with
two stats? all you will need to do is get rid of the prepare_reply()
above and NLA_PUT_U32() below (just like you do in a response to a GET.
> + NLA_PUT_U32(rep_skb, TASKSTATS_TYPE_TGID, (u32)tsk->tgid);
> + rc = fill_tgid(tsk->tgid, tsk, &stats);
> + if (rc < 0)
> + return;
> +
> + NLA_PUT_TYPE(rep_skb, struct taskstats, TASKSTATS_TYPE_STATS, stats);
> + send_reply(rep_skb, 0, TASKSTATS_MSG_MULTICAST);
> +
> +nla_put_failure:
> + genlmsg_cancel(rep_skb, reply);
> +}
Other than the above comments - I believe you have it right.
cheers,
jamal
^ permalink raw reply
* Re: [2.6.16-gitX] heavy performance regression in ipw2200 wireless driver
From: Alessandro Suardi @ 2006-03-23 14:02 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Zhu, Yi, James Ketrenos, netdev
In-Reply-To: <20060322191057.304962a4.akpm@osdl.org>
On 3/23/06, Andrew Morton <akpm@osdl.org> wrote:
> "Alessandro Suardi" <alessandro.suardi@gmail.com> wrote:
> >
>
> Pleeeeze try to cc the right people.
Sorry about that - should probably defer bug reporting to times
when I'm actually supposed to be awake (2:20am doesn't fit
the bill obviously :| )
> > Driver - or firmware ? Don't know - since the new git snapshots run
> > 1.1.1 which requires newer firmware from http://ipw2200.sourceforge.net.
> >
> > Symptom -> my new FC5 partition with 2.6.16-git kernels connects via
> > VNC viewer to my bittorrent box over wireless (ipw2200 to a D-Link
> > G604T router/AP); Dell D610 runs FC5, BT box is a K7-800 running
> > FC3 with a 2.6.16-rc5-git8 kernel (15+ days uptime...).
> >
> > I also run Firefox on the bittorrent box; noticed today (2.6.16-git5) that
> > the screen refresh of pages with images was from time to time very
> > slow (close to unusable).
> >
> > Rebooted into my FC4 partition with a 2.6.16 kernel, everything much
> > snappier. So I ran a scp test from my BT server to the laptop, three
> > times in a row the same file - a 38MB .flac with the laptop in the same
> > physical position (ie, no signal variation). Results...
> >
> > FC5 - 2.6.16-git3:
> >
> > [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> > Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> > asuardi@192.168.1.8's password:
> > KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 971.3KB/s 00:40
> > [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> > Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> > asuardi@192.168.1.8's password:
> > KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 1.3MB/s 00:29
> > [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> > Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> > asuardi@192.168.1.8's password:
> > KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 626.7KB/s 01:02
> >
> >
> > FC4 - 2.6.16:
> >
> > [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> > Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> > asuardi@192.168.1.8's password:
> > KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 1.5MB/s 00:25
> > [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> > Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> > asuardi@192.168.1.8's password:
> > KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 1.7MB/s 00:23
> > [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> > Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> > asuardi@192.168.1.8's password:
> > KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 1.7MB/s 00:22
> >
> > Bottom line - old driver has better performance than the new one,
> > but most noticeably delivers consistent performance.
> >
> > I will be available for testing starting Thursday 30th as I'll be on
> > the road since then. Of course if the problem is identified and
> > fixed earlier, I won't cry ;)
>
> Well. It's not a huge regression. It's a 50%ish regression. We've done
> worse ;)
That scp test shows 50%ish - but that was a quickie. The VNC
client even reported a 719Kbps throughput down from the more
usual 11500Kbps it starts off with. The first scp I tried when the
sluggishness was intolerable was going at 200KB/s - which
shows the problem can easily get in the neighborhood of an
order of magnitude.
Thanks,
--alessandro
"Dreamer ? Each one of us is a dreamer. We just push it down deep because
we are repeatedly told that we are not allowed to dream in real life"
(Reinhold Ziegler)
^ permalink raw reply
* Re: [2.6.16-gitX] heavy performance regression in ipw2200 wireless driver
From: Andrew Morton @ 2006-03-23 3:10 UTC (permalink / raw)
To: Alessandro Suardi; +Cc: linux-kernel, Zhu, Yi, James Ketrenos, netdev
In-Reply-To: <5a4c581d0603221724m391f5466l8a2af3ae7f0aacae@mail.gmail.com>
"Alessandro Suardi" <alessandro.suardi@gmail.com> wrote:
>
Pleeeeze try to cc the right people.
> Driver - or firmware ? Don't know - since the new git snapshots run
> 1.1.1 which requires newer firmware from http://ipw2200.sourceforge.net.
>
> Symptom -> my new FC5 partition with 2.6.16-git kernels connects via
> VNC viewer to my bittorrent box over wireless (ipw2200 to a D-Link
> G604T router/AP); Dell D610 runs FC5, BT box is a K7-800 running
> FC3 with a 2.6.16-rc5-git8 kernel (15+ days uptime...).
>
> I also run Firefox on the bittorrent box; noticed today (2.6.16-git5) that
> the screen refresh of pages with images was from time to time very
> slow (close to unusable).
>
> Rebooted into my FC4 partition with a 2.6.16 kernel, everything much
> snappier. So I ran a scp test from my BT server to the laptop, three
> times in a row the same file - a 38MB .flac with the laptop in the same
> physical position (ie, no signal variation). Results...
>
> FC5 - 2.6.16-git3:
>
> [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> asuardi@192.168.1.8's password:
> KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 971.3KB/s 00:40
> [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> asuardi@192.168.1.8's password:
> KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 1.3MB/s 00:29
> [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> asuardi@192.168.1.8's password:
> KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 626.7KB/s 01:02
>
>
> FC4 - 2.6.16:
>
> [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> asuardi@192.168.1.8's password:
> KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 1.5MB/s 00:25
> [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> asuardi@192.168.1.8's password:
> KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 1.7MB/s 00:23
> [asuardi@donkey melua_2004-09-23_Berlin]$ scp KM_9-23-04_17_The\
> Closest\ Thing\ to\ Crazy.flac 192.168.1.8:/tmp
> asuardi@192.168.1.8's password:
> KM_9-23-04_17_The Closest Thing to Crazy.flac 100% 38MB 1.7MB/s 00:22
>
> Bottom line - old driver has better performance than the new one,
> but most noticeably delivers consistent performance.
>
> I will be available for testing starting Thursday 30th as I'll be on
> the road since then. Of course if the problem is identified and
> fixed earlier, I won't cry ;)
Well. It's not a huge regression. It's a 50%ish regression. We've done
worse ;)
^ 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