* Re: 2.6.35-rc2, CONFIG_RPS is filling the dmesg log
From: Eric Dumazet @ 2010-06-09 13:42 UTC (permalink / raw)
To: tim.gardner; +Cc: netdev
In-Reply-To: <4C0F96B4.2000307@canonical.com>
Le mercredi 09 juin 2010 à 07:27 -0600, Tim Gardner a écrit :
> On 06/08/2010 02:55 PM, Tim Gardner wrote:
> > With 2.6.35-rc2 my dmesg log is being flooded with messages like this:
> >
> > br0 received packet on queue 4, but number of RX queues is 1
> >
> > This machine is bridged for KVM and has 2 igb network adapters.
> >
> > The root cause appears to be CONFIG_RPS=y and the fact that none of the
> > drivers that call skb_record_rx_queue() perform their net device
> > allocation using alloc_netdev_mq(), thereby initializing num_rx_queues
> > to a maximum of 1.
> >
> > Given that this is early RPS days, is the warning in get_rps_cpu()
> > really necessary? It would appear that _all_ of the multi-receive queue
> > devices that call skb_record_rx_queue() will cause this log noise.
> >
> > By the way, how do you turn off CONFIG_RPS? The only way I could get it
> > disabled was to change the default in net/Kconfig to 'n'.
> >
> > rtg
>
> This is the route that I'm taking with Ubuntu in the short term. I'll
> have lots of server testers complaining pretty soon if I don't take care
> of this now. It does keep my server logs from filling.
>
> rtg
>
Probably fine, but your commit message is not exact :
So far no users of skb_record_rx_queue() use alloc_netdev_mq() for
network device initialization, so don't print a warning about num_rx_queues
imbalances in get_rps_cpu() unless they have actually been allocated.
In fact, drivers that use skb_record_rx_queue() did use alloc_netdev_mq().
Problem is : packets going thru bridge/bonding that are not yet
multiqueue enabled. If R[PF]S enabled for these "virtual devices",
we trigger the get_rps_cpu() warning.
Also, in a bonding setup, we still have a problem
because all tx packets will go thru tx queue 0 (dev_pick_tx() job)
(That might be good to know that for Ubuntu server testers)
^ permalink raw reply
* Re: [PATCH v3] netfilter: Xtables: idletimer target implementation
From: Patrick McHardy @ 2010-06-09 13:45 UTC (permalink / raw)
To: luciano.coelho; +Cc: netfilter-devel, netdev, jengelh, Timo Teras
In-Reply-To: <1275592445-15555-1-git-send-email-luciano.coelho@nokia.com>
luciano.coelho@nokia.com wrote:
> From: Luciano Coelho <luciano.coelho@nokia.com>
>
> This patch implements an idletimer Xtables target that can be used to
> identify when interfaces have been idle for a certain period of time.
>
> Timers are identified by labels and are created when a rule is set with a new
> label. The rules also take a timeout value (in seconds) as an option. If
> more than one rule uses the same timer label, the timer will be restarted
> whenever any of the rules get a hit.
>
> One entry for each timer is created in sysfs. This attribute contains the
> timer remaining for the timer to expire. The attributes are located under
> the xt_idletimer class:
>
> /sys/class/xt_idletimer/timers/<label>
>
> When the timer expires, the target module sends a sysfs notification to the
> userspace, which can then decide what to do (eg. disconnect to save power).
>
Basically this seems fine to me, some minor comments below.
> +++ b/include/linux/netfilter/xt_IDLETIMER.h
> @@ -0,0 +1,40 @@
> +#ifndef _XT_IDLETIMER_H
> +#define _XT_IDLETIMER_H
> +
> +#define MAX_LABEL_SIZE 32
>
This seems a bit generic, maybe better use MAX_IDLETIMER_LABER_SIZE.
> +
> +struct idletimer_tg_info {
> + __u32 timeout;
> +
> + char label[MAX_LABEL_SIZE];
> +};
> +
> +#endif
> new file mode 100644
> index 0000000..65c195e
> --- /dev/null
> +++ b/net/netfilter/xt_IDLETIMER.c
> @@ -0,0 +1,356 @@
> +/*
> + * linux/net/netfilter/xt_IDLETIMER.c
> + *
> + * Netfilter module to trigger a timer when packet matches.
> + * After timer expires a kevent will be sent.
> + *
> + * Copyright (C) 2004, 2010 Nokia Corporation
> + * Written by Timo Teras <ext-timo.teras@nokia.com>
> + *
> + * Converted to x_tables and reworked for upstream inclusion
> + * by Luciano Coelho <luciano.coelho@nokia.com>
> + *
> + * Contact: Luciano Coelho <luciano.coelho@nokia.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * 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., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/timer.h>
> +#include <linux/list.h>
> +#include <linux/spinlock.h>
> +#include <linux/netfilter.h>
> +#include <linux/netfilter/x_tables.h>
> +#include <linux/netfilter/xt_IDLETIMER.h>
> +#include <linux/kobject.h>
> +#include <linux/workqueue.h>
> +#include <linux/sysfs.h>
> +
> +struct idletimer_tg {
> + struct list_head entry;
> + struct timer_list timer;
> + struct work_struct work;
> +
> + struct kobject *kobj;
> + struct idletimer_tg_attr *attr;
> +
> + unsigned int refcnt;
> +};
> +
> +struct idletimer_tg_attr {
> + struct attribute attr;
> + ssize_t (*show)(struct kobject *kobj,
> + struct attribute *attr, char *buf);
> +};
> +
> +static LIST_HEAD(idletimer_tg_list);
>
How does this work with multiple namespaces? It seems every namespace
can bind to any timer.
> +static DEFINE_SPINLOCK(list_lock);
> +
> +static struct kobject *idletimer_tg_kobj;
> +
> +static
> +struct idletimer_tg *__idletimer_tg_find_by_label(const char *label)
> +{
> + struct idletimer_tg *entry;
> +
> + BUG_ON(!label);
> +
> + list_for_each_entry(entry, &idletimer_tg_list, entry) {
> + if (!strcmp(label, entry->attr->attr.name))
> + return entry;
> + }
> +
> + return NULL;
> +}
> +
> +static ssize_t idletimer_tg_show(struct kobject *kobj, struct attribute *attr,
> + char *buf)
> +{
> + struct idletimer_tg *timer;
> + unsigned long expires = 0;
> +
> + spin_lock_bh(&list_lock);
> + timer = __idletimer_tg_find_by_label(attr->name);
> + if (timer)
> + expires = timer->timer.expires;
> + spin_unlock_bh(&list_lock);
> +
> + if (expires > jiffies)
>
time_after()?
> + return sprintf(buf, "%u\n",
> + jiffies_to_msecs(expires - jiffies) / 1000);
> +
> + return sprintf(buf, "0\n");
> +}
> +
> +static void idletimer_tg_delete(const struct idletimer_tg_info *info)
> +{
>
The only caller is the target cleanup function, why don't you just move
everything there?
> + struct idletimer_tg *timer;
> +
> + spin_lock_bh(&list_lock);
> + timer = __idletimer_tg_find_by_label(info->label);
> + if (!timer) {
> + spin_unlock_bh(&list_lock);
> + return;
> + }
> +
> + if (--timer->refcnt == 0) {
> + pr_debug("deleting timer %s\n", info->label);
> +
> + list_del(&timer->entry);
> + del_timer_sync(&timer->timer);
> + spin_unlock_bh(&list_lock);
> +
> + sysfs_remove_file(idletimer_tg_kobj, &timer->attr->attr);
> + kfree(timer->attr->attr.name);
> + kfree(timer->attr);
> + kfree(timer);
> + } else {
> + spin_unlock_bh(&list_lock);
> + pr_debug("decreased refcnt of timer %s to %u\n",
> + info->label, timer->refcnt);
> + }
> +}
> +
> +static void idletimer_tg_work(struct work_struct *work)
> +{
> + struct idletimer_tg *timer = container_of(work, struct idletimer_tg,
> + work);
> +
> + sysfs_notify(idletimer_tg_kobj, NULL,
> + timer->attr->attr.name);
> +}
> +
> +static void idletimer_tg_expired(unsigned long data)
> +{
> + struct idletimer_tg *timer = (struct idletimer_tg *) data;
> +
> + pr_debug("timer %s expired\n",
> + timer->attr->attr.name);
> +
> + schedule_work(&timer->work);
> +}
> +
> +static
> +struct idletimer_tg *idletimer_tg_create(const struct idletimer_tg_info *info)
> +{
> + struct idletimer_tg *timer;
> + struct idletimer_tg_attr *attr;
> +
> + attr = kzalloc(sizeof(attr), GFP_KERNEL);
>
sizeof(*attr)
> + if (!attr) {
> + pr_debug("couldn't alloc attribute\n");
> + return NULL;
> + }
> +
> + attr->attr.name = kstrdup(info->label, GFP_KERNEL);
> + if (!attr->attr.name) {
> + pr_debug("couldn't alloc attribute name\n");
> + goto out_free_attr;
> + }
> + attr->attr.mode = S_IRUGO;
> + attr->show = idletimer_tg_show;
> +
> + if (sysfs_create_file(idletimer_tg_kobj, &attr->attr)) {
> + pr_debug("couldn't add attr to sysfs\n");
> + goto out_free_name;
> + }
> +
> + timer = kmalloc(sizeof(struct idletimer_tg), GFP_KERNEL);
> + if (!timer) {
> + pr_debug("couldn't alloc timer\n");
> + goto out_free_file;
> + }
> +
> + spin_lock_bh(&list_lock);
> + list_add(&timer->entry, &idletimer_tg_list);
> +
> + init_timer(&timer->timer);
> + setup_timer(&timer->timer, idletimer_tg_expired, (unsigned long) timer);
> + mod_timer(&timer->timer,
> + msecs_to_jiffies(info->timeout * 1000) + jiffies);
> +
> + timer->attr = attr;
> + timer->refcnt = 0;
>
Better fully set up the timer before activating it.
> +
> + INIT_WORK(&timer->work, idletimer_tg_work);
> + spin_unlock_bh(&list_lock);
> +
> + return timer;
> +
> +out_free_file:
> + sysfs_remove_file(idletimer_tg_kobj, &attr->attr);
> +out_free_name:
> + kfree(attr->attr.name);
> +out_free_attr:
> + kfree(attr);
> + return NULL;
> +}
> +
> +static void idletimer_tg_cleanup(void)
> +{
> + struct idletimer_tg *timer;
> +
> + spin_lock(&list_lock);
> + list_for_each_entry(timer, &idletimer_tg_list, entry) {
>
list_for_each_entry_safe(). This function seems unnecessary though, the
module
can't be unloaded while its still in use and cleanup will already happen
when the
rules are removed.
> + pr_debug("deleting timer %s\n", timer->attr->attr.name);
> +
> + list_del(&timer->entry);
> + del_timer_sync(&timer->timer);
> + kfree(timer->attr->attr.name);
> + kfree(timer->attr);
> + kfree(timer);
> + }
> + spin_unlock(&list_lock);
> +}
> +
> +/*
> + * The actual xt_tables plugin.
> + */
> +static unsigned int idletimer_tg_target(struct sk_buff *skb,
> + const struct xt_action_param *par)
> +{
> + const struct idletimer_tg_info *info = par->targinfo;
> + struct idletimer_tg *timer;
> +
> + pr_debug("resetting timer %s, timeout period %u\n",
> + info->label, info->timeout);
> +
> + spin_lock(&list_lock);
>
You need BH protection here as well for the output path.
> + timer = __idletimer_tg_find_by_label(info->label);
> +
> + BUG_ON(!timer);
> +
> + mod_timer(&timer->timer,
> + msecs_to_jiffies(info->timeout * 1000) + jiffies);
> + spin_unlock(&list_lock);
> +
> + return XT_CONTINUE;
> +}
> +
> +static int idletimer_tg_checkentry(const struct xt_tgchk_param *par)
> +{
> + const struct idletimer_tg_info *info = par->targinfo;
> + struct idletimer_tg *timer;
> +
> + pr_debug("checkentry targinfo %s\n", info->label);
> +
> + if (info->timeout == 0) {
> + pr_debug("timeout value is zero\n");
> + return -EINVAL;
> + }
> +
> + if (!info->label || strlen(info->label) == 0) {
>
!info->label is impossible. You should check for \0 termination instead.
> + pr_debug("label is missing\n");
> + return -EINVAL;
> + }
> +
> + spin_lock(&list_lock);
>
spin_lock_bh()
> + timer = __idletimer_tg_find_by_label(info->label);
> + if (!timer) {
> + spin_unlock(&list_lock);
> + timer = idletimer_tg_create(info);
>
How does this prevent creating the same timer twice?
> + if (!timer) {
> + pr_debug("failed to create timer\n");
> + return -ENOMEM;
> + }
> + spin_lock(&list_lock);
> + }
> +
> + timer->refcnt++;
> + mod_timer(&timer->timer,
> + msecs_to_jiffies(info->timeout * 1000) + jiffies);
> + spin_unlock(&list_lock);
> +
> + return 0;
> +}
> +
> +static void idletimer_tg_destroy(const struct xt_tgdtor_param *par)
> +{
> + const struct idletimer_tg_info *info = par->targinfo;
> +
> + pr_debug("destroy targinfo %s\n", info->label);
> +
> + idletimer_tg_delete(info);
> +}
> +
> +static struct xt_target idletimer_tg __read_mostly = {
> + .name = "IDLETIMER",
> + .family = NFPROTO_UNSPEC,
> + .target = idletimer_tg_target,
> + .targetsize = sizeof(struct idletimer_tg_info),
> + .checkentry = idletimer_tg_checkentry,
> + .destroy = idletimer_tg_destroy,
> + .me = THIS_MODULE,
> +};
> +
> +static struct class *idletimer_tg_class;
> +
> +static struct device *idletimer_tg_device;
> +
> +static int __init idletimer_tg_init(void)
> +{
> + int err;
> +
> + idletimer_tg_class = class_create(THIS_MODULE, "xt_idletimer");
> + err = PTR_ERR(idletimer_tg_class);
> + if (IS_ERR(idletimer_tg_class)) {
> + pr_debug("couldn't register device class\n");
> + goto out;
> + }
> +
> + idletimer_tg_device = device_create(idletimer_tg_class, NULL,
> + MKDEV(0, 0), NULL, "timers");
> + err = PTR_ERR(idletimer_tg_device);
> + if (IS_ERR(idletimer_tg_device)) {
> + pr_debug("couldn't register system device\n");
> + goto out_class;
> + }
> +
> + idletimer_tg_kobj = &idletimer_tg_device->kobj;
> +
> + err = xt_register_target(&idletimer_tg);
> + if (err < 0) {
> + pr_debug("couldn't register xt target\n");
> + goto out_dev;
> + }
> +
> + return 0;
> +out_dev:
> + device_destroy(idletimer_tg_class, MKDEV(0, 0));
> +out_class:
> + class_destroy(idletimer_tg_class);
> +out:
> + return err;
> +}
> +
> +static void __exit idletimer_tg_exit(void)
> +{
> + xt_unregister_target(&idletimer_tg);
> +
> + device_destroy(idletimer_tg_class, MKDEV(0, 0));
> + class_destroy(idletimer_tg_class);
> +
> + idletimer_tg_cleanup();
> +}
> +
> +module_init(idletimer_tg_init);
> +module_exit(idletimer_tg_exit);
> +
> +MODULE_AUTHOR("Timo Teras <ext-timo.teras@nokia.com>");
> +MODULE_AUTHOR("Luciano Coelho <luciano.coelho@nokia.com>");
> +MODULE_DESCRIPTION("Xtables: idle time monitor");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply
* Re: [ath5k-devel] [PATCH] [ath5k][leds] Ability to disable leds support. If leds support enabled do not force mac802.11 leds layer selection.
From: Bob Copeland @ 2010-06-09 13:58 UTC (permalink / raw)
To: Dmytro Milinevskyy
Cc: ath5k-devel, Kalle Valo, linux-wireless, GeunSik Lim, Jiri Slaby,
Greg Kroah-Hartman, John W. Linville, Keng-Yu Lin, netdev,
Jiri Kosina, Shahar Or, linux-kernel, Luca Verdesca
In-Reply-To: <4c0f4326.0a41df0a.7bde.ffffeef2@mx.google.com>
On Wed, Jun 9, 2010 at 3:31 AM, Dmytro Milinevskyy
<milinevskyy@gmail.com> wrote:
> However if the leds support was enabled do not force selection of 802.11 leds layer.
I don't understand this part. What's the point of enabling software LEDs
if nothing triggers them?
Also, we can probably build hardware LEDs (hw_set_ledstate) in regardless.
It's a pure register write and doesn't require the rest of the LED machinery.
I assume you are doing this to reduce the size of the module. If so, can
you include size(1) output in the commit message?
--
Bob Copeland %% www.bobcopeland.com
^ permalink raw reply
* Re: [v2 Patch 1/2] s2io: add dynamic LRO disable support
From: Ben Hutchings @ 2010-06-09 14:00 UTC (permalink / raw)
To: Amerigo Wang
Cc: netdev, nhorman, sgruszka, herbert.xu, Ramkrishna.Vepa, davem
In-Reply-To: <20100609100928.6573.14199.sendpatchset@localhost.localdomain>
On Wed, 2010-06-09 at 06:05 -0400, Amerigo Wang wrote:
[...]
> +static int s2io_ethtool_set_flags(struct net_device *dev, u32 data)
> +{
> + struct s2io_nic *sp = netdev_priv(dev);
> + int rc = 0;
> + int changed = 0;
> +
> + if (data & ETH_FLAG_LRO) {
> + if (lro_enable) {
> + if (!(dev->features & NETIF_F_LRO)) {
> + dev->features |= NETIF_F_LRO;
> + changed = 1;
> + }
> + } else
> + rc = -EINVAL;
> + } else if (dev->features & NETIF_F_LRO) {
> + dev->features &= ~NETIF_F_LRO;
> + changed = 1;
> + }
> +
> + if (changed && netif_running(dev)) {
> + s2io_stop_all_tx_queue(sp);
> + s2io_card_down(sp);
> + sp->lro = dev->features & NETIF_F_LRO;
> + rc = s2io_card_up(sp);
> + s2io_start_all_tx_queue(sp);
[...]
Is it safe to call s2io_start_all_tx_queue() if s2io_card_up() failed?
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] ipvs: Add missing locking during connection table hashing and unhashing
From: Patrick McHardy @ 2010-06-09 14:12 UTC (permalink / raw)
To: Sven Wegener
Cc: Julian Anastasov, Simon Horman, Wensong Zhang, netdev, lvs-devel
In-Reply-To: <alpine.LNX.2.00.1006080828290.18946@titan.stealer.net>
Sven Wegener wrote:
> The code that hashes and unhashes connections from the connection table
> is missing locking of the connection being modified, which opens up a
> race condition and results in memory corruption when this race condition
> is hit.
>
> Here is what happens in pretty verbose form:
>
> CPU 0 CPU 1
> ------------ ------------
> An active connection is terminated and
> we schedule ip_vs_conn_expire() on this
> CPU to expire this connection.
>
> IRQ assignment is changed to this CPU,
> but the expire timer stays scheduled on
> the other CPU.
>
> New connection from same ip:port comes
> in right before the timer expires, we
> find the inactive connection in our
> connection table and get a reference to
> it. We proper lock the connection in
> tcp_state_transition() and read the
> connection flags in set_tcp_state().
>
> ip_vs_conn_expire() gets called, we
> unhash the connection from our
> connection table and remove the hashed
> flag in ip_vs_conn_unhash(), without
> proper locking!
>
> While still holding proper locks we
> write the connection flags in
> set_tcp_state() and this sets the hashed
> flag again.
>
> ip_vs_conn_expire() fails to expire the
> connection, because the other CPU has
> incremented the reference count. We try
> to re-insert the connection into our
> connection table, but this fails in
> ip_vs_conn_hash(), because the hashed
> flag has been set by the other CPU. We
> re-schedule execution of
> ip_vs_conn_expire(). Now this connection
> has the hashed flag set, but isn't
> actually hashed in our connection table
> and has a dangling list_head.
>
> We drop the reference we held on the
> connection and schedule the expire timer
> for timeouting the connection on this
> CPU. Further packets won't be able to
> find this connection in our connection
> table.
>
> ip_vs_conn_expire() gets called again,
> we think it's already hashed, but the
> list_head is dangling and while removing
> the connection from our connection table
> we write to the memory location where
> this list_head points to.
>
> The result will probably be a kernel oops at some other point in time.
>
> Signed-off-by: Sven Wegener <sven.wegener@stealer.net>
> Cc: stable@kernel.org
> Acked-by: Simon Horman <horms@verge.net.au>
> ---
> net/netfilter/ipvs/ip_vs_conn.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> This race condition is pretty subtle, but it can be triggered remotely.
> It needs the IRQ assignment change or another circumstance where packets
> coming from the same ip:port for the same service are being processed on
> different CPUs. And it involves hitting the exact time at which
> ip_vs_conn_expire() gets called. It can be avoided by making sure that
> all packets from one connection are always processed on the same CPU and
> can be made harder to exploit by changing the connection timeouts to
> some custom values.
>
>
Applied, thanks.
^ permalink raw reply
* Re: no reassembly for outgoing packets on RAW socket
From: Patrick McHardy @ 2010-06-09 14:16 UTC (permalink / raw)
To: Jiri Olsa; +Cc: netdev, Netfilter Developer Mailing List
In-Reply-To: <20100607145558.GA1939@jolsa.lab.eng.brq.redhat.com>
Jiri Olsa wrote:
> On Fri, Jun 04, 2010 at 02:03:17PM +0200, Patrick McHardy wrote:
>
>> Jiri Olsa wrote:
>>
>>> hi,
>>>
>>> I'd like to be able to sendout a single IP packet with MF flag set.
>>>
>>> When using RAW sockets the packet will get stuck in the
>>> netfilter (NF_INET_LOCAL_OUT nf_defrag_ipv4 reassembly unit)
>>> and wont ever make it out..
>>>
>>> I made a change which bypass the outgoing reassembly for
>>> RAW sockets, but I'm not sure wether it's too invasive..
>>>
>> That would break reassembly (and thus connection tracking) for cases
>> where its really intended.
>>
>>
>>> Is there any standard for RAW sockets behaviour?
>>> Or another way around? :)
>>>
>> You could use the NOTRACK target to bypass connection tracking.
>>
>
> ok,
>
> I tried the NOTRACK target, but the packet is still going
> throught reassembly, because the RAW filter has lower priority
> then the connection track defragmentation..
>
Right.
> I was able to get it bypassed by attached patch and following
> command:
>
> iptables -v -t raw -A OUTPUT -p icmp -j NOTRACK
>
> again, not sure if this is too invasive ;)
>
Well, we can't change it in the mainline kernel.
> If this is not the way, I'd appreciatte any hint.. my goal is
> to put malformed packet on the wire (more frags bit set for a
> non fragmented packet)
I don't have any good suggestions besides adding a flag to the IPCB
and skipping defragmentation based on that.
^ permalink raw reply
* Re: 2.6.35-rc2-git2: Reported regressions from 2.6.34
From: Linus Torvalds @ 2010-06-09 14:24 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Carl Worth, Eric Anholt, Venkatesh Pallipadi, Jens Axboe,
Dave Airlie, Jesse Barnes, Ingo Molnar, David Härdeman,
Mauro Carvalho Chehab, Eric Dumazet, Linux Kernel Mailing List,
Maciej Rutecki, Andrew Morton, Kernel Testers List,
Network Development, Linux ACPI, Linux PM List, Linux SCSI List,
Linux Wireless List, DRI
In-Reply-To: <201006091106.11232.rjw@sisk.pl>
On Wed, 9 Jun 2010, Rafael J. Wysocki wrote:
> >
> > That has a "reverting the commit fixes it", and a confirmation from Nick
> > Bowler.
> >
> > Eric, Carl: should I just revert that commit? Or do you have a fix?
>
> This one is reported to have been fixed already. Closed now.
Heh. That "fixed already" is actually the revert. Carl acked it.
> This should be fixed by commit f8ed8b4c5d30b5214f185997131b06e35f6f7113, so
> closing now.
Good, that was in yesterday's drm pull.
Linus
^ permalink raw reply
* Re: [ath5k-devel] [PATCH] [ath5k][leds] Ability to disable leds support. If leds support enabled do not force mac802.11 leds layer selection.
From: Dmytro Milinevskyy @ 2010-06-09 14:43 UTC (permalink / raw)
To: Bob Copeland
Cc: ath5k-devel, Kalle Valo, linux-wireless, GeunSik Lim, Jiri Slaby,
Greg Kroah-Hartman, John W. Linville, Keng-Yu Lin, netdev,
Jiri Kosina, Shahar Or, linux-kernel, Luca Verdesca
In-Reply-To: <AANLkTilrYYHjcMEhhrP7zjcLoo-TNczozxJSNRkDvSZy@mail.gmail.com>
Hi, Bob.
For instance I don't use 802.11 leds layer and trigger leds from
userspace according to my purposes.
-- Dima
On Wed, Jun 9, 2010 at 4:58 PM, Bob Copeland <me@bobcopeland.com> wrote:
> On Wed, Jun 9, 2010 at 3:31 AM, Dmytro Milinevskyy
> <milinevskyy@gmail.com> wrote:
>> However if the leds support was enabled do not force selection of 802.11 leds layer.
>
> I don't understand this part. What's the point of enabling software LEDs
> if nothing triggers them?
>
> Also, we can probably build hardware LEDs (hw_set_ledstate) in regardless.
> It's a pure register write and doesn't require the rest of the LED machinery.
>
> I assume you are doing this to reduce the size of the module. If so, can
> you include size(1) output in the commit message?
>
> --
> Bob Copeland %% www.bobcopeland.com
>
^ permalink raw reply
* Re: [PATCH 1/2] Export firmware assigned labels of network devices to sysfs
From: Greg KH @ 2010-06-09 15:02 UTC (permalink / raw)
To: Matt Domsch
Cc: K, Narendra, netdev@vger.kernel.org,
linux-hotplug@vger.kernel.org, linux-pci@vger.kernel.org,
Hargrave, Jordan, Rose, Charles, Nijhawan, Vijay
In-Reply-To: <20100609041709.GA7280@auslistsprd01.us.dell.com>
On Tue, Jun 08, 2010 at 11:17:09PM -0500, Matt Domsch wrote:
> On Fri, May 28, 2010 at 11:51:40PM -0500, Domsch, Matt wrote:
> > On Fri, May 28, 2010 at 05:27:45PM -0500, Greg KH wrote:
> > > Care to post that ECN publically? And no, the Linux Foundation does not
> > > have a PCI-SIG membership, the PCI-SIG keeps forbidding it. Other
> > > operating systems are allowed to join but not Linux. Strange but
> > > true...
> >
> > I'm looking into it, and should know more next week.
>
> I'm advised that I cannot post the ECN publically, due to it being an
> in-progress work item of a SIG working group, and therefore falls
> under the confidentiality rules that SIG members agree to. Members of
> the PCI SIG have access, which unfortunately is not everyone.
Then we can't properly review this, sorry. How about waiting until the
ECN is finalized? Then we could review and possibly accept this.
thanks,
greg k-h
^ permalink raw reply
* Re: 2.6.35-rc2-git1 - include/linux/cgroup.h:534 invoked rcu_dereference_check() without protection!
From: Miles Lane @ 2010-06-09 15:11 UTC (permalink / raw)
To: Peter Zijlstra
Cc: paulmck, Vivek Goyal, Eric Paris, Lai Jiangshan, Ingo Molnar,
LKML, nauman, eric.dumazet, netdev, Jens Axboe, Gui Jianfeng,
Li Zefan, Johannes Berg
In-Reply-To: <1276004075.2987.208.camel@twins>
On Tue, Jun 8, 2010 at 9:34 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, 2010-06-08 at 09:14 -0400, Miles Lane wrote:
>
>> CC kernel/sched.o
>> kernel/sched.c: In function ‘task_group’:
>> kernel/sched.c:321: error: implicit declaration of function ‘task_rq’
>> kernel/sched.c:321: error: invalid type argument of ‘->’ (have ‘int’)
>> make[1]: *** [kernel/sched.o] Error 1
>>
>> I had to apply with fuzz. Did it mess up?
>
>
> No, I probably did.. task_rq() is defined on line 636 or thereabouts,
> and this function landed around line 320.
>
> Ahh, and it compiled here because I have CGROUP_SCHED=y, but
> PROVE_RCU=n, so that whole check expression disappears and is never
> evaluated...
>
> /me fixes
>
> ---
> Subject: sched: PROVE_RCU vs cpu_cgroup
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Tue Jun 08 11:40:42 CEST 2010
>
> PROVE_RCU has a few issues with the cpu_cgroup because the scheduler
> typically holds rq->lock around the css rcu derefs but the generic
> cgroup code doesn't (and can't) know about that lock.
>
> Provide means to add extra checks to the css dereference and use that
> in the scheduler to annotate its users.
>
> The addition of rq->lock to these checks is correct because the
> cgroup_subsys::attach() method takes the rq->lock for each task it
> moves, therefore by holding that lock, we ensure the task is pinned to
> the current cgroup and the RCU dereference is valid.
>
> That leaves one genuine race in __sched_setscheduler() where we used
> task_group() without holding any of the required locks and thus raced
> with the cgroup code. Solve this by moving the check under the rq->lock.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> ---
> include/linux/cgroup.h | 20 +++++---
> kernel/sched.c | 115 +++++++++++++++++++++++++------------------------
> 2 files changed, 73 insertions(+), 62 deletions(-)
>
> Index: linux-2.6/include/linux/cgroup.h
> ===================================================================
> --- linux-2.6.orig/include/linux/cgroup.h
> +++ linux-2.6/include/linux/cgroup.h
> @@ -525,13 +525,21 @@ static inline struct cgroup_subsys_state
> return cgrp->subsys[subsys_id];
> }
>
> -static inline struct cgroup_subsys_state *task_subsys_state(
> - struct task_struct *task, int subsys_id)
> +/*
> + * function to get the cgroup_subsys_state which allows for extra
> + * rcu_dereference_check() conditions, such as locks used during the
> + * cgroup_subsys::attach() methods.
> + */
> +#define task_subsys_state_check(task, subsys_id, __c) \
> + rcu_dereference_check(task->cgroups->subsys[subsys_id], \
> + rcu_read_lock_held() || \
> + lockdep_is_held(&task->alloc_lock) || \
> + cgroup_lock_is_held() || (__c))
> +
> +static inline struct cgroup_subsys_state *
> +task_subsys_state(struct task_struct *task, int subsys_id)
> {
> - return rcu_dereference_check(task->cgroups->subsys[subsys_id],
> - rcu_read_lock_held() ||
> - lockdep_is_held(&task->alloc_lock) ||
> - cgroup_lock_is_held());
> + return task_subsys_state_check(task, subsys_id, false);
> }
>
> static inline struct cgroup* task_cgroup(struct task_struct *task,
> Index: linux-2.6/kernel/sched.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched.c
> +++ linux-2.6/kernel/sched.c
> @@ -306,52 +306,6 @@ static int init_task_group_load = INIT_T
> */
> struct task_group init_task_group;
>
> -/* return group to which a task belongs */
> -static inline struct task_group *task_group(struct task_struct *p)
> -{
> - struct task_group *tg;
> -
> -#ifdef CONFIG_CGROUP_SCHED
> - tg = container_of(task_subsys_state(p, cpu_cgroup_subsys_id),
> - struct task_group, css);
> -#else
> - tg = &init_task_group;
> -#endif
> - return tg;
> -}
> -
> -/* Change a task's cfs_rq and parent entity if it moves across CPUs/groups */
> -static inline void set_task_rq(struct task_struct *p, unsigned int cpu)
> -{
> - /*
> - * Strictly speaking this rcu_read_lock() is not needed since the
> - * task_group is tied to the cgroup, which in turn can never go away
> - * as long as there are tasks attached to it.
> - *
> - * However since task_group() uses task_subsys_state() which is an
> - * rcu_dereference() user, this quiets CONFIG_PROVE_RCU.
> - */
> - rcu_read_lock();
> -#ifdef CONFIG_FAIR_GROUP_SCHED
> - p->se.cfs_rq = task_group(p)->cfs_rq[cpu];
> - p->se.parent = task_group(p)->se[cpu];
> -#endif
> -
> -#ifdef CONFIG_RT_GROUP_SCHED
> - p->rt.rt_rq = task_group(p)->rt_rq[cpu];
> - p->rt.parent = task_group(p)->rt_se[cpu];
> -#endif
> - rcu_read_unlock();
> -}
> -
> -#else
> -
> -static inline void set_task_rq(struct task_struct *p, unsigned int cpu) { }
> -static inline struct task_group *task_group(struct task_struct *p)
> -{
> - return NULL;
> -}
> -
> #endif /* CONFIG_CGROUP_SCHED */
>
> /* CFS-related fields in a runqueue */
> @@ -644,6 +598,49 @@ static inline int cpu_of(struct rq *rq)
> #define cpu_curr(cpu) (cpu_rq(cpu)->curr)
> #define raw_rq() (&__raw_get_cpu_var(runqueues))
>
> +#ifdef CONFIG_CGROUP_SCHED
> +
> +/*
> + * Return the group to which this tasks belongs.
> + *
> + * We use task_subsys_state_check() and extend the RCU verification
> + * with lockdep_is_held(&task_rq(p)->lock) because cpu_cgroup_attach()
> + * holds that lock for each task it moves into the cgroup. Therefore
> + * by holding that lock, we pin the task to the current cgroup.
> + */
> +static inline struct task_group *task_group(struct task_struct *p)
> +{
> + struct cgroup_subsys_state *css;
> +
> + css = task_subsys_state_check(p, cpu_cgroup_subsys_id,
> + lockdep_is_held(&task_rq(p)->lock));
> + return container_of(css, struct task_group, css);
> +}
> +
> +/* Change a task's cfs_rq and parent entity if it moves across CPUs/groups */
> +static inline void set_task_rq(struct task_struct *p, unsigned int cpu)
> +{
> +#ifdef CONFIG_FAIR_GROUP_SCHED
> + p->se.cfs_rq = task_group(p)->cfs_rq[cpu];
> + p->se.parent = task_group(p)->se[cpu];
> +#endif
> +
> +#ifdef CONFIG_RT_GROUP_SCHED
> + p->rt.rt_rq = task_group(p)->rt_rq[cpu];
> + p->rt.parent = task_group(p)->rt_se[cpu];
> +#endif
> +}
> +
> +#else /* CONFIG_CGROUP_SCHED */
> +
> +static inline void set_task_rq(struct task_struct *p, unsigned int cpu) { }
> +static inline struct task_group *task_group(struct task_struct *p)
> +{
> + return NULL;
> +}
> +
> +#endif /* CONFIG_CGROUP_SCHED */
> +
> inline void update_rq_clock(struct rq *rq)
> {
> if (!rq->skip_clock_update)
> @@ -4465,16 +4462,6 @@ recheck:
> }
>
> if (user) {
> -#ifdef CONFIG_RT_GROUP_SCHED
> - /*
> - * Do not allow realtime tasks into groups that have no runtime
> - * assigned.
> - */
> - if (rt_bandwidth_enabled() && rt_policy(policy) &&
> - task_group(p)->rt_bandwidth.rt_runtime == 0)
> - return -EPERM;
> -#endif
> -
> retval = security_task_setscheduler(p, policy, param);
> if (retval)
> return retval;
> @@ -4490,6 +4477,22 @@ recheck:
> * runqueue lock must be held.
> */
> rq = __task_rq_lock(p);
> +
> +#ifdef CONFIG_RT_GROUP_SCHED
> + if (user) {
> + /*
> + * Do not allow realtime tasks into groups that have no runtime
> + * assigned.
> + */
> + if (rt_bandwidth_enabled() && rt_policy(policy) &&
> + task_group(p)->rt_bandwidth.rt_runtime == 0) {
> + __task_rq_unlock(rq);
> + raw_spin_unlock_irqrestore(&p->pi_lock, flags);
> + return -EPERM;
> + }
> + }
> +#endif
> +
> /* recheck policy now with rq lock held */
> if (unlikely(oldpolicy != -1 && oldpolicy != p->policy)) {
> policy = oldpolicy = -1;
>
>
Sorry. I misunderstood this message when I first read it. I didn't
realize this message include a new version of the patch.
Anyhow, I just tried to apply the patch to 2.6.35-rc2-git3 and got this:
# patch -p1 -l -F 20 --dry-run < ../5.patch
patching file include/linux/cgroup.h
patching file kernel/sched.c
Hunk #1 succeeded at 306 with fuzz 1.
Hunk #3 FAILED at 4462.
Hunk #4 succeeded at 4487 with fuzz 3.
1 out of 4 hunks FAILED -- saving rejects to file kernel/sched.c.rej
^ permalink raw reply
* Re: [PATCH v3] netfilter: Xtables: idletimer target implementation
From: Luciano Coelho @ 2010-06-09 15:11 UTC (permalink / raw)
To: ext Patrick McHardy
Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
jengelh@medozas.de, Timo Teras
In-Reply-To: <4C0F9B17.10504@trash.net>
On Wed, 2010-06-09 at 15:45 +0200, ext Patrick McHardy wrote:
> luciano.coelho@nokia.com wrote:
> > From: Luciano Coelho <luciano.coelho@nokia.com>
> >
> > This patch implements an idletimer Xtables target that can be used to
> > identify when interfaces have been idle for a certain period of time.
> >
> > Timers are identified by labels and are created when a rule is set with a new
> > label. The rules also take a timeout value (in seconds) as an option. If
> > more than one rule uses the same timer label, the timer will be restarted
> > whenever any of the rules get a hit.
> >
> > One entry for each timer is created in sysfs. This attribute contains the
> > timer remaining for the timer to expire. The attributes are located under
> > the xt_idletimer class:
> >
> > /sys/class/xt_idletimer/timers/<label>
> >
> > When the timer expires, the target module sends a sysfs notification to the
> > userspace, which can then decide what to do (eg. disconnect to save power).
> >
>
> Basically this seems fine to me, some minor comments below.
Thanks a lot for reviewing this again. My replies below.
> > +++ b/include/linux/netfilter/xt_IDLETIMER.h
> > @@ -0,0 +1,40 @@
> > +#ifndef _XT_IDLETIMER_H
> > +#define _XT_IDLETIMER_H
> > +
> > +#define MAX_LABEL_SIZE 32
> >
>
> This seems a bit generic, maybe better use MAX_IDLETIMER_LABER_SIZE.
True, I'll change it.
> > +struct idletimer_tg {
> > + struct list_head entry;
> > + struct timer_list timer;
> > + struct work_struct work;
> > +
> > + struct kobject *kobj;
> > + struct idletimer_tg_attr *attr;
> > +
> > + unsigned int refcnt;
> > +};
> > +
> > +struct idletimer_tg_attr {
> > + struct attribute attr;
> > + ssize_t (*show)(struct kobject *kobj,
> > + struct attribute *attr, char *buf);
> > +};
> > +
> > +static LIST_HEAD(idletimer_tg_list);
> >
>
> How does this work with multiple namespaces? It seems every namespace
> can bind to any timer.
I was implementing this solution for multiple namespaces (see the
previous versions of my patch), but the code started getting really
complicated. Then I found out that sysfs and multiple namespaces are
not working very well together yet and decided to drop it for the time
being. Of course this doesn't matter anymore, since the timers belong
to an independent class in sysfs, so I can easily add multiple namespace
support by adding struct net *net as part of the list key together with
the label.
Do you think it's okay to leave it like this for now and extend it for
multiple namespace support with a future patch?
> > +static ssize_t idletimer_tg_show(struct kobject *kobj, struct attribute *attr,
> > + char *buf)
> > +{
> > + struct idletimer_tg *timer;
> > + unsigned long expires = 0;
> > +
> > + spin_lock_bh(&list_lock);
> > + timer = __idletimer_tg_find_by_label(attr->name);
> > + if (timer)
> > + expires = timer->timer.expires;
> > + spin_unlock_bh(&list_lock);
> > +
> > + if (expires > jiffies)
> >
>
> time_after()?
Of course!
> > + return sprintf(buf, "%u\n",
> > + jiffies_to_msecs(expires - jiffies) / 1000);
> > +
> > + return sprintf(buf, "0\n");
> > +}
> > +
> > +static void idletimer_tg_delete(const struct idletimer_tg_info *info)
> > +{
> >
>
> The only caller is the target cleanup function, why don't you just move
> everything there?
Good point. It probably remained as a separate function as vestigial
code.
> > +static
> > +struct idletimer_tg *idletimer_tg_create(const struct idletimer_tg_info *info)
> > +{
> > + struct idletimer_tg *timer;
> > + struct idletimer_tg_attr *attr;
> > +
> > + attr = kzalloc(sizeof(attr), GFP_KERNEL);
> >
>
> sizeof(*attr)
Ugh! Nice catch, I'll fix it.
> > + if (!attr) {
> > + pr_debug("couldn't alloc attribute\n");
> > + return NULL;
> > + }
> > +
> > + attr->attr.name = kstrdup(info->label, GFP_KERNEL);
> > + if (!attr->attr.name) {
> > + pr_debug("couldn't alloc attribute name\n");
> > + goto out_free_attr;
> > + }
> > + attr->attr.mode = S_IRUGO;
> > + attr->show = idletimer_tg_show;
> > +
> > + if (sysfs_create_file(idletimer_tg_kobj, &attr->attr)) {
> > + pr_debug("couldn't add attr to sysfs\n");
> > + goto out_free_name;
> > + }
> > +
> > + timer = kmalloc(sizeof(struct idletimer_tg), GFP_KERNEL);
I will also change this to kmalloc(sizeof(*timer), GFP_KERNEL) for
consistency.
> > + if (!timer) {
> > + pr_debug("couldn't alloc timer\n");
> > + goto out_free_file;
> > + }
> > +
> > + spin_lock_bh(&list_lock);
> > + list_add(&timer->entry, &idletimer_tg_list);
> > +
> > + init_timer(&timer->timer);
> > + setup_timer(&timer->timer, idletimer_tg_expired, (unsigned long) timer);
> > + mod_timer(&timer->timer,
> > + msecs_to_jiffies(info->timeout * 1000) + jiffies);
> > +
> > + timer->attr = attr;
> > + timer->refcnt = 0;
> >
>
> Better fully set up the timer before activating it.
Ok. Also, I don't need init_timer() here, since setup_timer() already
calls it.
> > +
> > + INIT_WORK(&timer->work, idletimer_tg_work);
> > + spin_unlock_bh(&list_lock);
> > +
> > + return timer;
> > +
> > +out_free_file:
> > + sysfs_remove_file(idletimer_tg_kobj, &attr->attr);
> > +out_free_name:
> > + kfree(attr->attr.name);
> > +out_free_attr:
> > + kfree(attr);
> > + return NULL;
> > +}
> > +
> > +static void idletimer_tg_cleanup(void)
> > +{
> > + struct idletimer_tg *timer;
> > +
> > + spin_lock(&list_lock);
> > + list_for_each_entry(timer, &idletimer_tg_list, entry) {
> >
>
> list_for_each_entry_safe(). This function seems unnecessary though, the
> module
> can't be unloaded while its still in use and cleanup will already happen
> when the
> rules are removed.
Right. I'll remove the cleanup function, since it's unnecessary.
> > +static unsigned int idletimer_tg_target(struct sk_buff *skb,
> > + const struct xt_action_param *par)
> > +{
> > + const struct idletimer_tg_info *info = par->targinfo;
> > + struct idletimer_tg *timer;
> > +
> > + pr_debug("resetting timer %s, timeout period %u\n",
> > + info->label, info->timeout);
> > +
> > + spin_lock(&list_lock);
> >
>
> You need BH protection here as well for the output path.
True. I'll add it.
> > +static int idletimer_tg_checkentry(const struct xt_tgchk_param *par)
> > +{
> > + const struct idletimer_tg_info *info = par->targinfo;
> > + struct idletimer_tg *timer;
> > +
> > + pr_debug("checkentry targinfo %s\n", info->label);
> > +
> > + if (info->timeout == 0) {
> > + pr_debug("timeout value is zero\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (!info->label || strlen(info->label) == 0) {
> >
>
> !info->label is impossible. You should check for \0 termination instead.
Ooops! <blush> I'll fix it.
>
> > + pr_debug("label is missing\n");
> > + return -EINVAL;
> > + }
> > +
> > + spin_lock(&list_lock);
> >
> spin_lock_bh()
Yes, I'll add BH here too.
> > + timer = __idletimer_tg_find_by_label(info->label);
> > + if (!timer) {
> > + spin_unlock(&list_lock);
> > + timer = idletimer_tg_create(info);
> >
>
> How does this prevent creating the same timer twice?
The timer will only be created if __idletimer_tg_find_by_label() returns
NULL, which means that no timer with that label has been found. "info"
won't be the same if info->label is different, right? Or can it change
on the fly?
I'll submit v4 with these changes later this evening.
--
Cheers,
Luca.
^ permalink raw reply
* Re: no reassembly for outgoing packets on RAW socket
From: Jan Engelhardt @ 2010-06-09 15:15 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Jiri Olsa, netdev, Netfilter Developer Mailing List
In-Reply-To: <4C0FA24A.7060907@trash.net>
On Wednesday 2010-06-09 16:16, Patrick McHardy wrote:
>>>> I'd like to be able to sendout a single IP packet with MF flag set.
>>>>
>>>> When using RAW sockets the packet will get stuck in the
>>>> netfilter (NF_INET_LOCAL_OUT nf_defrag_ipv4 reassembly unit)
>>>> and wont ever make it out..
>>>>
>>>> I made a change which bypass the outgoing reassembly for
>>>> RAW sockets, but I'm not sure wether it's too invasive..
>>>>
>>> That would break reassembly (and thus connection tracking) for cases
>>> where its really intended.
>>>
>>>> Is there any standard for RAW sockets behaviour?
>>>> Or another way around? :)
>>>>
>>> You could use the NOTRACK target to bypass connection tracking.
>>
>> I tried the NOTRACK target, but the packet is still going
>> throught reassembly, because the RAW filter has lower priority
>> then the connection track defragmentation..
>
>Right.
Blech. That reminds me of
http://marc.info/?l=netfilter-devel&m=126581823826735&w=2
^ permalink raw reply
* Re: no reassembly for outgoing packets on RAW socket
From: Patrick McHardy @ 2010-06-09 15:16 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: Jiri Olsa, netdev, Netfilter Developer Mailing List
In-Reply-To: <alpine.LSU.2.01.1006091713260.30265@obet.zrqbmnf.qr>
Jan Engelhardt wrote:
> On Wednesday 2010-06-09 16:16, Patrick McHardy wrote:
>>>> You could use the NOTRACK target to bypass connection tracking.
>>>>
>>> I tried the NOTRACK target, but the packet is still going
>>> throught reassembly, because the RAW filter has lower priority
>>> then the connection track defragmentation..
>>>
>> Right.
>>
>
> Blech. That reminds me of
> http://marc.info/?l=netfilter-devel&m=126581823826735&w=2
>
We already fixed that.
^ permalink raw reply
* Re: [PATCH v3] netfilter: Xtables: idletimer target implementation
From: Jan Engelhardt @ 2010-06-09 15:18 UTC (permalink / raw)
To: Luciano Coelho
Cc: ext Patrick McHardy, netfilter-devel@vger.kernel.org,
netdev@vger.kernel.org, Timo Teras
In-Reply-To: <1276096275.31892.117.camel@chilepepper>
On Wednesday 2010-06-09 17:11, Luciano Coelho wrote:
>>
>> How does this work with multiple namespaces? It seems every namespace
>> can bind to any timer.
>
>I was implementing this solution for multiple namespaces (see the
>previous versions of my patch), but the code started getting really
>complicated. Then I found out that sysfs and multiple namespaces are
>not working very well together yet and decided to drop it for the time
>being. Of course this doesn't matter anymore, since the timers belong
>to an independent class in sysfs, so I can easily add multiple namespace
>support by adding struct net *net as part of the list key together with
>the label.
>
>Do you think it's okay to leave it like this for now and extend it for
>multiple namespace support with a future patch?
Yes. Least thing we need is one humongous patch. :)
>> > + timer = __idletimer_tg_find_by_label(info->label);
>> > + if (!timer) {
>> > + spin_unlock(&list_lock);
>> > + timer = idletimer_tg_create(info);
>> >
>>
>> How does this prevent creating the same timer twice?
>
>The timer will only be created if __idletimer_tg_find_by_label() returns
>NULL, which means that no timer with that label has been found. "info"
>won't be the same if info->label is different, right? Or can it change
>on the fly?
One thing to be generally aware about is that things could potentially
be instantiated by another entity between the time a label was looked up
with negative result and the time one tries to add it.
It may thus be required to extend keeping the lock until after
idletimer_tg_create, in other words, lookup and create must be atomic
to the rest of the world.
^ permalink raw reply
* Re: no reassembly for outgoing packets on RAW socket
From: Jan Engelhardt @ 2010-06-09 15:20 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Jiri Olsa, netdev, Netfilter Developer Mailing List
In-Reply-To: <4C0FB068.9090700@trash.net>
On Wednesday 2010-06-09 17:16, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> On Wednesday 2010-06-09 16:16, Patrick McHardy wrote:
>>>>> You could use the NOTRACK target to bypass connection tracking.
>>>>>
>>>> I tried the NOTRACK target, but the packet is still going
>>>> throught reassembly, because the RAW filter has lower priority
>>>> then the connection track defragmentation..
>>>
>>> Right.
>>
>> Blech. That reminds me of
>> http://marc.info/?l=netfilter-devel&m=126581823826735&w=2
>
>We already fixed that.
I know, and I posted it for the understanding of the OP
as to why RAW is after DEFRAG.
^ permalink raw reply
* Re: 2.6.35-rc2, CONFIG_RPS is filling the dmesg log
From: Tim Gardner @ 2010-06-09 15:22 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1276090953.2442.140.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 2011 bytes --]
On 06/09/2010 07:42 AM, Eric Dumazet wrote:
> Le mercredi 09 juin 2010 à 07:27 -0600, Tim Gardner a écrit :
>> On 06/08/2010 02:55 PM, Tim Gardner wrote:
>>> With 2.6.35-rc2 my dmesg log is being flooded with messages like this:
>>>
>>> br0 received packet on queue 4, but number of RX queues is 1
>>>
>>> This machine is bridged for KVM and has 2 igb network adapters.
>>>
>>> The root cause appears to be CONFIG_RPS=y and the fact that none of the
>>> drivers that call skb_record_rx_queue() perform their net device
>>> allocation using alloc_netdev_mq(), thereby initializing num_rx_queues
>>> to a maximum of 1.
>>>
>>> Given that this is early RPS days, is the warning in get_rps_cpu()
>>> really necessary? It would appear that _all_ of the multi-receive queue
>>> devices that call skb_record_rx_queue() will cause this log noise.
>>>
>>> By the way, how do you turn off CONFIG_RPS? The only way I could get it
>>> disabled was to change the default in net/Kconfig to 'n'.
>>>
>>> rtg
>>
>> This is the route that I'm taking with Ubuntu in the short term. I'll
>> have lots of server testers complaining pretty soon if I don't take care
>> of this now. It does keep my server logs from filling.
>>
>> rtg
>>
>
> Probably fine, but your commit message is not exact :
>
> So far no users of skb_record_rx_queue() use alloc_netdev_mq() for
> network device initialization, so don't print a warning about num_rx_queues
> imbalances in get_rps_cpu() unless they have actually been allocated.
>
> In fact, drivers that use skb_record_rx_queue() did use alloc_netdev_mq().
>
> Problem is : packets going thru bridge/bonding that are not yet
> multiqueue enabled. If R[PF]S enabled for these "virtual devices",
> we trigger the get_rps_cpu() warning.
>
> Also, in a bonding setup, we still have a problem
> because all tx packets will go thru tx queue 0 (dev_pick_tx() job)
>
> (That might be good to know that for Ubuntu server testers)
>
How about this?
--
Tim Gardner tim.gardner@canonical.com
[-- Attachment #2: 0001-UBUNTU-SAUCE-net-Print-num_rx_queues-imbalance-warni.patch --]
[-- Type: text/x-patch, Size: 1443 bytes --]
>From ad76786a1a0c7b7b3c9bfeb4116fa0e2742f6328 Mon Sep 17 00:00:00 2001
From: Tim Gardner <tim.gardner@canonical.com>
Date: Tue, 8 Jun 2010 17:51:27 -0600
Subject: [PATCH] net: Print num_rx_queues imbalance warning only when there are allocated queues
BugLink: http://bugs.launchpad.net/bugs/591416
There are a number of network drivers (bridge, bonding, etc) that are not yet
receive multi-queue enabled and use alloc_netdev(), so don't print a
num_rx_queues imbalance warning in that case.
Also, only print the warning once for those drivers that _are_ multi-queue
enabled.
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
net/core/dev.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index d03470f..14a8568 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2253,11 +2253,9 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
if (skb_rx_queue_recorded(skb)) {
u16 index = skb_get_rx_queue(skb);
if (unlikely(index >= dev->num_rx_queues)) {
- if (net_ratelimit()) {
- pr_warning("%s received packet on queue "
- "%u, but number of RX queues is %u\n",
- dev->name, index, dev->num_rx_queues);
- }
+ WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
+ "on queue %u, but number of RX queues is %u\n",
+ dev->name, index, dev->num_rx_queues);
goto done;
}
rxqueue = dev->_rx + index;
--
1.7.0.4
^ permalink raw reply related
* Re: 2.6.35-rc2-git1 - include/linux/cgroup.h:534 invoked rcu_dereference_check() without protection!
From: Peter Zijlstra @ 2010-06-09 15:24 UTC (permalink / raw)
To: Miles Lane
Cc: paulmck, Vivek Goyal, Eric Paris, Lai Jiangshan, Ingo Molnar,
LKML, nauman, eric.dumazet, netdev, Jens Axboe, Gui Jianfeng,
Li Zefan, Johannes Berg
In-Reply-To: <AANLkTil6ucLZ0A6tWx-OQt_2EeKnx8nSx6rIg2t1L2t5@mail.gmail.com>
On Wed, 2010-06-09 at 11:11 -0400, Miles Lane wrote:
>
> Sorry. I misunderstood this message when I first read it. I didn't
> realize this message include a new version of the patch.
> Anyhow, I just tried to apply the patch to 2.6.35-rc2-git3 and got this:
>
> # patch -p1 -l -F 20 --dry-run < ../5.patch
> patching file include/linux/cgroup.h
> patching file kernel/sched.c
> Hunk #1 succeeded at 306 with fuzz 1.
> Hunk #3 FAILED at 4462.
> Hunk #4 succeeded at 4487 with fuzz 3.
> 1 out of 4 hunks FAILED -- saving rejects to file kernel/sched.c.rej
Weird.. it seems to apply without trouble to Linus' git tree.
root@twins:/usr/src/linux-2.6# git checkout -f origin/master
HEAD is now at 84f7586... Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6
root@twins:/usr/src/linux-2.6# quilt push
Applying patch patches/sched-rcu-validation.patch
patching file include/linux/cgroup.h
patching file kernel/sched.c
Now at patch patches/sched-rcu-validation.patch
root@twins:/usr/src/linux-2.6# git describe
v2.6.35-rc2-54-g84f7586
^ permalink raw reply
* Re: 2.6.35-rc2, CONFIG_RPS is filling the dmesg log
From: Eric Dumazet @ 2010-06-09 15:27 UTC (permalink / raw)
To: tim.gardner; +Cc: netdev
In-Reply-To: <4C0FB1D3.60901@canonical.com>
Le mercredi 09 juin 2010 à 09:22 -0600, Tim Gardner a écrit :
> On 06/09/2010 07:42 AM, Eric Dumazet wrote:
> > Le mercredi 09 juin 2010 à 07:27 -0600, Tim Gardner a écrit :
> >> On 06/08/2010 02:55 PM, Tim Gardner wrote:
> >>> With 2.6.35-rc2 my dmesg log is being flooded with messages like this:
> >>>
> >>> br0 received packet on queue 4, but number of RX queues is 1
> >>>
> >>> This machine is bridged for KVM and has 2 igb network adapters.
> >>>
> >>> The root cause appears to be CONFIG_RPS=y and the fact that none of the
> >>> drivers that call skb_record_rx_queue() perform their net device
> >>> allocation using alloc_netdev_mq(), thereby initializing num_rx_queues
> >>> to a maximum of 1.
> >>>
> >>> Given that this is early RPS days, is the warning in get_rps_cpu()
> >>> really necessary? It would appear that _all_ of the multi-receive queue
> >>> devices that call skb_record_rx_queue() will cause this log noise.
> >>>
> >>> By the way, how do you turn off CONFIG_RPS? The only way I could get it
> >>> disabled was to change the default in net/Kconfig to 'n'.
> >>>
> >>> rtg
> >>
> >> This is the route that I'm taking with Ubuntu in the short term. I'll
> >> have lots of server testers complaining pretty soon if I don't take care
> >> of this now. It does keep my server logs from filling.
> >>
> >> rtg
> >>
> >
> > Probably fine, but your commit message is not exact :
> >
> > So far no users of skb_record_rx_queue() use alloc_netdev_mq() for
> > network device initialization, so don't print a warning about num_rx_queues
> > imbalances in get_rps_cpu() unless they have actually been allocated.
> >
> > In fact, drivers that use skb_record_rx_queue() did use alloc_netdev_mq().
> >
> > Problem is : packets going thru bridge/bonding that are not yet
> > multiqueue enabled. If R[PF]S enabled for these "virtual devices",
> > we trigger the get_rps_cpu() warning.
> >
> > Also, in a bonding setup, we still have a problem
> > because all tx packets will go thru tx queue 0 (dev_pick_tx() job)
> >
> > (That might be good to know that for Ubuntu server testers)
> >
>
> How about this?
>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Thanks !
^ permalink raw reply
* Re: 2.6.35-rc2-git1 - include/linux/cgroup.h:534 invoked rcu_dereference_check() without protection!
From: Miles Lane @ 2010-06-09 15:29 UTC (permalink / raw)
To: Peter Zijlstra
Cc: paulmck, Vivek Goyal, Eric Paris, Lai Jiangshan, Ingo Molnar,
LKML, nauman, eric.dumazet, netdev, Jens Axboe, Gui Jianfeng,
Li Zefan, Johannes Berg
In-Reply-To: <1276097068.1745.13.camel@laptop>
On Wed, Jun 9, 2010 at 11:24 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, 2010-06-09 at 11:11 -0400, Miles Lane wrote:
>>
>> Sorry. I misunderstood this message when I first read it. I didn't
>> realize this message include a new version of the patch.
>> Anyhow, I just tried to apply the patch to 2.6.35-rc2-git3 and got this:
>>
>> # patch -p1 -l -F 20 --dry-run < ../5.patch
>> patching file include/linux/cgroup.h
>> patching file kernel/sched.c
>> Hunk #1 succeeded at 306 with fuzz 1.
>> Hunk #3 FAILED at 4462.
>> Hunk #4 succeeded at 4487 with fuzz 3.
>> 1 out of 4 hunks FAILED -- saving rejects to file kernel/sched.c.rej
>
> Weird.. it seems to apply without trouble to Linus' git tree.
>
> root@twins:/usr/src/linux-2.6# git checkout -f origin/master
> HEAD is now at 84f7586... Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6
> root@twins:/usr/src/linux-2.6# quilt push
> Applying patch patches/sched-rcu-validation.patch
> patching file include/linux/cgroup.h
> patching file kernel/sched.c
>
> Now at patch patches/sched-rcu-validation.patch
> root@twins:/usr/src/linux-2.6# git describe
> v2.6.35-rc2-54-g84f7586
Oh. Duh. I know what is going on. I had received another patch to
sched.c. They must conflict. I will test with just your patch now.
^ permalink raw reply
* Re: [RFC PATCH 1/2] mac80211: make max_network_latency notifier atomic safe
From: Florian Mickler @ 2010-06-09 15:37 UTC (permalink / raw)
To: Johannes Berg
Cc: pm list, james.bottomley-l3A5Bk7waGM,
markgross-FexmcLCpFRVAfugRpC6u6w, mgross-VuQAYsv1563Yd54FQh9/CA,
John W. Linville, David S. Miller, Javier Cardona, Jouni Malinen,
Rui Paulo, Kalle Valo, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Thomas Gleixner
In-Reply-To: <1276086425.14580.14.camel-8upI4CBIZJIJvtFkdXX2HixXY32XiHfO@public.gmane.org>
On Wed, 09 Jun 2010 14:27:05 +0200
Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> On Wed, 2010-06-09 at 14:16 +0200, Florian Mickler wrote:
>
> > That was also my first idea, but then I thought about qos and thought
> > atomic notification are necessary.
> > Do you see any value in having atomic notification?
> >
> > I have the following situation before my eyes:
> >
> > Driver A gets an interrupt and needs (to service that
> > interrupt) the cpu to guarantee a latency of X because the
> > device is a bit icky.
> >
> > Now, in that situation, if we don't immediately (without scheduling in
> > between) notify the system to be in that latency-mode the driver won't
> > function properly. Is this a realistic scene?
> >
> > At the moment we only have process context notification and only 2
> > listeners.
> >
> > I think providing for atomic as well as "relaxed" notification could be
> > useful.
> >
> > If atomic notification is deemed unnecessary, I have no
> > problems to just use schedule_work() in update request.
> > Anyway, it is probably best to split this. I.e. first make
> > update_request callable from atomic contexts with doing the
> > schedule_work in update_request and then
> > as an add on provide for constraints_objects with atomic notifications.
>
> Well I remember http://thread.gmane.org/gmane.linux.kernel/979935 where
> Mark renamed things to "request" which seems to imply to me more of a
> "please do this" than "I NEED IT NOW!!!!!".
>
> johannes
Yes. I just posted a version which uses schedule_work().
Just FYI, James has also posted his version which uses either a blocking
or an atomic notifier chain.
http://article.gmane.org/gmane.linux.kernel/996813
Cheers,
Flo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH iproute2] ip: add support for multicast rules
From: Patrick McHardy @ 2010-06-09 15:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Linux Netdev List
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 01.diff --]
[-- Type: text/x-diff, Size: 2681 bytes --]
commit 44a5293c1c47b8c32d9bb0756660ea5d4802acf2
Author: Patrick McHardy <kaber@trash.net>
Date: Tue Apr 13 17:03:47 2010 +0200
ip: add support for multicast rules
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index e94981d..0d8ef9e 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -7,6 +7,13 @@
#include <linux/if_addr.h>
#include <linux/neighbour.h>
+/* rtnetlink families. Values up to 127 are reserved for real address
+ * families, values above 128 may be used arbitrarily.
+ */
+#define RTNL_FAMILY_IPMR 128
+#define RTNL_FAMILY_IP6MR 129
+#define RTNL_FAMILY_MAX 129
+
/****
* Routing/neighbour discovery messages.
****/
diff --git a/ip/ip.c b/ip/ip.c
index e0cf175..9f29533 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -42,7 +42,7 @@ static void usage(void)
"Usage: ip [ OPTIONS ] OBJECT { COMMAND | help }\n"
" ip [ -force ] -batch filename\n"
"where OBJECT := { link | addr | addrlabel | route | rule | neigh | ntable |\n"
-" tunnel | tuntap | maddr | mroute | monitor | xfrm }\n"
+" tunnel | tuntap | maddr | mroute | mrule | monitor | xfrm }\n"
" OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[esolve] |\n"
" -f[amily] { inet | inet6 | ipx | dnet | link } |\n"
" -o[neline] | -t[imestamp] | -b[atch] [filename] |\n"
@@ -76,6 +76,7 @@ static const struct cmd {
{ "monitor", do_ipmonitor },
{ "xfrm", do_xfrm },
{ "mroute", do_multiroute },
+ { "mrule", do_multirule },
{ "help", do_help },
{ 0 }
};
diff --git a/ip/ip_common.h b/ip/ip_common.h
index c857667..a114186 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -37,6 +37,7 @@ extern int do_iplink(int argc, char **argv);
extern int do_ipmonitor(int argc, char **argv);
extern int do_multiaddr(int argc, char **argv);
extern int do_multiroute(int argc, char **argv);
+extern int do_multirule(int argc, char **argv);
extern int do_xfrm(int argc, char **argv);
static inline int rtm_get_table(struct rtmsg *r, struct rtattr **tb)
diff --git a/ip/iprule.c b/ip/iprule.c
index 7140375..9c8c6ef 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -436,3 +436,20 @@ int do_iprule(int argc, char **argv)
exit(-1);
}
+int do_multirule(int argc, char **argv)
+{
+ switch (preferred_family) {
+ case AF_UNSPEC:
+ case AF_INET:
+ preferred_family = RTNL_FAMILY_IPMR;
+ break;
+ case AF_INET6:
+ preferred_family = RTNL_FAMILY_IP6MR;
+ break;
+ default:
+ fprintf(stderr, "Multicast rules are only supported for IPv4/IPv6\n");
+ exit(-1);
+ }
+
+ return do_iprule(argc, argv);
+}
^ permalink raw reply related
* Re: [PATCH iproute2] ip: add support for multicast rules
From: Stephen Hemminger @ 2010-06-09 15:50 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Linux Netdev List
In-Reply-To: <4C0FB616.3000303@trash.net>
Applied
^ permalink raw reply
* Re: 2.6.35-rc2-git1 - include/linux/cgroup.h:534 invoked rcu_dereference_check() without protection!
From: Miles Lane @ 2010-06-09 17:00 UTC (permalink / raw)
To: Peter Zijlstra
Cc: paulmck, Vivek Goyal, Eric Paris, Lai Jiangshan, Ingo Molnar,
LKML, nauman, eric.dumazet, netdev, Jens Axboe, Gui Jianfeng,
Li Zefan, Johannes Berg
In-Reply-To: <AANLkTinl2JKUX6859Ioa1y0DqHWXH5TrkissQ8BAF4cP@mail.gmail.com>
On Wed, Jun 9, 2010 at 11:29 AM, Miles Lane <miles.lane@gmail.com> wrote:
> On Wed, Jun 9, 2010 at 11:24 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Wed, 2010-06-09 at 11:11 -0400, Miles Lane wrote:
>>>
>>> Sorry. I misunderstood this message when I first read it. I didn't
>>> realize this message include a new version of the patch.
>>> Anyhow, I just tried to apply the patch to 2.6.35-rc2-git3 and got this:
>>>
>>> # patch -p1 -l -F 20 --dry-run < ../5.patch
>>> patching file include/linux/cgroup.h
>>> patching file kernel/sched.c
>>> Hunk #1 succeeded at 306 with fuzz 1.
>>> Hunk #3 FAILED at 4462.
>>> Hunk #4 succeeded at 4487 with fuzz 3.
>>> 1 out of 4 hunks FAILED -- saving rejects to file kernel/sched.c.rej
>>
>> Weird.. it seems to apply without trouble to Linus' git tree.
>>
>> root@twins:/usr/src/linux-2.6# git checkout -f origin/master
>> HEAD is now at 84f7586... Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6
>> root@twins:/usr/src/linux-2.6# quilt push
>> Applying patch patches/sched-rcu-validation.patch
>> patching file include/linux/cgroup.h
>> patching file kernel/sched.c
>>
>> Now at patch patches/sched-rcu-validation.patch
>> root@twins:/usr/src/linux-2.6# git describe
>> v2.6.35-rc2-54-g84f7586
>
> Oh. Duh. I know what is going on. I had received another patch to
> sched.c. They must conflict. I will test with just your patch now.
>
ACPI: Core revision 20100428
[ 0.061088]
[ 0.061090] ===================================================
[ 0.062009] [ INFO: suspicious rcu_dereference_check() usage. ]
[ 0.062138] ---------------------------------------------------
[ 0.062268] kernel/sched.c:616 invoked rcu_dereference_check()
without protection!
[ 0.062470]
[ 0.062471] other info that might help us debug this:
[ 0.062472]
[ 0.062835]
[ 0.062836] rcu_scheduler_active = 1, debug_locks = 1
[ 0.063009] no locks held by swapper/0.
[ 0.063134]
[ 0.063135] stack backtrace:
[ 0.063378] Pid: 0, comm: swapper Not tainted 2.6.35-rc2-git3 #3
[ 0.063507] Call Trace:
[ 0.063638] [<ffffffff81072205>] lockdep_rcu_dereference+0x9d/0xa5
[ 0.063773] [<ffffffff810379f9>] task_group+0x7b/0x8a
[ 0.064012] [<ffffffff81037a1d>] set_task_rq+0x15/0x6e
[ 0.064143] [<ffffffff8103e50f>] set_task_cpu+0xa9/0xba
[ 0.064274] [<ffffffff81042dbb>] sched_fork+0x10a/0x1b3
[ 0.064405] [<ffffffff810446f9>] copy_process+0x617/0x10e6
[ 0.064537] [<ffffffff8104533d>] do_fork+0x175/0x39b
[ 0.064670] [<ffffffff8106589b>] ? up+0xf/0x39
[ 0.064800] [<ffffffff8106589b>] ? up+0xf/0x39
[ 0.065013] [<ffffffff811dbf73>] ? do_raw_spin_lock+0x79/0x13e
[ 0.065148] [<ffffffff81011526>] kernel_thread+0x70/0x72
[ 0.065279] [<ffffffff816cc5e4>] ? kernel_init+0x0/0x1ce
[ 0.065411] [<ffffffff8100aba0>] ? kernel_thread_helper+0x0/0x10
[ 0.065545] [<ffffffff81096bea>] ? rcu_scheduler_starting+0x2a/0x4c
[ 0.065679] [<ffffffff813a8a4d>] rest_init+0x21/0xde
[ 0.065810] [<ffffffff816cce28>] start_kernel+0x448/0x453
[ 0.066013] [<ffffffff816cc2c8>] x86_64_start_reservations+0xb3/0xb7
[ 0.066148] [<ffffffff816cc418>] x86_64_start_kernel+0x14c/0x15b
[ 0.066499] Setting APIC routing to flat
^ permalink raw reply
* Re: linux-next: Tree for June 9 (niu)
From: Randy Dunlap @ 2010-06-09 17:36 UTC (permalink / raw)
To: Stephen Rothwell, netdev; +Cc: linux-next, LKML, davem
In-Reply-To: <20100609133443.38f1f957.sfr@canb.auug.org.au>
On Wed, 9 Jun 2010 13:34:43 +1000 Stephen Rothwell wrote:
> Hi all,
>
> Changes since 20100608:
>
> My fixes tree contains:
> v4l-dvb: update gfp/slab.h includes
> arm: update gfp/slab.h includes
> davinci: update gfp/slab.h includes
> ocfs2: update gfp/slab.h includes
> acpi: update gfp/slab.h includes
on x86_64 or i386, CONFIG_OF_DEVICE is not enabled:
drivers/net/niu.c:9700: warning: 'struct of_device' declared inside parameter list
drivers/net/niu.c:9700: warning: its scope is only this definition or declaration, which is probably not what you want
drivers/net/niu.c:9716: warning: assignment from incompatible pointer type
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply
* Re: [PATCH v3] netfilter: Xtables: idletimer target implementation
From: Luciano Coelho @ 2010-06-09 17:48 UTC (permalink / raw)
To: ext Jan Engelhardt
Cc: ext Patrick McHardy, netfilter-devel@vger.kernel.org,
netdev@vger.kernel.org, Timo Teras
In-Reply-To: <alpine.LSU.2.01.1006091716170.30265@obet.zrqbmnf.qr>
Hi Jan,
On Wed, 2010-06-09 at 17:18 +0200, ext Jan Engelhardt wrote:
> On Wednesday 2010-06-09 17:11, Luciano Coelho wrote:
> >Do you think it's okay to leave it like this for now and extend it for
> >multiple namespace support with a future patch?
>
> Yes. Least thing we need is one humongous patch. :)
Thanks! That was my concern too. One huge and very complex patch is not
a nice thing to have. ;)
> >> > + timer = __idletimer_tg_find_by_label(info->label);
> >> > + if (!timer) {
> >> > + spin_unlock(&list_lock);
> >> > + timer = idletimer_tg_create(info);
> >> >
> >>
> >> How does this prevent creating the same timer twice?
> >
> >The timer will only be created if __idletimer_tg_find_by_label() returns
> >NULL, which means that no timer with that label has been found. "info"
> >won't be the same if info->label is different, right? Or can it change
> >on the fly?
>
> One thing to be generally aware about is that things could potentially
> be instantiated by another entity between the time a label was looked up
> with negative result and the time one tries to add it.
> It may thus be required to extend keeping the lock until after
> idletimer_tg_create, in other words, lookup and create must be atomic
> to the rest of the world.
Ahh, sure! I missed the actual point of Patrick's question. I had the
idletimer_tg_create() inside the lock, but when I added the
sysfs_create_file() there (which can sleep), I screwed up with the
locking.
I'll move the sysfs file creation to outside that function so I can keep
the lock until after the timer is added to the list. Thanks for
clarifying!
--
Cheers,
Luca.
^ 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