* Re: [PATCH net 0/3] qmi_wwan: simplify device matching and add a few new devices
From: David Miller @ 2012-08-14 21:46 UTC (permalink / raw)
To: bjorn; +Cc: netdev, linux-usb
In-Reply-To: <1344798992-10176-1-git-send-email-bjorn@mork.no>
From: Bjørn Mork <bjorn@mork.no>
Date: Sun, 12 Aug 2012 21:16:29 +0200
> The home cooked whitelisting code can be removed now that
> the USB core supports interface number matching.
>
> The second patch adds a few new devices.
>
> The third patch improves device list readability by using
> existing macros where possible.
>
> I hope this can go in 3.6-rc2/3. The series is based on
> the current net/master (commit 2359a476)
Ok I changed my mind and applied this to 'net'.
^ permalink raw reply
* Re: [PATCH net] net: sierra_net: replace whitelist with ifnumber match
From: David Miller @ 2012-08-14 21:47 UTC (permalink / raw)
To: bjorn-yOkvZcmFvRU
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1344801218-11041-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Date: Sun, 12 Aug 2012 21:53:38 +0200
> Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Applied to 'net'
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Re: [PATCH] ipv4: Cache local output routes
From: David Miller @ 2012-08-14 21:47 UTC (permalink / raw)
To: zheng.z.yan; +Cc: netdev, alex.shi
In-Reply-To: <50289A37.6000209@intel.com>
From: "Yan, Zheng" <zheng.z.yan@intel.com>
Date: Mon, 13 Aug 2012 14:09:59 +0800
> Commit caacf05e5ad1abf causes big drop of UDP loop back performance.
> The cause of the regression is that we do not cache the local output
> routes. Each time we send a datagram from unconnected UDP socket,
> the kernel allocates a dst_entry and adds it to the rt_uncached_list.
> It creates lock contention on the rt_uncached_lock.
>
> Reported-by: Alex Shi <alex.shi@intel.com>
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
I was pretty sure not using 'fi' was necessary for semantic
reasons here, but I can't find any such requirement, so I've
applied this, thanks.
^ permalink raw reply
* Re: [PATCH net] net: ipv6: proc: Fix error handling
From: David Miller @ 2012-08-14 21:47 UTC (permalink / raw)
To: igorm; +Cc: netdev
In-Reply-To: <1344846718-4263-1-git-send-email-igorm@etf.rs>
From: igorm@etf.rs
Date: Mon, 13 Aug 2012 10:31:58 +0200
> From: Igor Maravic <igorm@etf.rs>
>
> Fix error handling in case making of dir dev_snmp6 failes
>
> Signed-off-by: Igor Maravic <igorm@etf.rs>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/85xx: add Fman MDIO muxing support to the P4080DS
From: Timur Tabi @ 2012-08-14 21:48 UTC (permalink / raw)
To: Kumar Gala; +Cc: Andy Fleming, Scott Wood, ddaney.cavm, linuxppc-dev, netdev
In-Reply-To: <5F0028FE-C555-47DE-B69A-888E7322A6E1@kernel.crashing.org>
Kumar Gala wrote:
>> > + {
>> > + /*
>> > + * Warning: this entry might need to be located before those
>> > + * for the Fman Ethernet nodes.
>> > + */
>> > + .compatible = "mdio-mux",
>> > + },
>> > {}
>> > };
> Under what condition would that be the case?
We had this discussion already. In my tests, I had to locate this entry
before these entries:
{
.compatible = "fsl,dpaa"
},
{
.compatible = "fsl,srio",
},
otherwise, the mdio-mux code would not prepare the mdio mus in time, and
there would be initialization failures. Now maybe this goes away with
-EPROBE_DEFER, or maybe it doesn't. But until we push the DPAA drivers
upstream, we won't know.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH v2] net-tcp: TCP/IP stack bypass for loopback connections
From: David Miller @ 2012-08-14 21:50 UTC (permalink / raw)
To: brutus; +Cc: edumazet, netdev
In-Reply-To: <CAEkNxbETGxFAHoNDK5qEmGo+PfLxmhCniLnOTkWxpK8Ycj_r0A@mail.gmail.com>
From: Bruce Curtis <brutus@google.com>
Date: Tue, 14 Aug 2012 14:45:09 -0700
> On Tue, Aug 14, 2012 at 2:22 PM, David Miller <davem@davemloft.net> wrote:
>>
>> Bruce, could you integrate (and unlike your submission, actually build
>> and run test) Weiping's bug fixes?
>>
>> Actually, I'm actually a little bit alarmed at Weiping's fixes,
>> because it makes it look as if you didn't test things at all under
>> net-next, as it appears that without his fixes any loopback TCP
>> connection would OOPS the kernel.
>>
> ??? rebase, build, boot a dev machine, run several hours of testing,
> take numbers from test and updated commit message (commit message test
> results changed from patch to patch submit because of this) so ???
I did it for every round of the patch series removing the entire
routing cache, so these kinds of excuses are going to fall on deaf
ears.
It's part of doing responsible development, and it's especially
critical when you are touching core parts of the kernel everyone
uses.
>
>> In fact, it wouldn't even build without the sysctl_tcp_friends typo.
>> Indeed:
>>
>> net/ipv4/tcp.c: In function ‘tcp_recvmsg’:
>> net/ipv4/tcp.c:1935:35: error: ‘friends’ undeclared (first use in this function)
>> net/ipv4/tcp.c:1935:35: note: each undeclared identifier is reported only once for each function it appears in
>>
> Didn't build with CONFIG_NET_DMA and wasn't caught as part of
> rebase/local var name change.
'allmodconfig' builds take less than 10 minutes on current hardware,
and should be part of your patch validation.
Especially because this is the first thing I'm personally going to
do with your patch.
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/85xx: add Fman MDIO muxing support to the P4080DS
From: Scott Wood @ 2012-08-14 21:53 UTC (permalink / raw)
To: Timur Tabi; +Cc: Kumar Gala, Andy Fleming, ddaney.cavm, linuxppc-dev, netdev
In-Reply-To: <502AC7C3.9030902@freescale.com>
On 08/14/2012 04:48 PM, Timur Tabi wrote:
> Kumar Gala wrote:
>>>> + {
>>>> + /*
>>>> + * Warning: this entry might need to be located before those
>>>> + * for the Fman Ethernet nodes.
>>>> + */
>>>> + .compatible = "mdio-mux",
>>>> + },
>>>> {}
>>>> };
>> Under what condition would that be the case?
>
> We had this discussion already.
I think that was internally, and not on this specific comment wording.
I don't think that code comment adequately explains things.
> otherwise, the mdio-mux code would not prepare the mdio mus in time, and
> there would be initialization failures. Now maybe this goes away with
> -EPROBE_DEFER, or maybe it doesn't. But until we push the DPAA drivers
> upstream, we won't know.
Do you know if it's theoretically supposed to be fixed and just can't
test it, or are you unsure of whether it's even supposed to work?
I don't think we should be relying on the order of this list to
determine probe order. For one thing, it won't work if the drivers
register after you create the platform devices (e.g. they're modules).
-Scott
^ permalink raw reply
* Re: [PATCH] XFRM: remove redundant parameter "int dir" in struct xfrm_mgr.acquire
From: David Miller @ 2012-08-14 21:56 UTC (permalink / raw)
To: fan.du; +Cc: steffen.klassert, netdev
In-Reply-To: <5028AB5D.9040200@windriver.com>
From: Fan Du <fan.du@windriver.com>
Date: Mon, 13 Aug 2012 15:23:09 +0800
>
>
> On 2012年08月13日 15:08, Steffen Klassert wrote:
>> On Mon, Aug 13, 2012 at 02:25:57PM +0800, Fan Du wrote:
>>>
>>> static int xfrm_send_acquire(struct xfrm_state *x, struct xfrm_tmpl
>>> *xt,
>>> - struct xfrm_policy *xp, int dir)
>>> + struct xfrm_policy *xp)
>>> {
>>> struct net *net = xs_net(x);
>>> struct sk_buff *skb;
>>> @@ -2614,7 +2614,7 @@ static int xfrm_send_acquire(struct xfrm_state
>>> *x, struct xfrm_tmpl *xt,
>>> if (skb == NULL)
>>> return -ENOMEM;
>>>
>>> - if (build_acquire(skb, x, xt, xp, dir)< 0)
>>> + if (build_acquire(skb, x, xt, xp, XFRM_POLICY_OUT)< 0)
>>> BUG();
>>
>> xfrm_send_acquire() is the only caller of build_acquire().
>> So if you remove the dir parameter from xfrm_send_acquire(),
>> you can remove it from build_acquire() too.
>>
> Yep, looks like we can only remove "dir" at build_acquire, not into
> copy_to_user_policy anymore :)
>
> I will adopt your approach in v2 if Dave say *YES* about this patch.
> thanks anyway.
Looks find to me.
^ permalink raw reply
* Re: [PATCH][XFRM][v3] Replace rwlock on xfrm_policy_afinfo with rcu
From: David Miller @ 2012-08-14 21:58 UTC (permalink / raw)
To: Priyanka.Jain; +Cc: netdev
In-Reply-To: <1344842549-25923-1-git-send-email-Priyanka.Jain@freescale.com>
From: Priyanka Jain <Priyanka.Jain@freescale.com>
Date: Mon, 13 Aug 2012 12:52:29 +0530
> xfrm_policy_afinfo is read mosly data structure.
> Write on xfrm_policy_afinfo is done only at the
> time of configuration.
> So rwlocks can be safely replaced with RCU.
>
> RCUs usage optimizes the performance.
>
> Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/85xx: add Fman MDIO muxing support to the P4080DS
From: Timur Tabi @ 2012-08-14 21:58 UTC (permalink / raw)
To: Scott Wood; +Cc: Kumar Gala, Andy Fleming, ddaney.cavm, linuxppc-dev, netdev
In-Reply-To: <502AC8D3.4010602@freescale.com>
Scott Wood wrote:
> I think that was internally, and not on this specific comment wording.
> I don't think that code comment adequately explains things.
I don't really have any more insight to add.
>> otherwise, the mdio-mux code would not prepare the mdio mus in time, and
>> there would be initialization failures. Now maybe this goes away with
>> -EPROBE_DEFER, or maybe it doesn't. But until we push the DPAA drivers
>> upstream, we won't know.
>
> Do you know if it's theoretically supposed to be fixed and just can't
> test it, or are you unsure of whether it's even supposed to work?
I'm not sure of anything. For one thing, we don't implement EPROBE_DEFER
in the DPAA drivers, so we'd probably have to fix that before anything.
And then, I'm just guessing that's the solution.
> I don't think we should be relying on the order of this list to
> determine probe order. For one thing, it won't work if the drivers
> register after you create the platform devices (e.g. they're modules).
I agree we should not be relying on the order, but I don't know what to
do. EPROBE_DEFER was designed to handle this situation, so my
recommendation is to worry about it later. I can beef up the comment to
talk about that, if you want.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] net: ipv4: fib_trie: Don't unnecessarily search for already found fib leaf
From: David Miller @ 2012-08-14 22:02 UTC (permalink / raw)
To: igorm; +Cc: netdev
In-Reply-To: <1344846368-4141-1-git-send-email-igorm@etf.rs>
From: igorm@etf.rs
Date: Mon, 13 Aug 2012 10:26:08 +0200
> From: Igor Maravic <igorm@etf.rs>
>
> We've already found leaf, don't search for it again. Same is for fib leaf info.
>
> Signed-off-by: Igor Maravic <igorm@etf.rs>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [patch net-next v2 01/15] net: introduce upper device lists
From: Ben Hutchings @ 2012-08-14 22:33 UTC (permalink / raw)
To: Jiri Pirko
Cc: bridge, ursula.braun, john.r.fastabend, edumazet, shemminger,
sean.hefty, therbert, roland, linux-s390, linux-rdma, fubar, fbl,
hal.rosenstock, faisal.latif, linux-driver, blaschka, sony.chacko,
gregory.v.rose, xiyou.wangcong, jitendra.kalsaria, divy, netdev,
linux-kernel, kaber, joe, linux390, davem
In-Reply-To: <1344956748-2099-2-git-send-email-jiri@resnulli.us>
On Tue, 2012-08-14 at 17:05 +0200, Jiri Pirko wrote:
> This lists are supposed to serve for storing pointers to all upper devices.
> Eventually it will replace dev->master pointer which is used for
> bonding, bridge, team but it cannot be used for vlan, macvlan where
> there might be multiple upper present. In case the upper link is
> replacement for dev->master, it is marked with "master" flag.
Something I found interesting is that the dev->master pointer and now
netdev_master_upper_dev_get{,_rcu}() are hardly used by the stackled
drivers that set the master. They also have to set an rx_handler on the
lower device (which is itself mutually exclusive) which gets its own
context pointer (rx_handler_data).
Instead, the master pointer is mostly used by device drivers to find out
about a bridge or bonding device above *their* devices. And that seems
to work only for those specific device drivers, not e.g. openvswitch or
team. I wonder if we could find a better way to encapsulate the things
they want do do, in a later step (not holding up this change!).
[...]
> +static int __netdev_upper_dev_link(struct net_device *dev,
> + struct net_device *upper_dev, bool master)
> +{
> + struct netdev_upper *upper;
> +
> + ASSERT_RTNL();
> +
> + if (dev == upper_dev)
> + return -EBUSY;
> + /*
> + * To prevent loops, check if dev is not upper device to upper_dev.
> + */
> + if (__netdev_has_upper_dev(upper_dev, dev, true))
> + return -EBUSY;
[...]
I think we will also need to limit the depth of the device stack so we
don't run out of stack space here. __netif_receive() implements a kind
of tail recursion whenever a packet is passed up, but
__netdev_has_upper_dev() can't avoid doing real recursion (without the
addition of a flag to net_device so it can mark its progress).
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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
* [net PATCH v3 1/3] net: netprio: fix files lock and remove useless d_path bits
From: John Fastabend @ 2012-08-14 22:34 UTC (permalink / raw)
To: viro, nhorman; +Cc: netdev, davem, linux-kernel, joe
Add lock to prevent a race with a file closing and also remove
useless and ugly sscanf code. The extra code was never needed
and the case it supposedly protected against is in fact handled
correctly by sock_from_file as pointed out by Al Viro.
CC: Neil Horman <nhorman@tuxdriver.com>
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
net/core/netprio_cgroup.c | 22 ++++------------------
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index ed0c043..f65dba3 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -277,12 +277,6 @@ out_free_devname:
void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
{
struct task_struct *p;
- char *tmp = kzalloc(sizeof(char) * PATH_MAX, GFP_KERNEL);
-
- if (!tmp) {
- pr_warn("Unable to attach cgrp due to alloc failure!\n");
- return;
- }
cgroup_taskset_for_each(p, cgrp, tset) {
unsigned int fd;
@@ -296,32 +290,24 @@ void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset)
continue;
}
- rcu_read_lock();
+ spin_lock(&files->file_lock);
fdt = files_fdtable(files);
for (fd = 0; fd < fdt->max_fds; fd++) {
- char *path;
struct file *file;
struct socket *sock;
- unsigned long s;
- int rv, err = 0;
+ int err;
file = fcheck_files(files, fd);
if (!file)
continue;
- path = d_path(&file->f_path, tmp, PAGE_SIZE);
- rv = sscanf(path, "socket:[%lu]", &s);
- if (rv <= 0)
- continue;
-
sock = sock_from_file(file, &err);
- if (!err)
+ if (sock)
sock_update_netprioidx(sock->sk, p);
}
- rcu_read_unlock();
+ spin_unlock(&files->file_lock);
task_unlock(p);
}
- kfree(tmp);
}
static struct cftype ss_files[] = {
^ permalink raw reply related
* [net PATCH v3 2/3] net: netprio: fd passed in SCM_RIGHTS datagram not set correctly
From: John Fastabend @ 2012-08-14 22:34 UTC (permalink / raw)
To: viro, nhorman; +Cc: netdev, davem, linux-kernel, joe
In-Reply-To: <20120814223424.3551.21608.stgit@jf-dev1-dcblab>
A socket fd passed in a SCM_RIGHTS datagram was not getting
updated with the new tasks cgrp prioidx. This leaves IO on
the socket tagged with the old tasks priority.
To fix this add a check in the scm recvmsg path to update the
sock cgrp prioidx with the new tasks value.
Thanks to Al Viro for catching this.
CC: Neil Horman <nhorman@tuxdriver.com>
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
net/core/scm.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/core/scm.c b/net/core/scm.c
index 8f6ccfd..040cebe 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -265,6 +265,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
for (i=0, cmfptr=(__force int __user *)CMSG_DATA(cm); i<fdmax;
i++, cmfptr++)
{
+ struct socket *sock;
int new_fd;
err = security_file_receive(fp[i]);
if (err)
@@ -281,6 +282,9 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
}
/* Bump the usage count and install the file. */
get_file(fp[i]);
+ sock = sock_from_file(fp[i], &err);
+ if (sock)
+ sock_update_netprioidx(sock->sk, current);
fd_install(new_fd, fp[i]);
}
^ permalink raw reply related
* [net PATCH v3 3/3] net: netprio: fix cgrp create and write priomap race
From: John Fastabend @ 2012-08-14 22:34 UTC (permalink / raw)
To: viro, nhorman; +Cc: netdev, davem, linux-kernel, joe
In-Reply-To: <20120814223424.3551.21608.stgit@jf-dev1-dcblab>
A race exists where creating cgroups and also updating the priomap
may result in losing a priomap update. This is because priomap
writers are not protected by rtnl_lock.
Move priority writer into rtnl_lock()/rtnl_unlock().
CC: Neil Horman <nhorman@tuxdriver.com>
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
net/core/netprio_cgroup.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index f65dba3..c75e3f9 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -101,12 +101,10 @@ static int write_update_netdev_table(struct net_device *dev)
u32 max_len;
struct netprio_map *map;
- rtnl_lock();
max_len = atomic_read(&max_prioidx) + 1;
map = rtnl_dereference(dev->priomap);
if (!map || map->priomap_len < max_len)
ret = extend_netdev_table(dev, max_len);
- rtnl_unlock();
return ret;
}
@@ -256,17 +254,17 @@ static int write_priomap(struct cgroup *cgrp, struct cftype *cft,
if (!dev)
goto out_free_devname;
+ rtnl_lock();
ret = write_update_netdev_table(dev);
if (ret < 0)
goto out_put_dev;
- rcu_read_lock();
- map = rcu_dereference(dev->priomap);
+ map = rtnl_dereference(dev->priomap);
if (map)
map->priomap[prioidx] = priority;
- rcu_read_unlock();
out_put_dev:
+ rtnl_unlock();
dev_put(dev);
out_free_devname:
^ permalink raw reply related
* Re: blog post
From: Luis R. Rodriguez @ 2012-08-14 22:44 UTC (permalink / raw)
To: Ann Davis, netdev
In-Reply-To: <502A3384020000D6000E532B@novprvoes0310.provo.novell.com>
On Tue, Aug 14, 2012 at 10:16 AM, Ann Davis <andavis@suse.com> wrote:
> Luis,
>
>>>> On 8/13/2012 at 10:45 PM, in message
> <CAB=NE6VFsJrXznJDU4w2Oq1gQkMjLTtJPtTk2hhL0V1FPLpaYQ@mail.gmail.com>, "Luis R.
> Rodriguez" <mcgrof@frijolero.org> wrote:
>> Here is:
>>
>> http://www.do-not-panic.com/2012/08/automatically-backporting-linux-kernel.h
>> tml
>>
>> Does this work?
>
> Yes, I think this is great. Are you ok with me sending it on to Amanda and Jennifer along with a short "Here's some new stuff happening in the Driver Backport Workgroup" intro? I don't know if they will want to just pull in the text or have a link to your mcgrof site; do you have a preference? I think they should just pull in the text and reference your site if needed (less clicks means more likelihood of people reading).
Whatever works, so I just updated the blog to reflect all the
documentation updates I've made. The backports.wiki.kernel.org page
now has all the documentation necessary and is very agnostic to any
subsystem. I've also renamed compat-wireless.git tree to
compat-drivers.git and also set information on the wiki about the
first G+ hangout scheduled for Sept 4th.
Luis
^ permalink raw reply
* Re: [PATCH 0/5] Call netif_carrier_off() after register_netdev()
From: Ben Hutchings @ 2012-08-14 22:47 UTC (permalink / raw)
To: David Miller; +Cc: shchepetkov, netdev, linux-kernel, ldv-project
In-Reply-To: <20120814.140019.1016184364798213214.davem@davemloft.net>
On Tue, 2012-08-14 at 14:00 -0700, David Miller wrote:
> From: Ilya Shchepetkov <shchepetkov@ispras.ru>
> Date: Tue, 14 Aug 2012 14:28:50 +0400
>
> > Hi,
> >
> > There are several patches on the subject:
> >
> > 31bde1ceaa873bcaecd49e829bfabceacc4c512d
> > c55ad8e56b983f03589b38b4504b5d1f41161ff8
> > e826eafa65c6f1f7c8db5a237556cebac57ebcc5
> > 0d672e9f8ac320c6d1ea9103db6df7f99ea20361
> > 6a3c869a6021f4abcd69aa5fbb15c63f69eb36fe
> >
> > In 2008, David Miller wrote in his commit:
> > (b47300168e770b60ab96c8924854c3b0eb4260eb)
> >
> >>net: Do not fire linkwatch events until the device is registered.
> >
> >>Several device drivers try to do things like netif_carrier_off()
> >>before register_netdev() is invoked. This is bogus, but too many
> >>drivers do this to fix them all up in one go.
> >
> > But I don't understand what will happen in this case?
>
> Sigh... I would strongly suggest that when you don't understand
> something you leave it alone until you do.
>
> You can't do the netif_carrier_off() after the device register because
> at the precise moment the device is registered it can be openned in
> parallel on another cpu and thus cause the entire carrier state
> to be changed.
>
> Therefore if you do the netif_carrier_off() afterwards, it might
> be overwriting state changes made in another context.
>
> Please just leave this code alone.
But if you do it beforehand then it doesn't have the intended effect.
(Supposed to be fixed by 22604c866889c4b2e12b73cbf1683bda1b72a313, which
had to be reverted: c276e098d3ee33059b4a1c747354226cec58487c.)
So you have to do it after, but without dropping the RTNL lock in
between.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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 net-next v2 01/15] net: introduce upper device lists
From: Stephen Hemminger @ 2012-08-14 23:00 UTC (permalink / raw)
To: Ben Hutchings
Cc: bridge, ursula.braun, john.r.fastabend, edumazet, sean.hefty,
therbert, roland, linux-s390, linux-rdma, fubar, fbl,
hal.rosenstock, Jiri Pirko, faisal.latif, linux-driver, blaschka,
sony.chacko, gregory.v.rose, xiyou.wangcong, jitendra.kalsaria,
divy, netdev, linux-kernel, kaber, joe, linux390, davem
In-Reply-To: <1344983624.2690.77.camel@bwh-desktop.uk.solarflarecom.com>
On Tue, 14 Aug 2012 23:33:44 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Tue, 2012-08-14 at 17:05 +0200, Jiri Pirko wrote:
> > This lists are supposed to serve for storing pointers to all upper devices.
> > Eventually it will replace dev->master pointer which is used for
> > bonding, bridge, team but it cannot be used for vlan, macvlan where
> > there might be multiple upper present. In case the upper link is
> > replacement for dev->master, it is marked with "master" flag.
>
> Something I found interesting is that the dev->master pointer and now
> netdev_master_upper_dev_get{,_rcu}() are hardly used by the stackled
> drivers that set the master. They also have to set an rx_handler on the
> lower device (which is itself mutually exclusive) which gets its own
> context pointer (rx_handler_data).
>
> Instead, the master pointer is mostly used by device drivers to find out
> about a bridge or bonding device above *their* devices. And that seems
> to work only for those specific device drivers, not e.g. openvswitch or
> team. I wonder if we could find a better way to encapsulate the things
> they want do do, in a later step (not holding up this change!).
The concept is master is very useful to user level config things like
Vyatta for seeing parent/child relationship. Since is in ABI now, it
must stay.
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/85xx: add Fman MDIO muxing support to the P4080DS
From: Scott Wood @ 2012-08-14 23:06 UTC (permalink / raw)
To: Timur Tabi; +Cc: Kumar Gala, Andy Fleming, ddaney.cavm, linuxppc-dev, netdev
In-Reply-To: <502ACA09.6070906@freescale.com>
On 08/14/2012 04:58 PM, Timur Tabi wrote:
> Scott Wood wrote:
>
>> I think that was internally, and not on this specific comment wording.
>> I don't think that code comment adequately explains things.
>
> I don't really have any more insight to add.
My point (at least, this part of it) was that more of the insight you've
already provided should be moved from e-mail discussion to the code comment.
>>> otherwise, the mdio-mux code would not prepare the mdio mus in time, and
>>> there would be initialization failures. Now maybe this goes away with
>>> -EPROBE_DEFER, or maybe it doesn't. But until we push the DPAA drivers
>>> upstream, we won't know.
>>
>> Do you know if it's theoretically supposed to be fixed and just can't
>> test it, or are you unsure of whether it's even supposed to work?
>
> I'm not sure of anything. For one thing, we don't implement EPROBE_DEFER
> in the DPAA drivers, so we'd probably have to fix that before anything.
> And then, I'm just guessing that's the solution.
I feel confident saying it is the solution, at least until it is
demonstrated otherwise.
>> I don't think we should be relying on the order of this list to
>> determine probe order. For one thing, it won't work if the drivers
>> register after you create the platform devices (e.g. they're modules).
>
> I agree we should not be relying on the order, but I don't know what to
> do. EPROBE_DEFER was designed to handle this situation, so my
> recommendation is to worry about it later. I can beef up the comment to
> talk about that, if you want.
If the DPAA driver doesn't implement it when it's submitted, it's a bug
in the DPAA driver and we should insist it be fixed. I don't think we
should at all entertain the notion that careful device id list ordering
is even a potential solution.
If anything, I'd make the ordering be "wrong" to force that code path to
be tested -- though ideally there would be a more systematic approach to
such testing, that doesn't require inefficiency during normal boot.
-Scott
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/85xx: add Fman MDIO muxing support to the P4080DS
From: Timur Tabi @ 2012-08-14 23:08 UTC (permalink / raw)
To: Scott Wood; +Cc: Kumar Gala, Andy Fleming, ddaney.cavm, linuxppc-dev, netdev
In-Reply-To: <502AD9F4.10903@freescale.com>
Scott Wood wrote:
>
> If anything, I'd make the ordering be "wrong" to force that code path to
> be tested -- though ideally there would be a more systematic approach to
> such testing, that doesn't require inefficiency during normal boot.
I can't force the ordering to be wrong, because it's the only entry in the
list. The DPAA entries are not there yet.
This is what I have now:
static const struct of_device_id of_device_ids[] __devinitconst = {
{
.compatible = "simple-bus"
},
{
.compatible = "fsl,srio",
},
{
.compatible = "fsl,p4080-pcie",
},
{
.compatible = "fsl,qoriq-pcie-v2.2",
},
{
.compatible = "fsl,qoriq-pcie-v2.3",
},
{
.compatible = "fsl,qoriq-pcie-v2.4",
},
/* The following two are for the Freescale hypervisor */
{
.name = "hypervisor",
},
{
.name = "handles",
},
{
/*
* Warning: this entry might need to be located before those
* for the Fman Ethernet nodes, although using EPROBE_DEFER
* in the DPAA drivers could fix that.
*/
.compatible = "mdio-mux",
},
{}
};
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/85xx: add Fman MDIO muxing support to the P4080DS
From: Scott Wood @ 2012-08-14 23:12 UTC (permalink / raw)
To: Timur Tabi; +Cc: Kumar Gala, Andy Fleming, ddaney.cavm, linuxppc-dev, netdev
In-Reply-To: <502ADA75.7020200@freescale.com>
On 08/14/2012 06:08 PM, Timur Tabi wrote:
> Scott Wood wrote:
>>
>> If anything, I'd make the ordering be "wrong" to force that code path to
>> be tested -- though ideally there would be a more systematic approach to
>> such testing, that doesn't require inefficiency during normal boot.
>
> I can't force the ordering to be wrong, because it's the only entry in the
> list. The DPAA entries are not there yet.
Right, I mean once the DPAA entries are added.
> This is what I have now:
>
> static const struct of_device_id of_device_ids[] __devinitconst = {
> {
> .compatible = "simple-bus"
> },
> {
> .compatible = "fsl,srio",
> },
> {
> .compatible = "fsl,p4080-pcie",
> },
> {
> .compatible = "fsl,qoriq-pcie-v2.2",
> },
> {
> .compatible = "fsl,qoriq-pcie-v2.3",
> },
> {
> .compatible = "fsl,qoriq-pcie-v2.4",
> },
> /* The following two are for the Freescale hypervisor */
> {
> .name = "hypervisor",
> },
> {
> .name = "handles",
> },
> {
> /*
> * Warning: this entry might need to be located before those
> * for the Fman Ethernet nodes, although using EPROBE_DEFER
> * in the DPAA drivers could fix that.
> */
> .compatible = "mdio-mux",
> },
I'd either say nothing here or say only "The ethernet driver should use
EPROBE_DEFER to ensure that the mdio-mux is probed first". Don't give
whoever submits the DPAA ethernet driver the idea that relying on list
order is an acceptable solution.
-Scott
^ permalink raw reply
* [PATCH 1/2] dt: introduce for_each_available_child_of_node, of_get_next_available_child
From: Timur Tabi @ 2012-08-14 23:20 UTC (permalink / raw)
To: Grant Likely, David Miller, david.daney-YGCgFSpz5w/QT0dZR+AlfA,
netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
Macro for_each_child_of_node() makes it easy to iterate over all of the
children for a given device tree node, including those nodes that are
marked as unavailable (i.e. status = "disabled").
Introduce for_each_available_child_of_node(), which is like
for_each_child_of_node(), but it automatically skips unavailable nodes.
This also requires the introduction of helper function
of_get_next_available_child(), which returns the next available child
node.
Signed-off-by: Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
drivers/of/base.c | 27 +++++++++++++++++++++++++++
include/linux/of.h | 7 +++++++
2 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index c181b94..d4a1c9a 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -364,6 +364,33 @@ struct device_node *of_get_next_child(const struct device_node *node,
EXPORT_SYMBOL(of_get_next_child);
/**
+ * of_get_next_available_child - Find the next available child node
+ * @node: parent node
+ * @prev: previous child of the parent node, or NULL to get first
+ *
+ * This function is like of_get_next_child(), except that it
+ * automatically skips any disabled nodes (i.e. status = "disabled").
+ */
+struct device_node *of_get_next_available_child(const struct device_node *node,
+ struct device_node *prev)
+{
+ struct device_node *next;
+
+ read_lock(&devtree_lock);
+ next = prev ? prev->sibling : node->child;
+ for (; next; next = next->sibling) {
+ if (!of_device_is_available(next))
+ continue;
+ if (of_node_get(next))
+ break;
+ }
+ of_node_put(prev);
+ read_unlock(&devtree_lock);
+ return next;
+}
+EXPORT_SYMBOL(of_get_next_available_child);
+
+/**
* of_find_node_by_path - Find a node matching a full OF path
* @path: The full path to match
*
diff --git a/include/linux/of.h b/include/linux/of.h
index 5919ee3..1b11632 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -190,10 +190,17 @@ extern struct device_node *of_get_parent(const struct device_node *node);
extern struct device_node *of_get_next_parent(struct device_node *node);
extern struct device_node *of_get_next_child(const struct device_node *node,
struct device_node *prev);
+extern struct device_node *of_get_next_available_child(
+ const struct device_node *node, struct device_node *prev);
+
#define for_each_child_of_node(parent, child) \
for (child = of_get_next_child(parent, NULL); child != NULL; \
child = of_get_next_child(parent, child))
+#define for_each_available_child_of_node(parent, child) \
+ for (child = of_get_next_available_child(parent, NULL); child != NULL; \
+ child = of_get_next_available_child(parent, child))
+
static inline int of_get_child_count(const struct device_node *np)
{
struct device_node *child;
--
1.7.3.4
^ permalink raw reply related
* [PATCH 2/2] [v2] netdev/phy: skip disabled mdio-mux nodes
From: Timur Tabi @ 2012-08-14 23:20 UTC (permalink / raw)
To: Grant Likely, David Miller, david.daney-YGCgFSpz5w/QT0dZR+AlfA,
netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <1344986424-14360-1-git-send-email-timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
The mdio-mux driver scans all child mdio nodes, without regard to whether
the node is actually used. Some device trees include all possible
mdio-mux nodes and rely on the boot loader to disable those that are not
present, based on some run-time configuration. Those nodes need to be
skipped.
Signed-off-by: Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
drivers/net/phy/mdio-mux.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/phy/mdio-mux.c b/drivers/net/phy/mdio-mux.c
index 5c12018..4d4d25e 100644
--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -132,7 +132,7 @@ int mdio_mux_init(struct device *dev,
pb->mii_bus = parent_bus;
ret_val = -ENODEV;
- for_each_child_of_node(dev->of_node, child_bus_node) {
+ for_each_available_child_of_node(dev->of_node, child_bus_node) {
u32 v;
r = of_property_read_u32(child_bus_node, "reg", &v);
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH 01/16] hashtable: introduce a small and naive hashtable
From: NeilBrown @ 2012-08-14 23:25 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
tj-DgEjT+Ai2ygdnm+yROfE0A, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mingo-X9Un+BFzKDI, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, josh-iaAMLnmF4UmaiuxdJuQwMA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w,
axboe-tSWWG44O7X1aa/9Udqfwiw, agk-H+wXaHxf7aLQT0dZR+AlfA,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <1344961490-4068-2-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1142 bytes --]
On Tue, 14 Aug 2012 18:24:35 +0200 Sasha Levin <levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:
> +static inline void hash_init_size(struct hlist_head *hashtable, int bits)
> +{
> + int i;
> +
> + for (i = 0; i < HASH_SIZE(bits); i++)
> + INIT_HLIST_HEAD(hashtable + i);
> +}
This seems like an inefficient way to do "memset(hashtable, 0, ...);".
And in many cases it isn't needed as the hash table is static and initialised
to zero.
I note that in the SUNRPC/cache patch you call hash_init(), but in the lockd
patch you don't. You don't actually need to in either case.
I realise that any optimisation here is for code that is only executed once
per boot, so no big deal, and even the presence of extra code making the
kernel bigger is unlikely to be an issue. But I'd at least like to see
consistency: Either use hash_init everywhere, even when not needed, or only
use it where absolutely needed which might be no-where because static tables
are already initialised, and dynamic tables can use GFP_ZERO.
And if you keep hash_init_size I would rather see a memset(0)....
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH 02/16] user_ns: use new hashtable implementation
From: Eric W. Biederman @ 2012-08-14 23:52 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, aarcange, ericvh, netdev, josh, eric.dumazet,
mathieu.desnoyers, axboe, agk, dm-devel, neilb, ccaulfie,
teigland, Trond.Myklebust, bfields, fweisbec, jesse,
venkat.x.venkatsubra, ejt, snitzer, edumazet, linux-nfs, dev,
rds-devel, lw
In-Reply-To: <1344961490-4068-3-git-send-email-levinsasha928@gmail.com>
Sasha Levin <levinsasha928@gmail.com> writes:
> Switch user_ns to use the new hashtable implementation. This reduces the amount of
> generic unrelated code in user_ns.
Two concerns here.
1) When adding a new entry you recompute the hash where previously that
was not done. I believe that will slow down adding of new entries.
2) Using hash_32 for uids is an interesting choice. hash_32 discards
the low bits. Last I checked for uids the low bits were the bits
that were most likely to be different and had the most entropy.
I'm not certain how multiplying by the GOLDEN_RATION_PRIME_32 will
affect things but I would be surprised if it shifted all of the
randomness from the low bits to the high bits.
And just a nit. struct user is essentially orthogonal to the user namespace
at this point, making the description of the patch a little weird.
Eric
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> kernel/user.c | 33 +++++++++++++--------------------
> 1 files changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/kernel/user.c b/kernel/user.c
> index b815fef..d10c484 100644
> --- a/kernel/user.c
> +++ b/kernel/user.c
> @@ -16,6 +16,7 @@
> #include <linux/interrupt.h>
> #include <linux/export.h>
> #include <linux/user_namespace.h>
> +#include <linux/hashtable.h>
>
> /*
> * userns count is 1 for root user, 1 for init_uts_ns,
> @@ -52,13 +53,9 @@ EXPORT_SYMBOL_GPL(init_user_ns);
> */
>
> #define UIDHASH_BITS (CONFIG_BASE_SMALL ? 3 : 7)
> -#define UIDHASH_SZ (1 << UIDHASH_BITS)
> -#define UIDHASH_MASK (UIDHASH_SZ - 1)
> -#define __uidhashfn(uid) (((uid >> UIDHASH_BITS) + uid) & UIDHASH_MASK)
> -#define uidhashentry(uid) (uidhash_table + __uidhashfn((__kuid_val(uid))))
>
> static struct kmem_cache *uid_cachep;
> -struct hlist_head uidhash_table[UIDHASH_SZ];
> +static DEFINE_HASHTABLE(uidhash_table, UIDHASH_BITS)
>
> /*
> * The uidhash_lock is mostly taken from process context, but it is
> @@ -84,22 +81,22 @@ struct user_struct root_user = {
> /*
> * These routines must be called with the uidhash spinlock held!
> */
> -static void uid_hash_insert(struct user_struct *up, struct hlist_head *hashent)
> +static void uid_hash_insert(struct user_struct *up)
> {
> - hlist_add_head(&up->uidhash_node, hashent);
> + hash_add(uidhash_table, &up->uidhash_node, __kuid_val(up->uid));
> }
>
> static void uid_hash_remove(struct user_struct *up)
> {
> - hlist_del_init(&up->uidhash_node);
> + hash_del(&up->uidhash_node);
> }
>
> -static struct user_struct *uid_hash_find(kuid_t uid, struct hlist_head *hashent)
> +static struct user_struct *uid_hash_find(kuid_t uid)
> {
> struct user_struct *user;
> struct hlist_node *h;
>
> - hlist_for_each_entry(user, h, hashent, uidhash_node) {
> + hash_for_each_possible(uidhash_table, user, h, uidhash_node, __kuid_val(uid)) {
> if (uid_eq(user->uid, uid)) {
> atomic_inc(&user->__count);
> return user;
> @@ -135,7 +132,7 @@ struct user_struct *find_user(kuid_t uid)
> unsigned long flags;
>
> spin_lock_irqsave(&uidhash_lock, flags);
> - ret = uid_hash_find(uid, uidhashentry(uid));
> + ret = uid_hash_find(uid);
> spin_unlock_irqrestore(&uidhash_lock, flags);
> return ret;
> }
> @@ -156,11 +153,10 @@ void free_uid(struct user_struct *up)
>
> struct user_struct *alloc_uid(kuid_t uid)
> {
> - struct hlist_head *hashent = uidhashentry(uid);
> struct user_struct *up, *new;
>
> spin_lock_irq(&uidhash_lock);
> - up = uid_hash_find(uid, hashent);
> + up = uid_hash_find(uid);
> spin_unlock_irq(&uidhash_lock);
>
> if (!up) {
> @@ -176,13 +172,13 @@ struct user_struct *alloc_uid(kuid_t uid)
> * on adding the same user already..
> */
> spin_lock_irq(&uidhash_lock);
> - up = uid_hash_find(uid, hashent);
> + up = uid_hash_find(uid);
> if (up) {
> key_put(new->uid_keyring);
> key_put(new->session_keyring);
> kmem_cache_free(uid_cachep, new);
> } else {
> - uid_hash_insert(new, hashent);
> + uid_hash_insert(new);
> up = new;
> }
> spin_unlock_irq(&uidhash_lock);
> @@ -196,17 +192,14 @@ out_unlock:
>
> static int __init uid_cache_init(void)
> {
> - int n;
> -
> uid_cachep = kmem_cache_create("uid_cache", sizeof(struct user_struct),
> 0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
>
> - for(n = 0; n < UIDHASH_SZ; ++n)
> - INIT_HLIST_HEAD(uidhash_table + n);
> + hash_init(uidhash_table);
>
> /* Insert the root user immediately (init already runs as root) */
> spin_lock_irq(&uidhash_lock);
> - uid_hash_insert(&root_user, uidhashentry(GLOBAL_ROOT_UID));
> + uid_hash_insert(&root_user);
> spin_unlock_irq(&uidhash_lock);
>
> return 0;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ 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