* Re: Scalability of interface creation and deletion
From: Eric Dumazet @ 2011-05-08 7:12 UTC (permalink / raw)
To: Alex Bligh; +Cc: netdev, Paul E. McKenney
In-Reply-To: <1304793749.3207.26.camel@edumazet-laptop>
Le samedi 07 mai 2011 à 20:42 +0200, Eric Dumazet a écrit :
> Here is my trace here for one device deletion on one 8 core machine
>
> [ 800.447012] synchronize_rcu() in 15787 us
> [ 800.455013] synchronize_rcu() in 7682 us
> [ 800.464019] rcu_barrier() in 8487 us
>
> Not that bad.
>
> $ grep RCU .config
> # RCU Subsystem
> CONFIG_TREE_RCU=y
> # CONFIG_PREEMPT_RCU is not set
> CONFIG_RCU_TRACE=y
> CONFIG_RCU_FANOUT=32
> # CONFIG_RCU_FANOUT_EXACT is not set
> # CONFIG_RCU_FAST_NO_HZ is not set
> CONFIG_TREE_RCU_TRACE=y
>
By the way, if I change HZ from 1000 to 100 I now have ten times slower
result :
# ip link add link eth0 eth0.103 type vlan id 103
# time ip link del eth0.103
real 0m0.430s
user 0m0.000s
sys 0m0.000s
So all this is related to your HZ value, even in a CONFIG_NO_HZ=y
kernel. Alex, I guess you have HZ=250 ?
# uname -a
Linux svivoipvnx021 2.6.39-rc6-00214-g5511a34-dirty #574 SMP Sun May 8
08:44:14 CEST 2011 x86_64 x86_64 x86_64 GNU/Linux
# cat /proc/cmdline
I enabled CONFIG_RCU_FAST_NO_HZ and got worse results (but not
alsways... its very variable)
# time ip link del eth0.103
real 0m0.544s
user 0m0.000s
sys 0m0.000s
# time ip link del eth0.103
real 0m0.414s
user 0m0.000s
sys 0m0.000s
^ permalink raw reply
* [PATCH] netconsole: switch to kstrto*() functions
From: Alexey Dobriyan @ 2011-05-08 6:33 UTC (permalink / raw)
To: davem; +Cc: netdev
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
drivers/net/netconsole.c | 62 ++++++++++-------------------------------------
1 file changed, 14 insertions(+), 48 deletions(-)
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -242,34 +242,6 @@ static struct netconsole_target *to_target(struct config_item *item)
}
/*
- * Wrapper over simple_strtol (base 10) with sanity and range checking.
- * We return (signed) long only because we may want to return errors.
- * Do not use this to convert numbers that are allowed to be negative.
- */
-static long strtol10_check_range(const char *cp, long min, long max)
-{
- long ret;
- char *p = (char *) cp;
-
- WARN_ON(min < 0);
- WARN_ON(max < min);
-
- ret = simple_strtol(p, &p, 10);
-
- if (*p && (*p != '\n')) {
- printk(KERN_ERR "netconsole: invalid input\n");
- return -EINVAL;
- }
- if ((ret < min) || (ret > max)) {
- printk(KERN_ERR "netconsole: input %ld must be between "
- "%ld and %ld\n", ret, min, max);
- return -EINVAL;
- }
-
- return ret;
-}
-
-/*
* Attribute operations for netconsole_target.
*/
@@ -327,12 +299,14 @@ static ssize_t store_enabled(struct netconsole_target *nt,
const char *buf,
size_t count)
{
+ int enabled;
int err;
- long enabled;
- enabled = strtol10_check_range(buf, 0, 1);
- if (enabled < 0)
- return enabled;
+ err = kstrtoint(buf, 10, &enabled);
+ if (err < 0)
+ return err;
+ if (enabled < 0 || enabled > 1)
+ return -EINVAL;
if (enabled) { /* 1 */
@@ -384,8 +358,7 @@ static ssize_t store_local_port(struct netconsole_target *nt,
const char *buf,
size_t count)
{
- long local_port;
-#define __U16_MAX ((__u16) ~0U)
+ int rv;
if (nt->enabled) {
printk(KERN_ERR "netconsole: target (%s) is enabled, "
@@ -394,12 +367,9 @@ static ssize_t store_local_port(struct netconsole_target *nt,
return -EINVAL;
}
- local_port = strtol10_check_range(buf, 0, __U16_MAX);
- if (local_port < 0)
- return local_port;
-
- nt->np.local_port = local_port;
-
+ rv = kstrtou16(buf, 10, &nt->np.local_port);
+ if (rv < 0)
+ return rv;
return strnlen(buf, count);
}
@@ -407,8 +377,7 @@ static ssize_t store_remote_port(struct netconsole_target *nt,
const char *buf,
size_t count)
{
- long remote_port;
-#define __U16_MAX ((__u16) ~0U)
+ int rv;
if (nt->enabled) {
printk(KERN_ERR "netconsole: target (%s) is enabled, "
@@ -417,12 +386,9 @@ static ssize_t store_remote_port(struct netconsole_target *nt,
return -EINVAL;
}
- remote_port = strtol10_check_range(buf, 0, __U16_MAX);
- if (remote_port < 0)
- return remote_port;
-
- nt->np.remote_port = remote_port;
-
+ rv = kstrtou16(buf, 10, &nt->np.remote_port);
+ if (rv < 0)
+ return rv;
return strnlen(buf, count);
}
^ permalink raw reply
* Re: [net-next-2.6 0/5][pull request] Intel Wired LAN Driver Update
From: David Miller @ 2011-05-08 5:59 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1304763923-6839-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sat, 7 May 2011 03:25:18 -0700
> The following series contains updates to e100, e1000, igb and ixgbe.
>
> Sorry for the delay on the e100/e1000/igb convert to set_phys_id patches,
> it was due to me falling ill and not completing the patches in a timely
> manner.
>
> The following are changes since commit 706527280ec38fcdcd0466f10b607105fd23801b:
> ipv4: Initialize cork->opt using NULL not 0
> and are available in the git repository at:
> master.kernel.org:/pub/scm/linux/kernel/git/jkirsher/net-next-2.6 master
Pulled, thanks Jeff!
^ permalink raw reply
* Re: [PATCH 7/7] ns: Wire up the setns system call
From: James Bottomley @ 2011-05-08 4:02 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, jamal,
Daniel Lezcano, Linux Containers, Renato Westphal
In-Reply-To: <m162pm53ye.fsf@fess.ebiederm.org>
On Sat, 2011-05-07 at 19:19 -0700, Eric W. Biederman wrote:
> James Bottomley <James.Bottomley@HansenPartnership.com> writes:
>
> > On Fri, 2011-05-06 at 19:25 -0700, Eric W. Biederman wrote:
> >> v2: Most of the architecture support added by Daniel Lezcano <dlezcano@fr.ibm.com>
> >> v3: ported to v2.6.36-rc4 by: Eric W. Biederman <ebiederm@xmission.com>
> >> v4: Moved wiring up of the system call to another patch
> >> v5: ported to v2.6.39-rc6
> >>
> >> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> >
> > We already have several syscalls queued up for parisc:
> >
> > http://git.kernel.org/?p=linux/kernel/git/jejb/parisc-2.6.git;a=shortlog;h=refs/heads/misc
> >
> > So if you could make this patch over them (or over linux-next), that
> > would help the merge process.
>
> I will take a look. I was rather pleasantly surprised that no other
> system call conflicts had shown up before now.
>
> This is unfortunately one of those areas where it is almost impossible
> to avoid conflicts.
>
> Do you know if there is any chance that the parisc tree might get
> rebased or anything horrible like that?
It shouldn't unless one of the wire ups is wrong and I have to replace
it, but I think the possibility of that will be minute.
> If not I think I will just pull the hunk of the parisc tree with the syscalls
> e38f5b745075828ac51b12c8c95c85a7be4a3ec7...2e7bad5f34b5beed47542490c760ed26574e38ba
> into my tree so I don't have to worry about merge order.
Yes, that should work fine.
James
^ permalink raw reply
* Re: [PATCH 2/7] ns: Introduce the setns syscall
From: Matt Helsley @ 2011-05-08 3:51 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, netdev, linux-kernel, Linux Containers, linux-fsdevel
In-Reply-To: <1304735101-1824-2-git-send-email-ebiederm@xmission.com>
On Fri, May 06, 2011 at 07:24:56PM -0700, Eric W. Biederman wrote:
> With the networking stack today there is demand to handle
> multiple network stacks at a time. Not in the context
> of containers but in the context of people doing interesting
> things with routing.
>
> There is also demand in the context of containers to have
> an efficient way to execute some code in the container itself.
> If nothing else it is very useful ad a debugging technique.
>
> Both problems can be solved by starting some form of login
> daemon in the namespaces people want access to, or you
> can play games by ptracing a process and getting the
> traced process to do things you want it to do. However
> it turns out that a login daemon or a ptrace puppet
> controller are more code, they are more prone to
> failure, and generally they are less efficient than
> simply changing the namespace of a process to a
> specified one.
>
> Pieces of this puzzle can also be solved by instead of
> coming up with a general purpose system call coming up
> with targed system calls perhaps socketat that solve
> a subset of the larger problem. Overall that appears
> to be more work for less reward.
>
> int setns(int fd, int nstype);
>
> The fd argument is a file descriptor referring to a proc
> file of the namespace you want to switch the process to.
>
> In the setns system call the nstype is 0 or specifies
> an clone flag of the namespace you intend to change
> to prevent changing a namespace unintentionally.
>
> v2: Most of the architecture support added by Daniel Lezcano <dlezcano@fr.ibm.com>
> v3: ported to v2.6.36-rc4 by: Eric W. Biederman <ebiederm@xmission.com>
> v4: Moved wiring up of the system call to another patch
>
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
> kernel/nsproxy.c | 37 +++++++++++++++++++++++++++++++++++++
> 1 files changed, 37 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
> index a05d191..96059d8 100644
> --- a/kernel/nsproxy.c
> +++ b/kernel/nsproxy.c
> @@ -22,6 +22,9 @@
> #include <linux/pid_namespace.h>
> #include <net/net_namespace.h>
> #include <linux/ipc_namespace.h>
> +#include <linux/proc_fs.h>
> +#include <linux/file.h>
> +#include <linux/syscalls.h>
>
> static struct kmem_cache *nsproxy_cachep;
>
> @@ -233,6 +236,40 @@ void exit_task_namespaces(struct task_struct *p)
> switch_task_namespaces(p, NULL);
> }
>
> +SYSCALL_DEFINE2(setns, int, fd, int, nstype)
> +{
> + const struct proc_ns_operations *ops;
> + struct task_struct *tsk = current;
> + struct nsproxy *new_nsproxy;
> + struct proc_inode *ei;
> + struct file *file;
> + int err;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + file = proc_ns_fget(fd);
> + if (IS_ERR(file))
> + return PTR_ERR(file);
> +
> + err = -EINVAL;
> + ei = PROC_I(file->f_dentry->d_inode);
> + ops = ei->ns_ops;
> + if (nstype && (ops->type != nstype))
> + goto out;
> +
> + new_nsproxy = create_new_namespaces(0, tsk, tsk->fs);
Doesn't this need some error checking like:
if (IS_ERR(new_nsproxy)) {
err = PTR_ERR(new_nsproxy);
goto out;
}
> + err = ops->install(new_nsproxy, ei->ns);
> + if (err) {
> + free_nsproxy(new_nsproxy);
> + goto out;
> + }
> + switch_task_namespaces(tsk, new_nsproxy);
> +out:
> + fput(file);
> + return err;
> +}
> +
> static int __init nsproxy_cache_init(void)
> {
> nsproxy_cachep = KMEM_CACHE(nsproxy, SLAB_PANIC);
> --
> 1.6.5.2.143.g8cc62
>
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers
^ permalink raw reply
* Re: Scalability of interface creation and deletion
From: Ben Greear @ 2011-05-08 3:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Alex Bligh, netdev
In-Reply-To: <1304787065.3207.17.camel@edumazet-laptop>
On 05/07/2011 09:51 AM, Eric Dumazet wrote:
> Le samedi 07 mai 2011 à 09:44 -0700, Ben Greear a écrit :
>> On 05/07/2011 09:37 AM, Eric Dumazet wrote:
>>> Le samedi 07 mai 2011 à 09:23 -0700, Ben Greear a écrit :
>>>
>>>> I wonder if it would be worth having a 'delete me soon'
>>>> method to delete interfaces that would not block on the
>>>> RCU code.
>>>>
>>>> The controlling programs could use netlink messages to
>>>> know exactly when an interface was truly gone.
>>>>
>>>> That should allow some batching in the sync-net logic
>>>> too, if user-space code deletes 1000 interfaces very
>>>> quickly, for instance...
>>>>
>>>
>>> I suggested in the past to have an extension of batch capabilities, so
>>> that one kthread could have 3 separate lists of devices being destroyed
>>> in //,
>>>
>>> This daemon would basically loop on one call to synchronize_rcu(), and
>>> transfert list3 to deletion, list2 to list3, list1 to list2, loop,
>>> eventually releasing RTNL while blocked in synchronize_rcu()
>>>
>>> This would need to allow as you suggest an asynchronous deletion method,
>>> or use a callback to wake the process blocked on device delete.
>>
>> I'd want to at least have the option to not block the calling
>> process...otherwise, it would be a lot more difficult to
>> quickly delete 1000 interfaces. You'd need 1000 threads, or
>> sockets, or something to parallelize it otherwise, eh?
>
> Yes, if you can afford not receive a final notification of device being
> fully freed, it should be possible.
Well, I'd hope to get a netlink message about the device being deleted, and
after that, be able to create another one with the same name, etc.
Whether the memory is actually freed in the kernel or not wouldn't matter
to me...
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 7/7] ns: Wire up the setns system call
From: Eric W. Biederman @ 2011-05-08 2:19 UTC (permalink / raw)
To: James Bottomley
Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, jamal,
Daniel Lezcano, Linux Containers, Renato Westphal
In-Reply-To: <1304798775.6123.18.camel@mulgrave.site>
James Bottomley <James.Bottomley@HansenPartnership.com> writes:
> On Fri, 2011-05-06 at 19:25 -0700, Eric W. Biederman wrote:
>> v2: Most of the architecture support added by Daniel Lezcano <dlezcano@fr.ibm.com>
>> v3: ported to v2.6.36-rc4 by: Eric W. Biederman <ebiederm@xmission.com>
>> v4: Moved wiring up of the system call to another patch
>> v5: ported to v2.6.39-rc6
>>
>> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
>
> We already have several syscalls queued up for parisc:
>
> http://git.kernel.org/?p=linux/kernel/git/jejb/parisc-2.6.git;a=shortlog;h=refs/heads/misc
>
> So if you could make this patch over them (or over linux-next), that
> would help the merge process.
I will take a look. I was rather pleasantly surprised that no other
system call conflicts had shown up before now.
This is unfortunately one of those areas where it is almost impossible
to avoid conflicts.
Do you know if there is any chance that the parisc tree might get
rebased or anything horrible like that?
If not I think I will just pull the hunk of the parisc tree with the syscalls
e38f5b745075828ac51b12c8c95c85a7be4a3ec7...2e7bad5f34b5beed47542490c760ed26574e38ba
into my tree so I don't have to worry about merge order.
Eric
^ permalink raw reply
* Re: [PATCH 6/7] net: Allow setting the network namespace by fd
From: Daniel Lezcano @ 2011-05-07 22:46 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, jamal,
Linux Containers, Renato Westphal
In-Reply-To: <1304735101-1824-6-git-send-email-ebiederm@xmission.com>
On 05/07/2011 04:25 AM, Eric W. Biederman wrote:
> Take advantage of the new abstraction and allow network devices
> to be placed in any network namespace that we have a fd to talk
> about.
>
> Signed-off-by: Eric W. Biederman<ebiederm@xmission.com>
> ---
Acked-by: Daniel Lezcano <daniel.lezcano@free.fr>
^ permalink raw reply
* Re: [PATCH 5/7] ns proc: Add support for the ipc namespace
From: Daniel Lezcano @ 2011-05-07 22:44 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, jamal,
Linux Containers, Renato Westphal
In-Reply-To: <1304735101-1824-5-git-send-email-ebiederm@xmission.com>
On 05/07/2011 04:24 AM, Eric W. Biederman wrote:
> Signed-off-by: Eric W. Biederman<ebiederm@xmission.com>
> ---
Acked-by: Daniel Lezcano <daniel.lezcano@free.fr>
^ permalink raw reply
* Re: [PATCH 4/7] ns proc: Add support for the uts namespace
From: Daniel Lezcano @ 2011-05-07 22:42 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, jamal,
Linux Containers, Renato Westphal
In-Reply-To: <1304735101-1824-4-git-send-email-ebiederm@xmission.com>
On 05/07/2011 04:24 AM, Eric W. Biederman wrote:
> Signed-off-by: Eric W. Biederman<ebiederm@xmission.com>
> ---
Acked-by: Daniel Lezcano <daniel.lezcano@free.fr>
^ permalink raw reply
* Re: [PATCH 3/7] ns proc: Add support for the network namespace.
From: Daniel Lezcano @ 2011-05-07 22:41 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, jamal,
Linux Containers, Renato Westphal
In-Reply-To: <1304735101-1824-3-git-send-email-ebiederm@xmission.com>
On 05/07/2011 04:24 AM, Eric W. Biederman wrote:
> Implementing file descriptors for the network namespace
> is simple and straight forward.
>
> Signed-off-by: Eric W. Biederman<ebiederm@xmission.com>
> ---
Acked-by: Daniel Lezcano <daniel.lezcano@free.fr>
^ permalink raw reply
* Re: [PATCH 2/7] ns: Introduce the setns syscall
From: Daniel Lezcano @ 2011-05-07 22:39 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, jamal,
Linux Containers, Renato Westphal
In-Reply-To: <1304735101-1824-2-git-send-email-ebiederm@xmission.com>
On 05/07/2011 04:24 AM, Eric W. Biederman wrote:
> With the networking stack today there is demand to handle
> multiple network stacks at a time. Not in the context
> of containers but in the context of people doing interesting
> things with routing.
>
> There is also demand in the context of containers to have
> an efficient way to execute some code in the container itself.
> If nothing else it is very useful ad a debugging technique.
>
> Both problems can be solved by starting some form of login
> daemon in the namespaces people want access to, or you
> can play games by ptracing a process and getting the
> traced process to do things you want it to do. However
> it turns out that a login daemon or a ptrace puppet
> controller are more code, they are more prone to
> failure, and generally they are less efficient than
> simply changing the namespace of a process to a
> specified one.
>
> Pieces of this puzzle can also be solved by instead of
> coming up with a general purpose system call coming up
> with targed system calls perhaps socketat that solve
> a subset of the larger problem. Overall that appears
> to be more work for less reward.
>
> int setns(int fd, int nstype);
>
> The fd argument is a file descriptor referring to a proc
> file of the namespace you want to switch the process to.
>
> In the setns system call the nstype is 0 or specifies
> an clone flag of the namespace you intend to change
> to prevent changing a namespace unintentionally.
>
> v2: Most of the architecture support added by Daniel Lezcano<dlezcano@fr.ibm.com>
> v3: ported to v2.6.36-rc4 by: Eric W. Biederman<ebiederm@xmission.com>
> v4: Moved wiring up of the system call to another patch
>
> Signed-off-by: Eric W. Biederman<ebiederm@xmission.com>
> ---
Acked-by: Daniel Lezcano <daniel.lezcano@free.fr>
^ permalink raw reply
* Re: [PATCH 1/7] ns: proc files for namespace naming policy.
From: Daniel Lezcano @ 2011-05-07 22:37 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, jamal,
Linux Containers, Renato Westphal
In-Reply-To: <1304735101-1824-1-git-send-email-ebiederm@xmission.com>
On 05/07/2011 04:24 AM, Eric W. Biederman wrote:
> Create files under /proc/<pid>/ns/ to allow controlling the
> namespaces of a process.
>
> This addresses three specific problems that can make namespaces hard to
> work with.
> - Namespaces require a dedicated process to pin them in memory.
> - It is not possible to use a namespace unless you are the child
> of the original creator.
> - Namespaces don't have names that userspace can use to talk about
> them.
>
> The namespace files under /proc/<pid>/ns/ can be opened and the
> file descriptor can be used to talk about a specific namespace, and
> to keep the specified namespace alive.
>
> A namespace can be kept alive by either holding the file descriptor
> open or bind mounting the file someplace else. aka:
> mount --bind /proc/self/ns/net /some/filesystem/path
> mount --bind /proc/self/fd/<N> /some/filesystem/path
>
> This allows namespaces to be named with userspace policy.
>
> It requires additional support to make use of these filedescriptors
> and that will be comming in the following patches.
>
> Signed-off-by: Eric W. Biederman<ebiederm@xmission.com>
Acked-by: Daniel Lezcano <daniel.lezcano@free.fr>
^ permalink raw reply
* Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: Rafael Aquini @ 2011-05-07 21:37 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Joe Perches, kernel-janitors, David Miller, Andy Gospodarek,
shemminger, netdev, Nicolas Kaiser
In-Reply-To: <23896.1304803541@death>
Howdy Jay,
On Sat, May 07, 2011 at 02:25:41PM -0700, Jay Vosburgh wrote:
> My preference would be to only remove the "silence compiler"
> comments if the possibility of silent misbehavior is also eliminated.
> For __ad_timer_to_ticks, that would mean either a default: case in the
> current arrangement, or something like what Joe suggests above.
>
> If this is beyond the scope of what you, Rafeal, want to do,
> that's fine, but in that case leave the "silence" notes in place.
Yeah, re-factor that sort of code was beyond my intentions when I first
submit the patch. However, it is certainly feasible to be done, and as I wrote
before, I'm more than willing to help where is needed to help.
I'll review the places where "silence" notes are placed, and try to figure out a
proper way to get rid of them.
Thank you, folks, for keep hitting me with valuable feedback.
Cheers!
--
Rafael Aquini <aquini@linux.com>
^ permalink raw reply
* Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: Jay Vosburgh @ 2011-05-07 21:25 UTC (permalink / raw)
To: Joe Perches
Cc: aquini, kernel-janitors, David Miller, Andy Gospodarek,
shemminger, netdev, Nicolas Kaiser
In-Reply-To: <1304791360.1738.6.camel@Joe-Laptop>
Joe Perches <joe@perches.com> wrote:
>On Sat, 2011-05-07 at 14:31 -0300, Rafael Aquini wrote:
>> To exemplify my point, I'll taking that very __ad_timer_to_ticks() as an example:
>> static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
>> {
>> u16 retval = 0;
>>
>> switch (timer_type) {
>> case AD_CURRENT_WHILE_TIMER: /* for rx machine usage */
>> if (par)
>> retval = (AD_SHORT_TIMEOUT_TIME*ad_ticks_per_sec);
>> else
>> retval = (AD_LONG_TIMEOUT_TIME*ad_ticks_per_sec);
>> break;
>> case AD_ACTOR_CHURN_TIMER: /* for local churn machine */
>> retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
>> break;
>> case AD_PERIODIC_TIMER: /* for periodic machine */
>> retval = (par*ad_ticks_per_sec);
>> break;
>> case AD_PARTNER_CHURN_TIMER: /* for remote churn machine */
>> retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
>> break;
>> case AD_WAIT_WHILE_TIMER: /* for selection machine */
>> retval = (AD_AGGREGATE_WAIT_TIME*ad_ticks_per_sec);
>> break;
>> }
>> return retval;
>> }
>>
>> If, for some unknown reason timer_type receives an 'alien' value, and
>> we were
>> using retval uninitialized, this function, as it is, would return an
>> unpredictable value to its caller. Unless we have the switch block
>> re-factored, we cannot leave retval uninitialized. So, it's not just a
>> matter of leaving the variable uninitialized, or initialize it just to
>> get rid of a compiler warning. That's why those comments are not
>> helpful anyway.
The comments are helpful, because they mark places where the
code could use some improvement to better handle "impossible"
conditions. The only reason we need the initializer is because the
function doesn't handle all possible inputs. This is probably not the
comment's intended purpose, but they're useful for this nevertheless.
>I'd write this not using a retval variable at all as:
>
> switch (timer_type) {
> case AD_CURRENT_WHILE_TIMER: /* for rx machine usage */
> if (par)
> return AD_SHORT_TIMEOUT_TIME * ad_ticks_per_sec;
> return AD_LONG_TIMEOUT_TIME * ad_ticks_per_sec;
> case AD_ACTOR_CHURN_TIMER: /* for local churn machine */
> return AD_CHURN_DETECTION_TIME * ad_ticks_per_sec;
> ...
> }
> WARN(1, "Invalid timer type: %u\n", timer_type)
> return 0;
>}
Most (perhaps all) of the "silence compiler" comments in this
code exist in places that, like the above, will silently misbehave if
unexpected input arrives. Granted, for the __ad_timer_to_ticks
function, there aren't a lot of call sites, but the other similar cases
exist within the state machine handler code.
My preference would be to only remove the "silence compiler"
comments if the possibility of silent misbehavior is also eliminated.
For __ad_timer_to_ticks, that would mean either a default: case in the
current arrangement, or something like what Joe suggests above.
If this is beyond the scope of what you, Rafeal, want to do,
that's fine, but in that case leave the "silence" notes in place.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: Joe Perches @ 2011-05-07 20:24 UTC (permalink / raw)
To: matt mooney
Cc: David Miller, Jay Vosburgh, Andy Gospodarek, shemminger, netdev,
Nicolas Kaiser, kernel-janitors, aquini
In-Reply-To: <BANLkTimKs39m_htMFEygR+3Bfu7Butc-zQ@mail.gmail.com>
On Sat, 2011-05-07 at 12:35 -0700, matt mooney wrote:
> On Sat, May 7, 2011 at 11:02 AM, Joe Perches <joe@perches.com> wrote:
> > I'd write this not using a retval variable at all as:
[]
> But isn't the preferred style to have a single exit point?
I don't think so. It's not pascal.
^ permalink raw reply
* Re: [PATCH] [RESEND] iwl4965: drop a lone pr_err()
From: julie Sullivan @ 2011-05-07 20:14 UTC (permalink / raw)
To: Paul Bolle; +Cc: John W. Linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1304771519.2227.6.camel@x61.thuisdomein>
> iwl4965_rate_control_register() prints a message at KERN_ERR level.
> - pr_err("Registering 4965 rate control operations\n");
Yes, I noticed this message on boot too and found it a tad confusing...
('Rate control operations'?? 4,965 of them?)
:-) At least I know what it is now. Thanks, Paul.
Julie
^ permalink raw reply
* Re: [PATCH 7/7] ns: Wire up the setns system call
From: James Bottomley @ 2011-05-07 20:06 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, jamal,
Daniel Lezcano, Linux Containers, Renato Westphal
In-Reply-To: <1304735101-1824-7-git-send-email-ebiederm@xmission.com>
On Fri, 2011-05-06 at 19:25 -0700, Eric W. Biederman wrote:
> v2: Most of the architecture support added by Daniel Lezcano <dlezcano@fr.ibm.com>
> v3: ported to v2.6.36-rc4 by: Eric W. Biederman <ebiederm@xmission.com>
> v4: Moved wiring up of the system call to another patch
> v5: ported to v2.6.39-rc6
>
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
We already have several syscalls queued up for parisc:
http://git.kernel.org/?p=linux/kernel/git/jejb/parisc-2.6.git;a=shortlog;h=refs/heads/misc
So if you could make this patch over them (or over linux-next), that
would help the merge process.
Thanks,
James
^ permalink raw reply
* Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: matt mooney @ 2011-05-07 19:35 UTC (permalink / raw)
To: Joe Perches
Cc: aquini, kernel-janitors, David Miller, Jay Vosburgh,
Andy Gospodarek, shemminger, netdev, Nicolas Kaiser
In-Reply-To: <1304791360.1738.6.camel@Joe-Laptop>
On Sat, May 7, 2011 at 11:02 AM, Joe Perches <joe@perches.com> wrote:
> On Sat, 2011-05-07 at 14:31 -0300, Rafael Aquini wrote:
>> To exemplify my point, I'll taking that very __ad_timer_to_ticks() as an example:
>> static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
>> {
>> u16 retval = 0;
>>
>> switch (timer_type) {
>> case AD_CURRENT_WHILE_TIMER: /* for rx machine usage */
>> if (par)
>> retval = (AD_SHORT_TIMEOUT_TIME*ad_ticks_per_sec);
>> else
>> retval = (AD_LONG_TIMEOUT_TIME*ad_ticks_per_sec);
>> break;
>> case AD_ACTOR_CHURN_TIMER: /* for local churn machine */
>> retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
>> break;
>> case AD_PERIODIC_TIMER: /* for periodic machine */
>> retval = (par*ad_ticks_per_sec);
>> break;
>> case AD_PARTNER_CHURN_TIMER: /* for remote churn machine */
>> retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
>> break;
>> case AD_WAIT_WHILE_TIMER: /* for selection machine */
>> retval = (AD_AGGREGATE_WAIT_TIME*ad_ticks_per_sec);
>> break;
>> }
>> return retval;
>> }
>>
>> If, for some unknown reason timer_type receives an 'alien' value, and
>> we were
>> using retval uninitialized, this function, as it is, would return an
>> unpredictable value to its caller. Unless we have the switch block
>> re-factored, we cannot leave retval uninitialized. So, it's not just a
>> matter of leaving the variable uninitialized, or initialize it just to
>> get rid of a compiler warning. That's why those comments are not
>> helpful anyway.
>
> I'd write this not using a retval variable at all as:
But isn't the preferred style to have a single exit point?
> switch (timer_type) {
> case AD_CURRENT_WHILE_TIMER: /* for rx machine usage */
> if (par)
> return AD_SHORT_TIMEOUT_TIME * ad_ticks_per_sec;
> return AD_LONG_TIMEOUT_TIME * ad_ticks_per_sec;
> case AD_ACTOR_CHURN_TIMER: /* for local churn machine */
> return AD_CHURN_DETECTION_TIME * ad_ticks_per_sec;
> ...
> }
> WARN(1, "Invalid timer type: %u\n", timer_type)
> return 0;
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
GPG-Key: 9AFE00EA
^ permalink raw reply
* Re: Scalability of interface creation and deletion
From: Eric Dumazet @ 2011-05-07 19:24 UTC (permalink / raw)
To: Alex Bligh; +Cc: netdev
In-Reply-To: <270382A9E068495F7E8A14CC@Ximines.local>
Le samedi 07 mai 2011 à 19:51 +0100, Alex Bligh a écrit :
>
> --On 7 May 2011 20:32:54 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > Well, there is also one rcu_barrier() call that is expensive.
> > (It was changed from one synchronize_rcu() to one rcu_barrier() lately
> > in commit ef885afb , in 2.6.36 kernel)
>
> I think you are saying it may be waiting in rcu_barrier(). I'll
> instrument that later plus synchronize_sched().
>
> > http://git2.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commi
> > tdiff;h=ef885afbf8a37689afc1d9d545e2f3e7a8276c17
>
> OK, so in head, which I am using, rollback_registered_many which
> previously had 2 calls to synchronize_net(), now has one, followed
> by a call to rc_barrier() at the bottom.
>
each device dismantle needs 2 synchronize_rcu() and one rcu_barrier()
> Right, that's what I patched before (see patch attached to
> message from earlier today) to do an exponential backoff (see
> previous entry), i.e. do a 5ms sleep, then a 10ms, then a 20ms, but
> never more than 250ms. It made no difference.
>
Oh well. How many time are you going to tell us about this ?
We suggested to wait no more than 1 ms, or even shout asap.
If after synchronize_rcu() and rcu_barrier() calls, they are still
references to the device, then there is a BUG somewhere.
Since these bugs are usually not fatal, we just wait a bit.
^ permalink raw reply
* Re: Scalability of interface creation and deletion
From: Alex Bligh @ 2011-05-07 18:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Alex Bligh
In-Reply-To: <1304793174.3207.22.camel@edumazet-laptop>
--On 7 May 2011 20:32:54 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Well, there is also one rcu_barrier() call that is expensive.
> (It was changed from one synchronize_rcu() to one rcu_barrier() lately
> in commit ef885afb , in 2.6.36 kernel)
I think you are saying it may be waiting in rcu_barrier(). I'll
instrument that later plus synchronize_sched().
> http://git2.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commi
> tdiff;h=ef885afbf8a37689afc1d9d545e2f3e7a8276c17
OK, so in head, which I am using, rollback_registered_many which
previously had 2 calls to synchronize_net(), now has one, followed
by a call to rc_barrier() at the bottom.
> netdev_wait_allrefs() waits that all references to a device vanishes.
>
> It currently uses a _very_ pessimistic 250 ms delay between each probe.
> Some users reported that no more than 4 devices can be dismantled per
> second, this is a pretty serious problem for some setups.
Right, that's what I patched before (see patch attached to
message from earlier today) to do an exponential backoff (see
previous entry), i.e. do a 5ms sleep, then a 10ms, then a 20ms, but
never more than 250ms. It made no difference.
> time to remove 50 ipip tunnels on a UP machine :
>
> before patch : real 11.910s
> after patch : real 1.250s
Sadly I don't see that improvement!
--
Alex Bligh
^ permalink raw reply
* Re: Scalability of interface creation and deletion
From: Alex Bligh @ 2011-05-07 18:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Alex Bligh
In-Reply-To: <1304793749.3207.26.camel@edumazet-laptop>
--On 7 May 2011 20:42:29 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Here is my trace here for one device deletion on one 8 core machine
>
> [ 800.447012] synchronize_rcu() in 15787 us
> [ 800.455013] synchronize_rcu() in 7682 us
> [ 800.464019] rcu_barrier() in 8487 us
Would you mind trying it with my script to do veth devices? kill udev and do
unshare -n first.
I've done this on 2 different lots of hardware now, with 3 kernels 18
months apart.
$ grep RCU .config
# RCU Subsystem
CONFIG_TREE_RCU=y
# CONFIG_PREEMPT_RCU is not set
# CONFIG_RCU_TRACE is not set
CONFIG_RCU_FANOUT=64
# CONFIG_RCU_FANOUT_EXACT is not set
CONFIG_RCU_FAST_NO_HZ=y
# CONFIG_TREE_RCU_TRACE is not set
# CONFIG_SPARSE_RCU_POINTER is not set
# CONFIG_RCU_TORTURE_TEST is not set
# CONFIG_RCU_CPU_STALL_DETECTOR is not set
--
Alex Bligh
^ permalink raw reply
* Re: Scalability of interface creation and deletion
From: Eric Dumazet @ 2011-05-07 18:44 UTC (permalink / raw)
To: Alex Bligh; +Cc: netdev
In-Reply-To: <472A3314CA89E2E5270E320A@Ximines.local>
Le samedi 07 mai 2011 à 19:38 +0100, Alex Bligh a écrit :
>
> --On 7 May 2011 18:26:29 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> ># time rmmod dummy
> >
> > real 0m0.111s
> > user 0m0.000s
> > sys 0m0.000s
> >
> >
> > This removed my two dummy0/dummy1 devices.
>
> rmmod dummy even with numdummies=100 does only one synchronize_net() and
> is quick (0.8ms).
>
Yes, thanks to batching we added some time ago to speedup module unload.
And because you didnt setup IP addresses on them ;)
for i in `seq 0 99`
do
ifconfig dummy$i 192.168.$i.1 up
done
^ permalink raw reply
* Re: Scalability of interface creation and deletion
From: Eric Dumazet @ 2011-05-07 18:42 UTC (permalink / raw)
To: Alex Bligh; +Cc: netdev
In-Reply-To: <1304793174.3207.22.camel@edumazet-laptop>
Here is my trace here for one device deletion on one 8 core machine
[ 800.447012] synchronize_rcu() in 15787 us
[ 800.455013] synchronize_rcu() in 7682 us
[ 800.464019] rcu_barrier() in 8487 us
Not that bad.
$ grep RCU .config
# RCU Subsystem
CONFIG_TREE_RCU=y
# CONFIG_PREEMPT_RCU is not set
CONFIG_RCU_TRACE=y
CONFIG_RCU_FANOUT=32
# CONFIG_RCU_FANOUT_EXACT is not set
# CONFIG_RCU_FAST_NO_HZ is not set
CONFIG_TREE_RCU_TRACE=y
^ permalink raw reply
* Re: Scalability of interface creation and deletion
From: Eric Dumazet @ 2011-05-07 18:39 UTC (permalink / raw)
To: Alex Bligh; +Cc: netdev
In-Reply-To: <1304793174.3207.22.camel@edumazet-laptop>
Le samedi 07 mai 2011 à 20:32 +0200, Eric Dumazet a écrit :
Also you could patch synchronize_sched() itself instead of
synchronize_net()
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index dd4aea8..4af6e10 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1518,6 +1518,7 @@ EXPORT_SYMBOL_GPL(call_rcu_bh);
void synchronize_sched(void)
{
struct rcu_synchronize rcu;
+ ktime_t time_start = ktime_get();
if (rcu_blocking_is_gp())
return;
@@ -1529,6 +1530,7 @@ void synchronize_sched(void)
/* Wait for it. */
wait_for_completion(&rcu.completion);
destroy_rcu_head_on_stack(&rcu.head);
+ pr_err("synchronize_rcu() in %lld us\n", ktime_us_delta(ktime_get(), time_start));
}
EXPORT_SYMBOL_GPL(synchronize_sched);
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox