* Re: [PATCH] act_mirred: cleanup and optimization
From: Patrick McHardy @ 2009-11-13 7:13 UTC (permalink / raw)
To: xiaosuo; +Cc: Jamal Hadi Salim, Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <4AFCF06B.1090602@gmail.com>
Changli Gao wrote:
> act_mirred: cleanup and optimization.
>
> cleanup and optimization.
> 1. don't let go back using goto.
> 2. move checking if eaction is valid in tcf_mirred_init().
> 3. don't call skb_act_clone() until it is necessary.
> 4. one exit of the critical context.
> 5. allow eaction is TCA_INGRESS_MIRROR & TCA_INGRESS_REDIR.
>
> if (parm->ifindex) {
> - dev = __dev_get_by_index(&init_net, parm->ifindex);
> + dev = dev_get_by_index(&init_net, parm->ifindex);
> if (dev == NULL)
> return -ENODEV;
This change is not mentioned in the changelog and introduces a
leak. Please split your patches into reasonable portions doing
one change at a time.
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Patrick McHardy @ 2009-11-13 7:18 UTC (permalink / raw)
To: Changli Gao
Cc: David S. Miller, Stephen Hemminger, Eric Dumazet, Tom Herbert,
netdev
In-Reply-To: <412e6f7f0911121732j106f96a2rb77f68098b7fd47e@mail.gmail.com>
Changli Gao wrote:
> On Thu, Nov 12, 2009 at 11:11 PM, Patrick McHardy <kaber@trash.net> wrote:
>> Changli Gao wrote:
>>> The corresponding iprout2 patch is attached.
>> Printing the value during dumps is missing from this patch.
>>
>
> I doesn't know the rules about adding more dumps. Can I add it in the
> end of the corresponding line?
>
> ifb0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN qlen 32 ntxqs 4
Yes, sounds fine.
^ permalink raw reply
* Re: [PATCH] act_mirred: cleanup and optimization
From: Changli Gao @ 2009-11-13 7:22 UTC (permalink / raw)
To: Patrick McHardy
Cc: Jamal Hadi Salim, Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <4AFD0707.50207@trash.net>
On Fri, Nov 13, 2009 at 3:13 PM, Patrick McHardy <kaber@trash.net> wrote:
> Changli Gao wrote:
>> act_mirred: cleanup and optimization.
>>
>> cleanup and optimization.
>> 1. don't let go back using goto.
>> 2. move checking if eaction is valid in tcf_mirred_init().
>> 3. don't call skb_act_clone() until it is necessary.
>> 4. one exit of the critical context.
>> 5. allow eaction is TCA_INGRESS_MIRROR & TCA_INGRESS_REDIR.
>>
>
>> if (parm->ifindex) {
>> - dev = __dev_get_by_index(&init_net, parm->ifindex);
>> + dev = dev_get_by_index(&init_net, parm->ifindex);
>> if (dev == NULL)
>> return -ENODEV;
>
> This change is not mentioned in the changelog and introduces a
> leak. Please split your patches into reasonable portions doing
> one change at a time.
>
Leak? Do I forget to put it some where? I'll keep it as a whole until
Jamal thinks it is in the right form and direction.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] act_mirred: cleanup and optimization
From: Patrick McHardy @ 2009-11-13 7:25 UTC (permalink / raw)
To: Changli Gao; +Cc: Jamal Hadi Salim, Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <412e6f7f0911122322w754f5686jac6c93d39fccab46@mail.gmail.com>
Changli Gao wrote:
> On Fri, Nov 13, 2009 at 3:13 PM, Patrick McHardy <kaber@trash.net> wrote:
>> Changli Gao wrote:
>>> act_mirred: cleanup and optimization.
>>>
>>> cleanup and optimization.
>>> 1. don't let go back using goto.
>>> 2. move checking if eaction is valid in tcf_mirred_init().
>>> 3. don't call skb_act_clone() until it is necessary.
>>> 4. one exit of the critical context.
>>> 5. allow eaction is TCA_INGRESS_MIRROR & TCA_INGRESS_REDIR.
>>>
>>> if (parm->ifindex) {
>>> - dev = __dev_get_by_index(&init_net, parm->ifindex);
>>> + dev = dev_get_by_index(&init_net, parm->ifindex);
>>> if (dev == NULL)
>>> return -ENODEV;
>> This change is not mentioned in the changelog and introduces a
>> leak. Please split your patches into reasonable portions doing
>> one change at a time.
>>
>
> Leak? Do I forget to put it some where? I'll keep it as a whole until
> Jamal thinks it is in the right form and direction.
Ah sorry, I misread the patch.
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Jarek Poplawski @ 2009-11-13 7:45 UTC (permalink / raw)
To: Changli Gao
Cc: Eric Dumazet, David S. Miller, Stephen Hemminger, Patrick McHardy,
Tom Herbert, netdev
In-Reply-To: <412e6f7f0911122216u6880e855g6a15dac29ad6a100@mail.gmail.com>
On 13-11-2009 07:16, Changli Gao wrote:
> 2009/11/13 Eric Dumazet <eric.dumazet@gmail.com>:
>> Messy ? Because of few tests added in code, and branches always
>> correctly predicted ?
>>
>> Still some people might rely on tasklet instead of workqueues
>> and added scheduler stress and latency penalty. Tasklet are softirq
>> and normally are processed a few nanosecs later than RX softirq,
>> on the same CPU, while with your workqueue, I guess the scheduler will
>> try to not migrate it, so we add a penalty for light to moderate load.
>>
>> I guess this new ifb mode would be a regression for them ?
>>
>> If you dont want to maintain a compatibility mode, maybe you
>> should introduce a complete new driver, drivers/net/ifbmq.c or ifbwq.c
>>
>> (multiqueue or workqueue references)
>>
>
> It sounds a good idea.
I don't think so. There would be a lot of code duplication and later
maintenance problems only because of the scheduling method. The main
question is to establish if there is really no performance difference
(which I doubt) - unless Changli can show some tests for various
setups now. On the other hand, if there is a difference, why keep
ineffective solution - similar thing should be possible to do in the
softirq context as well.
So it should not be a big problem to do it a bit messy for some
testing time. Since we can use separate ->ndo_start_xmit() etc. it
shouldn't be too messy, I guess.
Jarek P.
^ permalink raw reply
* [patch 0/4] s390: networking patches for 2.6.33
From: Ursula Braun @ 2009-11-13 7:46 UTC (permalink / raw)
To: davem, netdev, linux-s390; +Cc: schwidefsky, heiko.carstens
Hi Dave,
here is a set of networking patches for 2.6.33.
shortlog:
Frank Blaschka (1)
ctcm: suspend has to wait for outstanding I/O
Ursula Braun (3)
iucv: add work_queue cleanup for suspend
s390: remove cu3088 layer for lcs and ctcm
netiucv: displayed TX bytes value much too high
Thanks,
Ursula
^ permalink raw reply
* [patch 1/4] [PATCH] iucv: add work_queue cleanup for suspend
From: Ursula Braun @ 2009-11-13 7:46 UTC (permalink / raw)
To: davem, netdev, linux-s390; +Cc: schwidefsky, heiko.carstens, Ursula Braun
In-Reply-To: <20091113074626.078348000@linux.vnet.ibm.com>
[-- Attachment #1: 603-iucv-suspend-cleanup.diff --]
[-- Type: text/plain, Size: 1730 bytes --]
From: Ursula Braun <ursula.braun@de.ibm.com>
If iucv_work_queue is not empty during kernel freeze, a kernel panic
occurs. This suspend-patch adds flushing of the work queue for
pending connection requests and severing of remaining pending
connections.
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---
net/iucv/iucv.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
Index: linux-next-uschi/net/iucv/iucv.c
===================================================================
--- linux-next-uschi.orig/net/iucv/iucv.c
+++ linux-next-uschi/net/iucv/iucv.c
@@ -1768,7 +1768,6 @@ static void iucv_tasklet_fn(unsigned lon
*/
static void iucv_work_fn(struct work_struct *work)
{
- typedef void iucv_irq_fn(struct iucv_irq_data *);
LIST_HEAD(work_queue);
struct iucv_irq_list *p, *n;
@@ -1878,14 +1877,25 @@ int iucv_path_table_empty(void)
static int iucv_pm_freeze(struct device *dev)
{
int cpu;
+ struct iucv_irq_list *p, *n;
int rc = 0;
#ifdef CONFIG_PM_DEBUG
printk(KERN_WARNING "iucv_pm_freeze\n");
#endif
+ if (iucv_pm_state != IUCV_PM_FREEZING) {
+ for_each_cpu_mask_nr(cpu, iucv_irq_cpumask)
+ smp_call_function_single(cpu, iucv_block_cpu_almost,
+ NULL, 1);
+ cancel_work_sync(&iucv_work);
+ list_for_each_entry_safe(p, n, &iucv_work_queue, list) {
+ list_del_init(&p->list);
+ iucv_sever_pathid(p->data.ippathid,
+ iucv_error_no_listener);
+ kfree(p);
+ }
+ }
iucv_pm_state = IUCV_PM_FREEZING;
- for_each_cpu_mask_nr(cpu, iucv_irq_cpumask)
- smp_call_function_single(cpu, iucv_block_cpu_almost, NULL, 1);
if (dev->driver && dev->driver->pm && dev->driver->pm->freeze)
rc = dev->driver->pm->freeze(dev);
if (iucv_path_table_empty())
^ permalink raw reply
* [patch 4/4] [PATCH] netiucv: displayed TX bytes value much too high
From: Ursula Braun @ 2009-11-13 7:46 UTC (permalink / raw)
To: davem, netdev, linux-s390; +Cc: schwidefsky, heiko.carstens, Ursula Braun
In-Reply-To: <20091113074626.078348000@linux.vnet.ibm.com>
[-- Attachment #1: 616-netiucv-tx-bytes.diff --]
[-- Type: text/plain, Size: 938 bytes --]
From: Ursula Braun <ursula.braun@de.ibm.com>
tx_bytes value must be updated by skb length before skb is freed.
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---
drivers/s390/net/netiucv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-next-uschi/drivers/s390/net/netiucv.c
===================================================================
--- linux-next-uschi.orig/drivers/s390/net/netiucv.c
+++ linux-next-uschi/drivers/s390/net/netiucv.c
@@ -739,13 +739,13 @@ static void conn_action_txdone(fsm_insta
if (single_flag) {
if ((skb = skb_dequeue(&conn->commit_queue))) {
atomic_dec(&skb->users);
- dev_kfree_skb_any(skb);
if (privptr) {
privptr->stats.tx_packets++;
privptr->stats.tx_bytes +=
(skb->len - NETIUCV_HDRLEN
- - NETIUCV_HDRLEN);
+ - NETIUCV_HDRLEN);
}
+ dev_kfree_skb_any(skb);
}
}
conn->tx_buff->data = conn->tx_buff->head;
^ permalink raw reply
* [patch 2/4] [PATCH] ctcm: suspend has to wait for outstanding I/O
From: Ursula Braun @ 2009-11-13 7:46 UTC (permalink / raw)
To: davem, netdev, linux-s390
Cc: schwidefsky, heiko.carstens, Frank Blaschka, Ursula Braun
In-Reply-To: <20091113074626.078348000@linux.vnet.ibm.com>
[-- Attachment #1: 614-ctcm-suspend-wait.diff --]
[-- Type: text/plain, Size: 2270 bytes --]
From: Frank Blaschka <frank.blaschka@de.ibm.com>
State transition to DEV_STATE_STOPPED indicates all outstanding I/O has
finished. Add wait queue to wait for this state.
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---
drivers/s390/net/ctcm_main.c | 5 +++++
drivers/s390/net/fsm.c | 1 +
drivers/s390/net/fsm.h | 2 ++
3 files changed, 8 insertions(+)
diff -urpN linux-2.6/drivers/s390/net/ctcm_main.c linux-2.6-patched/drivers/s390/net/ctcm_main.c
--- linux-2.6/drivers/s390/net/ctcm_main.c 2009-11-12 14:59:32.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/ctcm_main.c 2009-11-12 14:59:32.000000000 +0100
@@ -1720,6 +1720,11 @@ static int ctcm_pm_suspend(struct ccwgro
return 0;
netif_device_detach(priv->channel[READ]->netdev);
ctcm_close(priv->channel[READ]->netdev);
+ if (!wait_event_timeout(priv->fsm->wait_q,
+ fsm_getstate(priv->fsm) == DEV_STATE_STOPPED, CTCM_TIME_5_SEC)) {
+ netif_device_attach(priv->channel[READ]->netdev);
+ return -EBUSY;
+ }
ccw_device_set_offline(gdev->cdev[1]);
ccw_device_set_offline(gdev->cdev[0]);
return 0;
diff -urpN linux-2.6/drivers/s390/net/fsm.c linux-2.6-patched/drivers/s390/net/fsm.c
--- linux-2.6/drivers/s390/net/fsm.c 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/fsm.c 2009-11-12 14:59:32.000000000 +0100
@@ -27,6 +27,7 @@ init_fsm(char *name, const char **state_
return NULL;
}
strlcpy(this->name, name, sizeof(this->name));
+ init_waitqueue_head(&this->wait_q);
f = kzalloc(sizeof(fsm), order);
if (f == NULL) {
diff -urpN linux-2.6/drivers/s390/net/fsm.h linux-2.6-patched/drivers/s390/net/fsm.h
--- linux-2.6/drivers/s390/net/fsm.h 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/fsm.h 2009-11-12 14:59:32.000000000 +0100
@@ -66,6 +66,7 @@ typedef struct fsm_instance_t {
char name[16];
void *userdata;
int userint;
+ wait_queue_head_t wait_q;
#if FSM_DEBUG_HISTORY
int history_index;
int history_size;
@@ -197,6 +198,7 @@ fsm_newstate(fsm_instance *fi, int newst
printk(KERN_DEBUG "fsm(%s): New state %s\n", fi->name,
fi->f->state_names[newstate]);
#endif
+ wake_up(&fi->wait_q);
}
/**
^ permalink raw reply
* [patch 3/4] [PATCH] s390: remove cu3088 layer for lcs and ctcm
From: Ursula Braun @ 2009-11-13 7:46 UTC (permalink / raw)
To: davem, netdev, linux-s390
Cc: schwidefsky, heiko.carstens, Cornelia Huck, Ursula Braun
In-Reply-To: <20091113074626.078348000@linux.vnet.ibm.com>
[-- Attachment #1: 615-remove-cu3088.diff --]
[-- Type: text/plain, Size: 24426 bytes --]
From: Ursula Braun <ursula.braun@de.ibm.com>
The cu3088-driver used as common base for lcs- and ctcm-devices
makes it difficult to assign the appropriate driver to an lcs-device
or a ctcm-device. This patch eliminates the cu3088-driver and thus
the root device "cu3088". Path /sys/devices/cu3088 is replaced with
the pathes /sys/devices/lcs and /sys/devices/ctcm.
Patch is based on a proposal from Cornelia Huck.
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
---
drivers/s390/net/Makefile | 6 -
drivers/s390/net/claw.c | 82 ++++++++++++++++++++---
drivers/s390/net/claw.h | 12 +++
drivers/s390/net/ctcm_fsms.c | 1
drivers/s390/net/ctcm_fsms.h | 1
drivers/s390/net/ctcm_main.c | 104 +++++++++++++++++++++++------
drivers/s390/net/ctcm_main.h | 20 +++++
drivers/s390/net/ctcm_mpc.c | 1
drivers/s390/net/ctcm_sysfs.c | 11 ++-
drivers/s390/net/cu3088.c | 148 ------------------------------------------
drivers/s390/net/cu3088.h | 41 -----------
drivers/s390/net/lcs.c | 101 ++++++++++++++++++++++++----
drivers/s390/net/lcs.h | 18 +++++
13 files changed, 304 insertions(+), 242 deletions(-)
diff -urpN linux-2.6/drivers/s390/net/claw.c linux-2.6-patched/drivers/s390/net/claw.c
--- linux-2.6/drivers/s390/net/claw.c 2009-11-12 14:58:54.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/claw.c 2009-11-12 14:59:32.000000000 +0100
@@ -90,7 +90,6 @@
#include <linux/timer.h>
#include <linux/types.h>
-#include "cu3088.h"
#include "claw.h"
/*
@@ -258,6 +257,9 @@ static int claw_pm_prepare(struct ccwgro
return -EPERM;
}
+/* the root device for claw group devices */
+static struct device *claw_root_dev;
+
/* ccwgroup table */
static struct ccwgroup_driver claw_group_driver = {
@@ -272,6 +274,47 @@ static struct ccwgroup_driver claw_group
.prepare = claw_pm_prepare,
};
+static struct ccw_device_id claw_ids[] = {
+ {CCW_DEVICE(0x3088, 0x61), .driver_info = claw_channel_type_claw},
+ {},
+};
+MODULE_DEVICE_TABLE(ccw, claw_ids);
+
+static struct ccw_driver claw_ccw_driver = {
+ .owner = THIS_MODULE,
+ .name = "claw",
+ .ids = claw_ids,
+ .probe = ccwgroup_probe_ccwdev,
+ .remove = ccwgroup_remove_ccwdev,
+};
+
+static ssize_t
+claw_driver_group_store(struct device_driver *ddrv, const char *buf,
+ size_t count)
+{
+ int err;
+ err = ccwgroup_create_from_string(claw_root_dev,
+ claw_group_driver.driver_id,
+ &claw_ccw_driver, 3, buf);
+ return err ? err : count;
+}
+
+static DRIVER_ATTR(group, 0200, NULL, claw_driver_group_store);
+
+static struct attribute *claw_group_attrs[] = {
+ &driver_attr_group.attr,
+ NULL,
+};
+
+static struct attribute_group claw_group_attr_group = {
+ .attrs = claw_group_attrs,
+};
+
+static struct attribute_group *claw_group_attr_groups[] = {
+ &claw_group_attr_group,
+ NULL,
+};
+
/*
* Key functions
*/
@@ -3326,7 +3369,11 @@ claw_remove_files(struct device *dev)
static void __exit
claw_cleanup(void)
{
- unregister_cu3088_discipline(&claw_group_driver);
+ driver_remove_file(&claw_group_driver.driver,
+ &driver_attr_group);
+ ccwgroup_driver_unregister(&claw_group_driver);
+ ccw_driver_unregister(&claw_ccw_driver);
+ root_device_unregister(claw_root_dev);
claw_unregister_debug_facility();
pr_info("Driver unloaded\n");
@@ -3348,16 +3395,31 @@ claw_init(void)
if (ret) {
pr_err("Registering with the S/390 debug feature"
" failed with error code %d\n", ret);
- return ret;
+ goto out_err;
}
CLAW_DBF_TEXT(2, setup, "init_mod");
- ret = register_cu3088_discipline(&claw_group_driver);
- if (ret) {
- CLAW_DBF_TEXT(2, setup, "init_bad");
- claw_unregister_debug_facility();
- pr_err("Registering with the cu3088 device driver failed "
- "with error code %d\n", ret);
- }
+ claw_root_dev = root_device_register("qeth");
+ ret = IS_ERR(claw_root_dev) ? PTR_ERR(claw_root_dev) : 0;
+ if (ret)
+ goto register_err;
+ ret = ccw_driver_register(&claw_ccw_driver);
+ if (ret)
+ goto ccw_err;
+ claw_group_driver.driver.groups = claw_group_attr_groups;
+ ret = ccwgroup_driver_register(&claw_group_driver);
+ if (ret)
+ goto ccwgroup_err;
+ return 0;
+
+ccwgroup_err:
+ ccw_driver_unregister(&claw_ccw_driver);
+ccw_err:
+ root_device_unregister(claw_root_dev);
+register_err:
+ CLAW_DBF_TEXT(2, setup, "init_bad");
+ claw_unregister_debug_facility();
+out_err:
+ pr_err("Initializing the claw device driver failed\n");
return ret;
}
diff -urpN linux-2.6/drivers/s390/net/claw.h linux-2.6-patched/drivers/s390/net/claw.h
--- linux-2.6/drivers/s390/net/claw.h 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/claw.h 2009-11-12 14:59:32.000000000 +0100
@@ -129,6 +129,18 @@ static inline int claw_dbf_passes(debug_
} \
} while (0)
+/**
+ * Enum for classifying detected devices.
+ */
+enum claw_channel_types {
+ /* Device is not a channel */
+ claw_channel_type_none,
+
+ /* Device is a CLAW channel device */
+ claw_channel_type_claw
+};
+
+
/*******************************************************
* Define Control Blocks *
* *
diff -urpN linux-2.6/drivers/s390/net/ctcm_fsms.c linux-2.6-patched/drivers/s390/net/ctcm_fsms.c
--- linux-2.6/drivers/s390/net/ctcm_fsms.c 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_fsms.c 2009-11-12 14:59:32.000000000 +0100
@@ -44,7 +44,6 @@
#include <asm/idals.h>
#include "fsm.h"
-#include "cu3088.h"
#include "ctcm_dbug.h"
#include "ctcm_main.h"
diff -urpN linux-2.6/drivers/s390/net/ctcm_fsms.h linux-2.6-patched/drivers/s390/net/ctcm_fsms.h
--- linux-2.6/drivers/s390/net/ctcm_fsms.h 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_fsms.h 2009-11-12 14:59:32.000000000 +0100
@@ -39,7 +39,6 @@
#include <asm/idals.h>
#include "fsm.h"
-#include "cu3088.h"
#include "ctcm_main.h"
/*
diff -urpN linux-2.6/drivers/s390/net/ctcm_main.c linux-2.6-patched/drivers/s390/net/ctcm_main.c
--- linux-2.6/drivers/s390/net/ctcm_main.c 2009-11-12 14:59:32.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/ctcm_main.c 2009-11-12 14:59:32.000000000 +0100
@@ -51,12 +51,16 @@
#include <asm/idals.h>
-#include "cu3088.h"
#include "ctcm_fsms.h"
#include "ctcm_main.h"
/* Some common global variables */
+/**
+ * The root device for ctcm group devices
+ */
+static struct device *ctcm_root_dev;
+
/*
* Linked list of all detected channels.
*/
@@ -246,7 +250,7 @@ static void channel_remove(struct channe
*
* returns Pointer to a channel or NULL if no matching channel available.
*/
-static struct channel *channel_get(enum channel_types type,
+static struct channel *channel_get(enum ctcm_channel_types type,
char *id, int direction)
{
struct channel *ch = channels;
@@ -1342,7 +1346,7 @@ static int ctcm_probe_device(struct ccwg
*
* returns 0 on success, !0 on error.
*/
-static int add_channel(struct ccw_device *cdev, enum channel_types type,
+static int add_channel(struct ccw_device *cdev, enum ctcm_channel_types type,
struct ctcm_priv *priv)
{
struct channel **c = &channels;
@@ -1501,13 +1505,13 @@ free_return: /* note that all channel po
/*
* Return type of a detected device.
*/
-static enum channel_types get_channel_type(struct ccw_device_id *id)
+static enum ctcm_channel_types get_channel_type(struct ccw_device_id *id)
{
- enum channel_types type;
- type = (enum channel_types)id->driver_info;
+ enum ctcm_channel_types type;
+ type = (enum ctcm_channel_types)id->driver_info;
- if (type == channel_type_ficon)
- type = channel_type_escon;
+ if (type == ctcm_channel_type_ficon)
+ type = ctcm_channel_type_escon;
return type;
}
@@ -1525,7 +1529,7 @@ static int ctcm_new_device(struct ccwgro
char read_id[CTCM_ID_SIZE];
char write_id[CTCM_ID_SIZE];
int direction;
- enum channel_types type;
+ enum ctcm_channel_types type;
struct ctcm_priv *priv;
struct net_device *dev;
struct ccw_device *cdev0;
@@ -1749,6 +1753,22 @@ err_out:
return rc;
}
+static struct ccw_device_id ctcm_ids[] = {
+ {CCW_DEVICE(0x3088, 0x08), .driver_info = ctcm_channel_type_parallel},
+ {CCW_DEVICE(0x3088, 0x1e), .driver_info = ctcm_channel_type_ficon},
+ {CCW_DEVICE(0x3088, 0x1f), .driver_info = ctcm_channel_type_escon},
+ {},
+};
+MODULE_DEVICE_TABLE(ccw, ctcm_ids);
+
+static struct ccw_driver ctcm_ccw_driver = {
+ .owner = THIS_MODULE,
+ .name = "ctcm",
+ .ids = ctcm_ids,
+ .probe = ccwgroup_probe_ccwdev,
+ .remove = ccwgroup_remove_ccwdev,
+};
+
static struct ccwgroup_driver ctcm_group_driver = {
.owner = THIS_MODULE,
.name = CTC_DRIVER_NAME,
@@ -1763,6 +1783,33 @@ static struct ccwgroup_driver ctcm_group
.restore = ctcm_pm_resume,
};
+static ssize_t
+ctcm_driver_group_store(struct device_driver *ddrv, const char *buf,
+ size_t count)
+{
+ int err;
+
+ err = ccwgroup_create_from_string(ctcm_root_dev,
+ ctcm_group_driver.driver_id,
+ &ctcm_ccw_driver, 2, buf);
+ return err ? err : count;
+}
+
+static DRIVER_ATTR(group, 0200, NULL, ctcm_driver_group_store);
+
+static struct attribute *ctcm_group_attrs[] = {
+ &driver_attr_group.attr,
+ NULL,
+};
+
+static struct attribute_group ctcm_group_attr_group = {
+ .attrs = ctcm_group_attrs,
+};
+
+static struct attribute_group *ctcm_group_attr_groups[] = {
+ &ctcm_group_attr_group,
+ NULL,
+};
/*
* Module related routines
@@ -1776,7 +1823,10 @@ static struct ccwgroup_driver ctcm_group
*/
static void __exit ctcm_exit(void)
{
- unregister_cu3088_discipline(&ctcm_group_driver);
+ driver_remove_file(&ctcm_group_driver.driver, &driver_attr_group);
+ ccwgroup_driver_unregister(&ctcm_group_driver);
+ ccw_driver_unregister(&ctcm_ccw_driver);
+ root_device_unregister(ctcm_root_dev);
ctcm_unregister_dbf_views();
pr_info("CTCM driver unloaded\n");
}
@@ -1802,17 +1852,31 @@ static int __init ctcm_init(void)
channels = NULL;
ret = ctcm_register_dbf_views();
- if (ret) {
- return ret;
- }
- ret = register_cu3088_discipline(&ctcm_group_driver);
- if (ret) {
- ctcm_unregister_dbf_views();
- pr_err("%s / register_cu3088_discipline failed, ret = %d\n",
- __func__, ret);
- return ret;
- }
+ if (ret)
+ goto out_err;
+ ctcm_root_dev = root_device_register("ctcm");
+ ret = IS_ERR(ctcm_root_dev) ? PTR_ERR(ctcm_root_dev) : 0;
+ if (ret)
+ goto register_err;
+ ret = ccw_driver_register(&ctcm_ccw_driver);
+ if (ret)
+ goto ccw_err;
+ ctcm_group_driver.driver.groups = ctcm_group_attr_groups;
+ ret = ccwgroup_driver_register(&ctcm_group_driver);
+ if (ret)
+ goto ccwgroup_err;
print_banner();
+ return 0;
+
+ccwgroup_err:
+ ccw_driver_unregister(&ctcm_ccw_driver);
+ccw_err:
+ root_device_unregister(ctcm_root_dev);
+register_err:
+ ctcm_unregister_dbf_views();
+out_err:
+ pr_err("%s / Initializing the ctcm device driver failed, ret = %d\n",
+ __func__, ret);
return ret;
}
diff -urpN linux-2.6/drivers/s390/net/ctcm_main.h linux-2.6-patched/drivers/s390/net/ctcm_main.h
--- linux-2.6/drivers/s390/net/ctcm_main.h 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_main.h 2009-11-12 14:59:32.000000000 +0100
@@ -16,7 +16,6 @@
#include <linux/netdevice.h>
#include "fsm.h"
-#include "cu3088.h"
#include "ctcm_dbug.h"
#include "ctcm_mpc.h"
@@ -66,6 +65,23 @@
ctcmpc_dumpit(buf, len); \
} while (0)
+/**
+ * Enum for classifying detected devices
+ */
+enum ctcm_channel_types {
+ /* Device is not a channel */
+ ctcm_channel_type_none,
+
+ /* Device is a CTC/A */
+ ctcm_channel_type_parallel,
+
+ /* Device is a FICON channel */
+ ctcm_channel_type_ficon,
+
+ /* Device is a ESCON channel */
+ ctcm_channel_type_escon
+};
+
/*
* CCW commands, used in this driver.
*/
@@ -121,7 +137,7 @@ struct channel {
* Type of this channel.
* CTC/A or Escon for valid channels.
*/
- enum channel_types type;
+ enum ctcm_channel_types type;
/*
* Misc. flags. See CHANNEL_FLAGS_... below
*/
diff -urpN linux-2.6/drivers/s390/net/ctcm_mpc.c linux-2.6-patched/drivers/s390/net/ctcm_mpc.c
--- linux-2.6/drivers/s390/net/ctcm_mpc.c 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_mpc.c 2009-11-12 14:59:32.000000000 +0100
@@ -53,7 +53,6 @@
#include <linux/moduleparam.h>
#include <asm/idals.h>
-#include "cu3088.h"
#include "ctcm_mpc.h"
#include "ctcm_main.h"
#include "ctcm_fsms.h"
diff -urpN linux-2.6/drivers/s390/net/ctcm_sysfs.c linux-2.6-patched/drivers/s390/net/ctcm_sysfs.c
--- linux-2.6/drivers/s390/net/ctcm_sysfs.c 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/ctcm_sysfs.c 2009-11-12 14:59:32.000000000 +0100
@@ -158,6 +158,15 @@ static ssize_t ctcm_proto_store(struct d
return count;
}
+const char *ctcm_type[] = {
+ "not a channel",
+ "CTC/A",
+ "FICON channel",
+ "ESCON channel",
+ "unknown channel type",
+ "unsupported channel type",
+};
+
static ssize_t ctcm_type_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -168,7 +177,7 @@ static ssize_t ctcm_type_show(struct dev
return -ENODEV;
return sprintf(buf, "%s\n",
- cu3088_type[cgdev->cdev[0]->id.driver_info]);
+ ctcm_type[cgdev->cdev[0]->id.driver_info]);
}
static DEVICE_ATTR(buffer, 0644, ctcm_buffer_show, ctcm_buffer_write);
diff -urpN linux-2.6/drivers/s390/net/cu3088.c linux-2.6-patched/drivers/s390/net/cu3088.c
--- linux-2.6/drivers/s390/net/cu3088.c 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/cu3088.c 1970-01-01 01:00:00.000000000 +0100
@@ -1,148 +0,0 @@
-/*
- * CTC / LCS ccw_device driver
- *
- * Copyright (C) 2002 IBM Deutschland Entwicklung GmbH, IBM Corporation
- * Author(s): Arnd Bergmann <arndb@de.ibm.com>
- * Cornelia Huck <cornelia.huck@de.ibm.com>
- *
- * 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, 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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- *
- */
-
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/err.h>
-
-#include <asm/ccwdev.h>
-#include <asm/ccwgroup.h>
-
-#include "cu3088.h"
-
-const char *cu3088_type[] = {
- "not a channel",
- "CTC/A",
- "ESCON channel",
- "FICON channel",
- "OSA LCS card",
- "CLAW channel device",
- "unknown channel type",
- "unsupported channel type",
-};
-
-/* static definitions */
-
-static struct ccw_device_id cu3088_ids[] = {
- { CCW_DEVICE(0x3088, 0x08), .driver_info = channel_type_parallel },
- { CCW_DEVICE(0x3088, 0x1f), .driver_info = channel_type_escon },
- { CCW_DEVICE(0x3088, 0x1e), .driver_info = channel_type_ficon },
- { CCW_DEVICE(0x3088, 0x60), .driver_info = channel_type_osa2 },
- { CCW_DEVICE(0x3088, 0x61), .driver_info = channel_type_claw },
- { /* end of list */ }
-};
-
-static struct ccw_driver cu3088_driver;
-
-static struct device *cu3088_root_dev;
-
-static ssize_t
-group_write(struct device_driver *drv, const char *buf, size_t count)
-{
- int ret;
- struct ccwgroup_driver *cdrv;
-
- cdrv = to_ccwgroupdrv(drv);
- if (!cdrv)
- return -EINVAL;
- ret = ccwgroup_create_from_string(cu3088_root_dev, cdrv->driver_id,
- &cu3088_driver, 2, buf);
-
- return (ret == 0) ? count : ret;
-}
-
-static DRIVER_ATTR(group, 0200, NULL, group_write);
-
-/* Register-unregister for ctc&lcs */
-int
-register_cu3088_discipline(struct ccwgroup_driver *dcp)
-{
- int rc;
-
- if (!dcp)
- return -EINVAL;
-
- /* Register discipline.*/
- rc = ccwgroup_driver_register(dcp);
- if (rc)
- return rc;
-
- rc = driver_create_file(&dcp->driver, &driver_attr_group);
- if (rc)
- ccwgroup_driver_unregister(dcp);
-
- return rc;
-
-}
-
-void
-unregister_cu3088_discipline(struct ccwgroup_driver *dcp)
-{
- if (!dcp)
- return;
-
- driver_remove_file(&dcp->driver, &driver_attr_group);
- ccwgroup_driver_unregister(dcp);
-}
-
-static struct ccw_driver cu3088_driver = {
- .owner = THIS_MODULE,
- .ids = cu3088_ids,
- .name = "cu3088",
- .probe = ccwgroup_probe_ccwdev,
- .remove = ccwgroup_remove_ccwdev,
-};
-
-/* module setup */
-static int __init
-cu3088_init (void)
-{
- int rc;
-
- cu3088_root_dev = root_device_register("cu3088");
- if (IS_ERR(cu3088_root_dev))
- return PTR_ERR(cu3088_root_dev);
- rc = ccw_driver_register(&cu3088_driver);
- if (rc)
- root_device_unregister(cu3088_root_dev);
-
- return rc;
-}
-
-static void __exit
-cu3088_exit (void)
-{
- ccw_driver_unregister(&cu3088_driver);
- root_device_unregister(cu3088_root_dev);
-}
-
-MODULE_DEVICE_TABLE(ccw,cu3088_ids);
-MODULE_AUTHOR("Arnd Bergmann <arndb@de.ibm.com>");
-MODULE_LICENSE("GPL");
-
-module_init(cu3088_init);
-module_exit(cu3088_exit);
-
-EXPORT_SYMBOL_GPL(cu3088_type);
-EXPORT_SYMBOL_GPL(register_cu3088_discipline);
-EXPORT_SYMBOL_GPL(unregister_cu3088_discipline);
diff -urpN linux-2.6/drivers/s390/net/cu3088.h linux-2.6-patched/drivers/s390/net/cu3088.h
--- linux-2.6/drivers/s390/net/cu3088.h 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/cu3088.h 1970-01-01 01:00:00.000000000 +0100
@@ -1,41 +0,0 @@
-#ifndef _CU3088_H
-#define _CU3088_H
-
-/**
- * Enum for classifying detected devices.
- */
-enum channel_types {
- /* Device is not a channel */
- channel_type_none,
-
- /* Device is a CTC/A */
- channel_type_parallel,
-
- /* Device is a ESCON channel */
- channel_type_escon,
-
- /* Device is a FICON channel */
- channel_type_ficon,
-
- /* Device is a OSA2 card */
- channel_type_osa2,
-
- /* Device is a CLAW channel device */
- channel_type_claw,
-
- /* Device is a channel, but we don't know
- * anything about it */
- channel_type_unknown,
-
- /* Device is an unsupported model */
- channel_type_unsupported,
-
- /* number of type entries */
- num_channel_types
-};
-
-extern const char *cu3088_type[num_channel_types];
-extern int register_cu3088_discipline(struct ccwgroup_driver *);
-extern void unregister_cu3088_discipline(struct ccwgroup_driver *);
-
-#endif
diff -urpN linux-2.6/drivers/s390/net/lcs.c linux-2.6-patched/drivers/s390/net/lcs.c
--- linux-2.6/drivers/s390/net/lcs.c 2009-11-12 14:59:31.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/lcs.c 2009-11-12 14:59:32.000000000 +0100
@@ -47,7 +47,6 @@
#include <asm/ccwgroup.h>
#include "lcs.h"
-#include "cu3088.h"
#if !defined(CONFIG_NET_ETHERNET) && \
@@ -60,7 +59,11 @@
*/
static char version[] __initdata = "LCS driver";
-static char debug_buffer[255];
+
+/**
+ * the root device for lcs group devices
+ */
+static struct device *lcs_root_dev;
/**
* Some prototypes.
@@ -76,6 +79,7 @@ static int lcs_recovery(void *ptr);
/**
* Debug Facility Stuff
*/
+static char debug_buffer[255];
static debug_info_t *lcs_dbf_setup;
static debug_info_t *lcs_dbf_trace;
@@ -1968,6 +1972,15 @@ lcs_portno_store (struct device *dev, st
static DEVICE_ATTR(portno, 0644, lcs_portno_show, lcs_portno_store);
+const char *lcs_type[] = {
+ "not a channel",
+ "2216 parallel",
+ "2216 channel",
+ "OSA LCS card",
+ "unknown channel type",
+ "unsupported channel type",
+};
+
static ssize_t
lcs_type_show(struct device *dev, struct device_attribute *attr, char *buf)
{
@@ -1977,7 +1990,7 @@ lcs_type_show(struct device *dev, struct
if (!cgdev)
return -ENODEV;
- return sprintf(buf, "%s\n", cu3088_type[cgdev->cdev[0]->id.driver_info]);
+ return sprintf(buf, "%s\n", lcs_type[cgdev->cdev[0]->id.driver_info]);
}
static DEVICE_ATTR(type, 0444, lcs_type_show, NULL);
@@ -2370,6 +2383,22 @@ static int lcs_restore(struct ccwgroup_d
return lcs_pm_resume(card);
}
+static struct ccw_device_id lcs_ids[] = {
+ {CCW_DEVICE(0x3088, 0x08), .driver_info = lcs_channel_type_parallel},
+ {CCW_DEVICE(0x3088, 0x1f), .driver_info = lcs_channel_type_2216},
+ {CCW_DEVICE(0x3088, 0x60), .driver_info = lcs_channel_type_osa2},
+ {},
+};
+MODULE_DEVICE_TABLE(ccw, lcs_ids);
+
+static struct ccw_driver lcs_ccw_driver = {
+ .owner = THIS_MODULE,
+ .name = "lcs",
+ .ids = lcs_ids,
+ .probe = ccwgroup_probe_ccwdev,
+ .remove = ccwgroup_remove_ccwdev,
+};
+
/**
* LCS ccwgroup driver registration
*/
@@ -2389,6 +2418,33 @@ static struct ccwgroup_driver lcs_group_
.restore = lcs_restore,
};
+static ssize_t
+lcs_driver_group_store(struct device_driver *ddrv, const char *buf,
+ size_t count)
+{
+ int err;
+ err = ccwgroup_create_from_string(lcs_root_dev,
+ lcs_group_driver.driver_id,
+ &lcs_ccw_driver, 2, buf);
+ return err ? err : count;
+}
+
+static DRIVER_ATTR(group, 0200, NULL, lcs_driver_group_store);
+
+static struct attribute *lcs_group_attrs[] = {
+ &driver_attr_group.attr,
+ NULL,
+};
+
+static struct attribute_group lcs_group_attr_group = {
+ .attrs = lcs_group_attrs,
+};
+
+static struct attribute_group *lcs_group_attr_groups[] = {
+ &lcs_group_attr_group,
+ NULL,
+};
+
/**
* LCS Module/Kernel initialization function
*/
@@ -2400,17 +2456,30 @@ __init lcs_init_module(void)
pr_info("Loading %s\n", version);
rc = lcs_register_debug_facility();
LCS_DBF_TEXT(0, setup, "lcsinit");
- if (rc) {
- pr_err("Initialization failed\n");
- return rc;
- }
-
- rc = register_cu3088_discipline(&lcs_group_driver);
- if (rc) {
- pr_err("Initialization failed\n");
- return rc;
- }
+ if (rc)
+ goto out_err;
+ lcs_root_dev = root_device_register("lcs");
+ rc = IS_ERR(lcs_root_dev) ? PTR_ERR(lcs_root_dev) : 0;
+ if (rc)
+ goto register_err;
+ rc = ccw_driver_register(&lcs_ccw_driver);
+ if (rc)
+ goto ccw_err;
+ lcs_group_driver.driver.groups = lcs_group_attr_groups;
+ rc = ccwgroup_driver_register(&lcs_group_driver);
+ if (rc)
+ goto ccwgroup_err;
return 0;
+
+ccwgroup_err:
+ ccw_driver_unregister(&lcs_ccw_driver);
+ccw_err:
+ root_device_unregister(lcs_root_dev);
+register_err:
+ lcs_unregister_debug_facility();
+out_err:
+ pr_err("Initializing the lcs device driver failed\n");
+ return rc;
}
@@ -2422,7 +2491,11 @@ __exit lcs_cleanup_module(void)
{
pr_info("Terminating lcs module.\n");
LCS_DBF_TEXT(0, trace, "cleanup");
- unregister_cu3088_discipline(&lcs_group_driver);
+ driver_remove_file(&lcs_group_driver.driver,
+ &driver_attr_group);
+ ccwgroup_driver_unregister(&lcs_group_driver);
+ ccw_driver_unregister(&lcs_ccw_driver);
+ root_device_unregister(lcs_root_dev);
lcs_unregister_debug_facility();
}
diff -urpN linux-2.6/drivers/s390/net/lcs.h linux-2.6-patched/drivers/s390/net/lcs.h
--- linux-2.6/drivers/s390/net/lcs.h 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/lcs.h 2009-11-12 14:59:32.000000000 +0100
@@ -36,6 +36,24 @@ static inline int lcs_dbf_passes(debug_i
#define CARD_FROM_DEV(cdev) \
(struct lcs_card *) dev_get_drvdata( \
&((struct ccwgroup_device *)dev_get_drvdata(&cdev->dev))->dev);
+
+/**
+ * Enum for classifying detected devices.
+ */
+enum lcs_channel_types {
+ /* Device is not a channel */
+ lcs_channel_type_none,
+
+ /* Device is a 2216 channel */
+ lcs_channel_type_parallel,
+
+ /* Device is a 2216 channel */
+ lcs_channel_type_2216,
+
+ /* Device is a OSA2 card */
+ lcs_channel_type_osa2
+};
+
/**
* CCW commands used in this driver
*/
diff -urpN linux-2.6/drivers/s390/net/Makefile linux-2.6-patched/drivers/s390/net/Makefile
--- linux-2.6/drivers/s390/net/Makefile 2009-09-10 00:13:59.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/Makefile 2009-11-12 14:59:32.000000000 +0100
@@ -3,11 +3,11 @@
#
ctcm-y += ctcm_main.o ctcm_fsms.o ctcm_mpc.o ctcm_sysfs.o ctcm_dbug.o
-obj-$(CONFIG_CTCM) += ctcm.o fsm.o cu3088.o
+obj-$(CONFIG_CTCM) += ctcm.o fsm.o
obj-$(CONFIG_NETIUCV) += netiucv.o fsm.o
obj-$(CONFIG_SMSGIUCV) += smsgiucv.o
-obj-$(CONFIG_LCS) += lcs.o cu3088.o
-obj-$(CONFIG_CLAW) += claw.o cu3088.o
+obj-$(CONFIG_LCS) += lcs.o
+obj-$(CONFIG_CLAW) += claw.o
qeth-y += qeth_core_sys.o qeth_core_main.o qeth_core_mpc.o
obj-$(CONFIG_QETH) += qeth.o
qeth_l2-y += qeth_l2_main.o
^ permalink raw reply
* Re: [PATCH] netfilter: Should xt_osf_remove_callback() return negative on errors?
From: Patrick McHardy @ 2009-11-13 8:32 UTC (permalink / raw)
To: Roel Kluin; +Cc: Andrew Morton, LKML, David S. Miller, netfilter-devel, netdev
In-Reply-To: <4AFB3B9D.1080606@gmail.com>
Roel Kluin wrote:
> Return a negative error value
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] netfilter: nf_log: fix sleeping function called from invalid context in seq_show()
From: Patrick McHardy @ 2009-11-13 8:35 UTC (permalink / raw)
To: Wu Fengguang; +Cc: LKML, Yin, Kangkai, netdev
In-Reply-To: <20091113063640.GA29012@localhost>
Wu Fengguang wrote:
> [ 171.925285] BUG: sleeping function called from invalid context at kernel/mutex.c:280
> [ 171.925296] in_atomic(): 1, irqs_disabled(): 0, pid: 671, name: grep
> [ 171.925306] 2 locks held by grep/671:
> [ 171.925312] #0: (&p->lock){+.+.+.}, at: [<c10b8acd>] seq_read+0x25/0x36c
> [ 171.925340] #1: (rcu_read_lock){.+.+..}, at: [<c1391dac>] seq_start+0x0/0x44
> [ 171.925372] Pid: 671, comm: grep Not tainted 2.6.31.6-4-netbook #3
> [ 171.925380] Call Trace:
> [ 171.925398] [<c105104e>] ? __debug_show_held_locks+0x1e/0x20
> [ 171.925414] [<c10264ac>] __might_sleep+0xfb/0x102
> [ 171.925430] [<c1461521>] mutex_lock_nested+0x1c/0x2ad
> [ 171.925444] [<c1391c9e>] seq_show+0x74/0x127
> [ 171.925456] [<c10b8c5c>] seq_read+0x1b4/0x36c
> [ 171.925469] [<c10b8aa8>] ? seq_read+0x0/0x36c
> [ 171.925483] [<c10d5c8e>] proc_reg_read+0x60/0x74
> [ 171.925496] [<c10d5c2e>] ? proc_reg_read+0x0/0x74
> [ 171.925510] [<c10a4468>] vfs_read+0x87/0x110
> [ 171.925523] [<c10a458a>] sys_read+0x3b/0x60
> [ 171.925538] [<c1002a49>] syscall_call+0x7/0xb
>
> Fix it by replacing RCU with nf_log_mutex.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13 8:54 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Eric Dumazet, David S. Miller, Stephen Hemminger, Patrick McHardy,
Tom Herbert, netdev
In-Reply-To: <20091113074508.GA6605@ff.dom.local>
On Fri, Nov 13, 2009 at 3:45 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> On 13-11-2009 07:16, Changli Gao wrote:
>
> I don't think so. There would be a lot of code duplication and later
> maintenance problems only because of the scheduling method. The main
> question is to establish if there is really no performance difference
> (which I doubt) - unless Changli can show some tests for various
> setups now. On the other hand, if there is a difference, why keep
> ineffective solution - similar thing should be possible to do in the
> softirq context as well.
>
> So it should not be a big problem to do it a bit messy for some
> testing time. Since we can use separate ->ndo_start_xmit() etc. it
> shouldn't be too messy, I guess.
>
I have done a simple test. I run a simple program on computer A, which
sends SYN packets with random source ports to Computer B's 80 port (No
socket listens on that port, so tcp reset packets will be sent) in
90kpps. On computer B, I redirect the traffic to IFB. At the same
time, I ping from B to A to get the RTT between them. I can't see any
difference between the original IFB and my MQ version. They are both:
CPU idle: 50%
Latency: 0.3-0.4ms, burst 2ms.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Documentation: rw_lock lessons learned
From: Stefan Richter @ 2009-11-13 8:59 UTC (permalink / raw)
To: Stephen Hemminger
Cc: sclark46, William Allen Simpson, Paul E. McKenney, Linus Torvalds,
Linux Kernel Developers, Linux Kernel Network Developers,
Eric Dumazet
In-Reply-To: <20091112150029.1a26c641@nehalam>
Stephen Hemminger wrote:
> On Thu, 12 Nov 2009 14:13:03 -0500
> Stephen Clark <sclark46@earthlink.net> wrote:
>> How up to date is this doc?
>>
>> http://www.kernel.org/pub/linux/kernel/people/rusty/kernel-locking/index.html
>>
>
> Out of date.
> 1. Missing mutex's which have largely replaced semaphores.
>
> 2. Missing change to lock initialization in later kernels.
>
> 3. Missing description of lock dependency checker which should be in same guide.
4. The section on atomic reference counting should refer to <linux/kref.h>.
--
Stefan Richter
-=====-==--= =-== -==-=
http://arcgraph.de/sr/
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Jarek Poplawski @ 2009-11-13 9:18 UTC (permalink / raw)
To: Changli Gao
Cc: Eric Dumazet, David S. Miller, Stephen Hemminger, Patrick McHardy,
Tom Herbert, netdev
In-Reply-To: <412e6f7f0911130054i7a508a6ah16368f11bdc7353d@mail.gmail.com>
On Fri, Nov 13, 2009 at 04:54:50PM +0800, Changli Gao wrote:
> On Fri, Nov 13, 2009 at 3:45 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> > On 13-11-2009 07:16, Changli Gao wrote:
> >
> > I don't think so. There would be a lot of code duplication and later
> > maintenance problems only because of the scheduling method. The main
> > question is to establish if there is really no performance difference
> > (which I doubt) - unless Changli can show some tests for various
> > setups now. On the other hand, if there is a difference, why keep
> > ineffective solution - similar thing should be possible to do in the
> > softirq context as well.
> >
> > So it should not be a big problem to do it a bit messy for some
> > testing time. Since we can use separate ->ndo_start_xmit() etc. it
> > shouldn't be too messy, I guess.
> >
>
> I have done a simple test. I run a simple program on computer A, which
> sends SYN packets with random source ports to Computer B's 80 port (No
> socket listens on that port, so tcp reset packets will be sent) in
> 90kpps. On computer B, I redirect the traffic to IFB. At the same
> time, I ping from B to A to get the RTT between them. I can't see any
> difference between the original IFB and my MQ version. They are both:
>
> CPU idle: 50%
> Latency: 0.3-0.4ms, burst 2ms.
>
I'm mostly concerned with routers doing forwarding with 1Gb or 10Gb
NICs (including multiqueue). Alas/happily I don't have such a problem,
but can't help you with testing either.
Regards,
Jarek P.
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13 9:38 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Eric Dumazet, David S. Miller, Stephen Hemminger, Patrick McHardy,
Tom Herbert, netdev
In-Reply-To: <20091113091825.GA7449@ff.dom.local>
On Fri, Nov 13, 2009 at 5:18 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> On Fri, Nov 13, 2009 at 04:54:50PM +0800, Changli Gao wrote:
>> On Fri, Nov 13, 2009 at 3:45 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
>>
>> I have done a simple test. I run a simple program on computer A, which
>> sends SYN packets with random source ports to Computer B's 80 port (No
>> socket listens on that port, so tcp reset packets will be sent) in
>> 90kpps. On computer B, I redirect the traffic to IFB. At the same
>> time, I ping from B to A to get the RTT between them. I can't see any
>> difference between the original IFB and my MQ version. They are both:
>>
>> CPU idle: 50%
>> Latency: 0.3-0.4ms, burst 2ms.
>>
>
> I'm mostly concerned with routers doing forwarding with 1Gb or 10Gb
> NICs (including multiqueue). Alas/happily I don't have such a problem,
> but can't help you with testing either.
>
Oh, :) . I know more than one companies use kernel threads to forward
packets, and there isn't explicit extra overhead at all. And as you
know, as throughput increases, NAPI will bind the NIC to a CPU, and
softirqd will be waked up to do the work, which should be done in
SoftIRQ context. At that time, there isn't any difference between my
approach and the current kernel's.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* [net-next-2.6 PATCH] ixgbe: Make queue pairs on single MSI-X interrupts
From: Jeff Kirsher @ 2009-11-13 9:50 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Peter P Waskiewicz Jr, Jeff Kirsher
From: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>
This patch pairs similar-numbered Rx and Tx queues onto a single
MSI-X vector. For example, Tx queue 0 and Rx queue 0's interrupt
with be ethX-RxTx-0. This allows for more efficient cleanup, since
fewer interrupts will be firing during device operation. It also
helps with a cleaner CPU affinity for IRQ affinity.
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_main.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 0489286..884152d 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -3626,10 +3626,10 @@ static int ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
* It's easy to be greedy for MSI-X vectors, but it really
* doesn't do us much good if we have a lot more vectors
* than CPU's. So let's be conservative and only ask for
- * (roughly) twice the number of vectors as there are CPU's.
+ * (roughly) the same number of vectors as there are CPU's.
*/
v_budget = min(adapter->num_rx_queues + adapter->num_tx_queues,
- (int)(num_online_cpus() * 2)) + NON_Q_VECTORS;
+ (int)num_online_cpus()) + NON_Q_VECTORS;
/*
* At the same time, hardware can only support a maximum of
^ permalink raw reply related
* Re: [net-next-2.6 PATCH] net: fast consecutive name allocation
From: Octavian Purdila @ 2009-11-13 9:51 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Eric Dumazet, netdev
In-Reply-To: <20091112222608.79d90e9e@nehalam>
On Friday 13 November 2009 08:26:08 you wrote:
> On Fri, 13 Nov 2009 07:12:35 +0100
>
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Octavian Purdila a écrit :
> > > On Friday 13 November 2009 07:01:14 you wrote:
> > >> This patch speeds up the network device name allocation for the case
> > >> where a significant number of devices of the same type are created
> > >> consecutively.
> > >>
> > >> Tests performed on a PPC750 @ 800Mhz machine with per device sysctl
> > >> and sysfs entries disabled:
> > >>
> > >> Without the patch With the patch
> > >>
> > >> real 0m 43.43s real 0m 0.49s
> > >> user 0m 0.00s user 0m 0.00s
> > >> sys 0m 43.43s sys 0m 0.48s
>
> No one has give a reasonable use case for this network device name
> explosion, what is the benchmark doing this nosense, and how do I
> get paid to do it...
>
For us the usecase is creating interfaces that get used by applications that
generate all sorts of traffic. This allows us to simulate realistic end user
traffic (e.g. coming from a full blown stack). That sounds reasonable to us :)
Also, I've seen other people reporting here to use more then 8000 interfaces.
> But I have to say no for another reason. You cause the kernel to choose
> a different name for the case where a device is deleted or renamed.
> The old code would find and fill the hole when a new device was added.
>
> Since this is a semantic ABI change, the kind that drives users nuts.
>
The intent was to keep the old behavior. When the device is deleted we stop
fast allocation and we resume it only after we go through the old code once
again. Did I miss something?
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Jarek Poplawski @ 2009-11-13 9:57 UTC (permalink / raw)
To: Changli Gao
Cc: Eric Dumazet, David S. Miller, Stephen Hemminger, Patrick McHardy,
Tom Herbert, netdev
In-Reply-To: <412e6f7f0911130138td181935w36cab3119972753e@mail.gmail.com>
On Fri, Nov 13, 2009 at 05:38:56PM +0800, Changli Gao wrote:
> On Fri, Nov 13, 2009 at 5:18 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> > I'm mostly concerned with routers doing forwarding with 1Gb or 10Gb
> > NICs (including multiqueue). Alas/happily I don't have such a problem,
> > but can't help you with testing either.
> >
>
> Oh, :) . I know more than one companies use kernel threads to forward
> packets, and there isn't explicit extra overhead at all. And as you
> know, as throughput increases, NAPI will bind the NIC to a CPU, and
> softirqd will be waked up to do the work, which should be done in
> SoftIRQ context. At that time, there isn't any difference between my
> approach and the current kernel's.
I'm not against your solution at all. It only needs more proof... You
seem to forget the main networking paths now are just softirq, and
it's probably for some reason. If kernel threads are good enough, it
seems we should do more such changes.
Jarek P.
^ permalink raw reply
* Re: [net-next-2.6 PATCH] net: fast consecutive name allocation
From: Octavian Purdila @ 2009-11-13 9:55 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <4AFCF8D3.6090905@gmail.com>
On Friday 13 November 2009 08:12:35 you wrote:
> Octavian Purdila a écrit :
> > On Friday 13 November 2009 07:01:14 you wrote:
> >> This patch speeds up the network device name allocation for the case
> >> where a significant number of devices of the same type are created
> >> consecutively.
> >>
> >> Tests performed on a PPC750 @ 800Mhz machine with per device sysctl
> >> and sysfs entries disabled:
> >>
> >> Without the patch With the patch
> >>
> >> real 0m 43.43s real 0m 0.49s
> >> user 0m 0.00s user 0m 0.00s
> >> sys 0m 43.43s sys 0m 0.48s
> >
> > Oops, pasting root prompts (e.g. # modprobe ....) directly into the git
> > commit message is not a good idea :) Here it is again, with the full
> > commit message.
> >
> > [net-next-2.6 PATCH] net: fast consecutive name allocation
> >
> > This patch speeds up the network device name allocation for the case
> > where a significant number of devices of the same type are created
> > consecutively.
> >
> > Tests performed on a PPC750 @ 800Mhz machine with per device sysctl
> > and sysfs entries disabled:
> >
> > $ time insmod /lib/modules/dummy.ko numdummies=8000
> >
> > Without the patch With the patch
> >
> > real 0m 43.43s real 0m 0.49s
> > user 0m 0.00s user 0m 0.00s
> > sys 0m 43.43s sys 0m 0.48s
> >
> > Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> > ---
>
> Honestly I dont like this bloat.
>
> Changing dummy.c is trivial, and you can allocate 100.000.000 dummies if
> you want now :)
>
Yep we can do that - actually we are doing exactly this in our drivers. But in
that way, you get to "bloat" every driver which needs this.
^ permalink raw reply
* [net-next-2.6 PATCH] allow access to sysfs_groups member
From: Kurt Van Dijck @ 2009-11-13 10:51 UTC (permalink / raw)
To: Stephen Hemminger, Oliver Hartkopp, Wolfgang Grandegger; +Cc: netdev
In a recent post, I asked for way to hold the uevent that adds a
net_device until a driver had added some extra sysfs files.
The use-case for such thing is a (PCMCIA) CAN card, that has 2 (CAN)
network chips. The driver could add a 'channel' sysfs file that
indicates the channel on the card, but this sysfs file is not present
yet when the uevent is generated.
This patch applied to a 2.6.30 kernel did allow to use non-standard
sysfs properties in udev rules (together with a match on device/driver).
I believe the above use-case is not limited to CAN, but any other
network type.
This patch allows adding sysfs attribute groups during netdevice registration.
The idea is that before the register_netdev call, several calls to
netdev_sysfs_add_group can be done.
The existing sysfs groups that are added in netdev_register_kobject use the
same function too.
I did not touch the number of possible groups (currently still 3).
Signed-off-by: Kurt Van Dijck <kurt.van.dijck@eia.be>
---
include/linux/netdevice.h | 2 ++
net/core/net-sysfs.c | 28 +++++++++++++++++++++++-----
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8380009..9d16687 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1115,6 +1115,8 @@ extern int dev_open(struct net_device *dev);
extern int dev_close(struct net_device *dev);
extern void dev_disable_lro(struct net_device *dev);
extern int dev_queue_xmit(struct sk_buff *skb);
+extern int netdev_sysfs_add_group(struct net_device *net,
+ struct attribute_group *grp);
extern int register_netdevice(struct net_device *dev);
extern void unregister_netdevice(struct net_device *dev);
extern void free_netdev(struct net_device *dev);
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 753c420..2938fdc 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -527,27 +527,45 @@ void netdev_unregister_kobject(struct net_device * net)
device_del(dev);
}
+/* Add a sysfs group to the netdev groups */
+int netdev_sysfs_add_group(struct net_device *net,
+ struct attribute_group *grp)
+{
+ struct attribute_group **groups = net->sysfs_groups;
+ struct attribute_group **end;
+
+ /* end pointer, with room for null terminator */
+ end = &net->sysfs_groups[ARRAY_SIZE(net->sysfs_groups) - 1];
+ for (; groups < end; ++groups) {
+ if (!*groups) {
+ *groups = grp;
+ return 0;
+ }
+ }
+ return -ENOSPC;
+}
+EXPORT_SYMBOL(netdev_sysfs_add_group);
+
/* Create sysfs entries for network device. */
int netdev_register_kobject(struct net_device *net)
{
struct device *dev = &(net->dev);
- const struct attribute_group **groups = net->sysfs_groups;
dev->class = &net_class;
dev->platform_data = net;
- dev->groups = groups;
+ dev->groups = net->sysfs_groups;
dev_set_name(dev, "%s", net->name);
#ifdef CONFIG_SYSFS
- *groups++ = &netstat_group;
+ netdev_sysfs_add_group(net, &netstat_group);
#ifdef CONFIG_WIRELESS_EXT_SYSFS
if (net->ieee80211_ptr)
- *groups++ = &wireless_group;
+ netdev_sysfs_add_group(net, &wireless_group);
#ifdef CONFIG_WIRELESS_EXT
else if (net->wireless_handlers)
- *groups++ = &wireless_group;
+ netdev_sysfs_add_group(net, &wireless_group);
#endif
#endif
#endif /* CONFIG_SYSFS */
^ permalink raw reply related
* Re: Problem with VLANs and via-velocity driver
From: Séguier Régis @ 2009-11-13 10:36 UTC (permalink / raw)
To: Kevin Shanahan; +Cc: netdev
In-Reply-To: <4AFCFF67.3060802@trash.net>
Patrick McHardy a écrit :
> Kevin Shanahan wrote:
>
>> Hi,
>>
>> I've had some problems with getting a fairly simple (I thought) VLAN
>> configuration working with the on board Via NICs on my Via M700
>> board. Looks like as soon as a tagged VLAN interface is added, the
>> underlying "raw" (untagged) interface stops responding.
>>
>> ...
>>
>> A bit of searching found a few references to similar problems going
>> back a few years (2005, 2007). Sounded like there were some driver
>> issues, but it wasn't clear from the messages I found whether they
>> were believed to be fixed or not. I tried the same test using a
>> differnt NIC with the tg3 driver and there were no problems, so it
>> looks to me like it's still a via-velocity issue. Unfortunately I
>> don't have room to add NICs to this machine and need to use the on
>> board Via hardware.
>>
>
> There's some special-casing for VID 0 in velocity_init_cam_filter().
> Does "ip link add link eth0 type vlan id 0" make any difference?
> If not, does "ip link set eth0 promisc on"?
>
When we patch the driver to support multiple vlan, we decide to use the
vlan 0 to desactivate the vlan filtering.
via-velocity: fix vlan receipt
- vlans were using a single CAM register (see mac_set_vlan_cam)
- setting the address filtering registers for vlans is not
needed when there is no vlan
The non-tagged interface is filtered out as soon as a tagged
(!= 0) interface is created. Its traffic appears again when an
zero-tagged interface is created.
Tested on Via Epia SN (VT6130 chipset) with several vlans whose
tag was above or beyond 255.
Signed-off-by: Séguier Régis <rseguier@e-teleport.net>
Acked-by: Francois Romieu <romieu@fr.zoreil.com>
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
--
Régis Séguier
^ permalink raw reply
* Re: Problem with VLANs and via-velocity driver
From: Patrick McHardy @ 2009-11-13 11:11 UTC (permalink / raw)
To: technique; +Cc: Kevin Shanahan, netdev
In-Reply-To: <4AFD36C0.3090302@e-teleport.net>
Séguier Régis wrote:
> Patrick McHardy a écrit :
>> Kevin Shanahan wrote:
>>
>>> I've had some problems with getting a fairly simple (I thought) VLAN
>>> configuration working with the on board Via NICs on my Via M700
>>> board. Looks like as soon as a tagged VLAN interface is added, the
>>> underlying "raw" (untagged) interface stops responding.
>>>
>>>
>> There's some special-casing for VID 0 in velocity_init_cam_filter().
>> Does "ip link add link eth0 type vlan id 0" make any difference?
>> If not, does "ip link set eth0 promisc on"?
>>
> When we patch the driver to support multiple vlan, we decide to use the
> vlan 0 to desactivate the vlan filtering.
Why? The usual behaviour is to receive both tagged and untagged
frames.
> via-velocity: fix vlan receipt
>
> - vlans were using a single CAM register (see mac_set_vlan_cam)
> - setting the address filtering registers for vlans is not
> needed when there is no vlan
>
> The non-tagged interface is filtered out as soon as a tagged
> (!= 0) interface is created. Its traffic appears again when an
> zero-tagged interface is created.
VID 0 doesn't mean untagged, it means the tag contains only a priority
value.
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13 11:25 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Eric Dumazet, David S. Miller, Stephen Hemminger, Patrick McHardy,
Tom Herbert, netdev
In-Reply-To: <20091113095731.GA7749@ff.dom.local>
On Fri, Nov 13, 2009 at 5:57 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
>
> I'm not against your solution at all. It only needs more proof...
I know. :)
> You seem to forget the main networking paths now are just softirq, and
> it's probably for some reason. If kernel threads are good enough, it
> seems we should do more such changes.
>
I find there is still a softirqd kernel thread for each online CPU,
and these threads will be waked up if there are more SoftIRQs need to
be done after restarting the SoftIRQ processing many times, to keep
the whole system responsible. Did I miss sth.? And at the other side,
real time branch just wants to make all the activities based on kernel
threads, even ISR.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: Problem with VLANs and via-velocity driver
From: Séguier Régis @ 2009-11-13 11:40 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Kevin Shanahan, netdev
In-Reply-To: <4AFD3ED6.2040606@trash.net>
Patrick McHardy a écrit :
>
>>> There's some special-casing for VID 0 in velocity_init_cam_filter().
>>> Does "ip link add link eth0 type vlan id 0" make any difference?
>>> If not, does "ip link set eth0 promisc on"?
>>>
>>>
>> When we patch the driver to support multiple vlan, we decide to use the
>> vlan 0 to desactivate the vlan filtering.
>>
>
> Why? The usual behaviour is to receive both tagged and untagged
> frames.
>
We decide this with françois as an alternative to desactivate the NIC
vlan filtering instead of creating a modul parameter.
The vlan id 0 need to be created but not necessary activate.
In september 2008, we had a discution about exactly the same problem.
You speak about the possibility of an ALLVLAN device flag.
I think i proposed a patch to desactivate NIC vlan filtering when we are
in promisc mode but not used.
--
Régis Séguier
^ 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