* Re: [PATCH v8 01/16] hashtable: introduce a small and naive hashtable
From: Tejun Heo @ 2012-10-30 21:42 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, 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: <1351622772-16400-1-git-send-email-levinsasha928@gmail.com>
Hello,
Just some nitpicks.
On Tue, Oct 30, 2012 at 02:45:57PM -0400, Sasha Levin wrote:
> +/* Use hash_32 when possible to allow for fast 32bit hashing in 64bit kernels. */
> +#define hash_min(val, bits) \
> +({ \
> + sizeof(val) <= 4 ? \
> + hash_32(val, bits) : \
> + hash_long(val, bits); \
> +})
Doesn't the above fit in 80 column. Why is it broken into multiple
lines? Also, you probably want () around at least @val. In general,
it's a good idea to add () around any macro argument to avoid nasty
surprises.
Looks good to me otherwise.
Reviewed-by: Tejun Heo <tj@kernel.org>
Thanks.
--
tejun
--
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 4/4] arm/dts: am33xx: Add CPSW and MDIO module nodes for AM33XX
From: Peter Korsgaard @ 2012-10-30 21:35 UTC (permalink / raw)
To: Vaibhav Hiremath
Cc: netdev, paul, linux-arm-kernel, linux-omap, Mugunthan V N,
Richard Cochran, Benoit Cousson
In-Reply-To: <1351498881-32482-6-git-send-email-hvaibhav@ti.com>
>>>>> "Vaibhav" == Vaibhav Hiremath <hvaibhav@ti.com> writes:
Vaibhav> From: Mugunthan V N <mugunthanvnm@ti.com>
Vaibhav> Add CPSW and MDIO related device tree data for AM33XX.
Vaibhav> Also enable them into board/evm dts files by providing
Vaibhav> respective phy-id.
Vaibhav> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Vaibhav> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Vaibhav> Cc: Richard Cochran <richardcochran@gmail.com>
Vaibhav> Cc: Benoit Cousson <b-cousson@ti.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
--
Bye, Peter Korsgaard
^ permalink raw reply
* Re: [PATCH 3/4] ARM: OMAP3+: hwmod: Add AM33XX HWMOD data for davinci_mdio module
From: Peter Korsgaard @ 2012-10-30 21:35 UTC (permalink / raw)
To: Vaibhav Hiremath
Cc: netdev, paul, linux-arm-kernel, linux-omap, Mugunthan V N,
Richard Cochran
In-Reply-To: <1351498881-32482-5-git-send-email-hvaibhav@ti.com>
>>>>> "Vaibhav" == Vaibhav Hiremath <hvaibhav@ti.com> writes:
Vaibhav> From: Mugunthan V N <mugunthanvnm@ti.com>
Vaibhav> This patch adds hwmod entry for davinci MDIO module,
Vaibhav> creating parent<->child relationship between CPSW and MDIO module.
Vaibhav> This Parent-child relation is required in order to use common resources
Vaibhav> like, clock, but still maintaining the logical separation between them.
Vaibhav> CPGMAC SubSystem consist of various sub-modules, like, mdio, cpdma,
Vaibhav> cpsw, etc... These sub-modules are also used in some of Davinci
Vaibhav> family of devices, so separate and independent platform devices &
Vaibhav> drivers for CPSW and MDIO is implemented.
Vaibhav> In case of AM33XX, the resources are shared and common register
Vaibhav> bit-field is provided to control module/clock enable/disable,
Vaibhav> makes it difficult to handle common resources from both drivers.
Vaibhav> So the solution is, create parent<->child relationship between
Vaibhav> CPGMAC & MDIO modules.
Vaibhav> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Vaibhav> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Vaibhav> Cc: Richard Cochran <richardcochran@gmail.com>
Vaibhav> Cc: Paul Walmsley <paul@pwsan.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
--
Bye, Peter Korsgaard
^ permalink raw reply
* Re: [PATCH 2/4] net: cpsw: Add parent<->child relation support between cpsw and mdio
From: Peter Korsgaard @ 2012-10-30 21:34 UTC (permalink / raw)
To: Vaibhav Hiremath
Cc: netdev, paul, linux-arm-kernel, linux-omap, Mugunthan V N,
Richard Cochran
In-Reply-To: <1351498881-32482-4-git-send-email-hvaibhav@ti.com>
>>>>> "Vaibhav" == Vaibhav Hiremath <hvaibhav@ti.com> writes:
Vaibhav> CPGMAC SubSystem consist of various sub-modules, like, mdio,
Vaibhav> cpdma, cpsw, etc... These sub-modules are also used in some of
Vaibhav> Davinci family of devices. Now based on requirement, use-case
Vaibhav> and available technology nodes the integration of these
Vaibhav> sub-modules varies across devices.
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
--
Bye, Peter Korsgaard
^ permalink raw reply
* Re: [PATCH 1/4] net: davinci_mdio: Fix typo mistake in calling runtime-pm api
From: Peter Korsgaard @ 2012-10-30 21:33 UTC (permalink / raw)
To: Vaibhav Hiremath
Cc: netdev, paul, linux-arm-kernel, linux-omap, Mugunthan V N,
Richard Cochran
In-Reply-To: <1351498881-32482-3-git-send-email-hvaibhav@ti.com>
>>>>> "Vaibhav" == Vaibhav Hiremath <hvaibhav@ti.com> writes:
Vaibhav> By mistake (most likely a copy-paste), instead of pm_runtime_get_sync()
Vaibhav> api, driver is calling pm_runtime_put_sync() api in resume callback
Vaibhav> function. The bug was introduced by commit id (ae2c07aaf74:
Vaibhav> davinci_mdio: runtime PM support).
Vaibhav> Now, the reason why it didn't impact functionality is, the patch has
Vaibhav> been tested on AM335x-EVM and BeagleBone platform while submitting;
Vaibhav> and in case of AM335x the MDIO driver doesn't control the module
Vaibhav> enable/disable part, which is handled by CPSW driver.
Vaibhav> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Vaibhav> Cc: Mugunthan V N <mugunthanvnm@ti.com>
Vaibhav> Cc: Richard Cochran <richardcochran@gmail.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
--
Bye, Peter Korsgaard
^ permalink raw reply
* ping -f is broken in iputils 20121011
From: Mohammad Alsaleh @ 2012-10-30 20:52 UTC (permalink / raw)
To: netdev
As explained in this bug report(1). ping -f seems to be broken in
iputils 20121011.
(1) https://bugs.archlinux.org/task/32306
^ permalink raw reply
* Re: [PATCH net-next 1/8] bridge: bridge port parameters over netlink
From: Stephen Hemminger @ 2012-10-30 21:00 UTC (permalink / raw)
To: Tommy S. Christensen; +Cc: davem, netdev
In-Reply-To: <50902115.3090005@gurgi.dk>
On Tue, 30 Oct 2012 19:48:53 +0100
"Tommy S. Christensen" <tsc@gurgi.dk> wrote:
> On 10/30/2012 01:57 AM, Stephen Hemminger wrote:
> >
> > -/*
> > - * Change state of port (ie from forwarding to blocking etc)
> > +static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
> > + [IFLA_BRPORT_STATE] = { .type = NLA_U8 },
> > + [IFLA_BRPORT_COST] = { .type = NLA_U16 },
> > + [IFLA_BRPORT_PRIORITY] = { .type = NLA_U32 },
> > + [IFLA_BRPORT_MODE] = { .type = NLA_U8 },
> > +};
> > +
>
> Hey Stephen
>
> It seems you've swapped the sizes of COST and PRIORITY in this part.
>
> -Tommy
>
Ok, will fix in next version.
One alternative I considered would be to use previously unused IFLA_PRIORITY and IFLA_COST
which would be more generic. But these fields only really matter to bridge.
^ permalink raw reply
* Re: [PATCH] vxlan: allow a user to set TTL value
From: Stephen Hemminger @ 2012-10-30 20:54 UTC (permalink / raw)
To: Vincent Bernat; +Cc: David Miller, netdev
In-Reply-To: <1351628836-23094-1-git-send-email-bernat@luffy.cx>
On Tue, 30 Oct 2012 21:27:16 +0100
Vincent Bernat <bernat@luffy.cx> wrote:
> "ip link add ... type vxlan ... ttl X" allows a user to set the TTL
> used by a VXLAN for encapsulation. The provided value was ignored by
> vxlan module and the default value of 1 was used when encapsulating
> multicast packets.
>
> Signed-off-by: Vincent Bernat <bernat@luffy.cx>
> ---
> drivers/net/vxlan.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 607976c..030559d 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1107,6 +1107,9 @@ static int vxlan_newlink(struct net *net, struct net_device *dev,
> if (data[IFLA_VXLAN_TOS])
> vxlan->tos = nla_get_u8(data[IFLA_VXLAN_TOS]);
>
> + if (data[IFLA_VXLAN_TTL])
> + vxlan->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
> +
> if (!data[IFLA_VXLAN_LEARNING] || nla_get_u8(data[IFLA_VXLAN_LEARNING]))
> vxlan->learn = true;
>
^ permalink raw reply
* Re: [PATCH] vxlan: allow a user to set TTL value
From: Stephen Hemminger @ 2012-10-30 20:53 UTC (permalink / raw)
To: Vincent Bernat; +Cc: David Miller, netdev
In-Reply-To: <1351628836-23094-1-git-send-email-bernat@luffy.cx>
On Tue, 30 Oct 2012 21:27:16 +0100
Vincent Bernat <bernat@luffy.cx> wrote:
> "ip link add ... type vxlan ... ttl X" allows a user to set the TTL
> used by a VXLAN for encapsulation. The provided value was ignored by
> vxlan module and the default value of 1 was used when encapsulating
> multicast packets.
>
> Signed-off-by: Vincent Bernat <bernat@luffy.cx>
> ---
> drivers/net/vxlan.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 607976c..030559d 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1107,6 +1107,9 @@ static int vxlan_newlink(struct net *net, struct net_device *dev,
> if (data[IFLA_VXLAN_TOS])
> vxlan->tos = nla_get_u8(data[IFLA_VXLAN_TOS]);
>
> + if (data[IFLA_VXLAN_TTL])
> + vxlan->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
> +
> if (!data[IFLA_VXLAN_LEARNING] || nla_get_u8(data[IFLA_VXLAN_LEARNING]))
> vxlan->learn = true;
>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
^ permalink raw reply
* [PATCH] vxlan: allow a user to set TTL value
From: Vincent Bernat @ 2012-10-30 20:27 UTC (permalink / raw)
To: Stephen Hemminger, David Miller, netdev; +Cc: Vincent Bernat
"ip link add ... type vxlan ... ttl X" allows a user to set the TTL
used by a VXLAN for encapsulation. The provided value was ignored by
vxlan module and the default value of 1 was used when encapsulating
multicast packets.
Signed-off-by: Vincent Bernat <bernat@luffy.cx>
---
drivers/net/vxlan.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 607976c..030559d 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1107,6 +1107,9 @@ static int vxlan_newlink(struct net *net, struct net_device *dev,
if (data[IFLA_VXLAN_TOS])
vxlan->tos = nla_get_u8(data[IFLA_VXLAN_TOS]);
+ if (data[IFLA_VXLAN_TTL])
+ vxlan->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
+
if (!data[IFLA_VXLAN_LEARNING] || nla_get_u8(data[IFLA_VXLAN_LEARNING]))
vxlan->learn = true;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH firmware] rtl_nic: update firmware for RTL8168G
From: Ben Hutchings @ 2012-10-30 20:37 UTC (permalink / raw)
To: Hayes Wang; +Cc: dwmw2, romieu, netdev, linux-kernel
In-Reply-To: <1350975734-1608-1-git-send-email-hayeswang@realtek.com>
[-- Attachment #1: Type: text/plain, Size: 425 bytes --]
On Tue, 2012-10-23 at 15:02 +0800, Hayes Wang wrote:
> File: rtl_nic/rtl8168g-1.fw
> Version: 0.0.3
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
> WHENCE | 2 +-
> rtl_nic/rtl8168g-1.fw | Bin 4272 -> 4304 bytes
> 2 files changed, 1 insertion(+), 1 deletion(-)
[...]
Applied, thanks.
Ben.
--
Ben Hutchings
Humans are not rational beings; they are rationalising beings.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/3] pppoatm: don't send frames to destroyed vcc
From: Krzysztof Mazur @ 2012-10-30 20:19 UTC (permalink / raw)
To: David Woodhouse; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1351589700.17077.31.camel@shinybook.infradead.org>
On Tue, Oct 30, 2012 at 09:35:00AM +0000, David Woodhouse wrote:
> On Mon, 2012-10-22 at 19:14 +0200, Krzysztof Mazur wrote:
> > Now pppoatm_send(), like vcc_sendmsg(), checks for vcc flags that
> > indicate that vcc is not ready.
>
> I note that vcc_sendmsg() also checks for sock->state == SS_CONNECTED.
> Is that check not needed here? Otherwise, looks sane enough.
>
> Acked-By: David Woodhouse <David.Woodhouse@intel.com>
I don't think so. We never leave SS_CONNECTED state. This check is
done in vcc_sendmsg() because it's called from userspace.
However maybe we should check socket state before assigning vcc to
pppoatm (untested):
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index 0dcb5dc..df06d14 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -414,6 +414,8 @@ static int pppoatm_ioctl(struct socket *sock, unsigned int cmd,
return -ENOIOCTLCMD;
if (!capable(CAP_NET_ADMIN))
return -EPERM;
+ if (sock->state != SS_CONNECTED)
+ return -EINVAL;
return pppoatm_assign_vcc(atmvcc, argp);
}
case PPPIOCGCHAN:
Thanks.
Krzysiek
^ permalink raw reply related
* Re: [PATCH v2 2/3] pppoatm: fix race condition with destroying of vcc
From: Krzysztof Mazur @ 2012-10-30 19:52 UTC (permalink / raw)
To: David Woodhouse; +Cc: davem, netdev, linux-kernel
In-Reply-To: <20121030190725.GA14044@shrek.podlesie.net>
On Tue, Oct 30, 2012 at 08:07:25PM +0100, Krzysztof Mazur wrote:
> On Tue, Oct 30, 2012 at 09:37:48AM +0000, David Woodhouse wrote:
> >
> > Should we be locking it earlier, so that the atm_may_send() call is also
> > covered by the lock?
>
> Yes, but only to protect against concurent vcc_sendmsg().
>
> >
> > Either way, it's an obvious improvement on what we had before ??? and even
> > if the answer to my question above is 'yes', exceeding the configured
> > size by one packet is both harmless and almost never going to happen
> > since we now limit ourselves to two packets anyway. So:
> >
> > Acked-By: David Woodhouse <David.Woodhouse@intel.com>
> >
>
David, I think we should also fix the issue with sk_sndbuf < MTU,
which is described in comment in pppoatm_may_send() added by
your "pppoatm: Fix excessive queue bloat" patch.
The vcc_sendmsg() already does that.
Krzysiek
-- >8 --
Subject: [PATCH] pppoatm: fix sending packets when sk_sndbuf < MTU
Now pppoatm_send() works, when sk_sndbuf is smaller than MTU. This
issue was already pointed in comment:
/*
* It's not clear that we need to bother with using atm_may_send()
* to check we don't exceed sk->sk_sndbuf. If userspace sets a
* value of sk_sndbuf which is lower than the MTU, we're going to
* block for ever. But the code always did that before we introduced
* the packet count limit, so...
*/
The test is copied from alloc_tx() which is used by vcc_sendmsg().
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
net/atm/pppoatm.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index 4cc81b5..f25536b 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -306,12 +306,9 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
/*
* It's not clear that we need to bother with using atm_may_send()
- * to check we don't exceed sk->sk_sndbuf. If userspace sets a
- * value of sk_sndbuf which is lower than the MTU, we're going to
- * block for ever. But the code always did that before we introduced
- * the packet count limit, so...
+ * to check we don't exceed sk->sk_sndbuf.
*/
- if (!atm_may_send(vcc, skb->truesize))
+ if (sk_wmem_alloc_get(sk_atm(vcc)) && !atm_may_send(vcc, skb->truesize))
goto nospace_unlock_sock;
atomic_add(skb->truesize, &sk_atm(ATM_SKB(skb)->vcc)->sk_wmem_alloc);
--
1.8.0.172.g62af90c
^ permalink raw reply related
* Re: [PATCH] bonding: fix bond 6 mode change MAC of arp reply from vif to cause Domu's network unreachable intermittently
From: Jay Vosburgh @ 2012-10-30 19:39 UTC (permalink / raw)
To: Zheng Li; +Cc: netdev, andy, linux-kernel, davem, joe.jin
In-Reply-To: <1351565271-5888-1-git-send-email-zheng.x.li@oracle.com>
Zheng Li <zheng.x.li@oracle.com> wrote:
>This is a fix for a bug in bond_alb.c
>Rate of reproduced:100%
>Scenario: set Dom0 to bond 6 mode, Domu communicate with Dom0 through vif which
>is in bridge mode. The Dom0's bridge of xenbr0 contains vif and bond0, bond0
>contains eth0 and eth1. You can just need to ping a host which is in same LAN on
>Domu, some of packets will be lost intermittently.
>Analyse: When Dom0 set bond mode to 6, the bond alb will change MAC of every arp
>reply in rlb_arp_xmit function to affect receive packets, it is ok for normal
>NIC, but it's wrong to Domu, when Domu send arp reply through vif of Dom0, bond
>of alb replace Domu's MAC in arp reply with NIC's MAC address, that will cause
>remote host send packets to Domu using real NIC's MAC instead of Domu's MAC. Domu
>can't receive the packets whose dst MAC is not Domu's MAC.
If I understand correctly, the issue really isn't about Dom0 /
Domu specifically, but rather that ARP traffic passing through a bridge
and out via the bond (when the bond is a port of the bridge) should not
have its source MAC address adjusted by the receive load balance code in
rlb_arp_xmit.
Is my understanding correct?
If so, then the patch in principle seems reasonable (although it
could be coded without the sentinel; more on that in a minute), but the
subject and description aren't really an accurate description of the
root problem (just one manifestation of it). A better subject might be
something like "rlb mode should not alter ARP replies originating via
bridge."
>Cc: Jay Vosburgh <fubar@us.ibm.com>
>Cc: Andy Gospodarek <andy@greyhouse.net>
>Cc: "David S. Miller" <davem@davemloft.net>
>Signed-off-by: Zheng Li <zheng.x.li@oracle.com>
>
>---
> drivers/net/bonding/bond_alb.c | 13 ++++++++++++-
> 1 files changed, 12 insertions(+), 1 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index e15cc11..d6b134a 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -700,7 +700,18 @@ static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond)
> */
> tx_slave = rlb_choose_channel(skb, bond);
> if (tx_slave) {
>- memcpy(arp->mac_src,tx_slave->dev->dev_addr, ETH_ALEN);
>+ struct slave *tmp_slave = NULL;
>+ int i = 0, found_mac = 0;
>+ bond_for_each_slave(bond, tmp_slave, i) {
>+ if (ether_addr_equal_64bits(arp->mac_src,
>+ tmp_slave->dev->dev_addr)) {
>+ found_mac = 1;
>+ break;
>+ }
>+ }
>+ if (found_mac)
>+ memcpy(arp->mac_src, tx_slave->dev->dev_addr,
>+ ETH_ALEN);
I think this could be coded as:
bond_for_each_slave(bond, tmp_slave, i) {
if (ether_addr_equal_64bits(arp->mac_src,
tmp_slave->dev->dev_addr)) {
memcpy(arp->mac_src,
tx_slave->dev->dev_addr,
ETH_ALEN);
break;
}
}
or even create a "bond_slave_has_mac" helper, since this loop is
something done several times in the code, e.g.,
struct slave *bond_slave_has_mac(struct bonding *bond, const u8 *mac)
{
int i = 0;
struct slave *tmp;
bond_for_each_slave(bond, tmp, i)
if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr))
return tmp;
return NULL;
}
then your code would be
if (tx_slave) {
struct slave *tmp;
/* Only modify ARP's MAC if it originates locally;
* don't change ARPs arriving via a bridge.
*/
tmp = bond_slave_has_mac(bond, arp->mac_src);
if (tmp)
memcpy(arp->mac_src, tx_slave->dev->dev_addr,
ETH_ALEN);
... and so on. I haven't tested any of the above, I'm just
scribbling it out into email, but it seems reasonable.
> }
> pr_debug("Server sent ARP Reply packet\n");
> } else if (arp->op_code == htons(ARPOP_REQUEST)) {
>--
>1.7.6.5
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: Krzysztof Mazur @ 2012-10-30 19:26 UTC (permalink / raw)
To: David Woodhouse; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1351589962.17077.36.camel@shinybook.infradead.org>
On Tue, Oct 30, 2012 at 09:39:22AM +0000, David Woodhouse wrote:
> On Mon, 2012-10-22 at 19:14 +0200, Krzysztof Mazur wrote:
> > The pppoatm gets a reference to atmvcc, but does not increment vcc
> > usage count. The vcc uses vcc->sk socket for reference counting,
> > so sock_hold() and sock_put() should be used by pppoatm.
> >
> > Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
> > Cc: David Woodhouse <dwmw2@infradead.org>
>
> Acked-By: David Woodhouse <David.Woodhouse@intel.com>
This patch is not needed, because vcc_destroy_socket()
calls pppoatm_push(vcc, NULL) to indicate that vcc is now closed,
before vcc_release() calls sock_put() and it's properly handled
by pppoatm.
I will drop this patch.
>
> But did you spot what's in the end of the context of the first hunk...?
>
> > --- a/net/atm/pppoatm.c
> > +++ b/net/atm/pppoatm.c
> > @@ -154,6 +154,7 @@ static void pppoatm_unassign_vcc(struct atm_vcc
> > *atmvcc)
> > tasklet_kill(&pvcc->wakeup_tasklet);
> > ppp_unregister_channel(&pvcc->chan);
> > atmvcc->user_back = NULL;
> > + sock_put(sk_atm(pvcc->atmvcc));
> > kfree(pvcc);
> > /* Gee, I hope we have the big kernel lock here... */
> > module_put(THIS_MODULE);
>
> Fairly sure that hope is unfounded these days... :)
>
Yes, I saw that.
Thanks.
Krzysiek
^ permalink raw reply
* Re: [PATCH v8 16/16] tracing output: use new hashtable implementation
From: Steven Rostedt @ 2012-10-30 19:23 UTC (permalink / raw)
To: Sasha Levin
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, lw-BthXqXjhjHXQFUHtdCDX3A,
mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1351622772-16400-16-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Tue, 2012-10-30 at 14:46 -0400, Sasha Levin wrote:
> Switch tracing to use the new hashtable implementation. This reduces the
> amount of generic unrelated code in the tracing module.
>
> Signed-off-by: Sasha Levin <levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Steven Rostedt <rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org>
-- Steve
^ permalink raw reply
* Re: [PATCH v8 06/16] tracepoint: use new hashtable implementation
From: Mathieu Desnoyers @ 2012-10-30 19:20 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev, josh,
eric.dumazet, 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: <1351622772-16400-6-git-send-email-levinsasha928@gmail.com>
* Sasha Levin (levinsasha928@gmail.com) wrote:
> Switch tracepoints to use the new hashtable implementation. This reduces the
> amount of generic unrelated code in the tracepoints.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> ---
> kernel/tracepoint.c | 25 +++++++++----------------
> 1 file changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index d96ba22..5b599f1 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -26,6 +26,7 @@
> #include <linux/slab.h>
> #include <linux/sched.h>
> #include <linux/static_key.h>
> +#include <linux/hashtable.h>
>
> extern struct tracepoint * const __start___tracepoints_ptrs[];
> extern struct tracepoint * const __stop___tracepoints_ptrs[];
> @@ -49,8 +50,7 @@ static LIST_HEAD(tracepoint_module_list);
> * Protected by tracepoints_mutex.
> */
> #define TRACEPOINT_HASH_BITS 6
> -#define TRACEPOINT_TABLE_SIZE (1 << TRACEPOINT_HASH_BITS)
> -static struct hlist_head tracepoint_table[TRACEPOINT_TABLE_SIZE];
> +static DEFINE_HASHTABLE(tracepoint_table, TRACEPOINT_HASH_BITS);
>
> /*
> * Note about RCU :
> @@ -191,16 +191,15 @@ tracepoint_entry_remove_probe(struct tracepoint_entry *entry,
> */
> static struct tracepoint_entry *get_tracepoint(const char *name)
> {
> - struct hlist_head *head;
> struct hlist_node *node;
> struct tracepoint_entry *e;
> u32 hash = jhash(name, strlen(name), 0);
>
> - head = &tracepoint_table[hash & (TRACEPOINT_TABLE_SIZE - 1)];
> - hlist_for_each_entry(e, node, head, hlist) {
> + hash_for_each_possible(tracepoint_table, e, node, hlist, hash) {
> if (!strcmp(name, e->name))
> return e;
> }
> +
> return NULL;
> }
>
> @@ -210,19 +209,13 @@ static struct tracepoint_entry *get_tracepoint(const char *name)
> */
> static struct tracepoint_entry *add_tracepoint(const char *name)
> {
> - struct hlist_head *head;
> - struct hlist_node *node;
> struct tracepoint_entry *e;
> size_t name_len = strlen(name) + 1;
> u32 hash = jhash(name, name_len-1, 0);
>
> - head = &tracepoint_table[hash & (TRACEPOINT_TABLE_SIZE - 1)];
> - hlist_for_each_entry(e, node, head, hlist) {
> - if (!strcmp(name, e->name)) {
> - printk(KERN_NOTICE
> - "tracepoint %s busy\n", name);
> - return ERR_PTR(-EEXIST); /* Already there */
> - }
> + if (get_tracepoint(name)) {
> + printk(KERN_NOTICE "tracepoint %s busy\n", name);
> + return ERR_PTR(-EEXIST); /* Already there */
> }
> /*
> * Using kmalloc here to allocate a variable length element. Could
> @@ -234,7 +227,7 @@ static struct tracepoint_entry *add_tracepoint(const char *name)
> memcpy(&e->name[0], name, name_len);
> e->funcs = NULL;
> e->refcount = 0;
> - hlist_add_head(&e->hlist, head);
> + hash_add(tracepoint_table, &e->hlist, hash);
> return e;
> }
>
> @@ -244,7 +237,7 @@ static struct tracepoint_entry *add_tracepoint(const char *name)
> */
> static inline void remove_tracepoint(struct tracepoint_entry *e)
> {
> - hlist_del(&e->hlist);
> + hash_del(&e->hlist);
> kfree(e);
> }
>
> --
> 1.7.12.4
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
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 v8 01/16] hashtable: introduce a small and naive hashtable
From: Mathieu Desnoyers @ 2012-10-30 19:19 UTC (permalink / raw)
To: Sasha Levin
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A, teigland-H+wXaHxf7aLQT0dZR+AlfA,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1351622772-16400-1-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
* Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> This hashtable implementation is using hlist buckets to provide a simple
> hashtable to prevent it from getting reimplemented all over the kernel.
>
> Signed-off-by: Sasha Levin <levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
> ---
>
> Changes from v8:
>
> - Addressed comments from Tejun Heo and Mathieu Desnoyers.
>
>
> include/linux/hashtable.h | 196 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 196 insertions(+)
> create mode 100644 include/linux/hashtable.h
>
> diff --git a/include/linux/hashtable.h b/include/linux/hashtable.h
> new file mode 100644
> index 0000000..3c1a9cb
> --- /dev/null
> +++ b/include/linux/hashtable.h
> @@ -0,0 +1,196 @@
> +/*
> + * Statically sized hash table implementation
> + * (C) 2012 Sasha Levin <levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + */
> +
> +#ifndef _LINUX_HASHTABLE_H
> +#define _LINUX_HASHTABLE_H
> +
> +#include <linux/list.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/hash.h>
> +#include <linux/rculist.h>
> +
> +#define DEFINE_HASHTABLE(name, bits) \
> + struct hlist_head name[1 << (bits)] = \
> + { [0 ... ((1 << (bits)) - 1)] = HLIST_HEAD_INIT }
> +
> +#define DECLARE_HASHTABLE(name, bits) \
> + struct hlist_head name[1 << (bits)]
> +
> +#define HASH_SIZE(name) (ARRAY_SIZE(name))
> +#define HASH_BITS(name) ilog2(HASH_SIZE(name))
> +
> +/* Use hash_32 when possible to allow for fast 32bit hashing in 64bit kernels. */
> +#define hash_min(val, bits) \
> +({ \
> + sizeof(val) <= 4 ? \
> + hash_32(val, bits) : \
> + hash_long(val, bits); \
> +})
> +
> +static inline void __hash_init(struct hlist_head *ht, unsigned int sz)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < sz; i++)
> + INIT_HLIST_HEAD(&ht[i]);
> +}
> +
> +/**
> + * hash_init - initialize a hash table
> + * @hashtable: hashtable to be initialized
> + *
> + * Calculates the size of the hashtable from the given parameter, otherwise
> + * same as hash_init_size.
> + *
> + * This has to be a macro since HASH_BITS() will not work on pointers since
> + * it calculates the size during preprocessing.
> + */
> +#define hash_init(hashtable) __hash_init(hashtable, HASH_SIZE(hashtable))
> +
> +/**
> + * hash_add - add an object to a hashtable
> + * @hashtable: hashtable to add to
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add(hashtable, node, key) \
> + hlist_add_head(node, &hashtable[hash_min(key, HASH_BITS(hashtable))])
> +
> +/**
> + * hash_add_rcu - add an object to a rcu enabled hashtable
> + * @hashtable: hashtable to add to
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add_rcu(hashtable, node, key) \
> + hlist_add_head_rcu(node, &hashtable[hash_min(key, HASH_BITS(hashtable))])
> +
> +/**
> + * hash_hashed - check whether an object is in any hashtable
> + * @node: the &struct hlist_node of the object to be checked
> + */
> +static inline bool hash_hashed(struct hlist_node *node)
> +{
> + return !hlist_unhashed(node);
> +}
> +
> +static inline bool __hash_empty(struct hlist_head *ht, unsigned int sz)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < sz; i++)
> + if (!hlist_empty(&ht[i]))
> + return false;
> +
> + return true;
> +}
> +
> +/**
> + * hash_empty - check whether a hashtable is empty
> + * @hashtable: hashtable to check
> + *
> + * This has to be a macro since HASH_BITS() will not work on pointers since
> + * it calculates the size during preprocessing.
> + */
> +#define hash_empty(hashtable) __hash_empty(hashtable, HASH_SIZE(hashtable))
> +
> +/**
> + * hash_del - remove an object from a hashtable
> + * @node: &struct hlist_node of the object to remove
> + */
> +static inline void hash_del(struct hlist_node *node)
> +{
> + hlist_del_init(node);
> +}
> +
> +/**
> + * hash_del_rcu - remove an object from a rcu enabled hashtable
> + * @node: &struct hlist_node of the object to remove
> + */
> +static inline void hash_del_rcu(struct hlist_node *node)
> +{
> + hlist_del_init_rcu(node);
> +}
> +
> +/**
> + * hash_for_each - iterate over a hashtable
> + * @name: hashtable to iterate
> + * @bkt: integer to use as bucket loop cursor
> + * @node: the &struct list_head to use as a loop cursor for each entry
> + * @obj: the type * to use as a loop cursor for each entry
> + * @member: the name of the hlist_node within the struct
> + */
> +#define hash_for_each(name, bkt, node, obj, member) \
> + for ((bkt) = 0, node = NULL; node == NULL && (bkt) < HASH_SIZE(name); (bkt)++)\
> + hlist_for_each_entry(obj, node, &name[bkt], member)
> +
> +/**
> + * hash_for_each_rcu - iterate over a rcu enabled hashtable
> + * @name: hashtable to iterate
> + * @bkt: integer to use as bucket loop cursor
> + * @node: the &struct list_head to use as a loop cursor for each entry
> + * @obj: the type * to use as a loop cursor for each entry
> + * @member: the name of the hlist_node within the struct
> + */
> +#define hash_for_each_rcu(name, bkt, node, obj, member) \
> + for ((bkt) = 0, node = NULL; node == NULL && (bkt) < HASH_SIZE(name); (bkt)++)\
> + hlist_for_each_entry_rcu(obj, node, &name[bkt], member)
> +
> +/**
> + * hash_for_each_safe - iterate over a hashtable safe against removal of
> + * hash entry
> + * @name: hashtable to iterate
> + * @bkt: integer to use as bucket loop cursor
> + * @node: the &struct list_head to use as a loop cursor for each entry
> + * @tmp: a &struct used for temporary storage
> + * @obj: the type * to use as a loop cursor for each entry
> + * @member: the name of the hlist_node within the struct
> + */
> +#define hash_for_each_safe(name, bkt, node, tmp, obj, member) \
> + for ((bkt) = 0, node = NULL; node == NULL && (bkt) < HASH_SIZE(name); (bkt)++)\
> + hlist_for_each_entry_safe(obj, node, tmp, &name[bkt], member)
> +
> +/**
> + * hash_for_each_possible - iterate over all possible objects hashing to the
> + * same bucket
> + * @name: hashtable to iterate
> + * @obj: the type * to use as a loop cursor for each entry
> + * @node: the &struct list_head to use as a loop cursor for each entry
> + * @member: the name of the hlist_node within the struct
> + * @key: the key of the objects to iterate over
> + */
> +#define hash_for_each_possible(name, obj, node, member, key) \
> + hlist_for_each_entry(obj, node, &name[hash_min(key, HASH_BITS(name))], member)
> +
> +/**
> + * hash_for_each_possible_rcu - iterate over all possible objects hashing to the
> + * same bucket in an rcu enabled hashtable
> + * in a rcu enabled hashtable
> + * @name: hashtable to iterate
> + * @obj: the type * to use as a loop cursor for each entry
> + * @node: the &struct list_head to use as a loop cursor for each entry
> + * @member: the name of the hlist_node within the struct
> + * @key: the key of the objects to iterate over
> + */
> +#define hash_for_each_possible_rcu(name, obj, node, member, key) \
> + hlist_for_each_entry_rcu(obj, node, &name[hash_min(key, HASH_BITS(name))], member)
> +
> +/**
> + * hash_for_each_possible_safe - iterate over all possible objects hashing to the
> + * same bucket safe against removals
> + * @name: hashtable to iterate
> + * @obj: the type * to use as a loop cursor for each entry
> + * @node: the &struct list_head to use as a loop cursor for each entry
> + * @tmp: a &struct used for temporary storage
> + * @member: the name of the hlist_node within the struct
> + * @key: the key of the objects to iterate over
> + */
> +#define hash_for_each_possible_safe(name, obj, node, tmp, member, key) \
> + hlist_for_each_entry_safe(obj, node, tmp, \
> + &name[hash_min(key, HASH_BITS(name))], member)
> +
> +
> +#endif
> --
> 1.7.12.4
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v2 2/3] pppoatm: fix race condition with destroying of vcc
From: Krzysztof Mazur @ 2012-10-30 19:07 UTC (permalink / raw)
To: David Woodhouse; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1351589868.17077.34.camel@shinybook.infradead.org>
On Tue, Oct 30, 2012 at 09:37:48AM +0000, David Woodhouse wrote:
>
> Should we be locking it earlier, so that the atm_may_send() call is also
> covered by the lock?
Yes, but only to protect against concurent vcc_sendmsg().
>
> Either way, it's an obvious improvement on what we had before ??? and even
> if the answer to my question above is 'yes', exceeding the configured
> size by one packet is both harmless and almost never going to happen
> since we now limit ourselves to two packets anyway. So:
>
> Acked-By: David Woodhouse <David.Woodhouse@intel.com>
>
I'm sending proposed patch (not tested).
Should I squash it into original patch or send it later because it's
not really important?
Thanks.
Krzysiek
-- >8 --
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index a766d96..3081834 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -214,15 +214,7 @@ error:
static inline int pppoatm_may_send(struct pppoatm_vcc *pvcc, int size)
{
- /*
- * It's not clear that we need to bother with using atm_may_send()
- * to check we don't exceed sk->sk_sndbuf. If userspace sets a
- * value of sk_sndbuf which is lower than the MTU, we're going to
- * block for ever. But the code always did that before we introduced
- * the packet count limit, so...
- */
- if (atm_may_send(pvcc->atmvcc, size) &&
- atomic_inc_not_zero_hint(&pvcc->inflight, NONE_INFLIGHT))
+ if (atomic_inc_not_zero_hint(&pvcc->inflight, NONE_INFLIGHT))
return 1;
/*
@@ -251,8 +243,7 @@ static inline int pppoatm_may_send(struct pppoatm_vcc *pvcc, int size)
* code path that calls pppoatm_send(), and is thus going to
* wait for us to finish.
*/
- if (atm_may_send(pvcc->atmvcc, size) &&
- atomic_inc_not_zero(&pvcc->inflight))
+ if (atomic_inc_not_zero(&pvcc->inflight))
return 1;
return 0;
@@ -314,6 +305,16 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
|| !test_bit(ATM_VF_READY, &vcc->flags))
goto nospace_unlock_sock;
+ /*
+ * It's not clear that we need to bother with using atm_may_send()
+ * to check we don't exceed sk->sk_sndbuf. If userspace sets a
+ * value of sk_sndbuf which is lower than the MTU, we're going to
+ * block for ever. But the code always did that before we introduced
+ * the packet count limit, so...
+ */
+ if (!atm_may_send(vcc, skb->truesize))
+ goto nospace_unlock_sock;
+
atomic_add(skb->truesize, &sk_atm(ATM_SKB(skb)->vcc)->sk_wmem_alloc);
ATM_SKB(skb)->atm_options = ATM_SKB(skb)->vcc->atm_options;
pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n",
^ permalink raw reply related
* Re: [rfc net-next v6 0/3] Multiqueue virtio-net
From: Rick Jones @ 2012-10-30 19:05 UTC (permalink / raw)
To: Jason Wang
Cc: krkumar2, kvm, mst, netdev, linux-kernel, virtualization, davem
In-Reply-To: <1351591403-23065-1-git-send-email-jasowang@redhat.com>
On 10/30/2012 03:03 AM, Jason Wang wrote:
> Hi all:
>
> This series is an update version of multiqueue virtio-net driver based on
> Krishna Kumar's work to let virtio-net use multiple rx/tx queues to do the
> packets reception and transmission. Please review and comments.
>
> Changes from v5:
> - Align the implementation with the RFC spec update v4
> - Switch the mode between single mode and multiqueue mode without reset
> - Remove the 256 limitation of queues
> - Use helpers to do the mapping between virtqueues and tx/rx queues
> - Use commbined channels instead of separated rx/tx queus when do the queue
> number configuartion
> - Other coding style comments from Michael
>
> Reference:
> - A protype implementation of qemu-kvm support could by found in
> git://github.com/jasowang/qemu-kvm-mq.git
> - V5 could be found at http://lwn.net/Articles/505388/
> - V4 could be found at https://lkml.org/lkml/2012/6/25/120
> - V2 could be found at http://lwn.net/Articles/467283/
> - Michael virtio-spec: http://www.spinics.net/lists/netdev/msg209986.html
>
> Perf Numbers:
>
> - Pktgen test shows the receiving capability of the multiqueue virtio-net were
> dramatically improved.
> - Netperf result shows latency were greately improved according to the test
> result.
I suppose it is technically correct to say that latency was improved,
but usually for aggregate request/response tests I tend to talk about
the aggregate transactions per second.
Do you have a hypothesis as to why the improvement dropped going to 20
concurrent sessions from 10?
rick jones
> Netperf Local VM to VM test:
> - VM1 and its vcpu/vhost thread in numa node 0
> - VM2 and its vcpu/vhost thread in numa node 1
> - a script is used to lauch the netperf with demo mode and do the postprocessing
> to measure the aggreagte result with the help of timestamp
> - average of 3 runs
>
> TCP_RR:
> size/session/+lat%/+normalize%
> 1/ 1/ 0%/ 0%
> 1/ 10/ +52%/ +6%
> 1/ 20/ +27%/ +5%
> 64/ 1/ 0%/ 0%
> 64/ 10/ +45%/ +4%
> 64/ 20/ +28%/ +7%
> 256/ 1/ -1%/ 0%
> 256/ 10/ +38%/ +2%
> 256/ 20/ +27%/ +6%
> TCP_CRR:
> size/session/+lat%/+normalize%
> 1/ 1/ -7%/ -12%
> 1/ 10/ +34%/ +3%
> 1/ 20/ +3%/ -8%
> 64/ 1/ -7%/ -3%
> 64/ 10/ +32%/ +1%
> 64/ 20/ +4%/ -7%
> 256/ 1/ -6%/ -18%
> 256/ 10/ +33%/ 0%
> 256/ 20/ +4%/ -8%
> STREAM:
> size/session/+thu%/+normalize%
> 1/ 1/ -3%/ 0%
> 1/ 2/ -1%/ 0%
> 1/ 4/ -2%/ 0%
> 64/ 1/ 0%/ +1%
> 64/ 2/ -6%/ -6%
> 64/ 4/ -8%/ -14%
> 256/ 1/ 0%/ 0%
> 256/ 2/ -48%/ -52%
> 256/ 4/ -50%/ -55%
> 512/ 1/ +4%/ +5%
> 512/ 2/ -29%/ -33%
> 512/ 4/ -37%/ -49%
> 1024/ 1/ +6%/ +7%
> 1024/ 2/ -46%/ -51%
> 1024/ 4/ -15%/ -17%
> 4096/ 1/ +1%/ +1%
> 4096/ 2/ +16%/ -2%
> 4096/ 4/ +31%/ -10%
> 16384/ 1/ 0%/ 0%
> 16384/ 2/ +16%/ +9%
> 16384/ 4/ +17%/ -9%
>
> Netperf test between external host and guest over 10gb(ixgbe):
> - VM thread and vhost threads were pinned int the node 0
> - a script is used to lauch the netperf with demo mode and do the postprocessing
> to measure the aggreagte result with the help of timestamp
> - average of 3 runs
>
> TCP_RR:
> size/session/+lat%/+normalize%
> 1/ 1/ 0%/ +6%
> 1/ 10/ +41%/ +2%
> 1/ 20/ +10%/ -3%
> 64/ 1/ 0%/ -10%
> 64/ 10/ +39%/ +1%
> 64/ 20/ +22%/ +2%
> 256/ 1/ 0%/ +2%
> 256/ 10/ +26%/ -17%
> 256/ 20/ +24%/ +10%
> TCP_CRR:
> size/session/+lat%/+normalize%
> 1/ 1/ -3%/ -3%
> 1/ 10/ +34%/ -3%
> 1/ 20/ 0%/ -15%
> 64/ 1/ -3%/ -3%
> 64/ 10/ +34%/ -3%
> 64/ 20/ -1%/ -16%
> 256/ 1/ -1%/ -3%
> 256/ 10/ +38%/ -2%
> 256/ 20/ -2%/ -17%
> TCP_STREAM:(guest receiving)
> size/session/+thu%/+normalize%
> 1/ 1/ +1%/ +14%
> 1/ 2/ 0%/ +4%
> 1/ 4/ -2%/ -24%
> 64/ 1/ -6%/ +1%
> 64/ 2/ +1%/ +1%
> 64/ 4/ -1%/ -11%
> 256/ 1/ +3%/ +4%
> 256/ 2/ 0%/ -1%
> 256/ 4/ 0%/ -15%
> 512/ 1/ +4%/ 0%
> 512/ 2/ -10%/ -12%
> 512/ 4/ 0%/ -11%
> 1024/ 1/ -5%/ 0%
> 1024/ 2/ -11%/ -16%
> 1024/ 4/ +3%/ -11%
> 4096/ 1/ +27%/ +6%
> 4096/ 2/ 0%/ -12%
> 4096/ 4/ 0%/ -20%
> 16384/ 1/ 0%/ -2%
> 16384/ 2/ 0%/ -9%
> 16384/ 4/ +10%/ -2%
> TCP_MAERTS:(guest sending)
> 1/ 1/ -1%/ 0%
> 1/ 2/ 0%/ 0%
> 1/ 4/ -5%/ 0%
> 64/ 1/ 0%/ 0%
> 64/ 2/ -7%/ -8%
> 64/ 4/ -7%/ -8%
> 256/ 1/ 0%/ 0%
> 256/ 2/ -28%/ -28%
> 256/ 4/ -28%/ -29%
> 512/ 1/ 0%/ 0%
> 512/ 2/ -15%/ -13%
> 512/ 4/ -53%/ -59%
> 1024/ 1/ +4%/ +13%
> 1024/ 2/ -7%/ -18%
> 1024/ 4/ +1%/ -18%
> 4096/ 1/ +2%/ 0%
> 4096/ 2/ +3%/ -19%
> 4096/ 4/ -1%/ -19%
> 16384/ 1/ -3%/ -1%
> 16384/ 2/ 0%/ -12%
> 16384/ 4/ 0%/ -10%
>
> Jason Wang (2):
> virtio_net: multiqueue support
> virtio-net: change the number of queues through ethtool
>
> Krishna Kumar (1):
> virtio_net: Introduce VIRTIO_NET_F_MULTIQUEUE
>
> drivers/net/virtio_net.c | 790 ++++++++++++++++++++++++++++-----------
> include/uapi/linux/virtio_net.h | 19 +
> 2 files changed, 594 insertions(+), 215 deletions(-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net-next 1/8] bridge: bridge port parameters over netlink
From: Tommy S. Christensen @ 2012-10-30 18:48 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: davem, netdev
In-Reply-To: <20121030005835.289407930@vyatta.com>
On 10/30/2012 01:57 AM, Stephen Hemminger wrote:
>
> -/*
> - * Change state of port (ie from forwarding to blocking etc)
> +static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
> + [IFLA_BRPORT_STATE] = { .type = NLA_U8 },
> + [IFLA_BRPORT_COST] = { .type = NLA_U16 },
> + [IFLA_BRPORT_PRIORITY] = { .type = NLA_U32 },
> + [IFLA_BRPORT_MODE] = { .type = NLA_U8 },
> +};
> +
Hey Stephen
It seems you've swapped the sizes of COST and PRIORITY in this part.
-Tommy
^ permalink raw reply
* [PATCH v8 16/16] tracing output: use new hashtable implementation
From: Sasha Levin @ 2012-10-30 18:46 UTC (permalink / raw)
To: torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A,
mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w, Sasha Levin,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1351622772-16400-1-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Switch tracing to use the new hashtable implementation. This reduces the
amount of generic unrelated code in the tracing module.
Signed-off-by: Sasha Levin <levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
kernel/trace/trace_output.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 123b189..6af4879 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -8,15 +8,15 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/ftrace.h>
+#include <linux/hashtable.h>
#include "trace_output.h"
-/* must be a power of 2 */
-#define EVENT_HASHSIZE 128
+#define EVENT_HASH_BITS 7
DECLARE_RWSEM(trace_event_mutex);
-static struct hlist_head event_hash[EVENT_HASHSIZE] __read_mostly;
+static DEFINE_HASHTABLE(event_hash, EVENT_HASH_BITS);
static int next_event_type = __TRACE_LAST_TYPE + 1;
@@ -712,11 +712,8 @@ struct trace_event *ftrace_find_event(int type)
{
struct trace_event *event;
struct hlist_node *n;
- unsigned key;
- key = type & (EVENT_HASHSIZE - 1);
-
- hlist_for_each_entry(event, n, &event_hash[key], node) {
+ hash_for_each_possible(event_hash, event, n, node, type) {
if (event->type == type)
return event;
}
@@ -781,7 +778,6 @@ void trace_event_read_unlock(void)
*/
int register_ftrace_event(struct trace_event *event)
{
- unsigned key;
int ret = 0;
down_write(&trace_event_mutex);
@@ -833,9 +829,7 @@ int register_ftrace_event(struct trace_event *event)
if (event->funcs->binary == NULL)
event->funcs->binary = trace_nop_print;
- key = event->type & (EVENT_HASHSIZE - 1);
-
- hlist_add_head(&event->node, &event_hash[key]);
+ hash_add(event_hash, &event->node, event->type);
ret = event->type;
out:
@@ -850,7 +844,7 @@ EXPORT_SYMBOL_GPL(register_ftrace_event);
*/
int __unregister_ftrace_event(struct trace_event *event)
{
- hlist_del(&event->node);
+ hash_del(&event->node);
list_del(&event->list);
return 0;
}
--
1.7.12.4
^ permalink raw reply related
* [PATCH v8 15/16] openvswitch: use new hashtable implementation
From: Sasha Levin @ 2012-10-30 18:46 UTC (permalink / raw)
To: torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A,
mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w, Sasha Levin,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1351622772-16400-1-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Switch openvswitch to use the new hashtable implementation. This reduces the
amount of generic unrelated code in openvswitch.
Signed-off-by: Sasha Levin <levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
net/openvswitch/vport.c | 35 ++++++++++++-----------------------
1 file changed, 12 insertions(+), 23 deletions(-)
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index 03779e8..20fdbd4 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -28,6 +28,7 @@
#include <linux/rtnetlink.h>
#include <linux/compat.h>
#include <net/net_namespace.h>
+#include <linux/hashtable.h>
#include "datapath.h"
#include "vport.h"
@@ -41,8 +42,8 @@ static const struct vport_ops *vport_ops_list[] = {
};
/* Protected by RCU read lock for reading, RTNL lock for writing. */
-static struct hlist_head *dev_table;
-#define VPORT_HASH_BUCKETS 1024
+#define VPORT_HASH_BITS 10
+static DEFINE_HASHTABLE(dev_table, VPORT_HASH_BITS);
/**
* ovs_vport_init - initialize vport subsystem
@@ -51,11 +52,6 @@ static struct hlist_head *dev_table;
*/
int ovs_vport_init(void)
{
- dev_table = kzalloc(VPORT_HASH_BUCKETS * sizeof(struct hlist_head),
- GFP_KERNEL);
- if (!dev_table)
- return -ENOMEM;
-
return 0;
}
@@ -66,13 +62,6 @@ int ovs_vport_init(void)
*/
void ovs_vport_exit(void)
{
- kfree(dev_table);
-}
-
-static struct hlist_head *hash_bucket(struct net *net, const char *name)
-{
- unsigned int hash = jhash(name, strlen(name), (unsigned long) net);
- return &dev_table[hash & (VPORT_HASH_BUCKETS - 1)];
}
/**
@@ -84,13 +73,12 @@ static struct hlist_head *hash_bucket(struct net *net, const char *name)
*/
struct vport *ovs_vport_locate(struct net *net, const char *name)
{
- struct hlist_head *bucket = hash_bucket(net, name);
struct vport *vport;
struct hlist_node *node;
+ int key = full_name_hash(name, strlen(name));
- hlist_for_each_entry_rcu(vport, node, bucket, hash_node)
- if (!strcmp(name, vport->ops->get_name(vport)) &&
- net_eq(ovs_dp_get_net(vport->dp), net))
+ hash_for_each_possible_rcu(dev_table, vport, node, hash_node, key)
+ if (!strcmp(name, vport->ops->get_name(vport)))
return vport;
return NULL;
@@ -174,7 +162,8 @@ struct vport *ovs_vport_add(const struct vport_parms *parms)
for (i = 0; i < ARRAY_SIZE(vport_ops_list); i++) {
if (vport_ops_list[i]->type == parms->type) {
- struct hlist_head *bucket;
+ int key;
+ const char *name;
vport = vport_ops_list[i]->create(parms);
if (IS_ERR(vport)) {
@@ -182,9 +171,9 @@ struct vport *ovs_vport_add(const struct vport_parms *parms)
goto out;
}
- bucket = hash_bucket(ovs_dp_get_net(vport->dp),
- vport->ops->get_name(vport));
- hlist_add_head_rcu(&vport->hash_node, bucket);
+ name = vport->ops->get_name(vport);
+ key = full_name_hash(name, strlen(name));
+ hash_add_rcu(dev_table, &vport->hash_node, key);
return vport;
}
}
@@ -225,7 +214,7 @@ void ovs_vport_del(struct vport *vport)
{
ASSERT_RTNL();
- hlist_del_rcu(&vport->hash_node);
+ hash_del_rcu(&vport->hash_node);
vport->ops->destroy(vport);
}
--
1.7.12.4
^ permalink raw reply related
* [PATCH v8 14/16] net, rds: use new hashtable implementation
From: Sasha Levin @ 2012-10-30 18:46 UTC (permalink / raw)
To: torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A,
mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w, Sasha Levin,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1351622772-16400-1-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Switch rds to use the new hashtable implementation. This reduces the amount of
generic unrelated code in rds.
Signed-off-by: Sasha Levin <levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
net/rds/bind.c | 20 +++++------
net/rds/connection.c | 100 ++++++++++++++++++++++-----------------------------
2 files changed, 53 insertions(+), 67 deletions(-)
diff --git a/net/rds/bind.c b/net/rds/bind.c
index 637bde5..a99e524 100644
--- a/net/rds/bind.c
+++ b/net/rds/bind.c
@@ -36,16 +36,16 @@
#include <linux/if_arp.h>
#include <linux/jhash.h>
#include <linux/ratelimit.h>
+#include <linux/hashtable.h>
#include "rds.h"
-#define BIND_HASH_SIZE 1024
-static struct hlist_head bind_hash_table[BIND_HASH_SIZE];
+#define BIND_HASH_BITS 10
+static DEFINE_HASHTABLE(bind_hash_table, BIND_HASH_BITS);
static DEFINE_SPINLOCK(rds_bind_lock);
-static struct hlist_head *hash_to_bucket(__be32 addr, __be16 port)
+static u32 rds_hash(__be32 addr, __be16 port)
{
- return bind_hash_table + (jhash_2words((u32)addr, (u32)port, 0) &
- (BIND_HASH_SIZE - 1));
+ return jhash_2words((u32)addr, (u32)port, 0);
}
static struct rds_sock *rds_bind_lookup(__be32 addr, __be16 port,
@@ -53,12 +53,12 @@ static struct rds_sock *rds_bind_lookup(__be32 addr, __be16 port,
{
struct rds_sock *rs;
struct hlist_node *node;
- struct hlist_head *head = hash_to_bucket(addr, port);
+ u32 key = rds_hash(addr, port);
u64 cmp;
u64 needle = ((u64)be32_to_cpu(addr) << 32) | be16_to_cpu(port);
rcu_read_lock();
- hlist_for_each_entry_rcu(rs, node, head, rs_bound_node) {
+ hash_for_each_possible_rcu(bind_hash_table, rs, node, rs_bound_node, key) {
cmp = ((u64)be32_to_cpu(rs->rs_bound_addr) << 32) |
be16_to_cpu(rs->rs_bound_port);
@@ -74,13 +74,13 @@ static struct rds_sock *rds_bind_lookup(__be32 addr, __be16 port,
* make sure our addr and port are set before
* we are added to the list, other people
* in rcu will find us as soon as the
- * hlist_add_head_rcu is done
+ * hash_add_rcu is done
*/
insert->rs_bound_addr = addr;
insert->rs_bound_port = port;
rds_sock_addref(insert);
- hlist_add_head_rcu(&insert->rs_bound_node, head);
+ hash_add_rcu(bind_hash_table, &insert->rs_bound_node, key);
}
return NULL;
}
@@ -152,7 +152,7 @@ void rds_remove_bound(struct rds_sock *rs)
rs, &rs->rs_bound_addr,
ntohs(rs->rs_bound_port));
- hlist_del_init_rcu(&rs->rs_bound_node);
+ hash_del_rcu(&rs->rs_bound_node);
rds_sock_put(rs);
rs->rs_bound_addr = 0;
}
diff --git a/net/rds/connection.c b/net/rds/connection.c
index 9e07c75..a9afcb8 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -34,28 +34,24 @@
#include <linux/list.h>
#include <linux/slab.h>
#include <linux/export.h>
+#include <linux/hashtable.h>
#include <net/inet_hashtables.h>
#include "rds.h"
#include "loop.h"
#define RDS_CONNECTION_HASH_BITS 12
-#define RDS_CONNECTION_HASH_ENTRIES (1 << RDS_CONNECTION_HASH_BITS)
-#define RDS_CONNECTION_HASH_MASK (RDS_CONNECTION_HASH_ENTRIES - 1)
/* converting this to RCU is a chore for another day.. */
static DEFINE_SPINLOCK(rds_conn_lock);
static unsigned long rds_conn_count;
-static struct hlist_head rds_conn_hash[RDS_CONNECTION_HASH_ENTRIES];
+static DEFINE_HASHTABLE(rds_conn_hash, RDS_CONNECTION_HASH_BITS);
static struct kmem_cache *rds_conn_slab;
-static struct hlist_head *rds_conn_bucket(__be32 laddr, __be32 faddr)
+static unsigned long rds_conn_hashfn(__be32 laddr, __be32 faddr)
{
/* Pass NULL, don't need struct net for hash */
- unsigned long hash = inet_ehashfn(NULL,
- be32_to_cpu(laddr), 0,
- be32_to_cpu(faddr), 0);
- return &rds_conn_hash[hash & RDS_CONNECTION_HASH_MASK];
+ return inet_ehashfn(NULL, be32_to_cpu(laddr), 0, be32_to_cpu(faddr), 0);
}
#define rds_conn_info_set(var, test, suffix) do { \
@@ -64,14 +60,14 @@ static struct hlist_head *rds_conn_bucket(__be32 laddr, __be32 faddr)
} while (0)
/* rcu read lock must be held or the connection spinlock */
-static struct rds_connection *rds_conn_lookup(struct hlist_head *head,
- __be32 laddr, __be32 faddr,
+static struct rds_connection *rds_conn_lookup(__be32 laddr, __be32 faddr,
struct rds_transport *trans)
{
struct rds_connection *conn, *ret = NULL;
struct hlist_node *pos;
+ unsigned long key = rds_conn_hashfn(laddr, faddr);
- hlist_for_each_entry_rcu(conn, pos, head, c_hash_node) {
+ hash_for_each_possible_rcu(rds_conn_hash, conn, pos, c_hash_node, key) {
if (conn->c_faddr == faddr && conn->c_laddr == laddr &&
conn->c_trans == trans) {
ret = conn;
@@ -117,13 +113,12 @@ static struct rds_connection *__rds_conn_create(__be32 laddr, __be32 faddr,
int is_outgoing)
{
struct rds_connection *conn, *parent = NULL;
- struct hlist_head *head = rds_conn_bucket(laddr, faddr);
struct rds_transport *loop_trans;
unsigned long flags;
int ret;
rcu_read_lock();
- conn = rds_conn_lookup(head, laddr, faddr, trans);
+ conn = rds_conn_lookup(laddr, faddr, trans);
if (conn && conn->c_loopback && conn->c_trans != &rds_loop_transport &&
!is_outgoing) {
/* This is a looped back IB connection, and we're
@@ -224,13 +219,15 @@ static struct rds_connection *__rds_conn_create(__be32 laddr, __be32 faddr,
/* Creating normal conn */
struct rds_connection *found;
- found = rds_conn_lookup(head, laddr, faddr, trans);
+ found = rds_conn_lookup(laddr, faddr, trans);
if (found) {
trans->conn_free(conn->c_transport_data);
kmem_cache_free(rds_conn_slab, conn);
conn = found;
} else {
- hlist_add_head_rcu(&conn->c_hash_node, head);
+ unsigned long key = rds_conn_hashfn(laddr, faddr);
+
+ hash_add_rcu(rds_conn_hash, &conn->c_hash_node, key);
rds_cong_add_conn(conn);
rds_conn_count++;
}
@@ -303,7 +300,7 @@ void rds_conn_shutdown(struct rds_connection *conn)
* conn - the reconnect is always triggered by the active peer. */
cancel_delayed_work_sync(&conn->c_conn_w);
rcu_read_lock();
- if (!hlist_unhashed(&conn->c_hash_node)) {
+ if (hash_hashed(&conn->c_hash_node)) {
rcu_read_unlock();
rds_queue_reconnect(conn);
} else {
@@ -329,7 +326,7 @@ void rds_conn_destroy(struct rds_connection *conn)
/* Ensure conn will not be scheduled for reconnect */
spin_lock_irq(&rds_conn_lock);
- hlist_del_init_rcu(&conn->c_hash_node);
+ hash_del(&conn->c_hash_node);
spin_unlock_irq(&rds_conn_lock);
synchronize_rcu();
@@ -375,7 +372,6 @@ static void rds_conn_message_info(struct socket *sock, unsigned int len,
struct rds_info_lengths *lens,
int want_send)
{
- struct hlist_head *head;
struct hlist_node *pos;
struct list_head *list;
struct rds_connection *conn;
@@ -388,27 +384,24 @@ static void rds_conn_message_info(struct socket *sock, unsigned int len,
rcu_read_lock();
- for (i = 0, head = rds_conn_hash; i < ARRAY_SIZE(rds_conn_hash);
- i++, head++) {
- hlist_for_each_entry_rcu(conn, pos, head, c_hash_node) {
- if (want_send)
- list = &conn->c_send_queue;
- else
- list = &conn->c_retrans;
-
- spin_lock_irqsave(&conn->c_lock, flags);
-
- /* XXX too lazy to maintain counts.. */
- list_for_each_entry(rm, list, m_conn_item) {
- total++;
- if (total <= len)
- rds_inc_info_copy(&rm->m_inc, iter,
- conn->c_laddr,
- conn->c_faddr, 0);
- }
-
- spin_unlock_irqrestore(&conn->c_lock, flags);
+ hash_for_each_rcu(rds_conn_hash, i, pos, conn, c_hash_node) {
+ if (want_send)
+ list = &conn->c_send_queue;
+ else
+ list = &conn->c_retrans;
+
+ spin_lock_irqsave(&conn->c_lock, flags);
+
+ /* XXX too lazy to maintain counts.. */
+ list_for_each_entry(rm, list, m_conn_item) {
+ total++;
+ if (total <= len)
+ rds_inc_info_copy(&rm->m_inc, iter,
+ conn->c_laddr,
+ conn->c_faddr, 0);
}
+
+ spin_unlock_irqrestore(&conn->c_lock, flags);
}
rcu_read_unlock();
@@ -438,7 +431,6 @@ void rds_for_each_conn_info(struct socket *sock, unsigned int len,
size_t item_len)
{
uint64_t buffer[(item_len + 7) / 8];
- struct hlist_head *head;
struct hlist_node *pos;
struct rds_connection *conn;
size_t i;
@@ -448,23 +440,19 @@ void rds_for_each_conn_info(struct socket *sock, unsigned int len,
lens->nr = 0;
lens->each = item_len;
- for (i = 0, head = rds_conn_hash; i < ARRAY_SIZE(rds_conn_hash);
- i++, head++) {
- hlist_for_each_entry_rcu(conn, pos, head, c_hash_node) {
-
- /* XXX no c_lock usage.. */
- if (!visitor(conn, buffer))
- continue;
-
- /* We copy as much as we can fit in the buffer,
- * but we count all items so that the caller
- * can resize the buffer. */
- if (len >= item_len) {
- rds_info_copy(iter, buffer, item_len);
- len -= item_len;
- }
- lens->nr++;
+ hash_for_each_rcu(rds_conn_hash, i, pos, conn, c_hash_node) {
+ /* XXX no c_lock usage.. */
+ if (!visitor(conn, buffer))
+ continue;
+
+ /* We copy as much as we can fit in the buffer,
+ * but we count all items so that the caller
+ * can resize the buffer. */
+ if (len >= item_len) {
+ rds_info_copy(iter, buffer, item_len);
+ len -= item_len;
}
+ lens->nr++;
}
rcu_read_unlock();
}
@@ -525,8 +513,6 @@ void rds_conn_exit(void)
{
rds_loop_exit();
- WARN_ON(!hlist_empty(rds_conn_hash));
-
kmem_cache_destroy(rds_conn_slab);
rds_info_deregister_func(RDS_INFO_CONNECTIONS, rds_conn_info);
--
1.7.12.4
^ permalink raw reply related
* [PATCH v8 13/16] lockd: use new hashtable implementation
From: Sasha Levin @ 2012-10-30 18:46 UTC (permalink / raw)
To: torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A,
mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w, Sasha Levin,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1351622772-16400-1-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Switch lockd to use the new hashtable implementation. This reduces the amount
of generic unrelated code in lockd.
Signed-off-by: Sasha Levin <levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
fs/lockd/svcsubs.c | 58 ++++++++++++++++++++++++++----------------------------
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
index 0deb5f6..26c90c8 100644
--- a/fs/lockd/svcsubs.c
+++ b/fs/lockd/svcsubs.c
@@ -20,6 +20,7 @@
#include <linux/lockd/share.h>
#include <linux/module.h>
#include <linux/mount.h>
+#include <linux/hashtable.h>
#define NLMDBG_FACILITY NLMDBG_SVCSUBS
@@ -28,8 +29,7 @@
* Global file hash table
*/
#define FILE_HASH_BITS 7
-#define FILE_NRHASH (1<<FILE_HASH_BITS)
-static struct hlist_head nlm_files[FILE_NRHASH];
+static DEFINE_HASHTABLE(nlm_files, FILE_HASH_BITS);
static DEFINE_MUTEX(nlm_file_mutex);
#ifdef NFSD_DEBUG
@@ -68,7 +68,7 @@ static inline unsigned int file_hash(struct nfs_fh *f)
int i;
for (i=0; i<NFS2_FHSIZE;i++)
tmp += f->data[i];
- return tmp & (FILE_NRHASH - 1);
+ return tmp;
}
/*
@@ -86,17 +86,17 @@ nlm_lookup_file(struct svc_rqst *rqstp, struct nlm_file **result,
{
struct hlist_node *pos;
struct nlm_file *file;
- unsigned int hash;
+ unsigned int key;
__be32 nfserr;
nlm_debug_print_fh("nlm_lookup_file", f);
- hash = file_hash(f);
+ key = file_hash(f);
/* Lock file table */
mutex_lock(&nlm_file_mutex);
- hlist_for_each_entry(file, pos, &nlm_files[hash], f_list)
+ hash_for_each_possible(nlm_files, file, pos, f_list, file_hash(f))
if (!nfs_compare_fh(&file->f_handle, f))
goto found;
@@ -123,7 +123,7 @@ nlm_lookup_file(struct svc_rqst *rqstp, struct nlm_file **result,
goto out_free;
}
- hlist_add_head(&file->f_list, &nlm_files[hash]);
+ hash_add(nlm_files, &file->f_list, key);
found:
dprintk("lockd: found file %p (count %d)\n", file, file->f_count);
@@ -147,8 +147,8 @@ static inline void
nlm_delete_file(struct nlm_file *file)
{
nlm_debug_print_file("closing file", file);
- if (!hlist_unhashed(&file->f_list)) {
- hlist_del(&file->f_list);
+ if (hash_hashed(&file->f_list)) {
+ hash_del(&file->f_list);
nlmsvc_ops->fclose(file->f_file);
kfree(file);
} else {
@@ -253,27 +253,25 @@ nlm_traverse_files(void *data, nlm_host_match_fn_t match,
int i, ret = 0;
mutex_lock(&nlm_file_mutex);
- for (i = 0; i < FILE_NRHASH; i++) {
- hlist_for_each_entry_safe(file, pos, next, &nlm_files[i], f_list) {
- if (is_failover_file && !is_failover_file(data, file))
- continue;
- file->f_count++;
- mutex_unlock(&nlm_file_mutex);
-
- /* Traverse locks, blocks and shares of this file
- * and update file->f_locks count */
- if (nlm_inspect_file(data, file, match))
- ret = 1;
-
- mutex_lock(&nlm_file_mutex);
- file->f_count--;
- /* No more references to this file. Let go of it. */
- if (list_empty(&file->f_blocks) && !file->f_locks
- && !file->f_shares && !file->f_count) {
- hlist_del(&file->f_list);
- nlmsvc_ops->fclose(file->f_file);
- kfree(file);
- }
+ hash_for_each_safe(nlm_files, i, pos, next, file, f_list) {
+ if (is_failover_file && !is_failover_file(data, file))
+ continue;
+ file->f_count++;
+ mutex_unlock(&nlm_file_mutex);
+
+ /* Traverse locks, blocks and shares of this file
+ * and update file->f_locks count */
+ if (nlm_inspect_file(data, file, match))
+ ret = 1;
+
+ mutex_lock(&nlm_file_mutex);
+ file->f_count--;
+ /* No more references to this file. Let go of it. */
+ if (list_empty(&file->f_blocks) && !file->f_locks
+ && !file->f_shares && !file->f_count) {
+ hash_del(&file->f_list);
+ nlmsvc_ops->fclose(file->f_file);
+ kfree(file);
}
}
mutex_unlock(&nlm_file_mutex);
--
1.7.12.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox