* [PATCH 2/2] net/pch_gbe: Changed PTP supporting mode
From: Takahiro Shimizu @ 2012-03-23 2:18 UTC (permalink / raw)
To: jeffrey.t.kirsher, davem, lucas.demarchi, mirq-linux,
paul.gortmaker, jdmason, john.stultz, richardcochran, arnd, khc,
netdev, linux-kernel
Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Takahiro Shimizu
In-Reply-To: <1332469139-2654-1-git-send-email-tshimizu818@gmail.com>
Signed-off-by: Takahiro Shimizu <tshimizu818@gmail.com>
---
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h | 1 +
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 14 +++++++++++++-
2 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index dd14915..9f3dbc4 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -660,6 +660,7 @@ extern u32 pch_src_uuid_lo_read(struct pci_dev *pdev);
extern u32 pch_src_uuid_hi_read(struct pci_dev *pdev);
extern u64 pch_rx_snap_read(struct pci_dev *pdev);
extern u64 pch_tx_snap_read(struct pci_dev *pdev);
+extern int pch_set_station_address(u8 *addr, struct pci_dev *pdev);
#endif
/* pch_gbe_param.c */
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 56089ca..776bcf2 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -111,6 +111,9 @@ const char pch_driver_version[] = DRV_VERSION;
/* 0x44 Time Synchronization Channel Event Register Bits */
#define TX_SNAPSHOT_LOCKED (1<<0)
#define RX_SNAPSHOT_LOCKED (1<<1)
+
+#define PTP_L4_MULTICAST_SA "01:00:5e:00:01:81"
+#define PTP_L2_MULTICAST_SA "01:1b:19:00:00:00"
#endif
static unsigned int copybreak __read_mostly = PCH_GBE_COPYBREAK_DEFAULT;
@@ -235,6 +238,7 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
struct hwtstamp_config cfg;
struct pch_gbe_adapter *adapter = netdev_priv(netdev);
struct pci_dev *pdev;
+ u8 station[20];
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
@@ -268,9 +272,17 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
adapter->hwts_rx_en = 1;
pch_ch_control_write(pdev, MASTER_MODE | CAP_MODE0);
break;
- case HWTSTAMP_FILTER_PTP_V2_EVENT:
+ case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
+ adapter->hwts_rx_en = 1;
+ pch_ch_control_write(pdev, V2_MODE | CAP_MODE2);
+ strcpy(station, PTP_L4_MULTICAST_SA);
+ pch_set_station_address(station, pdev);
+ break;
+ case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
adapter->hwts_rx_en = 1;
pch_ch_control_write(pdev, V2_MODE | CAP_MODE2);
+ strcpy(station, PTP_L2_MULTICAST_SA);
+ pch_set_station_address(station, pdev);
break;
default:
return -ERANGE;
--
1.7.4.4
^ permalink raw reply related
* [PATCH 1/2] ptp/ptp_pch: Changed PTP supporting mode
From: Takahiro Shimizu @ 2012-03-23 2:18 UTC (permalink / raw)
To: jeffrey.t.kirsher, davem, lucas.demarchi, mirq-linux,
paul.gortmaker, jdmason, john.stultz, richardcochran, arnd, khc,
netdev, linux-kernel
Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Takahiro Shimizu
Signed-off-by: Takahiro Shimizu <tshimizu818@gmail.com>
---
drivers/ptp/Kconfig | 4 ++--
drivers/ptp/ptp_pch.c | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index cd9bc3b..6af221b 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -78,8 +78,8 @@ config PTP_1588_CLOCK_PCH
depends on PCH_GBE
help
This driver adds support for using the PCH EG20T as a PTP
- clock. This clock is only useful if your PTP programs are
- getting hardware time stamps on the PTP Ethernet packets
+ clock E2E mode. This clock is only useful if your PTP programs
+ are getting hardware time stamps on the PTP Ethernet packets
using the SO_TIMESTAMPING API.
To compile this driver as a module, choose M here: the module
diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c
index 212381e..08c3311 100644
--- a/drivers/ptp/ptp_pch.c
+++ b/drivers/ptp/ptp_pch.c
@@ -308,7 +308,7 @@ static void pch_reset(struct pch_dev *chip)
* traffic on the ethernet interface
* @addr: dress which contain the column separated address to be used.
*/
-static int pch_set_station_address(u8 *addr, struct pci_dev *pdev)
+int pch_set_station_address(u8 *addr, struct pci_dev *pdev)
{
s32 i;
struct pch_dev *chip = pci_get_drvdata(pdev);
@@ -352,6 +352,7 @@ static int pch_set_station_address(u8 *addr, struct pci_dev *pdev)
}
return 0;
}
+EXPORT_SYMBOL(pch_set_station_address);
/*
* Interrupt service routine
--
1.7.4.4
^ permalink raw reply related
* [PATCH] net/pch_gbe: Fixed the issue that the multicast address is lost when Rx FIFO overrun happened
From: Takahiro Shimizu @ 2012-03-23 2:16 UTC (permalink / raw)
To: jeffrey.t.kirsher, davem, lucas.demarchi, mirq-linux,
paul.gortmaker, jdmason, john.stultz, richardcochran, arnd, khc,
netdev, linux-kernel
Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Takahiro Shimizu
Signed-off-by: Takahiro Shimizu <tshimizu818@gmail.com>
---
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 60 +++++++++++++++++++-
1 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index d983e76..56089ca 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -382,31 +382,85 @@ static void pch_gbe_mac_mar_set(struct pch_gbe_hw *hw, u8 * addr, u32 index)
}
/**
+ * pch_gbe_mac_save_mac_addr_regs - Save MAC addresse registers
+ * @hw: Pointer to the HW structure
+ * @addr: Pointer to the MAC address
+ * @index: MAC address array register
+ */
+static void
+pch_gbe_mac_save_mac_addr_regs(struct pch_gbe_hw *hw,
+ struct pch_gbe_regs_mac_adr *mac_adr, u32 index)
+{
+ mac_adr->high = ioread32(&hw->reg->mac_adr[index].high);
+ mac_adr->low = ioread32(&hw->reg->mac_adr[index].low);
+}
+
+/**
+ * pch_gbe_mac_store_mac_addr_regs - Store MAC addresse registers
+ * @hw: Pointer to the HW structure
+ * @addr: Pointer to the MAC address
+ * @index: MAC address array register
+ */
+static void
+pch_gbe_mac_store_mac_addr_regs(struct pch_gbe_hw *hw,
+ struct pch_gbe_regs_mac_adr *mac_adr, u32 index)
+{
+ u32 adrmask;
+
+ adrmask = ioread32(&hw->reg->ADDR_MASK);
+ iowrite32((adrmask | (0x0001 << index)), &hw->reg->ADDR_MASK);
+ /* wait busy */
+ pch_gbe_wait_clr_bit(&hw->reg->ADDR_MASK, PCH_GBE_BUSY);
+ /* Set the MAC address to the MAC address xA/xB register */
+ iowrite32(mac_adr->high, &hw->reg->mac_adr[index].high);
+ iowrite32(mac_adr->low, &hw->reg->mac_adr[index].low);
+ iowrite32((adrmask & ~(0x0001 << index)), &hw->reg->ADDR_MASK);
+ /* wait busy */
+ pch_gbe_wait_clr_bit(&hw->reg->ADDR_MASK, PCH_GBE_BUSY);
+}
+
+#define MAC_ADDR_LIST_NUM 16
+/**
* pch_gbe_mac_reset_hw - Reset hardware
* @hw: Pointer to the HW structure
*/
static void pch_gbe_mac_reset_hw(struct pch_gbe_hw *hw)
{
+ struct pch_gbe_regs_mac_adr mac_addr_list[MAC_ADDR_LIST_NUM];
+ int i;
+
/* Read the MAC address. and store to the private data */
pch_gbe_mac_read_mac_addr(hw);
+ /* Read other MAC addresses */
+ for (i = 1; i < MAC_ADDR_LIST_NUM; i++)
+ pch_gbe_mac_save_mac_addr_regs(hw, &mac_addr_list[i], i);
iowrite32(PCH_GBE_ALL_RST, &hw->reg->RESET);
#ifdef PCH_GBE_MAC_IFOP_RGMII
iowrite32(PCH_GBE_MODE_GMII_ETHER, &hw->reg->MODE);
#endif
pch_gbe_wait_clr_bit(&hw->reg->RESET, PCH_GBE_ALL_RST);
- /* Setup the receive address */
+ /* Setup the receive addresses */
pch_gbe_mac_mar_set(hw, hw->mac.addr, 0);
+ for (i = 1; i < MAC_ADDR_LIST_NUM; i++)
+ pch_gbe_mac_store_mac_addr_regs(hw, &mac_addr_list[i], i);
return;
}
static void pch_gbe_mac_reset_rx(struct pch_gbe_hw *hw)
{
- /* Read the MAC address. and store to the private data */
+ struct pch_gbe_regs_mac_adr mac_addr_list[MAC_ADDR_LIST_NUM];
+ int i;
+
+ /* Read the MAC addresses. and store to the private data */
pch_gbe_mac_read_mac_addr(hw);
+ for (i = 1; i < MAC_ADDR_LIST_NUM; i++)
+ pch_gbe_mac_save_mac_addr_regs(hw, &mac_addr_list[i], i);
iowrite32(PCH_GBE_RX_RST, &hw->reg->RESET);
pch_gbe_wait_clr_bit_irq(&hw->reg->RESET, PCH_GBE_RX_RST);
- /* Setup the MAC address */
+ /* Setup the MAC addresses */
pch_gbe_mac_mar_set(hw, hw->mac.addr, 0);
+ for (i = 1; i < MAC_ADDR_LIST_NUM; i++)
+ pch_gbe_mac_store_mac_addr_regs(hw, &mac_addr_list[i], i);
return;
}
--
1.7.4.4
^ permalink raw reply related
* [PATCH net-next][v3] bonding: remove entries for master_ip and vlan_ip and query devices instead
From: Andy Gospodarek @ 2012-03-23 2:14 UTC (permalink / raw)
To: netdev; +Cc: Ralf Zeidler
The following patch aimed to resolve an issue where secondary, tertiary,
etc. addresses added to bond interfaces could overwrite the
bond->master_ip and vlan_ip values.
commit 917fbdb32f37e9a93b00bb12ee83532982982df3
Author: Henrik Saavedra Persson <henrik.e.persson@ericsson.com>
Date: Wed Nov 23 23:37:15 2011 +0000
bonding: only use primary address for ARP
That patch was good because it prevented bonds using ARP monitoring from
sending frames with an invalid source IP address. Unfortunately, it
didn't always work as expected.
When using an ioctl (like ifconfig does) to set the IP address and
netmask, 2 separate ioctls are actually called to set the IP and netmask
if the mask chosen doesn't match the standard mask for that class of
address. The first ioctl did not have a mask that matched the one in
the primary address and would still cause the device address to be
overwritten. The second ioctl that was called to set the mask would
then detect as secondary and ignored, but the damage was already done.
This was not an issue when using an application that used netlink
sockets as the setting of IP and netmask came down at once. The
inconsistent behavior between those two interfaces was something that
needed to be resolved.
While I was thinking about how I wanted to resolve this, Ralf Zeidler
came with a patch that resolved this on a RHEL kernel by keeping a full
shadow of the entries in dev->ifa_list for the bonding device and vlan
devices in the bonding driver. I didn't like the duplication of the
list as I want to see the 'bonding' struct and code shrink rather than
grow, but liked the general idea.
As the Subject indicates this patch drops the master_ip and vlan_ip
elements from the 'bonding' and 'vlan_entry' structs, respectively.
This can be done because a device's address-list is now traversed to
determine the optimal source IP address for ARP requests and for checks
to see if the bonding device has a particular IP address. This code
could have all be contained inside the bonding driver, but it made more
sense to me to EXPORT and call inet_confirm_addr since it did exactly
what was needed.
I tested this and a backported patch and everything works as expected.
Ralf also helped with verification of the backported patch.
Thanks to Ralf for all his help on this.
v2: Whitespace and organizational changes based on suggestions from Jay
Vosburgh and Dave Miller.
v3: Fixup incorrect usage of rcu_read_unlock based on Dave Miller's
suggestion.
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
CC: Ralf Zeidler <ralf.zeidler@nsn.com>
---
drivers/net/bonding/bond_main.c | 82 +++++++-------------------------------
drivers/net/bonding/bonding.h | 18 ++++++++-
net/ipv4/devinet.c | 1 +
3 files changed, 32 insertions(+), 69 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 435984a..d80a6ed 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2561,12 +2561,16 @@ re_arm:
static int bond_has_this_ip(struct bonding *bond, __be32 ip)
{
struct vlan_entry *vlan;
+ struct net_device *vlan_dev;
- if (ip == bond->master_ip)
+ if (ip == bond_confirm_addr(bond->dev, 0, ip))
return 1;
list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
- if (ip == vlan->vlan_ip)
+ rcu_read_lock();
+ vlan_dev = __vlan_find_dev_deep(bond->dev, vlan->vlan_id);
+ rcu_read_unlock();
+ if (vlan_dev && ip == bond_confirm_addr(vlan_dev, 0, ip))
return 1;
}
@@ -2608,17 +2612,19 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
int i, vlan_id;
__be32 *targets = bond->params.arp_targets;
struct vlan_entry *vlan;
- struct net_device *vlan_dev;
+ struct net_device *vlan_dev = NULL;
struct rtable *rt;
for (i = 0; (i < BOND_MAX_ARP_TARGETS); i++) {
+ __be32 addr;
if (!targets[i])
break;
pr_debug("basa: target %x\n", targets[i]);
if (!bond_vlan_used(bond)) {
pr_debug("basa: empty vlan: arp_send\n");
+ addr = bond_confirm_addr(bond->dev, targets[i], 0);
bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
- bond->master_ip, 0);
+ addr, 0);
continue;
}
@@ -2643,8 +2649,9 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
if (rt->dst.dev == bond->dev) {
ip_rt_put(rt);
pr_debug("basa: rtdev == bond->dev: arp_send\n");
+ addr = bond_confirm_addr(bond->dev, targets[i], 0);
bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
- bond->master_ip, 0);
+ addr, 0);
continue;
}
@@ -2662,10 +2669,11 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
}
}
- if (vlan_id) {
+ if (vlan_id && vlan_dev) {
ip_rt_put(rt);
+ addr = bond_confirm_addr(vlan_dev, targets[i], 0);
bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
- vlan->vlan_ip, vlan_id);
+ addr, vlan_id);
continue;
}
@@ -3287,68 +3295,10 @@ static int bond_netdev_event(struct notifier_block *this,
return NOTIFY_DONE;
}
-/*
- * bond_inetaddr_event: handle inetaddr notifier chain events.
- *
- * We keep track of device IPs primarily to use as source addresses in
- * ARP monitor probes (rather than spewing out broadcasts all the time).
- *
- * We track one IP for the main device (if it has one), plus one per VLAN.
- */
-static int bond_inetaddr_event(struct notifier_block *this, unsigned long event, void *ptr)
-{
- struct in_ifaddr *ifa = ptr;
- struct net_device *vlan_dev, *event_dev = ifa->ifa_dev->dev;
- struct bond_net *bn = net_generic(dev_net(event_dev), bond_net_id);
- struct bonding *bond;
- struct vlan_entry *vlan;
-
- /* we only care about primary address */
- if(ifa->ifa_flags & IFA_F_SECONDARY)
- return NOTIFY_DONE;
-
- list_for_each_entry(bond, &bn->dev_list, bond_list) {
- if (bond->dev == event_dev) {
- switch (event) {
- case NETDEV_UP:
- bond->master_ip = ifa->ifa_local;
- return NOTIFY_OK;
- case NETDEV_DOWN:
- bond->master_ip = 0;
- return NOTIFY_OK;
- default:
- return NOTIFY_DONE;
- }
- }
-
- list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
- vlan_dev = __vlan_find_dev_deep(bond->dev,
- vlan->vlan_id);
- if (vlan_dev == event_dev) {
- switch (event) {
- case NETDEV_UP:
- vlan->vlan_ip = ifa->ifa_local;
- return NOTIFY_OK;
- case NETDEV_DOWN:
- vlan->vlan_ip = 0;
- return NOTIFY_OK;
- default:
- return NOTIFY_DONE;
- }
- }
- }
- }
- return NOTIFY_DONE;
-}
-
static struct notifier_block bond_netdev_notifier = {
.notifier_call = bond_netdev_event,
};
-static struct notifier_block bond_inetaddr_notifier = {
- .notifier_call = bond_inetaddr_event,
-};
-
/*---------------------------- Hashing Policies -----------------------------*/
/*
@@ -4917,7 +4867,6 @@ static int __init bonding_init(void)
}
register_netdevice_notifier(&bond_netdev_notifier);
- register_inetaddr_notifier(&bond_inetaddr_notifier);
out:
return res;
err:
@@ -4931,7 +4880,6 @@ err_link:
static void __exit bonding_exit(void)
{
unregister_netdevice_notifier(&bond_netdev_notifier);
- unregister_inetaddr_notifier(&bond_inetaddr_notifier);
bond_destroy_debugfs();
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 1aecc37..9f2bae66 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -21,6 +21,7 @@
#include <linux/cpumask.h>
#include <linux/in6.h>
#include <linux/netpoll.h>
+#include <linux/inetdevice.h>
#include "bond_3ad.h"
#include "bond_alb.h"
@@ -166,7 +167,6 @@ struct bond_parm_tbl {
struct vlan_entry {
struct list_head vlan_list;
- __be32 vlan_ip;
unsigned short vlan_id;
};
@@ -232,7 +232,6 @@ struct bonding {
struct list_head bond_list;
struct netdev_hw_addr_list mc_list;
int (*xmit_hash_policy)(struct sk_buff *, int);
- __be32 master_ip;
u16 rr_tx_counter;
struct ad_bond_info ad_info;
struct alb_bond_info alb_info;
@@ -378,6 +377,21 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
return slave->inactive;
}
+static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be32 local)
+{
+ struct in_device *in_dev;
+ __be32 addr = 0;
+
+ rcu_read_lock();
+ in_dev = __in_dev_get_rcu(dev);
+
+ if (in_dev)
+ addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST);
+
+ rcu_read_unlock();
+ return addr;
+}
+
struct bond_net;
struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr);
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index e41c40f..d4fad5c 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1079,6 +1079,7 @@ __be32 inet_confirm_addr(struct in_device *in_dev,
return addr;
}
+EXPORT_SYMBOL(inet_confirm_addr);
/*
* Device notifier
--
1.7.6.5
^ permalink raw reply related
* [PATCH 1/2] ptp/ptp_pch: Modified according to the mainter's comments
From: Takahiro Shimizu @ 2012-03-23 2:12 UTC (permalink / raw)
To: jeffrey.t.kirsher, davem, lucas.demarchi, mirq-linux,
paul.gortmaker, jdmason, john.stultz, richardcochran, arnd, khc,
netdev, linux-kernel
Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Takahiro Shimizu
Signed-off-by: Takahiro Shimizu <tshimizu818@gmail.com>
---
drivers/ptp/ptp_pch.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c
index 375eb04..212381e 100644
--- a/drivers/ptp/ptp_pch.c
+++ b/drivers/ptp/ptp_pch.c
@@ -261,6 +261,7 @@ u64 pch_rx_snap_read(struct pci_dev *pdev)
ns = ((u64) hi) << 32;
ns |= lo;
+ ns <<= TICKS_NS_SHIFT;
return ns;
}
@@ -277,6 +278,7 @@ u64 pch_tx_snap_read(struct pci_dev *pdev)
ns = ((u64) hi) << 32;
ns |= lo;
+ ns <<= TICKS_NS_SHIFT;
return ns;
}
@@ -649,8 +651,6 @@ pch_probe(struct pci_dev *pdev, const struct pci_device_id *id)
iowrite32(1, &chip->regs->trgt_lo);
iowrite32(0, &chip->regs->trgt_hi);
iowrite32(PCH_TSE_TTIPEND, &chip->regs->event);
- /* Version: IEEE1588 v1 and IEEE1588-2008, Mode: All Evwnt, Locked */
- iowrite32(0x80020000, &chip->regs->ch_control);
pch_eth_enable_set(chip);
--
1.7.4.4
^ permalink raw reply related
* [PATCH 2/2] net/pch_gbe: Modified according to the mainter's comments
From: Takahiro Shimizu @ 2012-03-23 2:12 UTC (permalink / raw)
To: jeffrey.t.kirsher, davem, lucas.demarchi, mirq-linux,
paul.gortmaker, jdmason, john.stultz, richardcochran, arnd, khc,
netdev, linux-kernel
Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Takahiro Shimizu
In-Reply-To: <1332468760-2525-1-git-send-email-tshimizu818@gmail.com>
Signed-off-by: Takahiro Shimizu <tshimizu818@gmail.com>
---
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 35 ++++++++-----------
1 files changed, 15 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 8035e5f..d983e76 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -101,13 +101,11 @@ const char pch_driver_version[] = DRV_VERSION;
#ifdef CONFIG_PCH_PTP
/* Macros for ieee1588 */
-#define TICKS_NS_SHIFT 5
-
/* 0x40 Time Synchronization Channel Control Register Bits */
#define MASTER_MODE (1<<0)
-#define SLAVE_MODE (0<<0)
+#define SLAVE_MODE (0)
#define V2_MODE (1<<31)
-#define CAP_MODE0 (0<<16)
+#define CAP_MODE0 (0)
#define CAP_MODE2 (1<<17)
/* 0x44 Time Synchronization Channel Event Register Bits */
@@ -129,14 +127,13 @@ static struct sock_filter ptp_filter[] = {
static int pch_ptp_match(struct sk_buff *skb, u16 uid_hi, u32 uid_lo, u16 seqid)
{
u8 *data = skb->data;
- unsigned int offset;
+ unsigned int offset, sk_filter;
u16 *hi, *id;
u32 lo;
- if ((sk_run_filter(skb, ptp_filter) != PTP_CLASS_V2_IPV4) &&
- (sk_run_filter(skb, ptp_filter) != PTP_CLASS_V1_IPV4)) {
+ sk_filter = sk_run_filter(skb, ptp_filter);
+ if ((sk_filter != PTP_CLASS_V2) && (sk_filter != PTP_CLASS_V1))
return 0;
- }
offset = ETH_HLEN + IPV4_HLEN(data) + UDP_HLEN;
@@ -153,8 +150,8 @@ static int pch_ptp_match(struct sk_buff *skb, u16 uid_hi, u32 uid_lo, u16 seqid)
seqid == *id);
}
-static void pch_rx_timestamp(
- struct pch_gbe_adapter *adapter, struct sk_buff *skb)
+static void
+pch_rx_timestamp(struct pch_gbe_adapter *adapter, struct sk_buff *skb)
{
struct skb_shared_hwtstamps *shhwtstamps;
struct pci_dev *pdev;
@@ -183,7 +180,6 @@ static void pch_rx_timestamp(
goto out;
ns = pch_rx_snap_read(pdev);
- ns <<= TICKS_NS_SHIFT;
shhwtstamps = skb_hwtstamps(skb);
memset(shhwtstamps, 0, sizeof(*shhwtstamps));
@@ -192,8 +188,8 @@ out:
pch_ch_event_write(pdev, RX_SNAPSHOT_LOCKED);
}
-static void pch_tx_timestamp(
- struct pch_gbe_adapter *adapter, struct sk_buff *skb)
+static void
+pch_tx_timestamp(struct pch_gbe_adapter *adapter, struct sk_buff *skb)
{
struct skb_shared_hwtstamps shhwtstamps;
struct pci_dev *pdev;
@@ -202,11 +198,11 @@ static void pch_tx_timestamp(
u32 cnt, val;
shtx = skb_shinfo(skb);
- if (unlikely(shtx->tx_flags & SKBTX_HW_TSTAMP && adapter->hwts_tx_en))
- shtx->tx_flags |= SKBTX_IN_PROGRESS;
- else
+ if (likely(!(shtx->tx_flags & SKBTX_HW_TSTAMP && adapter->hwts_tx_en)))
return;
+ shtx->tx_flags |= SKBTX_IN_PROGRESS;
+
/* Get ieee1588's dev information */
pdev = adapter->ptp_pdev;
@@ -226,7 +222,6 @@ static void pch_tx_timestamp(
}
ns = pch_tx_snap_read(pdev);
- ns <<= TICKS_NS_SHIFT;
memset(&shhwtstamps, 0, sizeof(shhwtstamps));
shhwtstamps.hwtstamp = ns_to_ktime(ns);
@@ -267,15 +262,15 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
break;
case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
adapter->hwts_rx_en = 0;
- pch_ch_control_write(pdev, (SLAVE_MODE | CAP_MODE0));
+ pch_ch_control_write(pdev, SLAVE_MODE | CAP_MODE0);
break;
case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
adapter->hwts_rx_en = 1;
- pch_ch_control_write(pdev, (MASTER_MODE | CAP_MODE0));
+ pch_ch_control_write(pdev, MASTER_MODE | CAP_MODE0);
break;
case HWTSTAMP_FILTER_PTP_V2_EVENT:
adapter->hwts_rx_en = 1;
- pch_ch_control_write(pdev, (V2_MODE | CAP_MODE2));
+ pch_ch_control_write(pdev, V2_MODE | CAP_MODE2);
break;
default:
return -ERANGE;
--
1.7.4.4
^ permalink raw reply related
* Re: [RFC PATCH 1/1] NUMA aware scheduling per cpu vhost thread
From: Shirley Ma @ 2012-03-23 0:16 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, tahm, kvm
In-Reply-To: <1332460136.7730.19.camel@oc3660625478.ibm.com>
Resubmit it with the right format.
Signed-off-by: Shirley Ma <xma@us.ibm.com>
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
Tested-by: Tom Lendacky <toml@us.ibm.com>
---
drivers/vhost/net.c | 26 ++-
drivers/vhost/vhost.c | 300 ++++++++++++++++++++++++----------
drivers/vhost/vhost.h | 16 ++-
3 files changed, 243 insertions(+), 103 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 9dab1f5..4664e63 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -41,12 +41,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Experimental Zero Copy TX");
#define VHOST_MAX_PEND 128
#define VHOST_GOODCOPY_LEN 256
-enum {
- VHOST_NET_VQ_RX = 0,
- VHOST_NET_VQ_TX = 1,
- VHOST_NET_VQ_MAX = 2,
-};
-
enum vhost_net_poll_state {
VHOST_NET_POLL_DISABLED = 0,
VHOST_NET_POLL_STARTED = 1,
@@ -510,8 +504,10 @@ static int vhost_net_open(struct inode *inode, struct file *f)
return r;
}
- vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
- vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
+ vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT,
+ &n->vqs[VHOST_NET_VQ_TX]);
+ vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN,
+ &n->vqs[VHOST_NET_VQ_RX]);
n->tx_poll_state = VHOST_NET_POLL_DISABLED;
f->private_data = n;
@@ -863,15 +859,27 @@ static struct miscdevice vhost_net_misc = {
static int vhost_net_init(void)
{
+ int ret;
+
if (experimental_zcopytx)
vhost_enable_zcopy(VHOST_NET_VQ_TX);
- return misc_register(&vhost_net_misc);
+
+ ret = misc_register(&vhost_net_misc);
+ if (ret)
+ return ret;
+
+ ret = vhost_init();
+ if (ret)
+ misc_deregister(&vhost_net_misc);
+
+ return ret;
}
module_init(vhost_net_init);
static void vhost_net_exit(void)
{
misc_deregister(&vhost_net_misc);
+ vhost_cleanup();
}
module_exit(vhost_net_exit);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index c14c42b..9fabc5a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -24,7 +24,7 @@
#include <linux/highmem.h>
#include <linux/slab.h>
#include <linux/kthread.h>
-#include <linux/cgroup.h>
+#include <linux/cpu.h>
#include <linux/net.h>
#include <linux/if_packet.h>
@@ -42,6 +42,15 @@ static unsigned vhost_zcopy_mask __read_mostly;
#define vhost_used_event(vq) ((u16 __user *)&vq->avail->ring[vq->num])
#define vhost_avail_event(vq) ((u16 __user *)&vq->used->ring[vq->num])
+/* per cpu vhost struct */
+struct vhost {
+ struct task_struct *worker;
+ spinlock_t lock;
+ struct list_head work_list;
+};
+
+static DEFINE_PER_CPU(struct vhost, vhosts);
+
static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
poll_table *pt)
{
@@ -64,25 +73,28 @@ static int vhost_poll_wakeup(wait_queue_t *wait, unsigned mode, int sync,
return 0;
}
-static void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn)
+static void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn,
+ struct vhost_virtqueue *vq)
{
INIT_LIST_HEAD(&work->node);
work->fn = fn;
init_waitqueue_head(&work->done);
work->flushing = 0;
work->queue_seq = work->done_seq = 0;
+ work->vq = vq;
+ spin_lock_init(&work->lock);
}
/* Init poll structure */
void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
- unsigned long mask, struct vhost_dev *dev)
+ unsigned long mask, struct vhost_virtqueue *vq)
{
init_waitqueue_func_entry(&poll->wait, vhost_poll_wakeup);
init_poll_funcptr(&poll->table, vhost_poll_func);
poll->mask = mask;
- poll->dev = dev;
+ poll->dev = vq->dev;
- vhost_work_init(&poll->work, fn);
+ vhost_work_init(&poll->work, fn, vq);
}
/* Start polling a file. We add ourselves to file's wait queue. The caller must
@@ -108,25 +120,30 @@ static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
{
int left;
- spin_lock_irq(&dev->work_lock);
+ spin_lock_irq(&work->lock);
left = seq - work->done_seq;
- spin_unlock_irq(&dev->work_lock);
+ spin_unlock_irq(&work->lock);
return left <= 0;
}
-static void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
+/* only flushing this work? */
+static void vhost_work_flush(struct vhost_poll *poll)
{
unsigned seq;
int flushing;
+ struct vhost_dev *dev = poll->dev;
+ struct vhost_work *work = &poll->work;
- spin_lock_irq(&dev->work_lock);
+ if (list_empty(&work->node))
+ return;
+ spin_lock_irq(&work->lock);
seq = work->queue_seq;
work->flushing++;
- spin_unlock_irq(&dev->work_lock);
+ spin_unlock_irq(&work->lock);
wait_event(work->done, vhost_work_seq_done(dev, work, seq));
- spin_lock_irq(&dev->work_lock);
+ spin_lock_irq(&work->lock);
flushing = --work->flushing;
- spin_unlock_irq(&dev->work_lock);
+ spin_unlock_irq(&work->lock);
BUG_ON(flushing < 0);
}
@@ -134,21 +151,59 @@ static void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
* locks that are also used by the callback. */
void vhost_poll_flush(struct vhost_poll *poll)
{
- vhost_work_flush(poll->dev, &poll->work);
+ vhost_work_flush(poll);
+}
+
+/* schedule the cpu on the same socket but different cpu with the given one */
+static unsigned long sched_node_cpu(unsigned long cpu)
+{
+ int node, ncpus_node;
+ unsigned long sched_cpu = cpu;
+
+ node = cpu_to_node(cpu);
+ ncpus_node = nr_cpus_node(node);
+ if (ncpus_node != 1) {
+ /* pick up a random cpu on the same node, exclude
+ * the input one
+ */
+ sched_cpu = node * ncpus_node + random32() % (ncpus_node - 1);
+ if (sched_cpu >= cpu)
+ ++sched_cpu;
+ /* todo hotplug cpu race */
+ if (!cpu_online(sched_cpu))
+ sched_cpu = cpu;
+ }
+ return sched_cpu;
}
static inline void vhost_work_queue(struct vhost_dev *dev,
struct vhost_work *work)
{
- unsigned long flags;
-
- spin_lock_irqsave(&dev->work_lock, flags);
+ unsigned long cpu = work->vq->cpu;
+ struct vhost *vhost;
+
+ /* Is it safe to disable vq notify here ? */
+ vhost_disable_notify(dev, work->vq);
+
+ /* schedule the work on the cpu socket as the work has been delivered
+ * but different with the cpu the work is delivered on
+ */
+ preempt_disable();
+ if (cpu_to_node(cpu) != cpu_to_node(smp_processor_id())) {
+ cpu = sched_node_cpu(smp_processor_id());
+ work->vq->cpu = cpu;
+ }
+ preempt_enable();
+ vhost = &per_cpu(vhosts, cpu);
+ spin_lock_irq(&vhost->lock);
+ spin_lock(&work->lock);
if (list_empty(&work->node)) {
- list_add_tail(&work->node, &dev->work_list);
+ list_add_tail(&work->node, &vhost->work_list);
work->queue_seq++;
- wake_up_process(dev->worker);
+ wake_up_process(vhost->worker);
}
- spin_unlock_irqrestore(&dev->work_lock, flags);
+ spin_unlock(&work->lock);
+ spin_unlock_irq(&vhost->lock);
}
void vhost_poll_queue(struct vhost_poll *poll)
@@ -188,17 +243,18 @@ static void vhost_vq_reset(struct vhost_dev *dev,
static int vhost_worker(void *data)
{
- struct vhost_dev *dev = data;
- struct vhost_work *work = NULL;
+ struct vhost *vhost = &__get_cpu_var(vhosts);
+ struct list_head *work_list;
+ struct mm_struct *prev_mm = NULL;
unsigned uninitialized_var(seq);
+ struct vhost_work *work = NULL;
- use_mm(dev->mm);
-
+ work_list = &vhost->work_list;
for (;;) {
/* mb paired w/ kthread_stop */
set_current_state(TASK_INTERRUPTIBLE);
- spin_lock_irq(&dev->work_lock);
+ spin_lock_irq(&vhost->lock);
if (work) {
work->done_seq = seq;
if (work->flushing)
@@ -206,18 +262,26 @@ static int vhost_worker(void *data)
}
if (kthread_should_stop()) {
- spin_unlock_irq(&dev->work_lock);
+ spin_unlock_irq(&vhost->lock);
__set_current_state(TASK_RUNNING);
break;
}
- if (!list_empty(&dev->work_list)) {
- work = list_first_entry(&dev->work_list,
+ if (!list_empty(work_list)) {
+ work = list_first_entry(work_list,
struct vhost_work, node);
+ spin_lock(&work->lock);
list_del_init(&work->node);
+ spin_unlock(&work->lock);
seq = work->queue_seq;
+ if (prev_mm != work->vq->dev->mm) {
+ if (prev_mm)
+ unuse_mm(prev_mm);
+ prev_mm = work->vq->dev->mm;
+ use_mm(prev_mm);
+ }
} else
work = NULL;
- spin_unlock_irq(&dev->work_lock);
+ spin_unlock_irq(&vhost->lock);
if (work) {
__set_current_state(TASK_RUNNING);
@@ -226,7 +290,9 @@ static int vhost_worker(void *data)
schedule();
}
- unuse_mm(dev->mm);
+
+ if (prev_mm)
+ unuse_mm(prev_mm);
return 0;
}
@@ -298,9 +364,6 @@ long vhost_dev_init(struct vhost_dev *dev,
dev->log_file = NULL;
dev->memory = NULL;
dev->mm = NULL;
- spin_lock_init(&dev->work_lock);
- INIT_LIST_HEAD(&dev->work_list);
- dev->worker = NULL;
for (i = 0; i < dev->nvqs; ++i) {
dev->vqs[i].log = NULL;
@@ -312,7 +375,8 @@ long vhost_dev_init(struct vhost_dev *dev,
vhost_vq_reset(dev, dev->vqs + i);
if (dev->vqs[i].handle_kick)
vhost_poll_init(&dev->vqs[i].poll,
- dev->vqs[i].handle_kick, POLLIN, dev);
+ dev->vqs[i].handle_kick, POLLIN,
+ &dev->vqs[i]);
}
return 0;
@@ -325,71 +389,35 @@ long vhost_dev_check_owner(struct vhost_dev *dev)
return dev->mm == current->mm ? 0 : -EPERM;
}
-struct vhost_attach_cgroups_struct {
- struct vhost_work work;
- struct task_struct *owner;
- int ret;
-};
-
-static void vhost_attach_cgroups_work(struct vhost_work *work)
-{
- struct vhost_attach_cgroups_struct *s;
-
- s = container_of(work, struct vhost_attach_cgroups_struct, work);
- s->ret = cgroup_attach_task_all(s->owner, current);
-}
-
-static int vhost_attach_cgroups(struct vhost_dev *dev)
-{
- struct vhost_attach_cgroups_struct attach;
-
- attach.owner = current;
- vhost_work_init(&attach.work, vhost_attach_cgroups_work);
- vhost_work_queue(dev, &attach.work);
- vhost_work_flush(dev, &attach.work);
- return attach.ret;
-}
-
/* Caller should have device mutex */
static long vhost_dev_set_owner(struct vhost_dev *dev)
{
- struct task_struct *worker;
int err;
+ unsigned long txcpu, rxcpu;
/* Is there an owner already? */
if (dev->mm) {
err = -EBUSY;
- goto err_mm;
+ goto out;
}
- /* No owner, become one */
- dev->mm = get_task_mm(current);
- worker = kthread_create(vhost_worker, dev, "vhost-%d", current->pid);
- if (IS_ERR(worker)) {
- err = PTR_ERR(worker);
- goto err_worker;
- }
+ err = vhost_dev_alloc_iovecs(dev);
+ if (err)
+ goto out;
- dev->worker = worker;
- wake_up_process(worker); /* avoid contributing to loadavg */
+ /* initial txcpu, rxcpu on the same socket */
+ txcpu = sched_node_cpu(smp_processor_id());
+ rxcpu = sched_node_cpu(txcpu);
- err = vhost_attach_cgroups(dev);
- if (err)
- goto err_cgroup;
+ dev->vqs[VHOST_NET_VQ_TX].cpu = txcpu;
+ dev->vqs[VHOST_NET_VQ_RX].cpu = rxcpu;
- err = vhost_dev_alloc_iovecs(dev);
- if (err)
- goto err_cgroup;
+ /* No owner, become one */
+ dev->mm = get_task_mm(current);
return 0;
-err_cgroup:
- kthread_stop(worker);
- dev->worker = NULL;
-err_worker:
- if (dev->mm)
- mmput(dev->mm);
- dev->mm = NULL;
-err_mm:
+
+out:
return err;
}
@@ -474,11 +502,6 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
kfree(rcu_dereference_protected(dev->memory,
lockdep_is_held(&dev->mutex)));
RCU_INIT_POINTER(dev->memory, NULL);
- WARN_ON(!list_empty(&dev->work_list));
- if (dev->worker) {
- kthread_stop(dev->worker);
- dev->worker = NULL;
- }
if (dev->mm)
mmput(dev->mm);
dev->mm = NULL;
@@ -1605,3 +1628,104 @@ void vhost_zerocopy_callback(void *arg)
vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
}
+
+/* to do
+static int __cpuinit vhost_pool_callback(struct notifier_block *nfb,
+ unsigned long action,
+ void *hcpu)
+{
+ struct vhost *vhost = per_cpu(vhosts, hcpu);
+
+ action &= ~CPU_TASKS_FROZEN;
+
+ switch (action) {
+ case CPU_UP_PREPARE:
+ case CPU_UP_PREPARE_FROZEN:
+ if (!create_vhost_task(vhosts, hcpu))
+ return notifier_from_errno(-ENOMEM);
+ break;
+ case CPU_UP_CANCELED:
+ case CPU_UP_CANCELED_FROZEN:
+ kthread_bind(vhost->worker, cpumask_any(cpu_online_mask));
+ destory_vhost_task(vhost, hcpu);
+ break;
+ case CPU_ONLINE:
+ case CPU_ONLINE_FROZEN:
+ kthread_bind(vhost->worker, hcpu);
+ wake_up_process(vhost->worker);
+ break;
+ case CPU_DOWN_PREPARE:
+ case CPU_DOWN_PREPARE_FROZEN:
+ case CPU_DOWN_FAILED:
+ case CPU_DOWN_FAILED_FROZEN:
+ break;
+ case CPU_DEAD:
+ case CPU_DEAD_FROZEN:
+ destrory_vhost_task(vhosts, hcpu);
+ take_over_work(vhosts, hcpu);
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static struct notifier_block vhost_pool_callback_nb __cpuinitdata = {
+ .notifier_call = vhost_pool_callcack,
+ .priority = 0,
+}
+*/
+
+static void free_workers(void)
+{
+ unsigned long cpu;
+ struct vhost *vhost;
+
+ /* to do
+ * unregister_cpu_notifier(&vhost_pool_callback_nb);
+ */
+ get_online_cpus();
+ for_each_online_cpu(cpu) {
+ vhost = &per_cpu(vhosts, cpu);
+ if (!IS_ERR(vhost->worker)) {
+ kthread_stop(vhost->worker);
+ BUG_ON(!list_empty(&vhost->work_list));
+ }
+ }
+ put_online_cpus();
+}
+
+int vhost_init(void)
+{
+ int ret = -ENOMEM;
+ unsigned long cpu;
+ struct vhost *vhost;
+
+ get_online_cpus();
+ for_each_online_cpu(cpu) {
+ vhost = &per_cpu(vhosts, cpu);
+
+ INIT_LIST_HEAD(&vhost->work_list);
+ spin_lock_init(&vhost->lock);
+ vhost->worker = kthread_create_on_node(vhost_worker, NULL,
+ cpu_to_node(cpu),
+ "vhost-%lu", cpu);
+ if (IS_ERR(vhost->worker))
+ goto err;
+
+ kthread_bind(vhost->worker, cpu);
+ wake_up_process(vhost->worker);
+ }
+ put_online_cpus();
+
+ /* to do
+ * register_cpu_notifier(&vhost_pool_callback_nb);
+ */
+ return 0;
+err:
+ free_workers();
+ return ret;
+}
+
+void vhost_cleanup(void)
+{
+ free_workers();
+}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index a801e28..c6ecfb0 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -18,6 +18,12 @@
#define VHOST_DMA_DONE_LEN 1
#define VHOST_DMA_CLEAR_LEN 0
+enum {
+ VHOST_NET_VQ_RX = 0,
+ VHOST_NET_VQ_TX = 1,
+ VHOST_NET_VQ_MAX = 2,
+};
+
struct vhost_device;
struct vhost_work;
@@ -30,6 +36,8 @@ struct vhost_work {
int flushing;
unsigned queue_seq;
unsigned done_seq;
+ struct vhost_virtqueue *vq;
+ spinlock_t lock;
};
/* Poll a file (eventfd or socket) */
@@ -44,7 +52,7 @@ struct vhost_poll {
};
void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
- unsigned long mask, struct vhost_dev *dev);
+ unsigned long mask, struct vhost_virtqueue *vq);
void vhost_poll_start(struct vhost_poll *poll, struct file *file);
void vhost_poll_stop(struct vhost_poll *poll);
void vhost_poll_flush(struct vhost_poll *poll);
@@ -141,6 +149,7 @@ struct vhost_virtqueue {
/* Reference counting for outstanding ubufs.
* Protected by vq mutex. Writers must also take device mutex. */
struct vhost_ubuf_ref *ubufs;
+ unsigned long cpu;
};
struct vhost_dev {
@@ -155,9 +164,6 @@ struct vhost_dev {
int nvqs;
struct file *log_file;
struct eventfd_ctx *log_ctx;
- spinlock_t work_lock;
- struct list_head work_list;
- struct task_struct *worker;
};
long vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue *vqs, int nvqs);
@@ -190,6 +196,8 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
void vhost_zerocopy_callback(void *arg);
int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
+int vhost_init(void);
+void vhost_cleanup(void);
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
^ permalink raw reply related
* [RFC PATCH 1/1] NUMA aware scheduling per cpu vhost thread
From: Shirley Ma @ 2012-03-22 23:48 UTC (permalink / raw)
To: Michael S. Tsirkin, netdev, tahm, kvm
Signed-off-by: Shirley Ma <xma@us.ibm.com>
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
Tested-by: Tom Lendacky <toml@us.ibm.com>
---
drivers/vhost/net.c | 26 ++-
drivers/vhost/vhost.c | 300 ++++++++++++++++++++++++----------
drivers/vhost/vhost.h | 16 ++-
3 files changed, 243 insertions(+), 103 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 9dab1f5..4664e63 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -41,12 +41,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Experimental Zero Copy TX");
#define VHOST_MAX_PEND 128
#define VHOST_GOODCOPY_LEN 256
-enum {
- VHOST_NET_VQ_RX = 0,
- VHOST_NET_VQ_TX = 1,
- VHOST_NET_VQ_MAX = 2,
-};
-
enum vhost_net_poll_state {
VHOST_NET_POLL_DISABLED = 0,
VHOST_NET_POLL_STARTED = 1,
@@ -510,8 +504,10 @@ static int vhost_net_open(struct inode *inode, struct file *f)
return r;
}
- vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
- vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
+ vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT,
+ &n->vqs[VHOST_NET_VQ_TX]);
+ vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN,
+ &n->vqs[VHOST_NET_VQ_RX]);
n->tx_poll_state = VHOST_NET_POLL_DISABLED;
f->private_data = n;
@@ -863,15 +859,27 @@ static struct miscdevice vhost_net_misc = {
static int vhost_net_init(void)
{
+ int ret;
+
if (experimental_zcopytx)
vhost_enable_zcopy(VHOST_NET_VQ_TX);
- return misc_register(&vhost_net_misc);
+
+ ret = misc_register(&vhost_net_misc);
+ if (ret)
+ return ret;
+
+ ret = vhost_init();
+ if (ret)
+ misc_deregister(&vhost_net_misc);
+
+ return ret;
}
module_init(vhost_net_init);
static void vhost_net_exit(void)
{
misc_deregister(&vhost_net_misc);
+ vhost_cleanup();
}
module_exit(vhost_net_exit);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index c14c42b..9fabc5a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -24,7 +24,7 @@
#include <linux/highmem.h>
#include <linux/slab.h>
#include <linux/kthread.h>
-#include <linux/cgroup.h>
+#include <linux/cpu.h>
#include <linux/net.h>
#include <linux/if_packet.h>
@@ -42,6 +42,15 @@ static unsigned vhost_zcopy_mask __read_mostly;
#define vhost_used_event(vq) ((u16 __user *)&vq->avail->ring[vq->num])
#define vhost_avail_event(vq) ((u16 __user *)&vq->used->ring[vq->num])
+/* per cpu vhost struct */
+struct vhost {
+ struct task_struct *worker;
+ spinlock_t lock;
+ struct list_head work_list;
+};
+
+static DEFINE_PER_CPU(struct vhost, vhosts);
+
static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
poll_table *pt)
{
@@ -64,25 +73,28 @@ static int vhost_poll_wakeup(wait_queue_t *wait, unsigned mode, int sync,
return 0;
}
-static void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn)
+static void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn,
+ struct vhost_virtqueue *vq)
{
INIT_LIST_HEAD(&work->node);
work->fn = fn;
init_waitqueue_head(&work->done);
work->flushing = 0;
work->queue_seq = work->done_seq = 0;
+ work->vq = vq;
+ spin_lock_init(&work->lock);
}
/* Init poll structure */
void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
- unsigned long mask, struct vhost_dev *dev)
+ unsigned long mask, struct vhost_virtqueue *vq)
{
init_waitqueue_func_entry(&poll->wait, vhost_poll_wakeup);
init_poll_funcptr(&poll->table, vhost_poll_func);
poll->mask = mask;
- poll->dev = dev;
+ poll->dev = vq->dev;
- vhost_work_init(&poll->work, fn);
+ vhost_work_init(&poll->work, fn, vq);
}
/* Start polling a file. We add ourselves to file's wait queue. The caller must
@@ -108,25 +120,30 @@ static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
{
int left;
- spin_lock_irq(&dev->work_lock);
+ spin_lock_irq(&work->lock);
left = seq - work->done_seq;
- spin_unlock_irq(&dev->work_lock);
+ spin_unlock_irq(&work->lock);
return left <= 0;
}
-static void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
+/* only flushing this work? */
+static void vhost_work_flush(struct vhost_poll *poll)
{
unsigned seq;
int flushing;
+ struct vhost_dev *dev = poll->dev;
+ struct vhost_work *work = &poll->work;
- spin_lock_irq(&dev->work_lock);
+ if (list_empty(&work->node))
+ return;
+ spin_lock_irq(&work->lock);
seq = work->queue_seq;
work->flushing++;
- spin_unlock_irq(&dev->work_lock);
+ spin_unlock_irq(&work->lock);
wait_event(work->done, vhost_work_seq_done(dev, work, seq));
- spin_lock_irq(&dev->work_lock);
+ spin_lock_irq(&work->lock);
flushing = --work->flushing;
- spin_unlock_irq(&dev->work_lock);
+ spin_unlock_irq(&work->lock);
BUG_ON(flushing < 0);
}
@@ -134,21 +151,59 @@ static void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
* locks that are also used by the callback. */
void vhost_poll_flush(struct vhost_poll *poll)
{
- vhost_work_flush(poll->dev, &poll->work);
+ vhost_work_flush(poll);
+}
+
+/* schedule the cpu on the same socket but different cpu with the given one */
+static unsigned long sched_node_cpu(unsigned long cpu)
+{
+ int node, ncpus_node;
+ unsigned long sched_cpu = cpu;
+
+ node = cpu_to_node(cpu);
+ ncpus_node = nr_cpus_node(node);
+ if (ncpus_node != 1) {
+ /* pick up a random cpu on the same node, exclude
+ * the input one
+ */
+ sched_cpu = node * ncpus_node + random32() % (ncpus_node - 1);
+ if (sched_cpu >= cpu)
+ ++sched_cpu;
+ /* todo hotplug cpu race */
+ if (!cpu_online(sched_cpu))
+ sched_cpu = cpu;
+ }
+ return sched_cpu;
}
static inline void vhost_work_queue(struct vhost_dev *dev,
struct vhost_work *work)
{
- unsigned long flags;
-
- spin_lock_irqsave(&dev->work_lock, flags);
+ unsigned long cpu = work->vq->cpu;
+ struct vhost *vhost;
+
+ /* Is it safe to disable vq notify here ? */
+ vhost_disable_notify(dev, work->vq);
+
+ /* schedule the work on the cpu socket as the work has been delivered
+ * but different with the cpu the work is delivered on
+ */
+ preempt_disable();
+ if (cpu_to_node(cpu) != cpu_to_node(smp_processor_id())) {
+ cpu = sched_node_cpu(smp_processor_id());
+ work->vq->cpu = cpu;
+ }
+ preempt_enable();
+ vhost = &per_cpu(vhosts, cpu);
+ spin_lock_irq(&vhost->lock);
+ spin_lock(&work->lock);
if (list_empty(&work->node)) {
- list_add_tail(&work->node, &dev->work_list);
+ list_add_tail(&work->node, &vhost->work_list);
work->queue_seq++;
- wake_up_process(dev->worker);
+ wake_up_process(vhost->worker);
}
- spin_unlock_irqrestore(&dev->work_lock, flags);
+ spin_unlock(&work->lock);
+ spin_unlock_irq(&vhost->lock);
}
void vhost_poll_queue(struct vhost_poll *poll)
@@ -188,17 +243,18 @@ static void vhost_vq_reset(struct vhost_dev *dev,
static int vhost_worker(void *data)
{
- struct vhost_dev *dev = data;
- struct vhost_work *work = NULL;
+ struct vhost *vhost = &__get_cpu_var(vhosts);
+ struct list_head *work_list;
+ struct mm_struct *prev_mm = NULL;
unsigned uninitialized_var(seq);
+ struct vhost_work *work = NULL;
- use_mm(dev->mm);
-
+ work_list = &vhost->work_list;
for (;;) {
/* mb paired w/ kthread_stop */
set_current_state(TASK_INTERRUPTIBLE);
- spin_lock_irq(&dev->work_lock);
+ spin_lock_irq(&vhost->lock);
if (work) {
work->done_seq = seq;
if (work->flushing)
@@ -206,18 +262,26 @@ static int vhost_worker(void *data)
}
if (kthread_should_stop()) {
- spin_unlock_irq(&dev->work_lock);
+ spin_unlock_irq(&vhost->lock);
__set_current_state(TASK_RUNNING);
break;
}
- if (!list_empty(&dev->work_list)) {
- work = list_first_entry(&dev->work_list,
+ if (!list_empty(work_list)) {
+ work = list_first_entry(work_list,
struct vhost_work, node);
+ spin_lock(&work->lock);
list_del_init(&work->node);
+ spin_unlock(&work->lock);
seq = work->queue_seq;
+ if (prev_mm != work->vq->dev->mm) {
+ if (prev_mm)
+ unuse_mm(prev_mm);
+ prev_mm = work->vq->dev->mm;
+ use_mm(prev_mm);
+ }
} else
work = NULL;
- spin_unlock_irq(&dev->work_lock);
+ spin_unlock_irq(&vhost->lock);
if (work) {
__set_current_state(TASK_RUNNING);
@@ -226,7 +290,9 @@ static int vhost_worker(void *data)
schedule();
}
- unuse_mm(dev->mm);
+
+ if (prev_mm)
+ unuse_mm(prev_mm);
return 0;
}
@@ -298,9 +364,6 @@ long vhost_dev_init(struct vhost_dev *dev,
dev->log_file = NULL;
dev->memory = NULL;
dev->mm = NULL;
- spin_lock_init(&dev->work_lock);
- INIT_LIST_HEAD(&dev->work_list);
- dev->worker = NULL;
for (i = 0; i < dev->nvqs; ++i) {
dev->vqs[i].log = NULL;
@@ -312,7 +375,8 @@ long vhost_dev_init(struct vhost_dev *dev,
vhost_vq_reset(dev, dev->vqs + i);
if (dev->vqs[i].handle_kick)
vhost_poll_init(&dev->vqs[i].poll,
- dev->vqs[i].handle_kick, POLLIN, dev);
+ dev->vqs[i].handle_kick, POLLIN,
+ &dev->vqs[i]);
}
return 0;
@@ -325,71 +389,35 @@ long vhost_dev_check_owner(struct vhost_dev *dev)
return dev->mm == current->mm ? 0 : -EPERM;
}
-struct vhost_attach_cgroups_struct {
- struct vhost_work work;
- struct task_struct *owner;
- int ret;
-};
-
-static void vhost_attach_cgroups_work(struct vhost_work *work)
-{
- struct vhost_attach_cgroups_struct *s;
-
- s = container_of(work, struct vhost_attach_cgroups_struct, work);
- s->ret = cgroup_attach_task_all(s->owner, current);
-}
-
-static int vhost_attach_cgroups(struct vhost_dev *dev)
-{
- struct vhost_attach_cgroups_struct attach;
-
- attach.owner = current;
- vhost_work_init(&attach.work, vhost_attach_cgroups_work);
- vhost_work_queue(dev, &attach.work);
- vhost_work_flush(dev, &attach.work);
- return attach.ret;
-}
-
/* Caller should have device mutex */
static long vhost_dev_set_owner(struct vhost_dev *dev)
{
- struct task_struct *worker;
int err;
+ unsigned long txcpu, rxcpu;
/* Is there an owner already? */
if (dev->mm) {
err = -EBUSY;
- goto err_mm;
+ goto out;
}
- /* No owner, become one */
- dev->mm = get_task_mm(current);
- worker = kthread_create(vhost_worker, dev, "vhost-%d", current->pid);
- if (IS_ERR(worker)) {
- err = PTR_ERR(worker);
- goto err_worker;
- }
+ err = vhost_dev_alloc_iovecs(dev);
+ if (err)
+ goto out;
- dev->worker = worker;
- wake_up_process(worker); /* avoid contributing to loadavg */
+ /* initial txcpu, rxcpu on the same socket */
+ txcpu = sched_node_cpu(smp_processor_id());
+ rxcpu = sched_node_cpu(txcpu);
- err = vhost_attach_cgroups(dev);
- if (err)
- goto err_cgroup;
+ dev->vqs[VHOST_NET_VQ_TX].cpu = txcpu;
+ dev->vqs[VHOST_NET_VQ_RX].cpu = rxcpu;
- err = vhost_dev_alloc_iovecs(dev);
- if (err)
- goto err_cgroup;
+ /* No owner, become one */
+ dev->mm = get_task_mm(current);
return 0;
-err_cgroup:
- kthread_stop(worker);
- dev->worker = NULL;
-err_worker:
- if (dev->mm)
- mmput(dev->mm);
- dev->mm = NULL;
-err_mm:
+
+out:
return err;
}
@@ -474,11 +502,6 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
kfree(rcu_dereference_protected(dev->memory,
lockdep_is_held(&dev->mutex)));
RCU_INIT_POINTER(dev->memory, NULL);
- WARN_ON(!list_empty(&dev->work_list));
- if (dev->worker) {
- kthread_stop(dev->worker);
- dev->worker = NULL;
- }
if (dev->mm)
mmput(dev->mm);
dev->mm = NULL;
@@ -1605,3 +1628,104 @@ void vhost_zerocopy_callback(void *arg)
vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
}
+
+/* to do
+static int __cpuinit vhost_pool_callback(struct notifier_block *nfb,
+ unsigned long action,
+ void *hcpu)
+{
+ struct vhost *vhost = per_cpu(vhosts, hcpu);
+
+ action &= ~CPU_TASKS_FROZEN;
+
+ switch (action) {
+ case CPU_UP_PREPARE:
+ case CPU_UP_PREPARE_FROZEN:
+ if (!create_vhost_task(vhosts, hcpu))
+ return notifier_from_errno(-ENOMEM);
+ break;
+ case CPU_UP_CANCELED:
+ case CPU_UP_CANCELED_FROZEN:
+ kthread_bind(vhost->worker, cpumask_any(cpu_online_mask));
+ destory_vhost_task(vhost, hcpu);
+ break;
+ case CPU_ONLINE:
+ case CPU_ONLINE_FROZEN:
+ kthread_bind(vhost->worker, hcpu);
+ wake_up_process(vhost->worker);
+ break;
+ case CPU_DOWN_PREPARE:
+ case CPU_DOWN_PREPARE_FROZEN:
+ case CPU_DOWN_FAILED:
+ case CPU_DOWN_FAILED_FROZEN:
+ break;
+ case CPU_DEAD:
+ case CPU_DEAD_FROZEN:
+ destrory_vhost_task(vhosts, hcpu);
+ take_over_work(vhosts, hcpu);
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static struct notifier_block vhost_pool_callback_nb __cpuinitdata = {
+ .notifier_call = vhost_pool_callcack,
+ .priority = 0,
+}
+*/
+
+static void free_workers(void)
+{
+ unsigned long cpu;
+ struct vhost *vhost;
+
+ /* to do
+ * unregister_cpu_notifier(&vhost_pool_callback_nb);
+ */
+ get_online_cpus();
+ for_each_online_cpu(cpu) {
+ vhost = &per_cpu(vhosts, cpu);
+ if (!IS_ERR(vhost->worker)) {
+ kthread_stop(vhost->worker);
+ BUG_ON(!list_empty(&vhost->work_list));
+ }
+ }
+ put_online_cpus();
+}
+
+int vhost_init(void)
+{
+ int ret = -ENOMEM;
+ unsigned long cpu;
+ struct vhost *vhost;
+
+ get_online_cpus();
+ for_each_online_cpu(cpu) {
+ vhost = &per_cpu(vhosts, cpu);
+
+ INIT_LIST_HEAD(&vhost->work_list);
+ spin_lock_init(&vhost->lock);
+ vhost->worker = kthread_create_on_node(vhost_worker, NULL,
+ cpu_to_node(cpu),
+ "vhost-%lu", cpu);
+ if (IS_ERR(vhost->worker))
+ goto err;
+
+ kthread_bind(vhost->worker, cpu);
+ wake_up_process(vhost->worker);
+ }
+ put_online_cpus();
+
+ /* to do
+ * register_cpu_notifier(&vhost_pool_callback_nb);
+ */
+ return 0;
+err:
+ free_workers();
+ return ret;
+}
+
+void vhost_cleanup(void)
+{
+ free_workers();
+}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index a801e28..c6ecfb0 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -18,6 +18,12 @@
#define VHOST_DMA_DONE_LEN 1
#define VHOST_DMA_CLEAR_LEN 0
+enum {
+ VHOST_NET_VQ_RX = 0,
+ VHOST_NET_VQ_TX = 1,
+ VHOST_NET_VQ_MAX = 2,
+};
+
struct vhost_device;
struct vhost_work;
@@ -30,6 +36,8 @@ struct vhost_work {
int flushing;
unsigned queue_seq;
unsigned done_seq;
+ struct vhost_virtqueue *vq;
+ spinlock_t lock;
};
/* Poll a file (eventfd or socket) */
@@ -44,7 +52,7 @@ struct vhost_poll {
};
void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
- unsigned long mask, struct vhost_dev *dev);
+ unsigned long mask, struct vhost_virtqueue *vq);
void vhost_poll_start(struct vhost_poll *poll, struct file *file);
void vhost_poll_stop(struct vhost_poll *poll);
void vhost_poll_flush(struct vhost_poll *poll);
@@ -141,6 +149,7 @@ struct vhost_virtqueue {
/* Reference counting for outstanding ubufs.
* Protected by vq mutex. Writers must also take device mutex. */
struct vhost_ubuf_ref *ubufs;
+ unsigned long cpu;
};
struct vhost_dev {
@@ -155,9 +164,6 @@ struct vhost_dev {
int nvqs;
struct file *log_file;
struct eventfd_ctx *log_ctx;
- spinlock_t work_lock;
- struct list_head work_list;
- struct task_struct *worker;
};
long vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue *vqs, int nvqs);
@@ -190,6 +196,8 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
void vhost_zerocopy_callback(void *arg);
int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
+int vhost_init(void);
+void vhost_cleanup(void);
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
^ permalink raw reply related
* [RFC PATCH 0/1] NUMA aware scheduling per vhost thread patch
From: Shirley Ma @ 2012-03-22 23:48 UTC (permalink / raw)
To: Michael S. Tsirkin, netdev, kvm, tahm
Sorry for being late to submit this patch. I have spent lots of time
trying to find the best approach. This effort is still going on...
This patch is built against net-next tree.
This is an experimental RFC patch. The purpose of this patch is to
address KVM networking scalability and NUMA scheduling issue.
The existing implementation of vhost creats a vhost thread per-device
(virtio_net) based. RX and TX work of a VMs per-device is handled by
same vhost thread.
One of the limitation of this implementation is with increasing the
number VMs or the number of virtio-net interfces, more vhost threads are
created, it will consume more kernel resources, and induce more threads
context switches/scheduling overhead. We noticed that the KVM network
performance doesn't scale with increasing number of VMs.
The other limitation is to have single vhost thread to process both RX
and TX, the work will be blocked. So we create this per cpu vhost thread
implementation. The number of vhost cpu threads is limited to the number
of cpus on the host.
To address these limitations, we are propsing a per-cpu vhost thread
model where the number of vhost threads are limited and equal to the
number of online cpus on the host.
Based on our testing experience, the vcpus can be scheduled across cpu
sockets even when the number of vcpus is smaller than the number of
cores per cpu socket and there is no other activities besides KVM
networking workload. We found that if vhost thread is scheduled on the
same socket as the work is received, the performance will be better.
So in this per cpu vhost thread implementation, a vhost thread is
selected dynamically based on where the TX/RX work is initiated. A vhost
thread on the same cpu socket is selected but not on the same cpu as the
vcpu/interrupt thread that initizated the TX/RX work.
When we test this RFC patch, the other interesting thing we found is the
performance results also seem related to NIC flow steering. We are
spending time on evaluate different NICs flow director implementation
now. We will enhance this patch based on our findings later.
We have tried different scheduling: per-device based, per vq based and
per work type (tx_kick, rx_kick, tx_net, rx_net) based vhost scheduling,
we found that so far the per vq based scheduling is good enough for now.
We also tried different algorithm to select which cpu vhost thread will
running on a specific cpu socket: avg_load balance, and randomly...
>From our test results, we found that the scalability has been
significantly improved. And this patch is also helpful for small packets
performance.
Hoever, we are seeing some regressions in a local guest to guest
scenario on a 8 cpu NUMA system.
In one case, 24 VMs 256 bytes tcp_stream test shows it has improved from
810Mb/s to 9.1Gb/s. :)
(We created two local VMs, and each VM has 2 vcpus. W/o this patch, the
number of threads is 4 vcpus + 2 vhosts = 6, w/i this patch is 4 vcpus +
8 vhosts = 12. It causes more context switches. When I change the
scheduling to use 2-4 vhost threads, the regressions are gone. I am
continue investigation on how to make small number of VMs, local guest
to gues performance better. Once I find the clue, I will share here.)
The cpu hotplug support hasn't in place yet. I will post it later.
Since we have per cpu vhost thread, each vhost thread will handle
multiple vqs, so we will be able to reduce/remove vq notification when
the work is heavy loaded in future.
Here is my test results for remote host to guest test: tcp_rrs, udp_rrs,
tcp_stream with guest has 2 vpus, host has two cpu socket, each socket
has 4 cores.
TCP_STREAM 256 512 1K 2K 4K 8K 16K
--------------------------------------------------------------------
Original
H->Guest 2501 4238 4744 5256 7203 6975 5799 Patch
H->Guest 1676 2290 3149 8026 8439 8283 8216
Original
Guest->H 744 1773 5675 1397 8207 7296 8117
Patch
Guest->Host 1041 1386 5407 7057 8298 8127 8241
60 instances TCP_RRs: Patch 150K trans/s vs. 91K trans/sec
65% improved with taskset vcpus on the same socket
60 instances UDP_RRs: Patch 172K trans/s vs. 103K trans/s
67% improved with taskset vcpus on the same socket
Tom has run 1VM to 24 VMs test for different work. He will post it here
soon.
If the host scheduler ensures that the VM's vcpus are not scheduled to
another socket (i.e. cpu mask the vcpus on same socket) then the
performance will be better.
Signed-off-by: Shirley Ma <xma@us.ibm.com>
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
Tested-by: Tom Lendacky <toml@us.ibm.com>
---
drivers/vhost/net.c | 26 ++-
drivers/vhost/vhost.c | 289
+++++++++++++++++++++++----------
drivers/vhost/vhost.h | 16 ++-
3 files changed, 232 insertions(+), 103 deletions(-)
Thanks
Shirley
^ permalink raw reply
* Re: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
From: Roland Dreier @ 2012-03-22 23:45 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Parav.Pandit-laKkSmNT4hbQT0dZR+AlfA,
David.Laight-JxhZ9S5GRejQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20120322224429.GA12980-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Thu, Mar 22, 2012 at 3:44 PM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> For hardware facing structures I'd combine this with a static assert
> to verify structure size at compile time.
>
> So..
>
> 1) Avoid using attributes unless the structure has unaligned members.
> 2) Avoid creating structures with unaligned members (eg for userspace
> communication)
> 3) Frown at hardware/firmware developers who make communication
> structures with unaligned members :)
> 4) Be explicit about padding in your layout for 64/32
> compatibility.
Excellent summary, thanks Jason.
- R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Piggyback the final ACK of the three way TCP connection establishment with the data
From: Rick Jones @ 2012-03-22 23:44 UTC (permalink / raw)
To: Vincent Li; +Cc: Eric Dumazet, netdev
In-Reply-To: <CAK3+h2ydKQ15U1_2dqWsHAL_LKYm-1xbJvUbeaMJShoOCU_TsA@mail.gmail.com>
I like piggybacking as much as the next guy, and pushed for a bunch of
it in the past, but are the files being transferred really that small
relative to the RTT that the savings of the standalone ACK of the
SYN|ACK really buys that much?
I'm sure it does nice things for a default netperf TCP_CRR test, shaving
one segment out of 8 or so, and maybe even more on a TCP_CC test, but
how many sub-MSS files are transferred these days?
rick jones
^ permalink raw reply
* Re: [PULL] vhost-net/virtio: fixes for 3.4
From: David Miller @ 2012-03-22 23:34 UTC (permalink / raw)
To: mst; +Cc: kvm, virtualization, netdev, linux-kernel, levinsasha928, nyh,
nyh
In-Reply-To: <20120322221227.GA17849@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Fri, 23 Mar 2012 00:12:28 +0200
> OK, sorry about that. Can't fix right now as I'm not at
> the box that has the key but this works for me with an old git:
Please fix it up so I can pull properly.
^ permalink raw reply
* Re: [PATCH 2/2] usbnet: don't clear urb->dev in tx_complete
From: David Miller @ 2012-03-22 23:33 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: tom.leiming-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, stable-DgEjT+Ai2ygdnm+yROfE0A,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
oliver-GvhC2dPhHPQdnm+yROfE0A
In-Reply-To: <20120322143607.GD19835-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
From: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Date: Thu, 22 Mar 2012 07:36:07 -0700
> On Thu, Mar 22, 2012 at 09:22:38PM +0800, Ming Lei wrote:
>> URB unlinking is always racing with its completion and tx_complete
>> may be called before or during running usb_unlink_urb, so tx_complete
>> must not clear urb->dev since it will be used in unlink path,
>> otherwise invalid memory accesses or usb device leak may be caused
>> inside usb_unlink_urb.
>>
>> Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>> Cc: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
>> Cc: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] usbnet: increase URB reference count before usb_unlink_urb
From: David Miller @ 2012-03-22 23:33 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: tom.leiming-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, stable-DgEjT+Ai2ygdnm+yROfE0A,
bigeasy-hfZtesqFncYOwBW4kG4KsQ,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
oliver-GvhC2dPhHPQdnm+yROfE0A
In-Reply-To: <20120322143553.GC19835-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
From: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Date: Thu, 22 Mar 2012 07:35:53 -0700
> On Thu, Mar 22, 2012 at 09:22:18PM +0800, Ming Lei wrote:
>> Commit 4231d47e6fe69f061f96c98c30eaf9fb4c14b96d(net/usbnet: avoid
>> recursive locking in usbnet_stop()) fixes the recursive locking
>> problem by releasing the skb queue lock, but it makes usb_unlink_urb
>> racing with defer_bh, and the URB to being unlinked may be freed before
>> or during calling usb_unlink_urb, so use-after-free problem may be
>> triggerd inside usb_unlink_urb.
>>
>> The patch fixes the use-after-free problem by increasing URB
>> reference count with skb queue lock held before calling
>> usb_unlink_urb, so the URB won't be freed until return from
>> usb_unlink_urb.
>>
>> Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>> Cc: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>> Cc: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
>> Cc: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
>> Reported-by: Dave Jones <davej-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] xfrm: Access the replay notify functions via the registered callbacks
From: David Miller @ 2012-03-22 23:30 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, netdev
In-Reply-To: <20120322093613.GU29891@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 22 Mar 2012 10:36:13 +0100
> We call the wrong replay notify function when we use ESN replay
> handling. This leads to the fact that we don't send notifications
> if we use ESN. Fix this by calling the registered callbacks instead
> of xfrm_replay_notify().
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Applied.
^ permalink raw reply
* Re: [patch] RDS: use gfp flags from caller in conn_alloc()
From: David Miller @ 2012-03-22 23:30 UTC (permalink / raw)
To: venkat.x.venkatsubra; +Cc: dan.carpenter, rds-devel, netdev, kernel-janitors
In-Reply-To: <4F6B4316.5050508@oracle.com>
From: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
Date: Thu, 22 Mar 2012 10:19:50 -0500
> On 3/22/2012 1:44 AM, Dan Carpenter wrote:
>> We should be using the gfp flags the caller specified here, instead of
>> GFP_KERNEL. I think this might be a bugfix, depending on the value of
>> "sock->sk->sk_allocation" when we call rds_conn_create_outgoing() in
>> rds_sendmsg(). Otherwise, it's just a cleanup.
>>
>> Signed-off-by: Dan Carpenter<dan.carpenter@oracle.com>
> Acked-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
Applied.
^ permalink raw reply
* Re: [patch] netlabel: use GFP flags from caller instead of GFP_ATOMIC
From: David Miller @ 2012-03-22 23:30 UTC (permalink / raw)
To: dan.carpenter; +Cc: paul, netdev, kernel-janitors
In-Reply-To: <20120322064101.GD7369@elgon.mountain>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Thu, 22 Mar 2012 09:41:01 +0300
> This function takes a GFP flags as a parameter, but they are never used.
> We don't take a lock in this function so there is no reason to prefer
> GFP_ATOMIC over the caller's GFP flags.
>
> There is only one caller, cipso_v4_map_cat_rng_ntoh(), and it passes
> GFP_ATOMIC as the GFP flags so this doesn't change how the code works.
> It's just a cleanup.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply
* Re: [PATCH] xfrm: Remove unused xfrm_state from xfrm_state_check_space
From: David Miller @ 2012-03-22 23:30 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, netdev
In-Reply-To: <20120322093239.GT29891@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 22 Mar 2012 10:32:39 +0100
> The xfrm_state argument is unused in this function, so remove it.
> Also the name xfrm_state_check_space does not really match what this
> function does. It actually checks if we have enough head and tailroom
> on the skb. So we rename the function to xfrm_skb_check_space.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Applied.
^ permalink raw reply
* Re: Piggyback the final ACK of the three way TCP connection establishment with the data
From: Eric Dumazet @ 2012-03-22 23:17 UTC (permalink / raw)
To: Vincent Li; +Cc: netdev
In-Reply-To: <1332458037.6521.12.camel@edumazet-glaptop>
On Thu, 2012-03-22 at 16:13 -0700, Eric Dumazet wrote:
> A third possibility (reading the code) if you use non blocking IO, is to
> send() a message right after connect()
>
>
Or use the auto connect on sendto() more probably...
That combines the connect() and send()
^ permalink raw reply
* Re: Piggyback the final ACK of the three way TCP connection establishment with the data
From: Eric Dumazet @ 2012-03-22 23:13 UTC (permalink / raw)
To: Vincent Li; +Cc: netdev
In-Reply-To: <CAK3+h2ydKQ15U1_2dqWsHAL_LKYm-1xbJvUbeaMJShoOCU_TsA@mail.gmail.com>
On Thu, 2012-03-22 at 16:02 -0700, Vincent Li wrote:
> >
> > No kernel patch is needed, you already can do this on linux.
> >
> > Check file net/ipv4/tcp_input.c lines around 5722
> >
> >
>
> is this code snippet in tcp_rcv_synsent_state_process that you refer to?
>
> 5676 if (sk->sk_write_pending ||
> 5677 icsk->icsk_accept_queue.rskq_defer_accept ||
> 5678 icsk->icsk_ack.pingpong) {
> 5679 /* Save one ACK. Data will be ready after
> 5680 * several ticks, if write_pending is set.
> 5681 *
> 5682 * It may be deleted, but with this
> feature tcpdumps
> 5683 * look so _wonderfully_ clever, that I
> was not able
> 5684 * to stand against the temptation 8) --ANK
> 5685 */
> 5686 inet_csk_schedule_ack(sk);
> 5687 icsk->icsk_ack.lrcvtime = tcp_time_stamp;
> 5688 icsk->icsk_ack.ato = TCP_ATO_MIN;
> 5689 tcp_incr_quickack(sk);
> 5690 tcp_enter_quickack_mode(sk);
> 5691 inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
> 5692 TCP_DELACK_MAX,
> TCP_RTO_MAX);
> 5693
> 5694 discard:
> 5695 __kfree_skb(skb);
> 5696 return 0;
> 5697 } else {
> 5698 tcp_send_ack(sk);
> 5699 }
>
> if I understand it correct on linux, the application code need to set
> socket option with TCP_DEFER_ACCEPT or TCP_QUICKACK in order to
> trigger it, correct?
>
> We have user running wu-ftpd on HP Unix with tcp tunable
> tcp_delay_final_twh_ack on. so in active FTP situation, when wu-ftpd
> open up data connection to client, it sends SYN, client SYN/ACK, then
> ACK/PUSH with data. so on HU UNIX, it appears just turn on tcp tunable
> tcp_delay_final_twh_ack would make it happen.
>
> but on Linux, do I need to change wu-ftpd code and modify the socket
> option with TCP_DEFER_ACCEPT or TCP_QUICKACK in order to trigger the
> code snippet in tcp_rcv_synsent_state_process?
Yes.
A third possibility (reading the code) if you use non blocking IO, is to
send() a message right after connect()
^ permalink raw reply
* Re: [PATCH net V4 2/2] igb: offer a PTP Hardware Clock instead of the timecompare method
From: chetan loke @ 2012-03-22 23:13 UTC (permalink / raw)
To: Richard Cochran
Cc: Keller, Jacob E, netdev@vger.kernel.org,
e1000-devel@lists.sourceforge.net, Kirsher, Jeffrey T,
Ronciak, John, john.stultz@linaro.org, tglx@linutronix.de
In-Reply-To: <20120322064107.GB2182@netboy.at.omicron.at>
On Thu, Mar 22, 2012 at 2:41 AM, Richard Cochran
<richardcochran@gmail.com> wrote:
> On Wed, Mar 21, 2012 at 05:50:34PM -0400, chetan loke wrote:
>>
>> Richard - Intent is to make the readers(get_time) wait (or return last
>> read value if the seq_counter tripped because you know that this value
>> was recent) and let the tx/rx path continue. I haven't looked in more
>> details but as Jake mentioned you will also need to change the way you
>> read the values(by not using timecounter_read in get_time).
>
> I don't get what you guys are saying. How can you avoid the spin lock
> around the two time register reads? How about a patch or some pseudo
> code?
>
tmreg_lock now becomes seqlock_t instead of spinlock_t.
/* users can keep re-trying - dont really care */
igb_gettime_locking (...) {
unsigned int seq;
u64 ns;
do {
seq = read_seqbegin( &pigb->tmreg_seq_lock);
ns = timecounter_read(&pigb->tc);
} while (read_seqretry(&pigb->tmreg_seq_lock, seq));
// process ns
}
copyright 2012 - Chetan Loke <lokechetan@gmail.com>
// trip cnt will ensure/enforce - evil adjtime user-space code can
still not block us.
// called from igb_tx[rx]_hwtstamp
driver_rx_tx_path_locking( ... ) {
unsigned int seq, trip_cnt = 0;
u64 ns;
do {
seq = read_seqbegin( &pigb->tmreg_seq_lock);
trip_cnt++;
ns = timecounter_read(&pigb->tc);
} while (read_seqretry(&pigb->tmreg_seq_lock, seq) && trip_cnt < 2));
// process ns
}
igb_adjtime () {
/* just let first user of adjtime or settime succeed? */
if (write_try_seqlock(&pigb->tmreg_seq_lock)) {
// update NIC counter - here ...
write_sequnlock(&pigb->tmreg_seq_lock);
}
}
igb_settime () {
// let all of them do their thing ... or you can use trylock here
too if you like...
write_seqlock(&pigb->tmreg_seq_lock);
// update NIC counter - here ...
write_sequnlock(&pigb->tmreg_seq_lock);
}
write_seq_xxx will spinlock as usual but read_seq reads the atomically
incremented counter. Also notice the use of trip_cnt to further ensure
the driver never keeps re-trying.
The regular __irqsave flavors are also available for use for the
write_seq calls.
> Thanks,
> Richard
Chetan
^ permalink raw reply
* Re: Piggyback the final ACK of the three way TCP connection establishment with the data
From: David Miller @ 2012-03-22 23:07 UTC (permalink / raw)
To: vincent.mc.li; +Cc: eric.dumazet, netdev
In-Reply-To: <CAK3+h2ydKQ15U1_2dqWsHAL_LKYm-1xbJvUbeaMJShoOCU_TsA@mail.gmail.com>
From: Vincent Li <vincent.mc.li@gmail.com>
Date: Thu, 22 Mar 2012 16:02:41 -0700
> but on Linux, do I need to change wu-ftpd code and modify the socket
> option with TCP_DEFER_ACCEPT or TCP_QUICKACK in order to trigger the
> code snippet in tcp_rcv_synsent_state_process?
Yes.
^ permalink raw reply
* Re: Piggyback the final ACK of the three way TCP connection establishment with the data
From: Vincent Li @ 2012-03-22 23:02 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1332374452.9433.15.camel@edumazet-glaptop>
>
> No kernel patch is needed, you already can do this on linux.
>
> Check file net/ipv4/tcp_input.c lines around 5722
>
>
is this code snippet in tcp_rcv_synsent_state_process that you refer to?
5676 if (sk->sk_write_pending ||
5677 icsk->icsk_accept_queue.rskq_defer_accept ||
5678 icsk->icsk_ack.pingpong) {
5679 /* Save one ACK. Data will be ready after
5680 * several ticks, if write_pending is set.
5681 *
5682 * It may be deleted, but with this
feature tcpdumps
5683 * look so _wonderfully_ clever, that I
was not able
5684 * to stand against the temptation 8) --ANK
5685 */
5686 inet_csk_schedule_ack(sk);
5687 icsk->icsk_ack.lrcvtime = tcp_time_stamp;
5688 icsk->icsk_ack.ato = TCP_ATO_MIN;
5689 tcp_incr_quickack(sk);
5690 tcp_enter_quickack_mode(sk);
5691 inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
5692 TCP_DELACK_MAX,
TCP_RTO_MAX);
5693
5694 discard:
5695 __kfree_skb(skb);
5696 return 0;
5697 } else {
5698 tcp_send_ack(sk);
5699 }
if I understand it correct on linux, the application code need to set
socket option with TCP_DEFER_ACCEPT or TCP_QUICKACK in order to
trigger it, correct?
We have user running wu-ftpd on HP Unix with tcp tunable
tcp_delay_final_twh_ack on. so in active FTP situation, when wu-ftpd
open up data connection to client, it sends SYN, client SYN/ACK, then
ACK/PUSH with data. so on HU UNIX, it appears just turn on tcp tunable
tcp_delay_final_twh_ack would make it happen.
but on Linux, do I need to change wu-ftpd code and modify the socket
option with TCP_DEFER_ACCEPT or TCP_QUICKACK in order to trigger the
code snippet in tcp_rcv_synsent_state_process?
Thanks
Vincent
^ permalink raw reply
* Regression due to: net/usbnet: avoid recursive locking in usbnet_stop()
From: Stephen Warren @ 2012-03-22 22:56 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Oliver Neukum, David S. Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Greg Kroah-Hartman, Willy Tarreau, Alan Stern
It looks like commit 4231d47e6fe69f061f96c98c30eaf9fb4c14b96d
"net/usbnet: avoid recursive locking in usbnet_stop()" causes a problem
for me.
When I reboot or shutdown, I see the error spew below at least 50% of
the time. Reverting the above patch fixes this, at least for 10 reboot
attempts.
Other notes:
* I'm running on an NVIDIA Tegra ARM device, with smsc95xx USB ethernet.
* This first happens in next-20120309, not in next-20120308.
* The problematic commit appears to have been queued for backport to
various stable branches:-(
> Will now restart.
> [ 85.622493] Unable to handle kernel paging request at virtual address 6b6b6bfb
> [ 85.629748] pgd = eea68000
> [ 85.632477] [6b6b6bfb] *pgd=00000000
> [ 85.636096] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> [ 85.641426] Modules linked in:
> [ 85.644527] CPU: 0 Not tainted (3.3.0-rc7-next-20120319-00027-gee679a3 #30)
> [ 85.651879] PC is at kobject_put+0xc/0x74
> [ 85.655936] LR is at usb_hcd_unlink_urb+0x68/0xc4
> [ 85.660675] pc : [<c019fb14>] lr : [<c0228db4>] psr: 20000013
> [ 85.660695] sp : ee8c7e08 ip : ffffac43 fp : 00000000
> [ 85.672198] r10: c057e190 r9 : ee87e0c0 r8 : ee83ac00
> [ 85.677448] r7 : a0000013 r6 : ffffff98 r5 : ee87e0c0 r4 : 6b6b6bdb
> [ 85.684000] r3 : ee82c800 r2 : ef1b6800 r1 : a0000013 r0 : 6b6b6bdb
> [ 85.690554] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 85.697716] Control: 10c5387d Table: 2ea6804a DAC: 00000015
> [ 85.703485] Process reboot (pid: 1528, stack limit = 0xee8c62f0)
> [ 85.709516] Stack: (0xee8c7e08 to 0xee8c8000)
> [ 85.713908] 7e00: 6b6b6bdb a0000013 00000000 c0228db4 ee87e0c0 ffffff98
> [ 85.722132] 7e20: ee82c800 ee83acd4 00000000 ee926bb8 ee83acc8 c02180f8 ee83ac00 ee83acc8
> [ 85.730353] 7e40: ee83acdc ee8c6000 00000000 ee83ba00 c057e190 c02181fc 00000000 00000000
> [ 85.738575] 7e60: eea53a40 c004a820 ee8c7e78 ee8c7e78 00000000 00000000 ee8c7e68 ee8c7e68
> [ 85.746796] 7e80: 00000000 ee83a800 ee83ac00 c03bb8f8 00000001 00000000 ee83ba0c c02191c8
> [ 85.755017] 7ea0: ee83a800 00000200 ee8c7ed0 ee83a800 00001003 00000001 00000000 c02ba42c
> [ 85.763239] 7ec0: ee83a858 ee83a800 00001042 c02ba480 ee83a858 ee83a858 c02bd224 c02bd4c4
> [ 85.771459] 7ee0: 00000000 ee83a800 00001003 ee83a800 beca7468 c02bd5b4 ee917840 00000000
> [ 85.779679] 7f00: ee83a800 c0302dfc ee83a828 beca7468 30687465 00000000 00000000 00000000
> [ 85.787899] 7f20: 00001042 71cc140a 00000000 00000000 30687465 00008914 beca7468 00008914
> [ 85.796120] 7f40: 00000003 c000e264 ee8c6000 00000000 beca7470 c02a974c ee9a0080 beca7468
> [ 85.804342] 7f60: 00008914 c00be058 ee9a0080 00000003 00008914 ee9a0080 beca7468 00008914
> [ 85.812562] 7f80: 00000003 c00be0c4 00000003 00000000 beca7468 00000000 beca746a beca7468
> [ 85.820782] 7fa0: 00000036 c000e0e0 00000000 beca746a 00000003 00008914 beca7468 00001042
> [ 85.829003] 7fc0: 00000000 beca746a beca7468 00000036 beca7468 00000001 beca7468 beca7470
> [ 85.837224] 7fe0: 00000000 beca7460 000095bc b6f1bcec 20000010 00000003 e0836006 e2044fff
> [ 85.845470] [<c019fb14>] (kobject_put+0xc/0x74) from [<c0228db4>] (usb_hcd_unlink_urb+0x68/0xc4)
> [ 85.854336] [<c0228db4>] (usb_hcd_unlink_urb+0x68/0xc4) from [<c02180f8>] (unlink_urbs+0x44/0xa8)
> [ 85.863270] [<c02180f8>] (unlink_urbs+0x44/0xa8) from [<c02181fc>] (usbnet_terminate_urbs+0xa0/0x15c)
> [ 85.872547] [<c02181fc>] (usbnet_terminate_urbs+0xa0/0x15c) from [<c02191c8>] (usbnet_stop+0xe0/0x13c)
> [ 85.881935] [<c02191c8>] (usbnet_stop+0xe0/0x13c) from [<c02ba42c>] (__dev_close_many+0x88/0xb8)
> [ 85.890782] [<c02ba42c>] (__dev_close_many+0x88/0xb8) from [<c02ba480>] (__dev_close+0x24/0x34)
> [ 85.899541] [<c02ba480>] (__dev_close+0x24/0x34) from [<c02bd4c4>] (__dev_change_flags+0x8c/0x110)
> [ 85.908548] [<c02bd4c4>] (__dev_change_flags+0x8c/0x110) from [<c02bd5b4>] (dev_change_flags+0x10/0x44)
> [ 85.918010] [<c02bd5b4>] (dev_change_flags+0x10/0x44) from [<c0302dfc>] (devinet_ioctl+0x2b0/0x598)
> [ 85.927125] [<c0302dfc>] (devinet_ioctl+0x2b0/0x598) from [<c02a974c>] (sock_ioctl+0x1dc/0x230)
> [ 85.935883] [<c02a974c>] (sock_ioctl+0x1dc/0x230) from [<c00be058>] (do_vfs_ioctl+0x294/0x2cc)
> [ 85.944544] [<c00be058>] (do_vfs_ioctl+0x294/0x2cc) from [<c00be0c4>] (sys_ioctl+0x34/0x54)
> [ 85.952957] [<c00be0c4>] (sys_ioctl+0x34/0x54) from [<c000e0e0>] (ret_fast_syscall+0x0/0x30)
> [ 85.961436] Code: c045ccd1 e92d4013 e2504000 0a000015 (e5d43020)
> [ 85.971554] ---[ end trace 2a43ec4ab88bc4a2 ]---
> Segmentation fault
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
From: Jason Gunthorpe @ 2012-03-22 22:44 UTC (permalink / raw)
To: Parav.Pandit-iH1Dq9VlAzfQT0dZR+AlfA
Cc: David.Laight-ZS65k/vG3HxXrIkS9f7CXA,
roland-BHEL68pLQRGGvPXPguhicg, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <3ae9829d-f8dd-4268-918a-94616eff0915-nbYkmrCdWxmgMrCBcu8zE0EOCMrvLtNR@public.gmane.org>
On Thu, Mar 22, 2012 at 02:20:28PM -0700, Parav.Pandit-iH1Dq9VlAzfQT0dZR+AlfA@public.gmane.org wrote:
> I got a question here lately.
>
> aligned directive will ensure that it will fall on boundary. Say
> aligned(4) ensures that structure is aligned to 4 byte boundary.
> Compiler can (at least theoretically) still have 4 byte structure
> aligned to 8 byte boundary on 64-bit platform (which is 4 byte
> aligned too).
There are very specific rules defined in the platform's ABI for how C
structures are layed out in memory, each ABI (ie CPU) has its own
specific quirks, but broadly in Linux land you can boil it down to:
1) The alignment of a structure is the greatest alignment of all the
members
2) Each member is aligned to its alignment.
The alignment of the structure drives the total size of the structure,
and specifically the padding added at the end to reach that alignment.
So, no, a compiler that increased the alignment of a struct with one
u32 to 8 would violate the various ABIs and not be usuable for
Linux. It is important to bear in mind that Linux targets a particular
set of ABI conventions, and it is not 'anything goes'.
> struct {
> u32 field;
> };
So in this case: the u32 is aligned to 4, the structure is aligned to
4 and the total size of the structure is 4 on everything linux
supports.
> struct {
> u64 fielda
> u32 field;
> };
In this case: On 64 bit: the u64 is aligned to 8 and the u32 is aligned to 4. So
the structure is aligned to 8. A pad is inserted at the end of the
struct to bring it out. On 32 bit, the u64 is aligned to 4, so the
struct is aligned to 4, so no pad is added.
> struct {
> __float128 fielda
> u32 field;
> };
In this case the float128 is aligned to 16 and thus the structure is
aligned to 16 and 12 pad bytes are added.
> However requirement is to have this structure only 4 byte size(
> because adapter excepts it to be 4B sise) and therefor packed is
> used. I don't know the way to ensure size of 4 byte and alignment
> too. Or I am misunderstanding?
Yes, you are mis-understanding the rules for padding.. Structures are
only padded out to their alignment, which depends on their constituent
types. This is so arrays of structures have each array element
starting on its natural alignment.
The aligned attribute overrides the automatic determination of the
alignment based on the contents and just forces it.
So, as an example, if you have this hardware layout:
struct {
u32 fielda;
u64 fieldb;
} attribute ((aligned(4));
David is saying you will get a 12 byte struct and fieldb will be
unaligned. Since 12 is aligned to 4 no padding is added.
For hardware facing structures I'd combine this with a static assert
to verify structure size at compile time.
So..
1) Avoid using attributes unless the structure has unaligned members.
2) Avoid creating structures with unaligned members (eg for userspace
communication)
3) Frown at hardware/firmware developers who make communication
structures with unaligned members :)
4) Be explicit about padding in your layout for 64/32
compatibility.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
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