* 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
* Re: [PATCH net 1/3] llc2: Fix silent failure of llc_station_init()
From: David Miller @ 2012-08-14 23:52 UTC (permalink / raw)
To: ben; +Cc: acme, netdev
In-Reply-To: <1344862199.824.140.camel@deadeye.wl.decadent.org.uk>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Mon, 13 Aug 2012 13:49:59 +0100
> llc_station_init() creates and processes an event skb with no effect
> other than to change the state from DOWN to UP. Allocation failure is
> reported, but then ignored by its caller, llc2_init(). Remove this
> possibility by simply initialising the state as UP.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Applied.
^ permalink raw reply
* Re: [PATCH net 2/3] llc2: Call llc_station_exit() on llc2_init() failure path
From: David Miller @ 2012-08-14 23:52 UTC (permalink / raw)
To: ben; +Cc: acme, netdev
In-Reply-To: <1344862243.824.141.camel@deadeye.wl.decadent.org.uk>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Mon, 13 Aug 2012 13:50:43 +0100
> Otherwise the station packet handler will remain registered even though
> the module is unloaded.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
This will cause section mismatch errors, you'll need to remove
the __exit tag from llc_station_exit() if you want to start
invoking it from llc2_init() which is __init.
I took care of this when applying this patch to 'net'.
^ permalink raw reply
* Re: [PATCH net 3/3] llc: Fix races between llc2 handler use and (un)registration
From: David Miller @ 2012-08-14 23:52 UTC (permalink / raw)
To: ben; +Cc: acme, netdev
In-Reply-To: <1344862255.824.142.camel@deadeye.wl.decadent.org.uk>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Mon, 13 Aug 2012 13:50:55 +0100
> When registering the handlers, any state they rely on must be
> completely initialised first. When unregistering, we must wait until
> they are definitely no longer running. llc_rcv() must also avoid
> reading the handler pointers again after checking for NULL.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/4] packet: Sock-diag extension for packet sockets (beginning)
From: David Miller @ 2012-08-14 23:56 UTC (permalink / raw)
To: xemul; +Cc: netdev
In-Reply-To: <5029219C.20305@parallels.com>
From: Pavel Emelyanov <xemul@parallels.com>
Date: Mon, 13 Aug 2012 19:47:40 +0400
> There's a set of stuff residing on a AF_PACKET socket which is currently
> write-only -- rings, copy_thresh and mclist. But we need to know these
> while doing the checkpoint-restore.
>
> As a solution for the similar problem with unix sockets, the sock-diag
> engine was developed, so here's the basic implementation of the packet
> sockets extension. It doesn't report rings, fanout and stats, but it can
> be easily added later.
>
> I'd like to mention, that it's not a strict requirement, that the diag is
> used for getting info about sockets, it would be perfectly fine just to
> fix the packet getsockopt callback to return the desired info. So, if the
> diag for packet sockets is for any reason unacceptable, just let me know.
All applied, looks fine.
Please number your patches properly next time, patch #3 was listed as 2/4
^ permalink raw reply
* Re: [PATCH 4/5] drivers/net/ethernet/ti/davinci_cpdma.c: Remove potential NULL dereference
From: David Miller @ 2012-08-15 0:00 UTC (permalink / raw)
To: Julia.Lawall; +Cc: netdev, kernel-janitors, linux-kernel
In-Reply-To: <1344959388-19719-5-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Tue, 14 Aug 2012 17:49:47 +0200
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> If the NULL test is necessary, the initialization involving a dereference of
> the tested value should be moved after the NULL test.
>
> The sematic patch that fixes this problem is as follows:
> (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 3/5] drivers/net/ethernet/freescale/fs_enet: fix error return code
From: David Miller @ 2012-08-15 0:01 UTC (permalink / raw)
To: Julia.Lawall
Cc: pantelis.antoniou, kernel-janitors, vbordug, linuxppc-dev, netdev,
linux-kernel
In-Reply-To: <1344949115-13266-4-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Tue, 14 Aug 2012 14:58:33 +0200
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Convert a 0 error return code to a negative one, as returned elsewhere in the
> function.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Applied.
^ permalink raw reply
* Re: [PATCH 4/5] drivers/net/ethernet/mellanox/mlx4/mcg.c: fix error return code
From: David Miller @ 2012-08-15 0:01 UTC (permalink / raw)
To: Julia.Lawall; +Cc: netdev, kernel-janitors, linux-kernel
In-Reply-To: <1344949115-13266-5-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Tue, 14 Aug 2012 14:58:34 +0200
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Convert a 0 error return code to a negative one, as returned elsewhere in the
> function.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Applied.
^ 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