* Re: [PATCH v2] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Wolfgang Grandegger @ 2012-08-07 6:26 UTC (permalink / raw)
To: Olivier Sobrie; +Cc: Marc Kleine-Budde, linux-can, linux-usb, netdev
In-Reply-To: <1344230489-16595-1-git-send-email-olivier@sobrie.be>
On 08/06/2012 07:21 AM, Olivier Sobrie wrote:
> This driver provides support for several Kvaser CAN/USB devices.
> Such kind of devices supports up to three can network interfaces.
s/can/CAN/
> It has been tested with a Kvaser USB Leaf Light (one network interface)
> connected to a pch_can interface.
> The firmware version of the Kvaser device was 2.5.205.
>
> List of Kvaser devices supported by the driver:
> - Kvaser Leaf prototype (P010v2 and v3)
> - Kvaser Leaf Light (P010v3)
> - Kvaser Leaf Professional HS
> - Kvaser Leaf SemiPro HS
> - Kvaser Leaf Professional LS
> - Kvaser Leaf Professional SWC
> - Kvaser Leaf Professional LIN
> - Kvaser Leaf SemiPro LS
> - Kvaser Leaf SemiPro SWC
> - Kvaser Memorator II, Prototype
> - Kvaser Memorator II HS/HS
> - Kvaser USBcan Professional HS/HS
> - Kvaser Leaf Light GI
> - Kvaser Leaf Professional HS (OBD-II connector)
> - Kvaser Memorator Professional HS/LS
> - Kvaser Leaf Light "China"
> - Kvaser BlackBird SemiPro
> - Kvaser OEM Mercury
> - Kvaser OEM Leaf
> - Kvaser USBcan R
Impressive list! What CAN controllers are used inside the devices? SJA1000?
> Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
> ---
> Changes since v1:
> - added copyrights
> - kvaser_usb.h merged into kvaser.c
> - added kvaser_usb_get_endpoints to find eindpoints instead of
> hardcoding their address
> - some cleanup and comestic changes
> - fixed issues with errors handling
> - fixed restart-ms == 0 case
> - removed do_get_berr_counter method since the hardware doens't return
> good values for txerr and rxerr.
>
> If someone in the linux-usb mailing can review it, it would be nice.
>
> Concerning the errors, it behaves like that now:
>
> 1) Short-circuit CAN-H and CAN-L and restart-ms = 0
>
> t0: short-circuit + 'cansend can1 123#112233'
>
> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-warning,tx-error-warning}
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-error
> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-passive,tx-error-passive}
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-error
> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-off
> bus-error
>
> t1: remove short-circuit + 'ip link set can1 type can restart'
>
> can1 20000100 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
> restarted-after-bus-off
> can1 20000004 [8] 00 0C 00 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-warning,tx-error-warning}
Why do we get the last error message? Maybe the firmware does it that
way (going down passive->warning->active).
> 2) Short-circuit CAN-H and CAN-L and restart-ms = 100
>
> t0: short-circuit + cansend can1 123#112233
>
> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-warning,tx-error-warning}
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-error
> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-passive,tx-error-passive}
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-error
> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-off
> bus-error
> can1 2000018C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-warning,tx-error-warning}
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-error
> restarted-after-bus-off
> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-warning,tx-error-warning}
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-error
> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-passive,tx-error-passive}
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-error
> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-off
> bus-error
> ...
>
> can1 2000018C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-warning,tx-error-warning}
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-error
> restarted-after-bus-off
> can1 2000008C [8] 00 0C 90 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-warning,tx-error-warning}
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-error
> can1 2000008C [8] 00 30 90 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-passive,tx-error-passive}
> protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> bus-error
>
> t1: remove short-circuit
>
> can1 123 [3] 11 22 33
> can1 20000008 [8] 00 00 40 00 00 00 00 00 ERRORFRAME
> protocol-violation{{back-to-error-active}{}}
The order is still inverted but likely the firmware is doing it that way.
> 3) CAN-H and CAN-L disconnected
>
> t0: CAN-H and CAN-L disconnected + cansend can1 123#112233
>
> can1 20000004 [8] 00 30 00 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-passive,tx-error-passive}
> can1 2000008C [8] 00 30 80 1B 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-passive,tx-error-passive}
> protocol-violation{{error-on-tx}{acknowledge-delimiter}}
> bus-error
>
> t1: CAN-H and CAN-L reconnected
>
> can1 123 [3] 11 22 33
> can1 20000004 [8] 00 30 00 00 00 00 00 00 ERRORFRAME
> controller-problem{rx-error-passive,tx-error-passive}
Why do we get an error-passive message? Now I will have a closer look to
the code...
> Let me know if it is correct.
>
> Thanks,
>
> Olivier
>
> drivers/net/can/usb/Kconfig | 33 +
> drivers/net/can/usb/Makefile | 1 +
> drivers/net/can/usb/kvaser_usb.c | 1437 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1471 insertions(+)
> create mode 100644 drivers/net/can/usb/kvaser_usb.c
>
> diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
> index 0a68768..578955f 100644
> --- a/drivers/net/can/usb/Kconfig
> +++ b/drivers/net/can/usb/Kconfig
> @@ -13,6 +13,39 @@ config CAN_ESD_USB2
> This driver supports the CAN-USB/2 interface
> from esd electronic system design gmbh (http://www.esd.eu).
>
> +config CAN_KVASER_USB
> + tristate "Kvaser CAN/USB interface"
> + ---help---
> + This driver adds support for Kvaser CAN/USB devices like Kvaser
> + Leaf Light.
> +
> + The driver gives support for the following devices:
> + - Kvaser Leaf prototype (P010v2 and v3)
> + - Kvaser Leaf Light (P010v3)
> + - Kvaser Leaf Professional HS
> + - Kvaser Leaf SemiPro HS
> + - Kvaser Leaf Professional LS
> + - Kvaser Leaf Professional SWC
> + - Kvaser Leaf Professional LIN
> + - Kvaser Leaf SemiPro LS
> + - Kvaser Leaf SemiPro SWC
> + - Kvaser Memorator II, Prototype
> + - Kvaser Memorator II HS/HS
> + - Kvaser USBcan Professional HS/HS
> + - Kvaser Leaf Light GI
> + - Kvaser Leaf Professional HS (OBD-II connector)
> + - Kvaser Memorator Professional HS/LS
> + - Kvaser Leaf Light "China"
> + - Kvaser BlackBird SemiPro
> + - Kvaser OEM Mercury
> + - Kvaser OEM Leaf
> + - Kvaser USBcan R
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called kvaser_usb.
> +
> config CAN_PEAK_USB
> tristate "PEAK PCAN-USB/USB Pro interfaces"
> ---help---
> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
> index da6d1d3..80a2ee4 100644
> --- a/drivers/net/can/usb/Makefile
> +++ b/drivers/net/can/usb/Makefile
> @@ -4,6 +4,7 @@
>
> obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
> obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o
> +obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
> obj-$(CONFIG_CAN_PEAK_USB) += peak_usb/
>
> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> new file mode 100644
> index 0000000..eea0655
> --- /dev/null
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -0,0 +1,1437 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * Parts of this driver are based on the following:
> + * - Kvaser linux leaf driver (version 4.78)
> + * - CAN driver for esd CAN-USB/2
> + *
> + * Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved.
> + * Copyright (C) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
> + * Copyright (C) 2012 Olivier Sobrie <olivier@sobrie.be>
> + */
> +
> +#include <linux/init.h>
> +#include <linux/completion.h>
> +#include <linux/module.h>
> +#include <linux/netdevice.h>
> +#include <linux/usb.h>
> +
> +#include <linux/can.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +
> +#define MAX_TX_URBS 16
> +#define MAX_RX_URBS 4
> +#define START_TIMEOUT 1000
> +#define STOP_TIMEOUT 1000
> +#define USB_SEND_TIMEOUT 1000
> +#define USB_RECV_TIMEOUT 1000
A comment on the units would be nice!
> +#define RX_BUFFER_SIZE 3072
> +#define CAN_USB_CLOCK 8000000
> +#define MAX_NET_DEVICES 3
> +
> +/* Kvaser USB devices */
> +#define KVASER_VENDOR_ID 0x0bfd
> +#define USB_LEAF_DEVEL_PRODUCT_ID 10
> +#define USB_LEAF_LITE_PRODUCT_ID 11
> +#define USB_LEAF_PRO_PRODUCT_ID 12
> +#define USB_LEAF_SPRO_PRODUCT_ID 14
> +#define USB_LEAF_PRO_LS_PRODUCT_ID 15
> +#define USB_LEAF_PRO_SWC_PRODUCT_ID 16
> +#define USB_LEAF_PRO_LIN_PRODUCT_ID 17
> +#define USB_LEAF_SPRO_LS_PRODUCT_ID 18
> +#define USB_LEAF_SPRO_SWC_PRODUCT_ID 19
> +#define USB_MEMO2_DEVEL_PRODUCT_ID 22
> +#define USB_MEMO2_HSHS_PRODUCT_ID 23
> +#define USB_UPRO_HSHS_PRODUCT_ID 24
> +#define USB_LEAF_LITE_GI_PRODUCT_ID 25
> +#define USB_LEAF_PRO_OBDII_PRODUCT_ID 26
> +#define USB_MEMO2_HSLS_PRODUCT_ID 27
> +#define USB_LEAF_LITE_CH_PRODUCT_ID 28
> +#define USB_BLACKBIRD_SPRO_PRODUCT_ID 29
> +#define USB_OEM_MERCURY_PRODUCT_ID 34
> +#define USB_OEM_LEAF_PRODUCT_ID 35
> +#define USB_CAN_R_PRODUCT_ID 39
> +
> +/* USB devices features */
> +#define KVASER_HAS_SILENT_MODE BIT(0)
> +
> +/* Message header size */
> +#define MSG_HEADER_LEN 2
> +
> +/* Can message flags */
> +#define MSG_FLAG_ERROR_FRAME BIT(0)
> +#define MSG_FLAG_OVERRUN BIT(1)
> +#define MSG_FLAG_NERR BIT(2)
> +#define MSG_FLAG_WAKEUP BIT(3)
> +#define MSG_FLAG_REMOTE_FRAME BIT(4)
> +#define MSG_FLAG_RESERVED BIT(5)
> +#define MSG_FLAG_TX_ACK BIT(6)
> +#define MSG_FLAG_TX_REQUEST BIT(7)
> +
> +/* Can states */
> +#define M16C_STATE_BUS_RESET BIT(0)
> +#define M16C_STATE_BUS_ERROR BIT(4)
> +#define M16C_STATE_BUS_PASSIVE BIT(5)
> +#define M16C_STATE_BUS_OFF BIT(6)
> +
> +/* Can msg ids */
> +#define CMD_RX_STD_MESSAGE 12
> +#define CMD_TX_STD_MESSAGE 13
> +#define CMD_RX_EXT_MESSAGE 14
> +#define CMD_TX_EXT_MESSAGE 15
> +#define CMD_SET_BUS_PARAMS 16
> +#define CMD_GET_BUS_PARAMS 17
> +#define CMD_GET_BUS_PARAMS_REPLY 18
> +#define CMD_GET_CHIP_STATE 19
> +#define CMD_CHIP_STATE_EVENT 20
> +#define CMD_SET_CTRL_MODE 21
> +#define CMD_GET_CTRL_MODE 22
> +#define CMD_GET_CTRL_MODE_REPLY 23
> +#define CMD_RESET_CHIP 24
> +#define CMD_RESET_CARD 25
> +#define CMD_START_CHIP 26
> +#define CMD_START_CHIP_REPLY 27
> +#define CMD_STOP_CHIP 28
> +#define CMD_STOP_CHIP_REPLY 29
> +#define CMD_GET_CARD_INFO2 32
> +#define CMD_GET_CARD_INFO 34
> +#define CMD_GET_CARD_INFO_REPLY 35
> +#define CMD_GET_SOFTWARE_INFO 38
> +#define CMD_GET_SOFTWARE_INFO_REPLY 39
> +#define CMD_ERROR_EVENT 45
> +#define CMD_FLUSH_QUEUE 48
> +#define CMD_RESET_ERROR_COUNTER 49
> +#define CMD_TX_ACKNOWLEDGE 50
> +#define CMD_CAN_ERROR_EVENT 51
> +#define CMD_USB_THROTTLE 77
> +
> +/* error factors */
> +#define M16C_EF_ACKE BIT(0)
> +#define M16C_EF_CRCE BIT(1)
> +#define M16C_EF_FORME BIT(2)
> +#define M16C_EF_STFE BIT(3)
> +#define M16C_EF_BITE0 BIT(4)
> +#define M16C_EF_BITE1 BIT(5)
> +#define M16C_EF_RCVE BIT(6)
> +#define M16C_EF_TRE BIT(7)
> +
> +/* bittiming parameters */
> +#define KVASER_USB_TSEG1_MIN 1
> +#define KVASER_USB_TSEG1_MAX 16
> +#define KVASER_USB_TSEG2_MIN 1
> +#define KVASER_USB_TSEG2_MAX 8
> +#define KVASER_USB_SJW_MAX 4
> +#define KVASER_USB_BRP_MIN 1
> +#define KVASER_USB_BRP_MAX 64
> +#define KVASER_USB_BRP_INC 1
> +
> +/* ctrl modes */
> +#define KVASER_CTRL_MODE_NORMAL 1
> +#define KVASER_CTRL_MODE_SILENT 2
> +#define KVASER_CTRL_MODE_SELFRECEPTION 3
> +#define KVASER_CTRL_MODE_OFF 4
> +
> +struct kvaser_msg_simple {
> + u8 tid;
> + u8 channel;
> +} __packed;
> +
> +struct kvaser_msg_cardinfo {
> + u8 tid;
> + u8 nchannels;
> + __le32 serial_number;
> + __le32 padding;
> + __le32 clock_resolution;
> + __le32 mfgdate;
> + u8 ean[8];
> + u8 hw_revision;
> + u8 usb_hs_mode;
> + __le16 padding2;
> +} __packed;
> +
> +struct kvaser_msg_cardinfo2 {
> + u8 tid;
> + u8 channel;
> + u8 pcb_id[24];
> + __le32 oem_unlock_code;
> +} __packed;
> +
> +struct kvaser_msg_softinfo {
> + u8 tid;
> + u8 channel;
> + __le32 sw_options;
> + __le32 fw_version;
> + __le16 max_outstanding_tx;
> + __le16 padding[9];
> +} __packed;
> +
> +struct kvaser_msg_busparams {
> + u8 tid;
> + u8 channel;
> + __le32 bitrate;
> + u8 tseg1;
> + u8 tseg2;
> + u8 sjw;
> + u8 no_samp;
> +} __packed;
> +
> +struct kvaser_msg_tx_can {
> + u8 channel;
> + u8 tid;
> + u8 msg[14];
> + u8 padding;
> + u8 flags;
> +} __packed;
> +
> +struct kvaser_msg_rx_can {
> + u8 channel;
> + u8 flag;
> + __le16 time[3];
> + u8 msg[14];
> +} __packed;
> +
> +struct kvaser_msg_chip_state_event {
> + u8 tid;
> + u8 channel;
> + __le16 time[3];
> + u8 tx_errors_count;
> + u8 rx_errors_count;
> + u8 status;
> + u8 padding[3];
> +} __packed;
> +
> +struct kvaser_msg_tx_acknowledge {
> + u8 channel;
> + u8 tid;
> + __le16 time[3];
> + u8 flags;
> + u8 time_offset;
> +} __packed;
> +
> +struct kvaser_msg_error_event {
> + u8 tid;
> + u8 flags;
> + __le16 time[3];
> + u8 channel;
> + u8 padding;
> + u8 tx_errors_count;
> + u8 rx_errors_count;
> + u8 status;
> + u8 error_factor;
> +} __packed;
> +
> +struct kvaser_msg_ctrl_mode {
> + u8 tid;
> + u8 channel;
> + u8 ctrl_mode;
> + u8 padding[3];
> +} __packed;
> +
> +struct kvaser_msg_flush_queue {
> + u8 tid;
> + u8 channel;
> + u8 flags;
> + u8 padding[3];
> +} __packed;
> +
> +struct kvaser_msg {
> + u8 len;
> + u8 id;
> + union {
> + struct kvaser_msg_simple simple;
> + struct kvaser_msg_cardinfo cardinfo;
> + struct kvaser_msg_cardinfo2 cardinfo2;
> + struct kvaser_msg_softinfo softinfo;
> + struct kvaser_msg_busparams busparams;
> + struct kvaser_msg_tx_can tx_can;
> + struct kvaser_msg_rx_can rx_can;
> + struct kvaser_msg_chip_state_event chip_state_event;
> + struct kvaser_msg_tx_acknowledge tx_acknowledge;
> + struct kvaser_msg_error_event error_event;
> + struct kvaser_msg_ctrl_mode ctrl_mode;
> + struct kvaser_msg_flush_queue flush_queue;
> + } u;
> +} __packed;
> +
> +struct kvaser_usb_tx_urb_context {
> + struct kvaser_usb_net_priv *priv;
> + u32 echo_index;
> + int dlc;
> +};
> +
> +struct kvaser_usb {
> + struct usb_device *udev;
> + struct kvaser_usb_net_priv *nets[MAX_NET_DEVICES];
> +
> + struct usb_endpoint_descriptor *bulk_in, *bulk_out;
> + struct usb_anchor rx_submitted;
> +
> + u32 fw_version;
> + unsigned int nchannels;
> +
> + bool rxinitdone;
> +};
> +
> +struct kvaser_usb_net_priv {
> + struct can_priv can;
> +
> + atomic_t active_tx_urbs;
> + struct usb_anchor tx_submitted;
> + struct kvaser_usb_tx_urb_context tx_contexts[MAX_TX_URBS];
> +
> + struct completion start_comp, stop_comp;
> +
> + struct kvaser_usb *dev;
> + struct net_device *netdev;
> + int channel;
> +
> + struct can_frame cf_err_old;
> +};
> +
> +static struct usb_device_id kvaser_usb_table[] = {
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_SWC_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LIN_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_LS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_SWC_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_DEVEL_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSHS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_UPRO_HSHS_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_GI_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_OBDII_PRODUCT_ID),
> + .driver_info = KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSLS_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_CH_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_BLACKBIRD_SPRO_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_MERCURY_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_LEAF_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_CAN_R_PRODUCT_ID) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
> +
> +static inline int kvaser_usb_send_msg(const struct kvaser_usb *dev,
> + struct kvaser_msg *msg)
> +{
> + int actual_len;
> +
> + return usb_bulk_msg(dev->udev,
> + usb_sndbulkpipe(dev->udev,
> + dev->bulk_out->bEndpointAddress),
> + msg, msg->len, &actual_len,
> + USB_SEND_TIMEOUT);
> +}
> +
> +static int kvaser_usb_wait_msg(const struct kvaser_usb *dev, u8 id,
> + struct kvaser_msg *msg)
> +{
> + struct kvaser_msg *tmp;
> + void *buf;
> + int actual_len;
> + int err;
> + int pos = 0;
> +
> + buf = kzalloc(RX_BUFFER_SIZE, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + err = usb_bulk_msg(dev->udev,
> + usb_rcvbulkpipe(dev->udev,
> + dev->bulk_in->bEndpointAddress),
> + buf, RX_BUFFER_SIZE, &actual_len,
> + USB_RECV_TIMEOUT);
> + if (err < 0) {
> + kfree(buf);
> + return err;
> + }
goto might be more elegant/efficient here.
> +
> + while (pos <= actual_len - MSG_HEADER_LEN) {
> + tmp = buf + pos;
> +
> + if (!tmp->len)
> + break;
> +
> + if (pos + tmp->len > actual_len) {
> + dev_err(dev->udev->dev.parent, "Format error\n");
> + break;
> + }
> +
> + if (tmp->id == id) {
> + memcpy(msg, tmp, tmp->len);
> + kfree(buf);
> + return 0;
> + }
> +
> + pos += tmp->len;
> + }
> +
> + kfree(buf);
> +
> + return -EINVAL;
> +}
> +
> +static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
> + u8 msg_id, int channel)
> +{
> + struct kvaser_msg msg = {
> + .len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple),
> + .id = msg_id,
> + .u.simple.channel = channel,
> + .u.simple.tid = 0xff,
> + };
> +
> + return kvaser_usb_send_msg(dev, &msg);
> +}
> +
> +static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
> +{
> + struct kvaser_msg msg;
> + int err;
> +
> + err = kvaser_usb_send_simple_msg(dev, CMD_GET_SOFTWARE_INFO, 0);
> + if (err)
> + return err;
> +
> + err = kvaser_usb_wait_msg(dev, CMD_GET_SOFTWARE_INFO_REPLY, &msg);
> + if (err)
> + return err;
> +
> + dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
> +{
> + struct kvaser_msg msg;
> + int err;
> +
> + err = kvaser_usb_send_simple_msg(dev, CMD_GET_CARD_INFO, 0);
> + if (err)
> + return err;
> +
> + err = kvaser_usb_wait_msg(dev, CMD_GET_CARD_INFO_REPLY, &msg);
> + if (err)
> + return err;
> +
> + dev->nchannels = msg.u.cardinfo.nchannels;
> +
> + return 0;
> +}
> +
> +static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct net_device_stats *stats;
> + struct kvaser_usb_tx_urb_context *context;
> + struct kvaser_usb_net_priv *priv;
> + u8 channel = msg->u.tx_acknowledge.channel;
> + u8 tid = msg->u.tx_acknowledge.tid;
> +
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
> +
> + priv = dev->nets[channel];
> +
> + if (!netif_device_present(priv->netdev))
> + return;
> +
> + stats = &priv->netdev->stats;
> +
> + context = &priv->tx_contexts[tid % MAX_TX_URBS];
> +
> + /*
> + * It looks like the firmware never sets the flags field of the
> + * tx_acknowledge frame and never reports a transmit failure.
> + * If the can message can't be transmited (e.g. incompatible
> + * bitrates), a frame CMD_CAN_ERROR_EVENT is sent (with a null
> + * tid) and the firmware tries to transmit again the packet until
> + * it succeeds. Once the packet is successfully transmitted, then
> + * the tx_acknowledge frame is sent.
> + */
> +
> + stats->tx_packets++;
> + stats->tx_bytes += context->dlc;
> + can_get_echo_skb(priv->netdev, context->echo_index);
> +
> + context->echo_index = MAX_TX_URBS;
> + atomic_dec(&priv->active_tx_urbs);
> +
> + netif_wake_queue(priv->netdev);
> +}
> +
> +static void kvaser_usb_simple_msg_callback(struct urb *urb)
> +{
> + struct net_device *netdev = urb->context;
> +
> + if (urb->status)
> + netdev_warn(netdev, "urb status received: %d\n",
> + urb->status);
> +}
> +
> +static int kvaser_usb_simple_msg_async(struct kvaser_usb_net_priv *priv,
> + u8 msg_id)
> +{
> + struct kvaser_usb *dev = priv->dev;
> + struct net_device *netdev = priv->netdev;
> + struct kvaser_msg *msg;
> + struct urb *urb;
> + void *buf;
> + int err;
> +
> + urb = usb_alloc_urb(0, GFP_ATOMIC);
> + if (!urb) {
> + netdev_err(netdev, "No memory left for URBs\n");
> + return -ENOMEM;
> + }
> +
> + buf = usb_alloc_coherent(dev->udev, sizeof(struct kvaser_msg),
> + GFP_ATOMIC, &urb->transfer_dma);
> + if (!buf) {
> + netdev_err(netdev, "No memory left for USB buffer\n");
> + return -ENOMEM;
> + }
> +
> + msg = (struct kvaser_msg *)buf;
> + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple);
> + msg->id = msg_id;
> + msg->u.simple.channel = priv->channel;
> +
> + usb_fill_bulk_urb(urb, dev->udev,
> + usb_sndbulkpipe(dev->udev,
> + dev->bulk_out->bEndpointAddress),
> + buf, msg->len,
> + kvaser_usb_simple_msg_callback, priv);
> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> + usb_anchor_urb(urb, &priv->tx_submitted);
> +
> + err = usb_submit_urb(urb, GFP_ATOMIC);
> + if (err) {
> + netdev_err(netdev, "Error transmitting URB\n");
> + usb_unanchor_urb(urb);
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> +{
> + int i;
> +
> + usb_kill_anchored_urbs(&priv->tx_submitted);
> + atomic_set(&priv->active_tx_urbs, 0);
> +
> + for (i = 0; i < MAX_TX_URBS; i++)
> + priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> +}
> +
> +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + struct net_device_stats *stats;
> + struct kvaser_usb_net_priv *priv;
> + unsigned int new_state;
> + u8 channel, status;
> +
> + if (msg->id == CMD_CAN_ERROR_EVENT) {
> + channel = msg->u.error_event.channel;
> + status = msg->u.error_event.status;
> + } else {
> + channel = msg->u.chip_state_event.channel;
> + status = msg->u.chip_state_event.status;
> + }
> +
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
> +
> + priv = dev->nets[channel];
> + stats = &priv->netdev->stats;
> +
> + if (status & M16C_STATE_BUS_RESET) {
> + kvaser_usb_unlink_tx_urbs(priv);
> + return;
> + }
> + skb = alloc_can_err_skb(priv->netdev, &cf);
> + if (!skb) {
> + stats->rx_dropped++;
> + return;
Cleanup? kvaser_usb_unlink_tx_urbs()?
> + }
> +
> + if (status & M16C_STATE_BUS_OFF) {
> + cf->can_id |= CAN_ERR_BUSOFF;
> +
> + if (!priv->can.restart_ms)
> + kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> +
> + if (!priv->can.state != CAN_ERR_BUSOFF) {
> + priv->can.can_stats.bus_off++;
> + netif_carrier_off(priv->netdev);
> + }
> +
> + new_state = CAN_STATE_BUS_OFF;
> + } else if (status & M16C_STATE_BUS_PASSIVE) {
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
> + CAN_ERR_CRTL_RX_PASSIVE;
State changes should only be report when the state really changes.
Therefore it should go under the if block below.
> + if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
> + priv->can.can_stats.error_passive++;
> +
> + new_state = CAN_STATE_ERROR_PASSIVE;
> + } else if (status & M16C_STATE_BUS_ERROR) {
Hm, strange, a bus error is not a state change. You use here if...else
if... Isn't it possible that more than one bit is set.
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] = CAN_ERR_CRTL_TX_WARNING |
> + CAN_ERR_CRTL_RX_WARNING;
See above.
> + if (priv->can.state != CAN_STATE_ERROR_WARNING)
> + priv->can.can_stats.error_warning++;
> +
> + new_state = CAN_STATE_ERROR_WARNING;
> + } else {
> + cf->can_id |= CAN_ERR_PROT;
> + cf->data[2] = CAN_ERR_PROT_ACTIVE;
> +
> + new_state = CAN_STATE_ERROR_ACTIVE;
> + }
> +
> + if (priv->can.restart_ms &&
> + (priv->can.state >= CAN_STATE_BUS_OFF) &&
> + (new_state < CAN_STATE_BUS_OFF)) {
> + cf->can_id |= CAN_ERR_RESTARTED;
> + priv->can.can_stats.restarts++;
> + netif_carrier_on(priv->netdev);
> + }
> +
> + if (msg->id == CMD_CAN_ERROR_EVENT) {
> + u8 error_factor = msg->u.error_event.error_factor;
> +
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> +
> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +
> + if (error_factor & M16C_EF_ACKE)
> + cf->data[3] |= (CAN_ERR_PROT_LOC_ACK |
> + CAN_ERR_PROT_LOC_ACK_DEL);
> + if (error_factor & M16C_EF_CRCE)
> + cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> + CAN_ERR_PROT_LOC_CRC_DEL);
> + if (error_factor & M16C_EF_FORME)
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + if (error_factor & M16C_EF_STFE)
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + if (error_factor & M16C_EF_BITE0)
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> + if (error_factor & M16C_EF_BITE1)
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> + if (error_factor & M16C_EF_TRE)
> + cf->data[2] |= CAN_ERR_PROT_TX;
> + }
> +
> + priv->can.state = new_state;
> +
> + if (!memcmp(cf, &priv->cf_err_old, sizeof(*cf))) {
> + kfree_skb(skb);
> + return;
> + }
Hm, the firmware seems not to clear error conditions? Anyway, state
change and error reporting is magic on many CAN controllers. Just the
SJA1000 is doing it nicely.
> + netif_rx(skb);
> +
> + priv->cf_err_old = *cf;
> +
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct kvaser_usb_net_priv *priv;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + struct net_device_stats *stats;
> + u8 channel = msg->u.rx_can.channel;
> +
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
> +
> + priv = dev->nets[channel];
> + stats = &priv->netdev->stats;
> +
> + skb = alloc_can_skb(priv->netdev, &cf);
> + if (skb == NULL) {
s/skb == NULL)/!skb/ ?
> + stats->tx_dropped++;
> + return;
> + }
> +
> + cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
> + (msg->u.rx_can.msg[1] & 0x3f);
> + cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
> +
> + if (msg->id == CMD_RX_EXT_MESSAGE) {
> + cf->can_id <<= 18;
> + cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
> + ((msg->u.rx_can.msg[3] & 0xff) << 6) |
> + (msg->u.rx_can.msg[4] & 0x3f);
> + cf->can_id |= CAN_EFF_FLAG;
> + }
> +
> + if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME) {
> + cf->can_id |= CAN_RTR_FLAG;
> + } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> + MSG_FLAG_NERR)) {
> + cf->can_id |= CAN_ERR_FLAG;
> + cf->can_dlc = CAN_ERR_DLC;
> + cf->data[1] = CAN_ERR_CRTL_UNSPEC;
What is the meaning of such errors? A comment, netdev_err() or
netdev_dbg() would be nice.
> +
> + stats->rx_errors++;
> + } else if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> + cf->can_id = CAN_ERR_FLAG | CAN_ERR_CRTL;
> + cf->can_dlc = CAN_ERR_DLC;
> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> +
> + stats->rx_over_errors++;
> + stats->rx_errors++;
> + } else if (!msg->u.rx_can.flag) {
> + memcpy(cf->data, &msg->u.rx_can.msg[6], cf->can_dlc);
> + } else {
> + kfree_skb(skb);
> + return;
> + }
> +
> + netif_rx(skb);
> +
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static void kvaser_usb_start_chip_reply(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct kvaser_usb_net_priv *priv;
> + u8 channel = msg->u.simple.channel;
> +
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
> +
> + priv = dev->nets[channel];
> +
> + if (completion_done(&priv->start_comp) &&
> + netif_queue_stopped(priv->netdev)) {
> + netif_wake_queue(priv->netdev);
> + } else {
> + netif_start_queue(priv->netdev);
> + complete(&priv->start_comp);
> + }
> +}
> +
> +static void kvaser_usb_stop_chip_reply(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct kvaser_usb_net_priv *priv;
> + u8 channel = msg->u.simple.channel;
> +
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
> +
> + priv = dev->nets[channel];
> +
> + complete(&priv->stop_comp);
> +}
> +
> +static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + switch (msg->id) {
> + case CMD_START_CHIP_REPLY:
> + kvaser_usb_start_chip_reply(dev, msg);
> + break;
> +
> + case CMD_STOP_CHIP_REPLY:
> + kvaser_usb_stop_chip_reply(dev, msg);
> + break;
> +
> + case CMD_RX_STD_MESSAGE:
> + case CMD_RX_EXT_MESSAGE:
> + kvaser_usb_rx_can_msg(dev, msg);
> + break;
> +
> + case CMD_CHIP_STATE_EVENT:
> + case CMD_CAN_ERROR_EVENT:
> + kvaser_usb_rx_error(dev, msg);
> + break;
> +
> + case CMD_TX_ACKNOWLEDGE:
> + kvaser_usb_tx_acknowledge(dev, msg);
> + break;
> +
> + default:
> + dev_warn(dev->udev->dev.parent,
> + "Unhandled message (%d)\n", msg->id);
> + break;
> + }
> +}
> +
> +static void kvaser_usb_read_bulk_callback(struct urb *urb)
> +{
> + struct kvaser_usb *dev = urb->context;
> + struct kvaser_msg *msg;
> + int pos = 0;
> + int err, i;
> +
> + switch (urb->status) {
> + case 0:
> + break;
> + case -ENOENT:
> + case -ESHUTDOWN:
> + return;
> + default:
> + dev_info(dev->udev->dev.parent, "Rx URB aborted (%d)\n",
> + urb->status);
> + goto resubmit_urb;
> + }
> +
> + while (pos <= urb->actual_length - MSG_HEADER_LEN) {
> + msg = urb->transfer_buffer + pos;
> +
> + if (!msg->len)
> + break;
> +
> + if (pos + msg->len > urb->actual_length) {
> + dev_err(dev->udev->dev.parent, "Format error\n");
> + break;
> + }
> +
> + kvaser_usb_handle_message(dev, msg);
> +
> + pos += msg->len;
> + }
> +
> +resubmit_urb:
> + usb_fill_bulk_urb(urb, dev->udev,
> + usb_rcvbulkpipe(dev->udev,
> + dev->bulk_in->bEndpointAddress),
> + urb->transfer_buffer, RX_BUFFER_SIZE,
> + kvaser_usb_read_bulk_callback, dev);
> +
> + err = usb_submit_urb(urb, GFP_ATOMIC);
> + if (err == -ENODEV) {
> + for (i = 0; i < dev->nchannels; i++) {
> + if (!dev->nets[i])
> + continue;
> +
> + netif_device_detach(dev->nets[i]->netdev);
> + }
> + } else if (err) {
> + dev_err(dev->udev->dev.parent,
> + "Failed resubmitting read bulk urb: %d\n", err);
> + }
> +
> + return;
> +}
> +
> +static int kvaser_usb_setup_rx_urbs(struct kvaser_usb *dev)
> +{
> + int i, err = 0;
> +
> + if (dev->rxinitdone)
> + return 0;
> +
> + for (i = 0; i < MAX_RX_URBS; i++) {
> + struct urb *urb = NULL;
> + u8 *buf = NULL;
> +
> + urb = usb_alloc_urb(0, GFP_KERNEL);
> + if (!urb) {
> + dev_warn(dev->udev->dev.parent,
> + "No memory left for URBs\n");
> + err = -ENOMEM;
> + break;
> + }
> +
> + buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE,
> + GFP_KERNEL, &urb->transfer_dma);
> + if (!buf) {
> + dev_warn(dev->udev->dev.parent,
> + "No memory left for USB buffer\n");
> + usb_free_urb(urb);
> + err = -ENOMEM;
> + break;
> + }
> +
> + usb_fill_bulk_urb(urb, dev->udev,
> + usb_rcvbulkpipe(dev->udev,
> + dev->bulk_in->bEndpointAddress),
> + buf, RX_BUFFER_SIZE,
> + kvaser_usb_read_bulk_callback,
> + dev);
> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> + usb_anchor_urb(urb, &dev->rx_submitted);
> +
> + err = usb_submit_urb(urb, GFP_KERNEL);
> + if (err) {
> + usb_unanchor_urb(urb);
> + usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
> + urb->transfer_dma);
> + break;
> + }
> +
> + usb_free_urb(urb);
> + }
> +
> + if (i == 0) {
> + dev_warn(dev->udev->dev.parent,
> + "Cannot setup read URBs, error %d\n", err);
> + return err;
> + } else if (i < MAX_RX_URBS) {
> + dev_warn(dev->udev->dev.parent,
> + "RX performances may be slow\n");
> + }
> +
> + dev->rxinitdone = true;
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv)
> +{
> + struct kvaser_msg msg = {
> + .id = CMD_SET_CTRL_MODE,
> + .len = MSG_HEADER_LEN +
> + sizeof(struct kvaser_msg_ctrl_mode),
> + .u.ctrl_mode.tid = 0xff,
> + .u.ctrl_mode.channel = priv->channel,
> + };
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> + msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
> + else
> + msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
> +
> + return kvaser_usb_send_msg(priv->dev, &msg);
> +}
> +
> +static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv)
> +{
> + int err;
> +
> + init_completion(&priv->start_comp);
> +
> + err = kvaser_usb_send_simple_msg(priv->dev, CMD_START_CHIP,
> + priv->channel);
> + if (err)
> + return err;
> +
> + if (!wait_for_completion_timeout(&priv->start_comp,
> + msecs_to_jiffies(START_TIMEOUT)))
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_open(struct net_device *netdev)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> + struct kvaser_usb *dev = priv->dev;
> + int err;
> +
> + err = open_candev(netdev);
> + if (err)
> + return err;
> +
> + err = kvaser_usb_setup_rx_urbs(dev);
> + if (err)
> + return err;
close_candev()? Using "goto" is preferred for error handling.
> +
> + err = kvaser_usb_set_opt_mode(priv);
> + if (err)
> + return err;
> +
> + err = kvaser_usb_start_chip(priv);
> + if (err) {
> + netdev_warn(netdev, "Cannot start device, error %d\n", err);
> + close_candev(netdev);
> + return err;
> + }
> +
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> + return 0;
> +}
> +
> +static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
> +{
> + int i;
> +
> + usb_kill_anchored_urbs(&dev->rx_submitted);
> +
> + for (i = 0; i < MAX_NET_DEVICES; i++) {
> + struct kvaser_usb_net_priv *priv = dev->nets[i];
> +
> + if (priv)
> + kvaser_usb_unlink_tx_urbs(priv);
> + }
> +}
> +
> +static int kvaser_usb_stop_chip(struct kvaser_usb_net_priv *priv)
> +{
> + int err;
> +
> + init_completion(&priv->stop_comp);
> +
> + err = kvaser_usb_send_simple_msg(priv->dev, CMD_STOP_CHIP,
> + priv->channel);
> + if (err)
> + return err;
> +
> + if (!wait_for_completion_timeout(&priv->stop_comp,
> + msecs_to_jiffies(STOP_TIMEOUT)))
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv)
> +{
> + struct kvaser_msg msg = {
> + .id = CMD_FLUSH_QUEUE,
> + .len = MSG_HEADER_LEN +
> + sizeof(struct kvaser_msg_flush_queue),
> + .u.flush_queue.channel = priv->channel,
> + .u.flush_queue.flags = 0x00,
> + };
> +
> + return kvaser_usb_send_msg(priv->dev, &msg);
> +}
> +
> +static int kvaser_usb_close(struct net_device *netdev)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> + struct kvaser_usb *dev = priv->dev;
> + int err;
> +
> + netif_stop_queue(netdev);
> +
> + err = kvaser_usb_flush_queue(priv);
> + if (err)
> + netdev_warn(netdev, "Cannot flush queue, error %d\n", err);
> +
> + if (kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, priv->channel))
> + netdev_warn(netdev, "Cannot reset card, error %d\n", err);
> +
> + err = kvaser_usb_stop_chip(priv);
> + if (err) {
> + netdev_warn(netdev, "Cannot stop device, error %d\n", err);
> + return err;
close_candev()? Returning here is maybe not a good idea?
> + }
> +
> + priv->can.state = CAN_STATE_STOPPED;
> + close_candev(priv->netdev);
> +
> + return 0;
> +}
> +
> +static void kvaser_usb_write_bulk_callback(struct urb *urb)
> +{
> + struct kvaser_usb_tx_urb_context *context = urb->context;
> + struct kvaser_usb_net_priv *priv;
> + struct net_device *netdev;
> +
> + if (WARN_ON(!context))
> + return;
> +
> + priv = context->priv;
> + netdev = priv->netdev;
> +
> + usb_free_coherent(urb->dev, urb->transfer_buffer_length,
> + urb->transfer_buffer, urb->transfer_dma);
> +
> + if (!netif_device_present(netdev))
> + return;
> +
> + if (urb->status)
> + netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
> +}
> +
> +static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> + struct net_device *netdev)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> + struct kvaser_usb *dev = priv->dev;
> + struct net_device_stats *stats = &netdev->stats;
> + struct can_frame *cf = (struct can_frame *)skb->data;
> + struct kvaser_usb_tx_urb_context *context = NULL;
> + struct urb *urb;
> + void *buf;
> + struct kvaser_msg *msg;
> + int i, err;
> + int ret = NETDEV_TX_OK;
> +
> + if (can_dropped_invalid_skb(netdev, skb))
> + return NETDEV_TX_OK;
> +
> + urb = usb_alloc_urb(0, GFP_ATOMIC);
> + if (!urb) {
> + netdev_err(netdev, "No memory left for URBs\n");
> + stats->tx_dropped++;
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> + }
> +
> + buf = usb_alloc_coherent(dev->udev, sizeof(struct kvaser_msg),
> + GFP_ATOMIC, &urb->transfer_dma);
> + if (!buf) {
> + netdev_err(netdev, "No memory left for USB buffer\n");
> + stats->tx_dropped++;
> + dev_kfree_skb(skb);
> + goto nobufmem;
> + }
> +
> + msg = (struct kvaser_msg *)buf;
> + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
> + msg->u.tx_can.flags = 0;
> + msg->u.tx_can.channel = priv->channel;
> +
> + if (cf->can_id & CAN_EFF_FLAG) {
> + msg->id = CMD_TX_EXT_MESSAGE;
> + msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
> + msg->u.tx_can.msg[1] = (cf->can_id >> 18) & 0x3f;
> + msg->u.tx_can.msg[2] = (cf->can_id >> 14) & 0x0f;
> + msg->u.tx_can.msg[3] = (cf->can_id >> 6) & 0xff;
> + msg->u.tx_can.msg[4] = cf->can_id & 0x3f;
> + } else {
> + msg->id = CMD_TX_STD_MESSAGE;
> + msg->u.tx_can.msg[0] = (cf->can_id >> 6) & 0x1f;
> + msg->u.tx_can.msg[1] = cf->can_id & 0x3f;
> + }
> +
> + msg->u.tx_can.msg[5] = cf->can_dlc;
> + memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
> +
> + if (cf->can_id & CAN_RTR_FLAG)
> + msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
> +
> + for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) {
> + if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
> + context = &priv->tx_contexts[i];
> + break;
> + }
> + }
> +
> + if (!context) {
> + netdev_warn(netdev, "cannot find free context\n");
> + ret = NETDEV_TX_BUSY;
> + goto releasebuf;
> + }
> +
> + context->priv = priv;
> + context->echo_index = i;
> + context->dlc = cf->can_dlc;
> +
> + msg->u.tx_can.tid = context->echo_index;
> +
> + usb_fill_bulk_urb(urb, dev->udev,
> + usb_sndbulkpipe(dev->udev,
> + dev->bulk_out->bEndpointAddress),
> + buf, msg->len,
> + kvaser_usb_write_bulk_callback, context);
> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> + usb_anchor_urb(urb, &priv->tx_submitted);
> +
> + can_put_echo_skb(skb, netdev, context->echo_index);
> +
> + atomic_inc(&priv->active_tx_urbs);
> +
> + if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
> + netif_stop_queue(netdev);
> +
> + err = usb_submit_urb(urb, GFP_ATOMIC);
> + if (unlikely(err)) {
> + can_free_echo_skb(netdev, context->echo_index);
> +
> + atomic_dec(&priv->active_tx_urbs);
> + usb_unanchor_urb(urb);
> +
> + stats->tx_dropped++;
> +
> + if (err == -ENODEV)
> + netif_device_detach(netdev);
> + else
> + netdev_warn(netdev, "Failed tx_urb %d\n", err);
> +
> + goto releasebuf;
> + }
> +
> + netdev->trans_start = jiffies;
> +
> + usb_free_urb(urb);
> +
> + return NETDEV_TX_OK;
> +
> +releasebuf:
> + usb_free_coherent(dev->udev, sizeof(struct kvaser_msg),
> + buf, urb->transfer_dma);
> +nobufmem:
> + usb_free_urb(urb);
> + return ret;
> +}
> +
> +static const struct net_device_ops kvaser_usb_netdev_ops = {
> + .ndo_open = kvaser_usb_open,
> + .ndo_stop = kvaser_usb_close,
> + .ndo_start_xmit = kvaser_usb_start_xmit,
> +};
> +
> +static struct can_bittiming_const kvaser_usb_bittiming_const = {
> + .name = "kvaser_usb",
> + .tseg1_min = KVASER_USB_TSEG1_MIN,
> + .tseg1_max = KVASER_USB_TSEG1_MAX,
> + .tseg2_min = KVASER_USB_TSEG2_MIN,
> + .tseg2_max = KVASER_USB_TSEG2_MAX,
> + .sjw_max = KVASER_USB_SJW_MAX,
> + .brp_min = KVASER_USB_BRP_MIN,
> + .brp_max = KVASER_USB_BRP_MAX,
> + .brp_inc = KVASER_USB_BRP_INC,
> +};
> +
> +static int kvaser_usb_set_bittiming(struct net_device *netdev)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> + struct can_bittiming *bt = &priv->can.bittiming;
> + struct kvaser_usb *dev = priv->dev;
> + struct kvaser_msg msg = {
> + .id = CMD_SET_BUS_PARAMS,
> + .len = MSG_HEADER_LEN +
> + sizeof(struct kvaser_msg_busparams),
> + .u.busparams.channel = priv->channel,
> + .u.busparams.tid = 0xff,
> + .u.busparams.bitrate = cpu_to_le32(bt->bitrate),
> + .u.busparams.sjw = bt->sjw,
> + .u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1,
> + .u.busparams.tseg2 = bt->phase_seg2,
> + };
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
> + msg.u.busparams.no_samp = 3;
> + else
> + msg.u.busparams.no_samp = 1;
> +
> + return kvaser_usb_send_msg(dev, &msg);
> +}
> +
> +static int kvaser_usb_set_mode(struct net_device *netdev,
> + enum can_mode mode)
> +{
> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> + int err;
> +
> + switch (mode) {
> + case CAN_MODE_START:
> + err = kvaser_usb_simple_msg_async(priv, CMD_START_CHIP);
> + if (err)
> + return err;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int kvaser_usb_init_one(struct usb_interface *intf,
> + const struct usb_device_id *id, int channel)
> +{
> + struct kvaser_usb *dev = usb_get_intfdata(intf);
> + struct net_device *netdev;
> + struct kvaser_usb_net_priv *priv;
> + int i, err;
> +
> + netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
> + if (!netdev) {
> + dev_err(&intf->dev, "Cannot alloc candev\n");
> + return -ENOMEM;
> + }
> +
> + priv = netdev_priv(netdev);
> +
> + init_completion(&priv->start_comp);
> + init_completion(&priv->stop_comp);
> +
> + init_usb_anchor(&priv->tx_submitted);
> + atomic_set(&priv->active_tx_urbs, 0);
> +
> + for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++)
> + priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> +
> + priv->dev = dev;
> + priv->netdev = netdev;
> + priv->channel = channel;
> +
> + priv->can.state = CAN_STATE_STOPPED;
> + priv->can.clock.freq = CAN_USB_CLOCK;
> + priv->can.bittiming_const = &kvaser_usb_bittiming_const;
> + priv->can.do_set_bittiming = kvaser_usb_set_bittiming;
> + priv->can.do_set_mode = kvaser_usb_set_mode;
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
> + if (id->driver_info & KVASER_HAS_SILENT_MODE)
> + priv->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY;
> +
> + netdev->flags |= IFF_ECHO;
> +
> + netdev->netdev_ops = &kvaser_usb_netdev_ops;
> +
> + SET_NETDEV_DEV(netdev, &intf->dev);
> +
> + err = register_candev(netdev);
> + if (err) {
> + dev_err(&intf->dev, "Failed to register can device\n");
> + free_candev(netdev);
> + return err;
> + }
> +
> + dev->nets[channel] = priv;
> + netdev_dbg(netdev, "device registered\n");
> +
> + return 0;
> +}
> +
> +static void kvaser_usb_get_endpoints(const struct usb_interface *intf,
> + struct usb_endpoint_descriptor **in,
> + struct usb_endpoint_descriptor **out)
> +{
> + const struct usb_host_interface *iface_desc;
> + struct usb_endpoint_descriptor *endpoint;
> + int i;
> +
> + iface_desc = &intf->altsetting[0];
> +
> + for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
> + endpoint = &iface_desc->endpoint[i].desc;
> +
> + if (usb_endpoint_is_bulk_in(endpoint))
> + *in = endpoint;
> +
> + if (usb_endpoint_is_bulk_out(endpoint))
> + *out = endpoint;
> + }
> +}
> +
> +static int kvaser_usb_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> + struct kvaser_usb *dev;
> + int err = -ENOMEM;
> + int i;
> +
> + dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL);
> + if (!dev)
> + return -ENOMEM;
> +
> + kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
> + if (!dev->bulk_in || !dev->bulk_out) {
> + dev_err(&intf->dev, "Cannot get usb endpoint(s)");
> + return err;
> + }
> +
> + dev->udev = interface_to_usbdev(intf);
> +
> + init_usb_anchor(&dev->rx_submitted);
> +
> + usb_set_intfdata(intf, dev);
> +
> + for (i = 0; i < MAX_NET_DEVICES; i++)
> + kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
> +
> + err = kvaser_usb_get_software_info(dev);
> + if (err) {
> + dev_err(&intf->dev,
> + "Cannot get software infos, error %d\n", err);
> + return err;
> + }
> +
> + err = kvaser_usb_get_card_info(dev);
> + if (err) {
> + dev_err(&intf->dev,
> + "Cannot get card infos, error %d\n", err);
> + return err;
> + }
> +
> + dev_dbg(&intf->dev, "Firmware version: %d.%d.%d\n",
> + ((dev->fw_version >> 24) & 0xff),
> + ((dev->fw_version >> 16) & 0xff),
> + (dev->fw_version & 0xffff));
> +
> + for (i = 0; i < dev->nchannels; i++)
> + kvaser_usb_init_one(intf, id, i);
> +
> + return 0;
> +}
> +
> +static void kvaser_usb_disconnect(struct usb_interface *intf)
> +{
> + struct kvaser_usb *dev = usb_get_intfdata(intf);
> + int i;
> +
> + usb_set_intfdata(intf, NULL);
> +
> + if (!dev)
> + return;
> +
> + for (i = 0; i < dev->nchannels; i++) {
> + if (!dev->nets[i])
> + continue;
> +
> + unregister_netdev(dev->nets[i]->netdev);
> + free_candev(dev->nets[i]->netdev);
> + }
> +
> + kvaser_usb_unlink_all_urbs(dev);
> +}
> +
> +static struct usb_driver kvaser_usb_driver = {
> + .name = "kvaser_usb",
> + .probe = kvaser_usb_probe,
> + .disconnect = kvaser_usb_disconnect,
> + .id_table = kvaser_usb_table
> +};
> +
> +module_usb_driver(kvaser_usb_driver);
> +
> +MODULE_AUTHOR("Olivier Sobrie <olivier@sobrie.be>");
> +MODULE_DESCRIPTION("Can driver for Kvaser CAN/USB devices");
s/Can/CAN/
> +MODULE_LICENSE("GPL v2");
Wolfgang.
^ permalink raw reply
* [Resend PATCH 7/8] bridge: add some comments for NETDEV_RELEASE
From: Cong Wang @ 2012-08-07 5:59 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Miller, Stephen Hemminger
In-Reply-To: <20120806.135003.442648448779232464.davem@davemloft.net>
Add comments on why we don't notify NETDEV_RELEASE.
Cc: David Miller <davem@davemloft.net>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 171fd6b..bf47d4f 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -427,6 +427,9 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
if (!p || p->br != br)
return -EINVAL;
+ /* We don't notify NETDEV_RELEASE event, as this will
+ * stop netconsole on the bridge.
+ */
del_nbp(p);
spin_lock_bh(&br->lock);
^ permalink raw reply related
* [PATCH][XFRM] Replace rwlock on xfrm_policy_afinfo with rcu
From: Priyanka Jain @ 2012-08-07 5:21 UTC (permalink / raw)
To: netdev; +Cc: Priyanka Jain
xfrm_policy_afinfo is read mosly data structure.
Write on xfrm_policy_afinfo is done only at the
time of configuration.
So rwlocks can be safely replaced with RCU.
RCUs usage optimizes the performance.
Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com>
---
For IPSEC fwd test
-On p4080ds (8-core, SMP system)
Around 110% throughput increase in case of PREEMPT_RT enabled
Around 5-6% throughput increase in case of PREEMPT_RT disabled
-On p2020 (2-core, SMP system)
Around 4-5% throughput increase in case of PREEMPT_RT disabled
net/xfrm/xfrm_policy.c | 37 +++++++++++++++++++++----------------
1 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 5082bcf..0ca3258 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -41,13 +41,14 @@ static DEFINE_SPINLOCK(xfrm_policy_sk_bundle_lock);
static struct dst_entry *xfrm_policy_sk_bundles;
static DEFINE_RWLOCK(xfrm_policy_lock);
-static DEFINE_RWLOCK(xfrm_policy_afinfo_lock);
-static struct xfrm_policy_afinfo *xfrm_policy_afinfo[NPROTO];
+static DEFINE_SPINLOCK(xfrm_policy_afinfo_lock);
+static struct xfrm_policy_afinfo __rcu * xfrm_policy_afinfo[NPROTO]
+ __read_mostly;
static struct kmem_cache *xfrm_dst_cache __read_mostly;
static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family);
-static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo);
+static inline void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo);
static void xfrm_init_pmtu(struct dst_entry *dst);
static int stale_bundle(struct dst_entry *dst);
static int xfrm_bundle_ok(struct xfrm_dst *xdst);
@@ -2436,7 +2437,7 @@ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
return -EINVAL;
if (unlikely(afinfo->family >= NPROTO))
return -EAFNOSUPPORT;
- write_lock_bh(&xfrm_policy_afinfo_lock);
+ spin_lock_bh(&xfrm_policy_afinfo_lock);
if (unlikely(xfrm_policy_afinfo[afinfo->family] != NULL))
err = -ENOBUFS;
else {
@@ -2455,9 +2456,9 @@ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
dst_ops->link_failure = xfrm_link_failure;
if (likely(afinfo->garbage_collect == NULL))
afinfo->garbage_collect = __xfrm_garbage_collect;
- xfrm_policy_afinfo[afinfo->family] = afinfo;
+ rcu_assign_pointer(xfrm_policy_afinfo[afinfo->family], afinfo);
}
- write_unlock_bh(&xfrm_policy_afinfo_lock);
+ spin_unlock_bh(&xfrm_policy_afinfo_lock);
rtnl_lock();
for_each_net(net) {
@@ -2490,13 +2491,14 @@ int xfrm_policy_unregister_afinfo(struct xfrm_policy_afinfo *afinfo)
return -EINVAL;
if (unlikely(afinfo->family >= NPROTO))
return -EAFNOSUPPORT;
- write_lock_bh(&xfrm_policy_afinfo_lock);
+ spin_lock_bh(&xfrm_policy_afinfo_lock);
if (likely(xfrm_policy_afinfo[afinfo->family] != NULL)) {
if (unlikely(xfrm_policy_afinfo[afinfo->family] != afinfo))
err = -EINVAL;
else {
struct dst_ops *dst_ops = afinfo->dst_ops;
- xfrm_policy_afinfo[afinfo->family] = NULL;
+ rcu_assign_pointer(xfrm_policy_afinfo[afinfo->family],
+ NULL);
dst_ops->kmem_cachep = NULL;
dst_ops->check = NULL;
dst_ops->negative_advice = NULL;
@@ -2504,7 +2506,8 @@ int xfrm_policy_unregister_afinfo(struct xfrm_policy_afinfo *afinfo)
afinfo->garbage_collect = NULL;
}
}
- write_unlock_bh(&xfrm_policy_afinfo_lock);
+ spin_unlock_bh(&xfrm_policy_afinfo_lock);
+ synchronize_rcu();
return err;
}
EXPORT_SYMBOL(xfrm_policy_unregister_afinfo);
@@ -2513,8 +2516,9 @@ static void __net_init xfrm_dst_ops_init(struct net *net)
{
struct xfrm_policy_afinfo *afinfo;
- read_lock_bh(&xfrm_policy_afinfo_lock);
- afinfo = xfrm_policy_afinfo[AF_INET];
+ local_bh_disable();
+ rcu_read_lock();
+ afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
if (afinfo)
net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
@@ -2522,7 +2526,8 @@ static void __net_init xfrm_dst_ops_init(struct net *net)
if (afinfo)
net->xfrm.xfrm6_dst_ops = *afinfo->dst_ops;
#endif
- read_unlock_bh(&xfrm_policy_afinfo_lock);
+ rcu_read_unlock();
+ local_bh_enable();
}
static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family)
@@ -2530,16 +2535,16 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family)
struct xfrm_policy_afinfo *afinfo;
if (unlikely(family >= NPROTO))
return NULL;
- read_lock(&xfrm_policy_afinfo_lock);
- afinfo = xfrm_policy_afinfo[family];
+ rcu_read_lock();
+ afinfo = rcu_dereference(xfrm_policy_afinfo[family]);
if (unlikely(!afinfo))
- read_unlock(&xfrm_policy_afinfo_lock);
+ rcu_read_unlock();
return afinfo;
}
static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo)
{
- read_unlock(&xfrm_policy_afinfo_lock);
+ rcu_read_unlock();
}
static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void *ptr)
--
1.7.4.1
^ permalink raw reply related
* [PATCH] configure: Add search path for 64bit library.
From: Li Wei @ 2012-08-07 4:22 UTC (permalink / raw)
To: shemminger; +Cc: netdev, Li Wei
Signed-off-by: Li Wei <lw@cn.fujitsu.com>
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 0f4444f..997759c 100755
--- a/configure
+++ b/configure
@@ -149,7 +149,7 @@ check_ipt()
check_ipt_lib_dir()
{
IPT_LIB_DIR=""
- for dir in /lib /usr/lib /usr/local/lib
+ for dir in /lib /usr/lib /usr/local/lib /lib64 /usr/lib64 /usr/local/lib64
do
for file in $dir/{xtables,iptables}/lib*t_*so ; do
if [ -f $file ]; then
--
1.7.10.1
^ permalink raw reply related
* Re: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: Joseph Glanville @ 2012-08-07 3:37 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Ali Ayoub, David Miller, ogerlitz, roland, netdev, sean.hefty,
erezsh, dledford
In-Reply-To: <87obmnfs4p.fsf@xmission.com>
On 7 August 2012 10:44, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Ali Ayoub <ali@mellanox.com> writes:
>
>> Among other things, the main benefit we're targeting is to allow IPoE
>> traffic within the VM to go through the (Ethernet) vBridge down to the
>> eIPoIB PIF, and eventually to IPoIB and to the IB network.
>
> That works today without code changes. It is called routing.
>
>> In Para virtualized environment, the VM emulator sends/receives packets
>> with Ethernet header, and the vBridge also performs L2 switching based
>> on the Ethernet header, in addition to other tools that expect an
>> Ethernet link layer. We'd like to support them on top of IPoIB.
>
> See routing. The code is already done.
>
>> I don't see in other alternatives a solution for the problem we're
>> trying to solve. If there are changes/suggestions to improve eIPoIB
>> netdev driver to avoid "messing with the link layer" and make it
>> acceptable, we can discuss and apply them.
>
> Nothing needs to be applied the code is done. Routing from
> IPoE to IPoIB works.
>
> There is nothing in what anyone has posted as requirements that needs
> work to implement.
>
> I totally fail to see how getting packets of of the VM as ethernet
> frames, and then IP layer routing those packets over IP is not an
> option. What requirement am I missing.
>
> All VMs should suport that mode of operation, and certainly the kernel
> does.
>
> Implementations involving bridges like macvlan and macvtap are
> performance optimizations, and the optimizations don't even apply in
> areas like 802.11, where only one mac address is supported per adapter.
>
> Bridging can ocassionally also be an administrative simplification as
> well, but you should be able to achieve the a similar simplification
> with a dhcprelay and proxy arp.
>
> Eric
> --
> 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
Hi,
While I agree this driver doesn't offer an adequate solution to the
problem I think the problem still very much exists.
Ultimately if you have an Infiniband fabric and you want to use it for
inter-guest communication across hosts you are left with 2 options,
tunneling and routing.
Routing can be ok sometimes but it's not a great solution for the vast
majority of people, it requires considerable edge intelligence in the
hypervisor and ISIS routing setups.
The biggest issue is that it's just not L2, guest applications on
guest operating systems expect to have access to fully fledged
Ethernet devices.
This is especially an issue for hosting providers etc where they have
little to no control over what applications their customers want to
use.
Tunnelling solves some of these problems, generally done over L3 and
using existing IP fabric to transport Ethernet L2 frames to the
destination hypervisors.
Better because now you have true L2 encapsulation but to be honest
it's slow and chews alot of cycles and generally has poor PPS
performance.
IMO this driver doesn't make sense compared to a true Ethernet over IB
encapsulation, which isn't actually all that much extra work. (If I am
right you already have a driver that does just this)
The point about retaining compatibility with existing IPoIB boggles my
mind a little.
It means little if no benefit is really added because only IP and ARP
will work.. no other L2 protocol will work correctly as others have
noted.
With full encapsulation you can make use of all the existing
infrastructure, linux bridge, OpenvSwitch, ebtables/netfilter etc.
Joseph.
--
CTO | Orion Virtualisation Solutions | www.orionvm.com.au
Phone: 1300 56 99 52 | Mobile: 0428 754 846
^ permalink raw reply
* Re: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: Eric W. Biederman @ 2012-08-07 3:33 UTC (permalink / raw)
To: Ali Ayoub
Cc: David Miller, ogerlitz, roland, netdev, sean.hefty, erezsh,
dledford
In-Reply-To: <87obmnfs4p.fsf@xmission.com>
ebiederm@xmission.com (Eric W. Biederman) writes:
> Ali Ayoub <ali@mellanox.com> writes:
>
>> Among other things, the main benefit we're targeting is to allow IPoE
>> traffic within the VM to go through the (Ethernet) vBridge down to the
>> eIPoIB PIF, and eventually to IPoIB and to the IB network.
Oh yes. It just occurred to me there is huge problem with eIPoIB as
currently presented in these patches. It breaks DHCPv4 the same way
it breaks ARP, but DHCPv4 is not fixed up.
I am stunned to to realize there has been so much push for a solution
that doesn't even support dhcp.
How can anyone possibly argue that eIPoIB works or is useful? I just
can't imagine.
Totally stunned,
Eric
^ permalink raw reply
* Re: [RFC v3 1/7] hashtable: introduce a small and naive hashtable
From: Josh Triplett @ 2012-08-07 2:55 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev, eric.dumazet,
mathieu.desnoyers
In-Reply-To: <1344300317-23189-2-git-send-email-levinsasha928@gmail.com>
On Tue, Aug 07, 2012 at 02:45:10AM +0200, Sasha Levin wrote:
> +/**
> + * hash_add - add an object to a hashtable
> + * @hashtable: hashtable to add to
> + * @bits: bit count used for hashing
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add(hashtable, bits, node, key) \
> + hlist_add_head(node, &hashtable[hash_min(key, bits)]);
Any particular reason to make this a macro rather than a static inline?
Also, even if you do make it a macro, don't include the semicolon.
> +/**
> + * hash_for_each_possible - iterate over all possible objects for a giver key
s/giver/given/
> + * @name: hashtable to iterate
> + * @obj: the type * to use as a loop cursor for each bucke
s/bucke/bucket/
- Josh Triplett
--
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
* [PATCH] tc-tbf.8: Add parameter range to man page.
From: Li Wei @ 2012-08-07 2:30 UTC (permalink / raw)
To: shemminger; +Cc: netdev, Li Wei
Signed-off-by: Li Wei <lw@cn.fujitsu.com>
---
man/man8/tc-tbf.8 | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/man/man8/tc-tbf.8 b/man/man8/tc-tbf.8
index 3abb238..7b76146 100644
--- a/man/man8/tc-tbf.8
+++ b/man/man8/tc-tbf.8
@@ -76,6 +76,7 @@ latency parameter, which specifies the maximum amount of time a packet can
sit in the TBF. The latter calculation takes into account the size of the
bucket, the rate and possibly the peakrate (if set). These two parameters
are mutually exclusive.
+The range of parameter limit is [1, UINT32_MAX] bytes.
.TP
burst
Also known as buffer or maxburst.
@@ -85,6 +86,7 @@ if you want to reach your configured rate!
If your buffer is too small, packets may be dropped because more tokens arrive per timer tick than fit in your bucket.
The minimum buffer size can be calculated by dividing the rate by HZ.
+The range of this parameter is [1, UINT32_MAX] bytes.
Token usage calculations are performed using a table which by default has a resolution of 8 packets.
This resolution can be changed by specifying the
@@ -96,11 +98,13 @@ this. Must be an integral power of 2.
mpu
A zero-sized packet does not use zero bandwidth. For ethernet, no packet uses less than 64 bytes. The Minimum Packet Unit
determines the minimal token usage (specified in bytes) for a packet. Defaults to zero.
+The range of this parameter is [0, UINT32_MAX] bytes.
.TP
rate
The speed knob. See remarks above about limits! See
.BR tc (8)
for units.
+The range of this parameter is [1, UINT32_MAX] bps.
.PP
Furthermore, if a peakrate is desired, the following parameters are available:
@@ -108,12 +112,14 @@ Furthermore, if a peakrate is desired, the following parameters are available:
peakrate
Maximum depletion rate of the bucket. Limited to 1mbit/s on Intel, 10mbit/s on Alpha. The peakrate does
not need to be set, it is only necessary if perfect millisecond timescale shaping is required.
+The range of this parameter is [1, UINT32_MAX] bps.
.TP
mtu/minburst
Specifies the size of the peakrate bucket. For perfect accuracy, should be set to the MTU of the interface.
If a peakrate is needed, but some burstiness is acceptable, this size can be raised. A 3000 byte minburst
allows around 3mbit/s of peakrate, given 1000 byte packets.
+The range of this parameter is [1, UINT32_MAX] bytes.
Like the regular burstsize you can also specify a
.B cell
--
1.7.10.1
^ permalink raw reply related
* Re: [PATCH 1/8] netpoll: use GFP_ATOMIC in slave_enable_netpoll() and __netpoll_setup()
From: Cong Wang @ 2012-08-07 2:28 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David S. Miller
In-Reply-To: <1344266405.26674.30.camel@edumazet-glaptop>
On Mon, 2012-08-06 at 17:20 +0200, Eric Dumazet wrote:
> On Mon, 2012-08-06 at 22:23 +0800, Cong Wang wrote:
> > slave_enable_netpoll() and __netpoll_setup() may be called
> > with read_lock() held, so should use GFP_ATOMIC to allocate
> > memory. Eric suggested to pass gfp flags to __netpoll_setup().
> >
> > Cc: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Cong Wang <amwang@redhat.com>
> > ---
>
> You based this on net-next, but didnt add the net-next suffix in your
> [PATCH ...] description.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
>
Oh, it is default, isn't it? :-D Anyway, I will add it the next time
when I send network patches.
Thanks!
^ permalink raw reply
* Re: [PATCH 7/8] bridge: add some comments for NETDEV_RELEASE
From: Cong Wang @ 2012-08-07 2:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev, shemminger
In-Reply-To: <20120806.135003.442648448779232464.davem@davemloft.net>
On Mon, 2012-08-06 at 13:50 -0700, David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Mon, 6 Aug 2012 22:23:31 +0800
>
> > + /*
> > + * We don't notify NETDEV_RELEASE event, as this will
> > + * stop netconsole on the bridge.
> > + */
>
> Please format comments in the networking:
>
> /* Like
> * this.
> */
>
> Thanks.
Ok, I will update and resend this single patch. :)
^ permalink raw reply
* [PATCH] af_packet: Quiet sparse noise about using plain integer as NULL pointer
From: Ying Xue @ 2012-08-07 2:27 UTC (permalink / raw)
To: davem; +Cc: netdev
Quiets the sparse warning:
warning: Using plain integer as NULL pointer
Signed-off-by: Ying Xue <ying.xue@windriver.com>
---
net/packet/af_packet.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index ceaca7c..f016f66 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1079,7 +1079,7 @@ static void *packet_current_rx_frame(struct packet_sock *po,
default:
WARN(1, "TPACKET version not supported\n");
BUG();
- return 0;
+ return NULL;
}
}
--
1.7.1
^ permalink raw reply related
* [RESEND][PATCH] drivers: net: irda: bfin_sir: fix compile error
From: Bob Liu @ 2012-08-07 2:08 UTC (permalink / raw)
To: davem
Cc: samuel, rmk+kernel, gregkh, kuninori.morimoto.gx, linux-kernel,
uclinux-dist-devel, netdev, Sonic Zhang, Bob Liu
From: Sonic Zhang <sonic.zhang@analog.com>
Bit IREN is replaced by UMOD_IRDA and UMOD_MASK since blackfin 60x added, but
this driver didn't update which will cause bfin_sir build error:
drivers/net/irda/bfin_sir.c:161:9: error: 'IREN' undeclared (first use in this
function)
drivers/net/irda/bfin_sir.c:435:18: error: 'IREN' undeclared (first use in
this function)
drivers/net/irda/bfin_sir.c:521:11: error: 'IREN' undeclared (first use in
this function)
This patch fix it.
Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
Signed-off-by: Bob Liu <lliubbo@gmail.com>
Acked-by: Samuel Ortiz <samuel@sortiz.org>
---
drivers/net/irda/bfin_sir.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/irda/bfin_sir.c b/drivers/net/irda/bfin_sir.c
index a561ae4..c6a0299 100644
--- a/drivers/net/irda/bfin_sir.c
+++ b/drivers/net/irda/bfin_sir.c
@@ -158,7 +158,7 @@ static int bfin_sir_set_speed(struct bfin_sir_port *port, int speed)
/* If not add the 'RPOLC', we can't catch the receive interrupt.
* It's related with the HW layout and the IR transiver.
*/
- val |= IREN | RPOLC;
+ val |= UMOD_IRDA | RPOLC;
UART_PUT_GCTL(port, val);
return ret;
}
@@ -432,7 +432,7 @@ static void bfin_sir_shutdown(struct bfin_sir_port *port, struct net_device *dev
bfin_sir_stop_rx(port);
val = UART_GET_GCTL(port);
- val &= ~(UCEN | IREN | RPOLC);
+ val &= ~(UCEN | UMOD_MASK | RPOLC);
UART_PUT_GCTL(port, val);
#ifdef CONFIG_SIR_BFIN_DMA
@@ -518,10 +518,10 @@ static void bfin_sir_send_work(struct work_struct *work)
* reset all the UART.
*/
val = UART_GET_GCTL(port);
- val &= ~(IREN | RPOLC);
+ val &= ~(UMOD_MASK | RPOLC);
UART_PUT_GCTL(port, val);
SSYNC();
- val |= IREN | RPOLC;
+ val |= UMOD_IRDA | RPOLC;
UART_PUT_GCTL(port, val);
SSYNC();
/* bfin_sir_set_speed(port, self->speed); */
--
1.7.9.5
^ permalink raw reply related
* Re: [RFC v3 1/7] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-08-07 1:54 UTC (permalink / raw)
To: Li Wei
Cc: torvalds, tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev, josh,
eric.dumazet, mathieu.desnoyers
In-Reply-To: <502073E9.8050205@cn.fujitsu.com>
On 08/07/2012 03:48 AM, Li Wei wrote:
> On 08/07/2012 08:45 AM, Sasha Levin wrote:
>> +/**
>> + * hash_for_each - iterate over a hashtable
>> + * @name: hashtable to iterate
>> + * @bits: bit count of hashing function of the hashtable
>> + * @bkt: integer to use as bucket loop cursor
>> + * @node: the &struct list_head to use as a loop cursor for each bucket
>> + * @obj: the type * to use as a loop cursor for each bucket
>> + * @member: the name of the hlist_node within the struct
>> + */
>> +#define hash_for_each(name, bits, bkt, node, obj, member) \
>> + for (bkt = 0; bkt < HASH_SIZE(bits); bkt++) \
>> + hlist_for_each_entry(obj, node, &name[i], member)
>
> Where is the 'i' coming from? maybe &name[bkt]?
Heh, yeah. And the only place that uses this macro had 'i' declared as the loop counter, so it didn't trigger any issues during testing.
Thanks!
--
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: [RFC v3 1/7] hashtable: introduce a small and naive hashtable
From: Li Wei @ 2012-08-07 1:48 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, mathieu.desnoyers
In-Reply-To: <1344300317-23189-2-git-send-email-levinsasha928@gmail.com>
On 08/07/2012 08:45 AM, Sasha Levin 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@gmail.com>
> ---
> include/linux/hashtable.h | 82 +++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 82 insertions(+), 0 deletions(-)
> 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..394652b
> --- /dev/null
> +++ b/include/linux/hashtable.h
> @@ -0,0 +1,82 @@
> +/*
> + * Hash table implementation
> + * (C) 2012 Sasha Levin <levinsasha928@gmail.com>
> + */
> +
> +#ifndef _LINUX_HASHTABLE_H
> +#define _LINUX_HASHTABLE_H
> +
> +#include <linux/list.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/hash.h>
> +
> +#define DEFINE_HASHTABLE(name, bits) \
> + struct hlist_head name[HASH_SIZE(bits)];
> +
> +#define HASH_SIZE(bits) (1 << (bits))
> +
> +/* 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)))
> +
> +/**
> + * hash_init - initialize a hash table
> + * @hashtable: hashtable to be initialized
> + * @bits: bit count of hashing function
> + *
> + * Initializes a hash table with 2**bits buckets.
> + */
> +static inline void hash_init(struct hlist_head *hashtable, int bits)
> +{
> + int i;
> +
> + for (i = 0; i < HASH_SIZE(bits); i++)
> + INIT_HLIST_HEAD(hashtable + i);
> +}
> +
> +/**
> + * hash_add - add an object to a hashtable
> + * @hashtable: hashtable to add to
> + * @bits: bit count used for hashing
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add(hashtable, bits, node, key) \
> + hlist_add_head(node, &hashtable[hash_min(key, bits)]);
> +
> +/**
> + * 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_for_each - iterate over a hashtable
> + * @name: hashtable to iterate
> + * @bits: bit count of hashing function of the hashtable
> + * @bkt: integer to use as bucket loop cursor
> + * @node: the &struct list_head to use as a loop cursor for each bucket
> + * @obj: the type * to use as a loop cursor for each bucket
> + * @member: the name of the hlist_node within the struct
> + */
> +#define hash_for_each(name, bits, bkt, node, obj, member) \
> + for (bkt = 0; bkt < HASH_SIZE(bits); bkt++) \
> + hlist_for_each_entry(obj, node, &name[i], member)
Where is the 'i' coming from? maybe &name[bkt]?
> +
> +/**
> + * hash_for_each_possible - iterate over all possible objects for a giver key
> + * @name: hashtable to iterate
> + * @obj: the type * to use as a loop cursor for each bucke
> + * @bits: bit count of hashing function of the hashtable
> + * @node: the &struct list_head to use as a loop cursor for each bucket
> + * @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, bits, node, member, key) \
> + hlist_for_each_entry(obj, node, \
> + &name[hash_min(key, bits)], member)
> +
> +#endif
--
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
* First pass at MSG_FASTOPEN support in top-of-trunk netperf
From: Rick Jones @ 2012-08-07 1:47 UTC (permalink / raw)
To: netdev
Folks -
I have just checked-in to the top-of-trunk netperf
(http://www.netperf.org/svn/netperf2/trunk) some changes which enable
use of sendto(MSG_FASTOPEN) in a TCP_CRR test. While I was checking the
system calls I noticed that netperf was calling enable_enobufs() for
every migrated test, not just a UDP_STREAM test (the one where it is
needed), so I fixed that at the same time. Baseline is taken with the
fix in place.
MSG_FASTOPEN is used when one adds a test-specific -F option to the
netperf command line:
netperf -t TCP_CRR -H <destination> -- -F
Just testing the client side from a VM on my workstation (running a
net-next pulled just before 16:30 Pacific time) to my workstation itself
I notice the following:
Without MSG_FASTOPEN:
raj@tardy-ubuntu-1204:~/netperf2_trunk$ src/netperf -H tardy.usa.hp.com
-t TCP_CRR -i 3,30
MIGRATED TCP Connect/Request/Response TEST from 0.0.0.0 (0.0.0.0) port 0
AF_INET to tardy.usa.hp.com () port 0 AF_INET : +/-2.500% @ 99% conf. :
demo
Local /Remote
Socket Size Request Resp. Elapsed Trans.
Send Recv Size Size Time Rate
bytes Bytes bytes bytes secs. per sec
16384 87380 1 1 10.00 2092.33
16384 87380
With MSG_FASTOPEN
raj@tardy-ubuntu-1204:~/netperf2_trunk$ src/netperf -H tardy.usa.hp.com
-t TCP_CRR -i 3,30 -- -F
MIGRATED TCP Connect/Request/Response TEST from 0.0.0.0 (0.0.0.0) port 0
AF_INET to tardy.usa.hp.com () port 0 AF_INET : +/-2.500% @ 99% conf. :
demo
!!! WARNING
!!! Desired confidence was not achieved within the specified iterations.
!!! This implies that there was variability in the test environment that
!!! must be investigated before going further.
!!! Confidence intervals: Throughput : 6.565%
!!! Local CPU util : 0.000%
!!! Remote CPU util : 0.000%
Local /Remote
Socket Size Request Resp. Elapsed Trans.
Send Recv Size Size Time Rate
bytes Bytes bytes bytes secs. per sec
16384 87380 1 1 10.00 2590.18
16384 87380
There was a ~25% increase in TCP_CRR performance, even without the
server actually accepting the magic TCP option. Is that actually
expected? Admittedly, it eliminates a connect() call, but the sequence
before was:
socket()
getsockopt(SO_SNDBUF)
getsockopt(SO_RCVBUF)
setsockopt(SO_REUSEADDR)
bind()
connect()
sendto(4, "n", 1, 0, NULL, 0) = 1
recvfrom(4, "n", 1, 0, NULL, NULL) = 1
recvfrom(4, "", 1, 0, NULL, NULL) = 0
close(4)
and with MSG_FASTOPEN only the connect() goes away. Unless connect() is
really heavy compared to what sendto(MSG_FASTOPEN) does or something
else has changed wrt behaviour.
Anyway, feel free to kick the tires on the netperf changes and let me
know of any problems you encounter.
happy benchmarking,
rick jones
^ permalink raw reply
* Re[2]: ethtool 3.5 released
From: Naoto MATSUMOTO @ 2012-08-07 1:36 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev
In-Reply-To: <1344271283.2632.6.camel@bwh-desktop.uk.solarflarecom.com>
Hi, Ben. Thanks your reply for me.
On Mon, 6 Aug 2012 17:41:23 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> > Port: Twisted Pair
>
> 40G twisted-pair, eh?
We are using this one(cable).
http://www.mellanox.com/content/pages.php?pg=cables
> > PHYAD: 0
> > Transceiver: internal
> > Auto-negotiation: off
> > MDI-X: Unknown
> > Supports Wake-on: d
> > Wake-on: d
> > Current message level: 0x00000014 (20)
> > link ifdown
> > Link detected: yes
> >
> >
> > Our 40GbE NIC Firmware/Dirver status are here.
> > -----------------------------------------------------------------------
> > Mellanox ConnectX HCA Ethernet driver v1.5.8.3 (June 2012)
> > Ethernet controller: Mellanox Technologies MT27500 Family [ConnectX-3]
> > Firmware Version: 2.10.800
> > -----------------------------------------------------------------------
> >
> >
> > And We were testing 10GbE(Intel) NIC and your 40GbE NIC in our lab
> > using ethtool with added some debug code.
>
> I work for Solarflare, not Mellanox... and the patch adding 40G link
> modes came from Parav Pandit at Emulex.
>
> [...]
> > It's firmware problem I thinks. But Just Information for you ;-)
>
> Also, the 40G link mode flags were only just defined, so the driver
> probably has yet to be updated.
>
> Ben.
I agree with you. and we'll wait for the MLX_GUY works ;-)
--
SAKURA Internet Inc. / Senior Researcher
Naoto MATSUMOTO <n-matsumoto@sakura.ad.jp>
SAKURA Research Center <http://research.sakura.ad.jp/>
^ permalink raw reply
* Re[2]: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: Naoto MATSUMOTO @ 2012-08-07 1:21 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Ali Ayoub, David Miller, ogerlitz, roland, netdev, sean.hefty,
erezsh, dledford
In-Reply-To: <87obmnfs4p.fsf@xmission.com>
Hi Eric, Ali
On Mon, 06 Aug 2012 17:44:06 -0700
ebiederm@xmission.com (Eric W. Biederman) wrote:
> Implementations involving bridges like macvlan and macvtap are
> performance optimizations, and the optimizations don't even apply in
> areas like 802.11, where only one mac address is supported per adapter.
We had nearly equal situation perfomance result. share for you.
-----------------------------------------------------------------------
Ethernet over EoIB using gretap with "Raid STP" performance result.
http://twitpic.com/agd1pp
How to use EoIB with gretap.
http://twitpic.com/agd4io
ifconfig ib0 10.1.1.1/24 up
ip link add eoib0 type gretap remote 10.1.1.2 local 10.1.1.1
ip link add eoib1 type gretap remote 10.1.1.3 local 10.1.1.1
ip link add type veth
ifconfig eoib0 up up
ifconfig eoib1 up up
ifconfig veth0 up up
ifconfig veth1 up up
brctl addbr br0
brctl addif br0 eoib0
brctl addif br0 eoib1
bcrtl addif br0 veth1
rstpd
rstpctl rstp br0 on
rstpctl setbridgeprio br0 40960
rstpctl setportpathcost br0 vpn1 40000
rstpctl sethello br0 1
rstpctl setmaxage br0 6
rstpctl setfdelay br0 4
brctl stp br0 on
brctl sethello br0 1
brctl setfd br0 4
brctl setmaxage br0 6
-----------------------------------------------------------------------
regards,
--
SAKURA Internet Inc. / Senior Researcher
Naoto MATSUMOTO <n-matsumoto@sakura.ad.jp>
SAKURA Research Center <http://research.sakura.ad.jp/>
^ permalink raw reply
* Re: [RFC v3 4/7] workqueue: use new hashtable implementation
From: Joe Perches @ 2012-08-07 1:19 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, mathieu.desnoyers, Sasha Levin
In-Reply-To: <1344300317-23189-6-git-send-email-levinsasha928@gmail.com>
On Tue, 2012-08-07 at 02:45 +0200, Sasha Levin wrote:
> From: Sasha Levin <sasha.levin@oracle.com>
>
> Switch workqueues to use the new hashtable implementation. This reduces the amount of
> generic unrelated code in the workqueues.
Just style trivia:
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
[]
> @@ -897,8 +839,15 @@ static struct worker *__find_worker_executing_work(struct global_cwq *gcwq,
> static struct worker *find_worker_executing_work(struct global_cwq *gcwq,
> struct work_struct *work)
> {
> - return __find_worker_executing_work(gcwq, busy_worker_head(gcwq, work),
> - work);
> + struct worker *worker;
> + struct hlist_node *tmp;
> +
> + hash_for_each_possible(gcwq->busy_hash, worker, BUSY_WORKER_HASH_ORDER,
> + tmp, hentry, work)
> + if (worker->current_work == work)
> + return worker;
braces please:
hash_for_each_possible(gcwq->busy_hash, worker, BUSY_WORKER_HASH_ORDER,
tmp, hentry, work) {
if (worker->current_work == work)
return worker;
}
[]
@@ -1916,7 +1865,7 @@ static void cwq_dec_nr_in_flight(struct cpu_workqueue_struct *cwq, int color,
> * @worker: self
> * @work: work to process
> *
> - * Process @work. This function contains all the logics necessary to
> + * Process @work. This? function contains all the logics necessary to
Odd ? and the grammar also seems odd.
> * process a single work including synchronization against and
> * interaction with other workers on the same cpu, queueing and
> * flushing. As long as context requirement is met, any worker can
--
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: [RFC v3 1/7] hashtable: introduce a small and naive hashtable
From: Joe Perches @ 2012-08-07 1:19 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, mathieu.desnoyers
In-Reply-To: <1344300317-23189-2-git-send-email-levinsasha928@gmail.com>
On Tue, 2012-08-07 at 02:45 +0200, Sasha Levin wrote:
> This hashtable implementation is using hlist buckets to provide a simple
> hashtable to prevent it from getting reimplemented all over the kernel.
> diff --git a/include/linux/hashtable.h b/include/linux/hashtable.h
Just trivial style notes and a typo
> +/* 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)))
This is a pretty long line. It doesn't use normal kernel spacing
style and it has unnecessary parentheses.
Maybe:
#define hash_min(val, bits) \
(sizeof(val) == 4 ? hash_32(val, bits) : hash_long(val, bits))
> +
> +/**
> + * hash_init - initialize a hash table
> + * @hashtable: hashtable to be initialized
> + * @bits: bit count of hashing function
> + *
> + * Initializes a hash table with 2**bits buckets.
> + */
> +static inline void hash_init(struct hlist_head *hashtable, int bits)
> +{
> + int i;
> +
> + for (i = 0; i < HASH_SIZE(bits); i++)
> + INIT_HLIST_HEAD(hashtable + i);
> +}
Maybe use a struct hlist_head *last_hash_entry as a loop variable
{
struct hlist_head *eo_hash = hashtable + HASH_SIZE(bits);
while (hashtable < eo_hash)
INIT_HLIST_HEAD(hashtable++);
}
The compiler might generate the same code anyway...
[]
> +/**
> + * hash_for_each_possible - iterate over all possible objects for a giver key
> + * @name: hashtable to iterate
> + * @obj: the type * to use as a loop cursor for each bucke
bucket
--
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
* [RFC v3 7/7] net,9p: use new hashtable implementation
From: Sasha Levin @ 2012-08-07 0:45 UTC (permalink / raw)
To: torvalds
Cc: tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem, rostedt,
mingo, ebiederm, aarcange, ericvh, netdev, josh, eric.dumazet,
mathieu.desnoyers, Sasha Levin
In-Reply-To: <1344300317-23189-1-git-send-email-levinsasha928@gmail.com>
Switch 9p error table to use the new hashtable implementation. This reduces the amount of
generic unrelated code in 9p.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
net/9p/error.c | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/net/9p/error.c b/net/9p/error.c
index 2ab2de7..f712344d 100644
--- a/net/9p/error.c
+++ b/net/9p/error.c
@@ -34,7 +34,7 @@
#include <linux/jhash.h>
#include <linux/errno.h>
#include <net/9p/9p.h>
-
+#include <linux/hashtable.h>
/**
* struct errormap - map string errors from Plan 9 to Linux numeric ids
* @name: string sent over 9P
@@ -50,8 +50,8 @@ struct errormap {
struct hlist_node list;
};
-#define ERRHASHSZ 32
-static struct hlist_head hash_errmap[ERRHASHSZ];
+#define ERR_HASH_BITS 5
+static DEFINE_HASHTABLE(hash_errmap, ERR_HASH_BITS);
/* FixMe - reduce to a reasonable size */
static struct errormap errmap[] = {
@@ -193,18 +193,17 @@ static struct errormap errmap[] = {
int p9_error_init(void)
{
struct errormap *c;
- int bucket;
+ u32 hash;
/* initialize hash table */
- for (bucket = 0; bucket < ERRHASHSZ; bucket++)
- INIT_HLIST_HEAD(&hash_errmap[bucket]);
+ hash_init(hash_errmap, ERR_HASH_BITS);
/* load initial error map into hash table */
for (c = errmap; c->name != NULL; c++) {
c->namelen = strlen(c->name);
- bucket = jhash(c->name, c->namelen, 0) % ERRHASHSZ;
+ hash = jhash(c->name, c->namelen, 0);
INIT_HLIST_NODE(&c->list);
- hlist_add_head(&c->list, &hash_errmap[bucket]);
+ hash_add(hash_errmap, ERR_HASH_BITS, &c->list, hash);
}
return 1;
@@ -223,13 +222,13 @@ int p9_errstr2errno(char *errstr, int len)
int errno;
struct hlist_node *p;
struct errormap *c;
- int bucket;
+ u32 hash;
errno = 0;
p = NULL;
c = NULL;
- bucket = jhash(errstr, len, 0) % ERRHASHSZ;
- hlist_for_each_entry(c, p, &hash_errmap[bucket], list) {
+ hash = jhash(errstr, len, 0);
+ hash_for_each_possible(hash_errmap, c, ERR_HASH_BITS, p, list, hash) {
if (c->namelen == len && !memcmp(c->name, errstr, len)) {
errno = c->val;
break;
--
1.7.8.6
--
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 related
* [RFC v3 6/7] tracepoint: use new hashtable implementation
From: Sasha Levin @ 2012-08-07 0:45 UTC (permalink / raw)
To: torvalds
Cc: tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem, rostedt,
mingo, ebiederm, aarcange, ericvh, netdev, josh, eric.dumazet,
mathieu.desnoyers, Sasha Levin
In-Reply-To: <1344300317-23189-1-git-send-email-levinsasha928@gmail.com>
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>
---
kernel/tracepoint.c | 26 ++++++++++----------------
1 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index d96ba22..5a44949 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,16 @@ 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, TRACEPOINT_HASH_BITS,
+ node, hlist, hash) {
if (!strcmp(name, e->name))
return e;
}
+
return NULL;
}
@@ -210,19 +210,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 +228,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, TRACEPOINT_HASH_BITS, &e->hlist, hash);
return e;
}
@@ -244,7 +238,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.8.6
--
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 related
* [RFC v3 5/7] mm/huge_memory: use new hashtable implementation
From: Sasha Levin @ 2012-08-07 0:45 UTC (permalink / raw)
To: torvalds
Cc: tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem, rostedt,
mingo, ebiederm, aarcange, ericvh, netdev, josh, eric.dumazet,
mathieu.desnoyers, Sasha Levin
In-Reply-To: <1344300317-23189-1-git-send-email-levinsasha928@gmail.com>
Switch hugemem to use the new hashtable implementation. This reduces the amount of
generic unrelated code in the hugemem.
This also removes the dymanic allocation of the hash table. The size of the table is
constant so there's no point in paying the price of an extra dereference when accessing
it.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
mm/huge_memory.c | 56 +++++++++++++----------------------------------------
1 files changed, 14 insertions(+), 42 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 57c4b93..e4c752d 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -17,6 +17,7 @@
#include <linux/khugepaged.h>
#include <linux/freezer.h>
#include <linux/mman.h>
+#include <linux/hashtable.h>
#include <asm/tlb.h>
#include <asm/pgalloc.h>
#include "internal.h"
@@ -57,12 +58,12 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
static unsigned int khugepaged_max_ptes_none __read_mostly = HPAGE_PMD_NR-1;
static int khugepaged(void *none);
-static int mm_slots_hash_init(void);
static int khugepaged_slab_init(void);
static void khugepaged_slab_free(void);
-#define MM_SLOTS_HASH_HEADS 1024
-static struct hlist_head *mm_slots_hash __read_mostly;
+#define MM_SLOTS_HASH_BITS 10
+static DEFINE_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);
+
static struct kmem_cache *mm_slot_cache __read_mostly;
/**
@@ -140,7 +141,7 @@ static int start_khugepaged(void)
int err = 0;
if (khugepaged_enabled()) {
int wakeup;
- if (unlikely(!mm_slot_cache || !mm_slots_hash)) {
+ if (unlikely(!mm_slot_cache)) {
err = -ENOMEM;
goto out;
}
@@ -554,12 +555,6 @@ static int __init hugepage_init(void)
if (err)
goto out;
- err = mm_slots_hash_init();
- if (err) {
- khugepaged_slab_free();
- goto out;
- }
-
/*
* By default disable transparent hugepages on smaller systems,
* where the extra memory used could hurt more than TLB overhead
@@ -1562,47 +1557,24 @@ static inline void free_mm_slot(struct mm_slot *mm_slot)
kmem_cache_free(mm_slot_cache, mm_slot);
}
-static int __init mm_slots_hash_init(void)
-{
- mm_slots_hash = kzalloc(MM_SLOTS_HASH_HEADS * sizeof(struct hlist_head),
- GFP_KERNEL);
- if (!mm_slots_hash)
- return -ENOMEM;
- return 0;
-}
-
-#if 0
-static void __init mm_slots_hash_free(void)
-{
- kfree(mm_slots_hash);
- mm_slots_hash = NULL;
-}
-#endif
-
static struct mm_slot *get_mm_slot(struct mm_struct *mm)
{
- struct mm_slot *mm_slot;
- struct hlist_head *bucket;
+ struct mm_slot *slot;
struct hlist_node *node;
- bucket = &mm_slots_hash[((unsigned long)mm / sizeof(struct mm_struct))
- % MM_SLOTS_HASH_HEADS];
- hlist_for_each_entry(mm_slot, node, bucket, hash) {
- if (mm == mm_slot->mm)
- return mm_slot;
- }
+ hash_for_each_possible(mm_slots_hash, slot, MM_SLOTS_HASH_BITS, node,
+ hash, (unsigned long) mm)
+ if (slot->mm == mm)
+ return slot;
+
return NULL;
}
static void insert_to_mm_slots_hash(struct mm_struct *mm,
struct mm_slot *mm_slot)
{
- struct hlist_head *bucket;
-
- bucket = &mm_slots_hash[((unsigned long)mm / sizeof(struct mm_struct))
- % MM_SLOTS_HASH_HEADS];
mm_slot->mm = mm;
- hlist_add_head(&mm_slot->hash, bucket);
+ hash_add(mm_slots_hash, MM_SLOTS_HASH_BITS, &mm_slot->hash, (long)mm);
}
static inline int khugepaged_test_exit(struct mm_struct *mm)
@@ -1675,7 +1647,7 @@ void __khugepaged_exit(struct mm_struct *mm)
spin_lock(&khugepaged_mm_lock);
mm_slot = get_mm_slot(mm);
if (mm_slot && khugepaged_scan.mm_slot != mm_slot) {
- hlist_del(&mm_slot->hash);
+ hash_del(&mm_slot->hash);
list_del(&mm_slot->mm_node);
free = 1;
}
@@ -2089,7 +2061,7 @@ static void collect_mm_slot(struct mm_slot *mm_slot)
if (khugepaged_test_exit(mm)) {
/* free mm_slot */
- hlist_del(&mm_slot->hash);
+ hash_del(&mm_slot->hash);
list_del(&mm_slot->mm_node);
/*
--
1.7.8.6
--
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 related
* [RFC v3 4/7] workqueue: use new hashtable implementation
From: Sasha Levin @ 2012-08-07 0:45 UTC (permalink / raw)
To: torvalds
Cc: tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem, rostedt,
mingo, ebiederm, aarcange, ericvh, netdev, josh, eric.dumazet,
mathieu.desnoyers, Sasha Levin
In-Reply-To: <1344300317-23189-1-git-send-email-levinsasha928@gmail.com>
From: Sasha Levin <sasha.levin@oracle.com>
Switch workqueues to use the new hashtable implementation. This reduces the amount of
generic unrelated code in the workqueues.
Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
kernel/workqueue.c | 91 +++++++++++-----------------------------------------
1 files changed, 19 insertions(+), 72 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 692d976..edc7fd0 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -41,6 +41,7 @@
#include <linux/debug_locks.h>
#include <linux/lockdep.h>
#include <linux/idr.h>
+#include <linux/hashtable.h>
#include "workqueue_sched.h"
@@ -82,8 +83,6 @@ enum {
NR_WORKER_POOLS = 2, /* # worker pools per gcwq */
BUSY_WORKER_HASH_ORDER = 6, /* 64 pointers */
- BUSY_WORKER_HASH_SIZE = 1 << BUSY_WORKER_HASH_ORDER,
- BUSY_WORKER_HASH_MASK = BUSY_WORKER_HASH_SIZE - 1,
MAX_IDLE_WORKERS_RATIO = 4, /* 1/4 of busy can be idle */
IDLE_WORKER_TIMEOUT = 300 * HZ, /* keep idle ones for 5 mins */
@@ -180,7 +179,7 @@ struct global_cwq {
unsigned int flags; /* L: GCWQ_* flags */
/* workers are chained either in busy_hash or pool idle_list */
- struct hlist_head busy_hash[BUSY_WORKER_HASH_SIZE];
+ DEFINE_HASHTABLE(busy_hash, BUSY_WORKER_HASH_ORDER);
/* L: hash of busy workers */
struct worker_pool pools[2]; /* normal and highpri pools */
@@ -288,8 +287,8 @@ EXPORT_SYMBOL_GPL(system_nrt_freezable_wq);
(pool) < &(gcwq)->pools[NR_WORKER_POOLS]; (pool)++)
#define for_each_busy_worker(worker, i, pos, gcwq) \
- for (i = 0; i < BUSY_WORKER_HASH_SIZE; i++) \
- hlist_for_each_entry(worker, pos, &gcwq->busy_hash[i], hentry)
+ hash_for_each(gcwq->busy_hash, BUSY_WORKER_HASH_ORDER, i, pos, \
+ worker, hentry)
static inline int __next_gcwq_cpu(int cpu, const struct cpumask *mask,
unsigned int sw)
@@ -822,63 +821,6 @@ static inline void worker_clr_flags(struct worker *worker, unsigned int flags)
}
/**
- * busy_worker_head - return the busy hash head for a work
- * @gcwq: gcwq of interest
- * @work: work to be hashed
- *
- * Return hash head of @gcwq for @work.
- *
- * CONTEXT:
- * spin_lock_irq(gcwq->lock).
- *
- * RETURNS:
- * Pointer to the hash head.
- */
-static struct hlist_head *busy_worker_head(struct global_cwq *gcwq,
- struct work_struct *work)
-{
- const int base_shift = ilog2(sizeof(struct work_struct));
- unsigned long v = (unsigned long)work;
-
- /* simple shift and fold hash, do we need something better? */
- v >>= base_shift;
- v += v >> BUSY_WORKER_HASH_ORDER;
- v &= BUSY_WORKER_HASH_MASK;
-
- return &gcwq->busy_hash[v];
-}
-
-/**
- * __find_worker_executing_work - find worker which is executing a work
- * @gcwq: gcwq of interest
- * @bwh: hash head as returned by busy_worker_head()
- * @work: work to find worker for
- *
- * Find a worker which is executing @work on @gcwq. @bwh should be
- * the hash head obtained by calling busy_worker_head() with the same
- * work.
- *
- * CONTEXT:
- * spin_lock_irq(gcwq->lock).
- *
- * RETURNS:
- * Pointer to worker which is executing @work if found, NULL
- * otherwise.
- */
-static struct worker *__find_worker_executing_work(struct global_cwq *gcwq,
- struct hlist_head *bwh,
- struct work_struct *work)
-{
- struct worker *worker;
- struct hlist_node *tmp;
-
- hlist_for_each_entry(worker, tmp, bwh, hentry)
- if (worker->current_work == work)
- return worker;
- return NULL;
-}
-
-/**
* find_worker_executing_work - find worker which is executing a work
* @gcwq: gcwq of interest
* @work: work to find worker for
@@ -897,8 +839,15 @@ static struct worker *__find_worker_executing_work(struct global_cwq *gcwq,
static struct worker *find_worker_executing_work(struct global_cwq *gcwq,
struct work_struct *work)
{
- return __find_worker_executing_work(gcwq, busy_worker_head(gcwq, work),
- work);
+ struct worker *worker;
+ struct hlist_node *tmp;
+
+ hash_for_each_possible(gcwq->busy_hash, worker, BUSY_WORKER_HASH_ORDER,
+ tmp, hentry, work)
+ if (worker->current_work == work)
+ return worker;
+
+ return NULL;
}
/**
@@ -1916,7 +1865,7 @@ static void cwq_dec_nr_in_flight(struct cpu_workqueue_struct *cwq, int color,
* @worker: self
* @work: work to process
*
- * Process @work. This function contains all the logics necessary to
+ * Process @work. This? function contains all the logics necessary to
* process a single work including synchronization against and
* interaction with other workers on the same cpu, queueing and
* flushing. As long as context requirement is met, any worker can
@@ -1932,7 +1881,6 @@ __acquires(&gcwq->lock)
struct cpu_workqueue_struct *cwq = get_work_cwq(work);
struct worker_pool *pool = worker->pool;
struct global_cwq *gcwq = pool->gcwq;
- struct hlist_head *bwh = busy_worker_head(gcwq, work);
bool cpu_intensive = cwq->wq->flags & WQ_CPU_INTENSIVE;
work_func_t f = work->func;
int work_color;
@@ -1964,7 +1912,7 @@ __acquires(&gcwq->lock)
* already processing the work. If so, defer the work to the
* currently executing one.
*/
- collision = __find_worker_executing_work(gcwq, bwh, work);
+ collision = find_worker_executing_work(gcwq, work);
if (unlikely(collision)) {
move_linked_works(work, &collision->scheduled, NULL);
return;
@@ -1972,7 +1920,8 @@ __acquires(&gcwq->lock)
/* claim and process */
debug_work_deactivate(work);
- hlist_add_head(&worker->hentry, bwh);
+ hash_add(gcwq->busy_hash, BUSY_WORKER_HASH_ORDER, &worker->hentry,
+ (unsigned long)worker);
worker->current_work = work;
worker->current_cwq = cwq;
work_color = get_work_color(work);
@@ -2027,7 +1976,7 @@ __acquires(&gcwq->lock)
worker_clr_flags(worker, WORKER_CPU_INTENSIVE);
/* we're done with it, release */
- hlist_del_init(&worker->hentry);
+ hash_del(&worker->hentry);
worker->current_work = NULL;
worker->current_cwq = NULL;
cwq_dec_nr_in_flight(cwq, work_color, false);
@@ -3690,7 +3639,6 @@ out_unlock:
static int __init init_workqueues(void)
{
unsigned int cpu;
- int i;
cpu_notifier(workqueue_cpu_up_callback, CPU_PRI_WORKQUEUE_UP);
cpu_notifier(workqueue_cpu_down_callback, CPU_PRI_WORKQUEUE_DOWN);
@@ -3704,8 +3652,7 @@ static int __init init_workqueues(void)
gcwq->cpu = cpu;
gcwq->flags |= GCWQ_DISASSOCIATED;
- for (i = 0; i < BUSY_WORKER_HASH_SIZE; i++)
- INIT_HLIST_HEAD(&gcwq->busy_hash[i]);
+ hash_init(gcwq->busy_hash, BUSY_WORKER_HASH_ORDER);
for_each_worker_pool(pool, gcwq) {
pool->gcwq = gcwq;
--
1.7.8.6
--
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 related
* [RFC v3 4/7] workqueue: use new hashtable implementation
From: Sasha Levin @ 2012-08-07 0:45 UTC (permalink / raw)
To: torvalds
Cc: tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem, rostedt,
mingo, ebiederm, aarcange, ericvh, netdev, josh, eric.dumazet,
mathieu.desnoyers, Sasha Levin
In-Reply-To: <1344300317-23189-1-git-send-email-levinsasha928@gmail.com>
Switch workqueues to use the new hashtable implementation. This reduces the amount of
generic unrelated code in the workqueues.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
kernel/workqueue.c | 89 ++++++++++-----------------------------------------
1 files changed, 18 insertions(+), 71 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 692d976..548fa87 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -41,6 +41,7 @@
#include <linux/debug_locks.h>
#include <linux/lockdep.h>
#include <linux/idr.h>
+#include <linux/hashtable.h>
#include "workqueue_sched.h"
@@ -82,8 +83,6 @@ enum {
NR_WORKER_POOLS = 2, /* # worker pools per gcwq */
BUSY_WORKER_HASH_ORDER = 6, /* 64 pointers */
- BUSY_WORKER_HASH_SIZE = 1 << BUSY_WORKER_HASH_ORDER,
- BUSY_WORKER_HASH_MASK = BUSY_WORKER_HASH_SIZE - 1,
MAX_IDLE_WORKERS_RATIO = 4, /* 1/4 of busy can be idle */
IDLE_WORKER_TIMEOUT = 300 * HZ, /* keep idle ones for 5 mins */
@@ -180,7 +179,7 @@ struct global_cwq {
unsigned int flags; /* L: GCWQ_* flags */
/* workers are chained either in busy_hash or pool idle_list */
- struct hlist_head busy_hash[BUSY_WORKER_HASH_SIZE];
+ DEFINE_HASHTABLE(busy_hash, BUSY_WORKER_HASH_ORDER);
/* L: hash of busy workers */
struct worker_pool pools[2]; /* normal and highpri pools */
@@ -288,8 +287,8 @@ EXPORT_SYMBOL_GPL(system_nrt_freezable_wq);
(pool) < &(gcwq)->pools[NR_WORKER_POOLS]; (pool)++)
#define for_each_busy_worker(worker, i, pos, gcwq) \
- for (i = 0; i < BUSY_WORKER_HASH_SIZE; i++) \
- hlist_for_each_entry(worker, pos, &gcwq->busy_hash[i], hentry)
+ hash_for_each(gcwq->busy_hash, BUSY_WORKER_HASH_ORDER, i, pos, \
+ worker, hentry)
static inline int __next_gcwq_cpu(int cpu, const struct cpumask *mask,
unsigned int sw)
@@ -822,63 +821,6 @@ static inline void worker_clr_flags(struct worker *worker, unsigned int flags)
}
/**
- * busy_worker_head - return the busy hash head for a work
- * @gcwq: gcwq of interest
- * @work: work to be hashed
- *
- * Return hash head of @gcwq for @work.
- *
- * CONTEXT:
- * spin_lock_irq(gcwq->lock).
- *
- * RETURNS:
- * Pointer to the hash head.
- */
-static struct hlist_head *busy_worker_head(struct global_cwq *gcwq,
- struct work_struct *work)
-{
- const int base_shift = ilog2(sizeof(struct work_struct));
- unsigned long v = (unsigned long)work;
-
- /* simple shift and fold hash, do we need something better? */
- v >>= base_shift;
- v += v >> BUSY_WORKER_HASH_ORDER;
- v &= BUSY_WORKER_HASH_MASK;
-
- return &gcwq->busy_hash[v];
-}
-
-/**
- * __find_worker_executing_work - find worker which is executing a work
- * @gcwq: gcwq of interest
- * @bwh: hash head as returned by busy_worker_head()
- * @work: work to find worker for
- *
- * Find a worker which is executing @work on @gcwq. @bwh should be
- * the hash head obtained by calling busy_worker_head() with the same
- * work.
- *
- * CONTEXT:
- * spin_lock_irq(gcwq->lock).
- *
- * RETURNS:
- * Pointer to worker which is executing @work if found, NULL
- * otherwise.
- */
-static struct worker *__find_worker_executing_work(struct global_cwq *gcwq,
- struct hlist_head *bwh,
- struct work_struct *work)
-{
- struct worker *worker;
- struct hlist_node *tmp;
-
- hlist_for_each_entry(worker, tmp, bwh, hentry)
- if (worker->current_work == work)
- return worker;
- return NULL;
-}
-
-/**
* find_worker_executing_work - find worker which is executing a work
* @gcwq: gcwq of interest
* @work: work to find worker for
@@ -897,8 +839,15 @@ static struct worker *__find_worker_executing_work(struct global_cwq *gcwq,
static struct worker *find_worker_executing_work(struct global_cwq *gcwq,
struct work_struct *work)
{
- return __find_worker_executing_work(gcwq, busy_worker_head(gcwq, work),
- work);
+ struct worker *worker;
+ struct hlist_node *tmp;
+
+ hash_for_each_possible(gcwq->busy_hash, worker, BUSY_WORKER_HASH_ORDER,
+ tmp, hentry, (unsigned long)work)
+ if (worker->current_work == work)
+ return worker;
+
+ return NULL;
}
/**
@@ -1932,7 +1881,6 @@ __acquires(&gcwq->lock)
struct cpu_workqueue_struct *cwq = get_work_cwq(work);
struct worker_pool *pool = worker->pool;
struct global_cwq *gcwq = pool->gcwq;
- struct hlist_head *bwh = busy_worker_head(gcwq, work);
bool cpu_intensive = cwq->wq->flags & WQ_CPU_INTENSIVE;
work_func_t f = work->func;
int work_color;
@@ -1964,7 +1912,7 @@ __acquires(&gcwq->lock)
* already processing the work. If so, defer the work to the
* currently executing one.
*/
- collision = __find_worker_executing_work(gcwq, bwh, work);
+ collision = find_worker_executing_work(gcwq, work);
if (unlikely(collision)) {
move_linked_works(work, &collision->scheduled, NULL);
return;
@@ -1972,7 +1920,8 @@ __acquires(&gcwq->lock)
/* claim and process */
debug_work_deactivate(work);
- hlist_add_head(&worker->hentry, bwh);
+ hash_add(gcwq->busy_hash, BUSY_WORKER_HASH_ORDER, &worker->hentry,
+ (unsigned long)worker);
worker->current_work = work;
worker->current_cwq = cwq;
work_color = get_work_color(work);
@@ -2027,7 +1976,7 @@ __acquires(&gcwq->lock)
worker_clr_flags(worker, WORKER_CPU_INTENSIVE);
/* we're done with it, release */
- hlist_del_init(&worker->hentry);
+ hash_del(&worker->hentry);
worker->current_work = NULL;
worker->current_cwq = NULL;
cwq_dec_nr_in_flight(cwq, work_color, false);
@@ -3690,7 +3639,6 @@ out_unlock:
static int __init init_workqueues(void)
{
unsigned int cpu;
- int i;
cpu_notifier(workqueue_cpu_up_callback, CPU_PRI_WORKQUEUE_UP);
cpu_notifier(workqueue_cpu_down_callback, CPU_PRI_WORKQUEUE_DOWN);
@@ -3704,8 +3652,7 @@ static int __init init_workqueues(void)
gcwq->cpu = cpu;
gcwq->flags |= GCWQ_DISASSOCIATED;
- for (i = 0; i < BUSY_WORKER_HASH_SIZE; i++)
- INIT_HLIST_HEAD(&gcwq->busy_hash[i]);
+ hash_init(gcwq->busy_hash, BUSY_WORKER_HASH_ORDER);
for_each_worker_pool(pool, gcwq) {
pool->gcwq = gcwq;
--
1.7.8.6
--
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 related
* [RFC v3 3/7] mm,ksm: use new hashtable implementation
From: Sasha Levin @ 2012-08-07 0:45 UTC (permalink / raw)
To: torvalds
Cc: tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem, rostedt,
mingo, ebiederm, aarcange, ericvh, netdev, josh, eric.dumazet,
mathieu.desnoyers, Sasha Levin
In-Reply-To: <1344300317-23189-1-git-send-email-levinsasha928@gmail.com>
Switch ksm to use the new hashtable implementation. This reduces the amount of
generic unrelated code in the ksm module.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
mm/ksm.c | 31 +++++++++++++------------------
1 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/mm/ksm.c b/mm/ksm.c
index 47c8853..96d5f91 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -33,7 +33,7 @@
#include <linux/mmu_notifier.h>
#include <linux/swap.h>
#include <linux/ksm.h>
-#include <linux/hash.h>
+#include <linux/hashtable.h>
#include <linux/freezer.h>
#include <linux/oom.h>
@@ -156,9 +156,8 @@ struct rmap_item {
static struct rb_root root_stable_tree = RB_ROOT;
static struct rb_root root_unstable_tree = RB_ROOT;
-#define MM_SLOTS_HASH_SHIFT 10
-#define MM_SLOTS_HASH_HEADS (1 << MM_SLOTS_HASH_SHIFT)
-static struct hlist_head mm_slots_hash[MM_SLOTS_HASH_HEADS];
+#define MM_SLOTS_HASH_BITS 10
+static DEFINE_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);
static struct mm_slot ksm_mm_head = {
.mm_list = LIST_HEAD_INIT(ksm_mm_head.mm_list),
@@ -275,26 +274,22 @@ static inline void free_mm_slot(struct mm_slot *mm_slot)
static struct mm_slot *get_mm_slot(struct mm_struct *mm)
{
- struct mm_slot *mm_slot;
- struct hlist_head *bucket;
struct hlist_node *node;
+ struct mm_slot *slot;
+
+ hash_for_each_possible(mm_slots_hash, slot, MM_SLOTS_HASH_BITS,
+ node, link, (unsigned long)mm)
+ if (slot->mm == mm)
+ return slot;
- bucket = &mm_slots_hash[hash_ptr(mm, MM_SLOTS_HASH_SHIFT)];
- hlist_for_each_entry(mm_slot, node, bucket, link) {
- if (mm == mm_slot->mm)
- return mm_slot;
- }
return NULL;
}
static void insert_to_mm_slots_hash(struct mm_struct *mm,
struct mm_slot *mm_slot)
{
- struct hlist_head *bucket;
-
- bucket = &mm_slots_hash[hash_ptr(mm, MM_SLOTS_HASH_SHIFT)];
mm_slot->mm = mm;
- hlist_add_head(&mm_slot->link, bucket);
+ hash_add(mm_slots_hash, MM_SLOTS_HASH_BITS, &mm_slot->link, (long)mm);
}
static inline int in_stable_tree(struct rmap_item *rmap_item)
@@ -647,7 +642,7 @@ static int unmerge_and_remove_all_rmap_items(void)
ksm_scan.mm_slot = list_entry(mm_slot->mm_list.next,
struct mm_slot, mm_list);
if (ksm_test_exit(mm)) {
- hlist_del(&mm_slot->link);
+ hash_del(&mm_slot->link);
list_del(&mm_slot->mm_list);
spin_unlock(&ksm_mmlist_lock);
@@ -1385,7 +1380,7 @@ next_mm:
* or when all VM_MERGEABLE areas have been unmapped (and
* mmap_sem then protects against race with MADV_MERGEABLE).
*/
- hlist_del(&slot->link);
+ hash_del(&slot->link);
list_del(&slot->mm_list);
spin_unlock(&ksm_mmlist_lock);
@@ -1548,7 +1543,7 @@ void __ksm_exit(struct mm_struct *mm)
mm_slot = get_mm_slot(mm);
if (mm_slot && ksm_scan.mm_slot != mm_slot) {
if (!mm_slot->rmap_list) {
- hlist_del(&mm_slot->link);
+ hash_del(&mm_slot->link);
list_del(&mm_slot->mm_list);
easy_to_free = 1;
} else {
--
1.7.8.6
--
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 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