Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 7/8] net: Allow setting the network namespace by fd
From: jamal @ 2010-09-24 11:51 UTC (permalink / raw)
  To: David Lamparter
  Cc: Eric W. Biederman, linux-kernel, Linux Containers, netdev,
	netfilter-devel, linux-fsdevel, Daniel Lezcano, Linus Torvalds,
	Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
	Serge E. Hallyn, Pavel Emelyanov, Pavel Emelyanov, Ben Greear,
	Matt Helsley, Jonathan Corbet, Sukadev Bhattiprolu,
	Jan Engelhardt, Patrick McHardy
In-Reply-To: <20100923145856.GB1160234@jupiter.n2.diac24.net>

On Thu, 2010-09-23 at 16:58 +0200, David Lamparter wrote:

> migrating route table entries makes no sense because
> a) they refer to devices and configuration that does not exist in the
>    target namespace; they only make sense within their netns context
> b) they are purely virtual and you get the same result from deleting and
>    recreating them.
> 
> Network devices are special because they may have something attached to
> them, be it hardware or some daemon.

Routes functionally reside on top of netdevices, point to nexthop
neighbors across these netdevices etc. Underlying assumption is you take
care of that dependency when migrating.
We are talking about FIB entries here not the route cache; moving a few
pointers within the kernel is a hell lot faster than recreating a subset
of BGP entries from user space. 

Eric, I didnt follow the exposed-races arguement: Why would it involve
more than just some basic locking only while you change the struct net
pointer to the new namespace for these sub-subsystems?

cheers,
jamal


^ permalink raw reply

* Re: [RFC] bonding: fix workqueue re-arming races
From: Narendra K @ 2010-09-24 11:23 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Jiri Bohac, bonding-devel, markine, jarkao2, chavey, netdev
In-Reply-To: <25924.1284677073@death>

On Fri, Sep 17, 2010 at 04:14:33AM +0530, Jay Vosburgh wrote:
> Jay Vosburgh <fubar@us.ibm.com> wrote:
> [...]
> 
> 	I had some time to work on this, and I fixed a few nits in the
> most recent patch, and also modified it as I describe above (the
> new_link business).  This seems to do the right thing for the mii/arp
> commit functions.
> 
> 	The alb_promisc alb_promisc function, however, still has a race.
> The curr_active_slave could change between the time the function is
> scheduled and when it executes.  That window is pretty small, but does
> exist.  Losing the race means that some interface stays promisc when it
> shouldn't; I don't believe it will panic.  Fixing that is probably a
> matter of stashing a pointer to the slave to be de-promisc-ified
> somewhere, but that stash would have to be handled if the slave were to
> be removed from the bond.
> 
> 	I've tested this a bit, and it seems ok, but I can't reproduce
> the original problem, so I'm not entirely sure this doesn't break
> something very subtle.
> 
> 	Also, I'll be out of the office for the next two weeks, so I
> won't get back to this until I return.  If any interested parties could
> test this out and provide some feedback before then, it would be
> appreciated.
> 
Thanks.

Original issue was seen when the system was rebooted and while the
network was shutting down. I applied the patch to linux-next (branch-
20100811) and issued service network stop/start in quick succession.

The bond interface had 4 slaves, 3 with link up and 1 with link down
configured in balance-alb mode, miimon=100, bonding driver version:3.7.0

The follwing call trace was seen -

2.6.35.with.upstream.patch-next-20100811-0.7-default+
[14602.945876] ------------[ cut here ]------------
[14602.950474] kernel BUG at kernel/workqueue.c:2844!
[14602.955242] invalid opcode: 0000 [#1] SMP 
[14602.959341] last sysfs file: /sys/class/net/bonding_masters
[14602.964888] CPU 1 
[14602.966714] Modules linked in: af_packet bonding ipv6 cpufreq_conservative cpufreq_userspace cpufreq_powersave acpi_cpufreq mperf microcode fuse loop dm_mod joydev usbhid hid bnx2 tpm_tis tpm tpm_bios rtc_cmos iTCO_wdt iTCO_vendor_support sr_mod power_meter cdrom sg serio_raw mptctl pcspkr rtc_core usb_storage dcdbas rtc_lib button uhci_hcd ehci_hcd usbcore sd_mod crc_t10dif edd ext3 mbcache jbd fan processor ide_pci_generic ide_core ata_generic ata_piix libata mptsas mptscsih mptbase scsi_transport_sas scsi_mod thermal thermal_sys hwmon
[14603.015002] 
[14603.016524] Pid: 4006, comm: ifdown-bonding Not tainted 2.6.35.with.upstream.patch-next-20100811-0.7-default+ #2 0M233H/PowerEdge R710
[14603.028554] RIP: 0010:[<ffffffff81067b50>]  [<ffffffff81067b50>] destroy_workqueue+0x1d0/0x1e0
[14603.037144] RSP: 0018:ffff88022a379d88  EFLAGS: 00010286
[14603.042432] RAX: 000000000000003c RBX: ffff880228674240 RCX: ffff880228f0e800
[14603.049534] RDX: 0000000000001000 RSI: 0000000000000002 RDI: 000000000000001a
[14603.056638] RBP: ffff88022a379da8 R08: ffff88022a379cf8 R09: 0000000000000000
[14603.063741] R10: 00000000ffffffff R11: 0000000000000000 R12: 0000000000000002
[14603.070842] R13: ffffffff817b8560 R14: ffff8802299d1480 R15: ffff8802299d1488
[14603.077944] FS:  00007f8e6a28f700(0000) GS:ffff880001c00000(0000) knlGS:0000000000000000
[14603.085999] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[14603.091719] CR2: 00007f8e6a2c2000 CR3: 0000000127d1c000 CR4: 00000000000006e0
[14603.098822] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[14603.105924] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[14603.113026] Process ifdown-bonding (pid: 4006, threadinfo ffff88022a378000, task ffff8802299b0080)
[14603.121944] Stack:
[14603.123944]  ffff88022a379da8 ffff8802299d1000 ffff8802299d1000 000000010036b6a4
[14603.131182] <0> ffff88022a379dc8 ffffffffa030a91d ffff8802299d1000 000000010036b6a4
[14603.138857] <0> ffff88022a379e28 ffffffff812e0a08 ffff88022a379e38 ffff88022a379de8
[14603.146718] Call Trace:
[14603.149158]  [<ffffffffa030a91d>] bond_destructor+0x1d/0x30 [bonding]
[14603.155572]  [<ffffffff812e0a08>] netdev_run_todo+0x1a8/0x270
[14603.161293]  [<ffffffff812ee859>] rtnl_unlock+0x9/0x10
[14603.166411]  [<ffffffffa0317824>] bonding_store_bonds+0x1c4/0x1f0 [bonding]
[14603.173342]  [<ffffffff810f26be>] ? alloc_pages_current+0x9e/0x110
[14603.179497]  [<ffffffff81285c9e>] class_attr_store+0x1e/0x20
[14603.185132]  [<ffffffff8116e365>] sysfs_write_file+0xc5/0x140
[14603.190853]  [<ffffffff8110a68f>] vfs_write+0xcf/0x190
[14603.195967]  [<ffffffff8110a840>] sys_write+0x50/0x90
[14603.200996]  [<ffffffff81002ec2>] system_call_fastpath+0x16/0x1b
[14603.206974] Code: 00 7f 14 8b 3b eb 91 3d 00 10 00 00 89 c2 77 10 8b 3b e9 07 ff ff ff 3d 00 10 00 00 89 c2 76 f0 8b 3b e9 a9 fe ff ff 0f 0b eb fe <0f> 0b eb fe 66 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 8b 3d 00 
[14603.226419] RIP  [<ffffffff81067b50>] destroy_workqueue+0x1d0/0x1e0
[14603.232669]  RSP <ffff88022a379d88>
[    0.000000] Initializing cgroup subsys cpuset
[    0.000000] Initializing cgroup subsys cpu

With regards,
Narendra K

^ permalink raw reply

* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Stanislaw Gruszka @ 2010-09-24 11:18 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev
In-Reply-To: <20100923212012.GA10925@electric-eye.fr.zoreil.com>

On Thu, 23 Sep 2010 23:20:12 +0200
Francois Romieu <romieu@fr.zoreil.com> wrote:

> Stanislaw Gruszka <sgruszka@redhat.com> :
> > We have fedora bug report where driver fail to initialize after
> > suspend/resume because of memory allocation errors:
> > https://bugzilla.redhat.com/show_bug.cgi?id=629158
> > 
> > To fix use GFP_KERNEL allocation where possible. 
> 
> Feel free to add a Acked-by: Francois Romieu <romieu@fr.zoreil.com>
> as soon as it will have been explicitely reported to improve the
> situation (it is not clear in the PR above).

I'm pretty sure patch fix the problem, however yes, we do not have
confirmation from reporter yet. Anyway atomic allocation should not
be used in process context.

Stanislaw

^ permalink raw reply

* Re: [PATCH v4 2/2] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE  and HIDIOCSFEATURE
From: Antonio Ospite @ 2010-09-24 10:47 UTC (permalink / raw)
  To: Alan Ott
  Cc: Ville Tervo, Jiri Kosina, Stefan Achatz, Alexey Dobriyan,
	Tejun Heo, Alan Stern, Greg Kroah-Hartman, Marcel Holtmann,
	Stephane Chatty, Michael Poole, David S. Miller, Bastien Nocera,
	Eric Dumazet, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <89BCFDE1-0D39-4248-84B2-E02C1480E067@signal11.us>

[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]

On Thu, 23 Sep 2010 10:16:43 -0400
Alan Ott <alan@signal11.us> wrote:

> 
> On Sep 23, 2010, at 7:51 AM, Ville Tervo wrote:
> 
> > Hi Alan,
> >
> > One comment.
> >
> > How about a variable called ret and using that to return len or  
> > errno? It
> > would eliminate code dublication.
> >
> 
> Hi Ville,
> 
> Where specifically? In which function? I've gone through it a couple  
> of times and failed to find return statements which are superfluous.  
> Maybe I'm missing something fundamental?
> 

I guess he's referring to the return paths (normal, err_restartsys,
err_eio) in hidp_get_raw_report(), they could be merged into a
generic "out:" path by parametrizing the return value.

I don't know if Alan prefers explicit error paths, for me it's OK either
way.

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* [MeeGo-Dev][PATCH v3] Topcliff: Update PCH_CAN driver to 2.6.35
From: Masayuki Ohtak @ 2010-09-24 10:24 UTC (permalink / raw)
  To: Wolfgang Grandegger, David S. Miller, Wolfram Sang,
	Christian Pellegrin, Barry Song
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, Tomoya MORINAGA, Masayuki Ohtake, margie.foster

Hi Wolfgang and Marc,

We have modified a pretty amount of our driver based on other accepted Socket CAN driver.
Additionally, We have reduced the number of lines 3601 to 1444.
Please check below.

Thanks, Ohtake(OKISemi)

---
CAN driver of Topcliff PCH

Topcliff PCH is the platform controller hub that is going to be used in
Intel's upcoming general embedded platform. All IO peripherals in
Topcliff PCH are actually devices sitting on AMBA bus. 
Topcliff PCH has CAN I/F. This driver enables CAN function.

Signed-off-by: Masayuki Ohtake <masa-korg@dsn.okisemi.com>
---
 drivers/net/can/Kconfig   |    8 +
 drivers/net/can/Makefile  |    1 +
 drivers/net/can/pch_can.c | 1444 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1453 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/can/pch_can.c

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 2c5227c..5c98a20 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -73,6 +73,14 @@ config CAN_JANZ_ICAN3
 	  This driver can also be built as a module. If so, the module will be
 	  called janz-ican3.ko.
 
+config PCH_CAN
+	tristate "PCH CAN"
+	depends on  CAN_DEV
+	---help---
+	  This driver is for PCH CAN of Topcliff which is an IOH for x86
+	  embedded processor.
+	  This driver can access CAN bus.
+
 source "drivers/net/can/mscan/Kconfig"
 
 source "drivers/net/can/sja1000/Kconfig"
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 9047cd0..3ddc6a7 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -16,5 +16,6 @@ obj-$(CONFIG_CAN_TI_HECC)	+= ti_hecc.o
 obj-$(CONFIG_CAN_MCP251X)	+= mcp251x.o
 obj-$(CONFIG_CAN_BFIN)		+= bfin_can.o
 obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ican3.o
+obj-$(CONFIG_PCH_CAN)		+= pch_can.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
new file mode 100644
index 0000000..8c1731b
--- /dev/null
+++ b/drivers/net/can/pch_can.c
@@ -0,0 +1,1444 @@
+/*
+ * Copyright (C) 1999 - 2010 Intel Corporation.
+ * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
+ *
+ * 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 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+#include <linux/can.h>
+#include <linux/can/dev.h>
+#include <linux/can/error.h>
+
+#define MAX_BITRATE		0x3e8
+
+#define MAX_MSG_OBJ		32
+#define MSG_OBJ_RX		0 /* The receive message object flag. */
+#define MSG_OBJ_TX		1 /* The transmit message object flag. */
+
+#define ENABLE			1 /* The enable flag */
+#define DISABLE			0 /* The disable flag */
+#define CAN_CTRL_INIT		0x0001 /* The INIT bit of CANCONT register. */
+#define CAN_CTRL_IE		0x0002 /* The IE bit of CAN control register */
+#define CAN_CTRL_IE_SIE_EIE	0x000e
+#define CAN_CTRL_CCE		0x0040
+#define CAN_CTRL_OPT		0x0080 /* The OPT bit of CANCONT register. */
+#define CAN_OPT_SILENT		0x0008 /* The Silent bit of CANOPT reg. */
+#define CAN_OPT_LBACK		0x0010 /* The LoopBack bit of CANOPT reg. */
+#define CAN_CMASK_RX_TX_SET	0x00f3
+#define CAN_CMASK_RX_TX_GET	0x0073
+#define CAN_CMASK_ALL		0xff
+#define CAN_CMASK_RDWR		0x80
+#define CAN_CMASK_ARB		0x20
+#define CAN_CMASK_CTRL		0x10
+#define CAN_CMASK_MASK		0x40
+
+#define CAN_IF_MCONT_NEWDAT	0x8000
+#define CAN_IF_MCONT_INTPND	0x2000
+#define CAN_IF_MCONT_UMASK		0x1000
+#define CAN_IF_MCONT_TXIE		0x0800
+#define CAN_IF_MCONT_RXIE		0x0400
+#define CAN_IF_MCONT_RMTEN		0x0200
+#define CAN_IF_MCONT_TXRQXT		0x0100
+#define CAN_IF_MCONT_EOB		0x0080
+#define CAN_IF_MCONT_MSGLOST		0x4000
+#define CAN_MASK2_MDIR_MXTD		0xc000
+#define CAN_ID2_DIR			0x2000
+#define CAN_ID_MSGVAL			0x8000
+
+#define CAN_STATUS_INT			0x8000
+#define CAN_IF_CREQ_BUSY		0x8000
+#define CAN_ID2_XTD			0x4000
+
+#define CAN_REC				0x00007f00
+#define CAN_TEC				0x000000ff
+
+#define PCH_RX_OK			0x00000010
+#define PCH_TX_OK			0x00000008
+#define PCH_BUS_OFF			0x00000080
+#define PCH_EWARN			0x00000040
+#define PCH_EPASSIV			0x00000020
+#define PCH_LEC0			0x00000001
+#define PCH_LEC1			0x00000002
+#define PCH_LEC2			0x00000004
+#define PCH_LEC_ALL			(PCH_LEC0 | PCH_LEC1 | PCH_LEC2)
+#define PCH_STUF_ERR			PCH_LEC0
+#define PCH_FORM_ERR			PCH_LEC1
+#define PCH_ACK_ERR			(PCH_LEC0 | PCH_LEC1)
+#define PCH_BIT1_ERR			PCH_LEC2
+#define PCH_BIT0_ERR			(PCH_LEC0 | PCH_LEC2)
+#define PCH_CRC_ERR			(PCH_LEC1 | PCH_LEC2)
+
+/* bit position of certain controller bits. */
+#define BIT_BITT_BRP			0
+#define BIT_BITT_SJW			6
+#define BIT_BITT_TSEG1			8
+#define BIT_BITT_TSEG2			12
+#define BIT_IF1_MCONT_RXIE		10
+#define BIT_IF2_MCONT_TXIE		11
+#define BIT_BRPE_BRPE			6
+#define BIT_ES_TXERRCNT			0
+#define BIT_ES_RXERRCNT			8
+#define MSK_BITT_BRP			0x3f
+#define MSK_BITT_SJW			0xc0
+#define MSK_BITT_TSEG1			0xf00
+#define MSK_BITT_TSEG2			0x7000
+#define MSK_BRPE_BRPE			0x3c0
+#define MSK_BRPE_GET			0x0f
+#define MSK_CTRL_IE_SIE_EIE		0x07
+#define MSK_MCONT_TXIE			0x08
+#define MSK_MCONT_RXIE			0x10
+#define PCH_CAN_NO_TX_BUFF		1
+#define PCI_DEVICE_ID_INTEL_PCH1_CAN	0x8818
+#define COUNTER_LIMIT 0xFFFF
+#define PCH_CAN_CLK			50000	/* 50MHz */
+
+/* Total 32 OBJs */
+#define PCH_RX_OBJ_NUM	1
+#define PCH_TX_OBJ_NUM	1
+#define PCH_OBJ_NUM (PCH_TX_OBJ_NUM + PCH_RX_OBJ_NUM)
+
+#define	PCH_CAN_ACTIVE	0
+#define	PCH_CAN_LISTEN	1
+#define PCH_CAN_STOP	0
+#define PCH_CAN_RUN	1
+
+#define PCH_CAN_ENABLE	0
+#define PCH_CAN_DISABLE	1
+#define PCH_CAN_ALL	2
+#define PCH_CAN_NONE	3
+
+struct pch_can_regs {
+	u32 cont;
+	u32 stat;
+	u32 errc;
+	u32 bitt;
+	u32 intr;
+	u32 opt;
+	u32 brpe;
+	u32 reserve1;
+	u32 if1_creq;
+	u32 if1_cmask;
+	u32 if1_mask1;
+	u32 if1_mask2;
+	u32 if1_id1;
+	u32 if1_id2;
+	u32 if1_mcont;
+	u32 if1_dataa1;
+	u32 if1_dataa2;
+	u32 if1_datab1;
+	u32 if1_datab2;
+	u32 reserve2;
+	u32 reserve3[12];
+	u32 if2_creq;
+	u32 if2_cmask;
+	u32 if2_mask1;
+	u32 if2_mask2;
+	u32 if2_id1;
+	u32 if2_id2;
+	u32 if2_mcont;
+	u32 if2_dataa1;
+	u32 if2_dataa2;
+	u32 if2_datab1;
+	u32 if2_datab2;
+	u32 reserve4;
+	u32 reserve5[20];
+	u32 treq1;
+	u32 treq2;
+	u32 reserve6[2];
+	u32 reserve7[56];
+	u32 reserve8[3];
+	u32 srst;
+};
+
+struct pch_can_priv {
+	struct can_priv can;
+	void __iomem *base;
+	unsigned int can_num;
+	struct pci_dev *dev;
+	unsigned int tx_enable[MAX_MSG_OBJ];
+	unsigned int rx_enable[MAX_MSG_OBJ];
+	unsigned int rx_link[MAX_MSG_OBJ];
+	unsigned int int_enables;
+	unsigned int int_stat;
+	unsigned int bus_off_interrupt;
+	struct net_device *ndev;
+	spinlock_t msgif_reg_lock; /* Message Interface Registers Access Lock*/
+	unsigned int msg_obj[MAX_MSG_OBJ];
+	struct pch_can_regs *regs;
+};
+
+static struct can_bittiming_const pch_can_bittiming_const = {
+	.name = KBUILD_MODNAME,
+	.tseg1_min = 1,
+	.tseg1_max = 16,
+	.tseg2_min = 1,
+	.tseg2_max = 8,
+	.sjw_max = 4,
+	.brp_min = 1,
+	.brp_max = 1024, /* 6bit + extended 4bit */
+	.brp_inc = 1,
+};
+
+static const struct pci_device_id pch_can_pcidev_id[] __devinitdata = {
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PCH1_CAN)},
+	{}
+};
+
+static inline void pch_can_bit_set(u32 *addr, u32 mask)
+{
+	iowrite32((ioread32(addr) | mask), addr);
+}
+
+static inline void pch_can_bit_clear(u32 *addr, u32 mask)
+{
+	iowrite32((ioread32(addr) & ~(mask)), addr);
+}
+
+static void pch_can_set_run_mode(struct pch_can_priv *priv, u32 mode)
+{
+	switch (mode) {
+	case PCH_CAN_RUN:
+		pch_can_bit_clear(&(priv->regs)->cont, CAN_CTRL_INIT);
+		break;
+
+	case PCH_CAN_STOP:
+		pch_can_bit_set(&(priv->regs)->cont, CAN_CTRL_INIT);
+		break;
+
+	default:
+		dev_err(&priv->ndev->dev, "%s -> Invalid Mode.\n", __func__);
+		break;
+	}
+}
+
+static void pch_can_get_run_mode(struct pch_can_priv *priv, u32 *mode)
+{
+	u32 reg_val = ioread32(&(priv->regs)->cont);
+
+	if (reg_val & CAN_CTRL_INIT)
+		*mode = PCH_CAN_STOP;
+	else
+		*mode = PCH_CAN_RUN;
+}
+
+static void pch_can_set_optmode(struct pch_can_priv *priv)
+{
+	u32 reg_val = ioread32(&(priv->regs)->opt);
+
+	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
+		reg_val |= CAN_OPT_SILENT;
+
+	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
+		reg_val |= CAN_OPT_LBACK;
+
+	pch_can_bit_set(&(priv->regs)->cont, CAN_CTRL_OPT);
+	iowrite32(reg_val, &(priv->regs)->opt);
+}
+
+static void pch_can_set_int_custom(struct pch_can_priv *priv)
+{
+	/* Clearing the IE, SIE and EIE bits of Can control register. */
+	pch_can_bit_clear(&(priv->regs)->cont, CAN_CTRL_IE_SIE_EIE);
+
+	/* Appropriately setting them. */
+	pch_can_bit_set(&(priv->regs)->cont,
+			((priv->int_enables & MSK_CTRL_IE_SIE_EIE) << 1));
+}
+
+/* This function retrieves interrupt enabled for the CAN device. */
+static void pch_can_get_int_enables(struct pch_can_priv *priv, u32 *enables)
+{
+	u32 reg_ctrl_val = ioread32(&(priv->regs)->cont);
+
+	/* Obtaining the status of IE, SIE and EIE interrupt bits. */
+	*enables = ((reg_ctrl_val & CAN_CTRL_IE_SIE_EIE) >> 1);
+}
+
+static void pch_can_set_int_enables(struct pch_can_priv *priv, u32 interrupt_no)
+{
+	switch (interrupt_no) {
+	case PCH_CAN_ENABLE:
+		pch_can_bit_set(&(priv->regs)->cont, CAN_CTRL_IE);
+		break;
+
+	case PCH_CAN_DISABLE:
+		pch_can_bit_clear(&(priv->regs)->cont, CAN_CTRL_IE);
+		break;
+
+	case PCH_CAN_ALL:
+		pch_can_bit_set(&(priv->regs)->cont, CAN_CTRL_IE_SIE_EIE);
+		break;
+
+	case PCH_CAN_NONE:
+		pch_can_bit_clear(&(priv->regs)->cont, CAN_CTRL_IE_SIE_EIE);
+		break;
+
+	default:
+		dev_err(&priv->ndev->dev, "Invalid interrupt number.\n");
+		break;
+	}
+}
+
+static void pch_can_check_if1_busy(struct pch_can_priv *priv, u32 num)
+{
+	u32 counter = COUNTER_LIMIT;
+	u32 if1_creq;
+
+	iowrite32(num, &(priv->regs)->if1_creq);
+	while (counter) {
+		if1_creq = (ioread32(&(priv->regs)->if1_creq)) &
+				     CAN_IF_CREQ_BUSY;
+		if (!if1_creq)
+			break;
+		counter--;
+	}
+	if (!counter)
+		dev_err(&priv->ndev->dev, "IF1 BUSY Flag is set forever.\n");
+}
+
+static void pch_can_check_if2_busy(struct pch_can_priv *priv, u32 num)
+{
+	u32 counter = COUNTER_LIMIT;
+	u32 if2_creq;
+
+	iowrite32(num, &(priv->regs)->if2_creq);
+	while (counter) {
+		if2_creq = (ioread32(&(priv->regs)->if2_creq)) &
+				     CAN_IF_CREQ_BUSY;
+		if (!if2_creq)
+			break;
+		counter--;
+	}
+	if (!counter)
+		dev_err(&priv->ndev->dev, "IF2 BUSY Flag is set forever.\n");
+}
+
+static void pch_can_set_rx_enable(struct pch_can_priv *priv, u32 buff_num,
+				  u32 set)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
+	/*Reading the receive buffer data from RAM to Interface1 registers */
+	iowrite32(CAN_CMASK_RX_TX_GET, &(priv->regs)->if1_cmask);
+	pch_can_check_if1_busy(priv, buff_num); /* Read from MsgRAN */
+
+	/* Setting the IF1MASK1 register to access MsgVal and RxIE bits */
+	iowrite32((CAN_CMASK_RDWR | CAN_CMASK_ARB | CAN_CMASK_CTRL),
+		  (&(priv->regs)->if1_cmask));
+
+	if (set == ENABLE) {
+		/* Setting the MsgVal and RxIE bits */
+		pch_can_bit_set(&(priv->regs)->if1_mcont, CAN_IF_MCONT_RXIE);
+		pch_can_bit_set(&(priv->regs)->if1_id2, CAN_ID_MSGVAL);
+
+	} else if (set == DISABLE) {
+		/* Resetting the MsgVal and RxIE bits */
+		pch_can_bit_clear(&(priv->regs)->if1_mcont, CAN_IF_MCONT_RXIE);
+		pch_can_bit_clear(&(priv->regs)->if1_id2, CAN_ID_MSGVAL);
+	}
+
+	pch_can_check_if1_busy(priv, buff_num); /* Write to MsgRAM */
+	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+}
+
+static void pch_can_rx_enable_all(struct pch_can_priv *priv)
+{
+	u32 i;
+
+	/* Traversing to obtain the object configured as receivers. */
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if (priv->msg_obj[i] == MSG_OBJ_RX)
+			pch_can_set_rx_enable(priv, i + 1, ENABLE);
+	}
+}
+
+static void pch_can_rx_disable_all(struct pch_can_priv *priv)
+{
+	u32 i;
+
+	/* Traversing to obtain the object configured as receivers. */
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if (priv->msg_obj[i] == MSG_OBJ_RX)
+			pch_can_set_rx_enable(priv, (i + 1), DISABLE);
+	}
+}
+
+static void pch_can_set_tx_enable(struct pch_can_priv *priv, u32 buff_num,
+				 u32 set)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
+	/* Reading the Msg buffer from Message RAM to Interface2 registers. */
+	iowrite32(CAN_CMASK_RX_TX_GET, (&(priv->regs)->if1_cmask));
+	pch_can_check_if1_busy(priv, buff_num);
+
+	/* Setting the IF2CMASK register for accessing the
+		MsgVal and TxIE bits */
+	iowrite32((CAN_CMASK_RDWR | CAN_CMASK_ARB | CAN_CMASK_CTRL),
+		 (&(priv->regs)->if1_cmask));
+
+	if (set == ENABLE) {
+		/* Setting the MsgVal and TxIE bits */
+		pch_can_bit_set(&(priv->regs)->if1_mcont, CAN_IF_MCONT_TXIE);
+		pch_can_bit_set(&(priv->regs)->if1_id2, CAN_ID_MSGVAL);
+	} else if (set == DISABLE) {
+		/* Resetting the MsgVal and TxIE bits. */
+		pch_can_bit_clear(&(priv->regs)->if1_mcont, CAN_IF_MCONT_TXIE);
+		pch_can_bit_clear(&(priv->regs)->if1_id2, CAN_ID_MSGVAL);
+	}
+
+	pch_can_check_if1_busy(priv, buff_num); /* Write to MsgRAM */
+	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+}
+
+static void pch_can_tx_enable_all(struct pch_can_priv *priv)
+{
+	u32 i;
+
+	/* Traversing to obtain the object configured as transmit object. */
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if (priv->msg_obj[i] == MSG_OBJ_TX)
+			pch_can_set_tx_enable(priv, (i + 1), ENABLE);
+	}
+}
+
+static void pch_can_tx_disable_all(struct pch_can_priv *priv)
+{
+	u32 i;
+
+	/* Traversing to obtain the object configured as transmit object. */
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if (priv->msg_obj[i] == MSG_OBJ_TX)
+			pch_can_set_tx_enable(priv, (i + 1), DISABLE);
+	}
+}
+
+static void pch_can_get_rx_enable(struct pch_can_priv *priv, u32 buff_num,
+				 u32 *enable)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
+	iowrite32(CAN_CMASK_RX_TX_GET, (&(priv->regs)->if1_cmask));
+	pch_can_check_if1_busy(priv, buff_num);
+
+	if (((ioread32(&(priv->regs)->if1_id2)) & CAN_ID_MSGVAL) &&
+			((ioread32(&(priv->regs)->if1_mcont)) &
+			CAN_IF_MCONT_RXIE))
+		*enable = ENABLE;
+	else
+		*enable = DISABLE;
+	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+}
+
+static void pch_can_get_tx_enable(struct pch_can_priv *priv, u32 buff_num,
+				 u32 *enable)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
+	iowrite32(CAN_CMASK_RX_TX_GET, &(priv->regs)->if1_cmask);
+	pch_can_check_if1_busy(priv, buff_num);
+
+	if (((ioread32(&(priv->regs)->if1_id2)) & CAN_ID_MSGVAL) &&
+			((ioread32(&(priv->regs)->if1_mcont)) &
+			CAN_IF_MCONT_TXIE)) {
+		*enable = ENABLE;
+	} else {
+		*enable = DISABLE;
+	}
+	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+}
+
+static int pch_can_int_pending(struct pch_can_priv *priv)
+{
+	return ioread32(&(priv->regs)->intr) & 0xffff;
+}
+
+static void pch_can_set_rx_buffer_link(struct pch_can_priv *priv,
+				       u32 buffer_num, u32 set)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
+	iowrite32(CAN_CMASK_RX_TX_GET, &(priv->regs)->if1_cmask);
+	pch_can_check_if1_busy(priv, buffer_num);
+	iowrite32((CAN_CMASK_RDWR | CAN_CMASK_CTRL), &(priv->regs)->if1_cmask);
+	if (set == ENABLE)
+		pch_can_bit_clear(&(priv->regs)->if1_mcont, CAN_IF_MCONT_EOB);
+	else
+		pch_can_bit_set(&(priv->regs)->if1_mcont, CAN_IF_MCONT_EOB);
+
+	pch_can_check_if1_busy(priv, buffer_num);
+	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+}
+
+static void pch_can_get_rx_buffer_link(struct pch_can_priv *priv,
+				       u32 buffer_num, u32 *link)
+{
+	u32 reg_val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
+	iowrite32(CAN_CMASK_RX_TX_GET, &(priv->regs)->if1_cmask);
+	pch_can_check_if1_busy(priv, buffer_num);
+
+	reg_val = ioread32(&(priv->regs)->if1_mcont);
+	if (reg_val & CAN_IF_MCONT_EOB)
+		*link = DISABLE;
+	else
+		*link = ENABLE;
+	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+}
+
+static void pch_can_clear_buffers(struct pch_can_priv *priv)
+{
+	u32 i;
+	u32 rx_buff_num;
+	u32 tx_buff_num;
+
+	iowrite32(CAN_CMASK_RX_TX_SET, &(priv->regs)->if1_cmask);
+	iowrite32(CAN_CMASK_RX_TX_SET, &(priv->regs)->if2_cmask);
+	iowrite32(0xffff, &(priv->regs)->if1_mask1);
+	iowrite32(0xffff, &(priv->regs)->if1_mask2);
+	iowrite32(0xffff, &(priv->regs)->if2_mask1);
+	iowrite32(0xffff, &(priv->regs)->if2_mask2);
+
+	iowrite32(0x0, &(priv->regs)->if1_id1);
+	iowrite32(0x0, &(priv->regs)->if1_id2);
+	iowrite32(0x0, &(priv->regs)->if2_id1);
+	iowrite32(0x0, &(priv->regs)->if2_id2);
+	iowrite32(0x0, &(priv->regs)->if1_mcont);
+	iowrite32(0x0, &(priv->regs)->if2_mcont);
+	iowrite32(0x0, &(priv->regs)->if1_dataa1);
+	iowrite32(0x0, &(priv->regs)->if1_dataa2);
+	iowrite32(0x0, &(priv->regs)->if1_datab1);
+	iowrite32(0x0, &(priv->regs)->if1_datab2);
+	iowrite32(0x0, &(priv->regs)->if2_dataa1);
+	iowrite32(0x0, &(priv->regs)->if2_dataa2);
+	iowrite32(0x0, &(priv->regs)->if2_datab1);
+	iowrite32(0x0, &(priv->regs)->if2_datab2);
+
+	for (i = 1; i <= (MAX_MSG_OBJ / 2); i++) {
+		rx_buff_num = 2 * i;
+		tx_buff_num = (2 * i) - 1;
+
+		iowrite32(rx_buff_num, &(priv->regs)->if1_creq);
+		iowrite32(tx_buff_num, &(priv->regs)->if2_creq);
+
+		mdelay(10);
+	}
+}
+
+static void pch_can_config_rx_tx_buffers(struct pch_can_priv *priv)
+{
+	u32 i;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
+	/* For accssing MsgVal, ID and EOB bit */
+	iowrite32((CAN_CMASK_RDWR | CAN_CMASK_ARB | CAN_CMASK_CTRL),
+		 (&(priv->regs)->if1_cmask));
+	iowrite32((CAN_CMASK_RDWR | CAN_CMASK_ARB | CAN_CMASK_CTRL),
+		 (&(priv->regs)->if2_cmask));
+	iowrite32(0x0, (&(priv->regs)->if1_id1));
+	iowrite32(0x0, (&(priv->regs)->if1_id2));
+
+	/* Resetting DIR bit for reception */
+	iowrite32(0x0, (&(priv->regs)->if2_id1));
+
+	/* Setting DIR bit for transmission */
+	iowrite32((CAN_ID2_DIR | (0x7ff << 2)),
+				(&(priv->regs)->if2_id2));
+
+	/* Setting EOB bit for receiver */
+	iowrite32(CAN_IF_MCONT_EOB, &(priv->regs)->if1_mcont);
+
+	/* Setting EOB bit for transmitter */
+	iowrite32(CAN_IF_MCONT_EOB, (&(priv->regs)->if2_mcont));
+
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if (priv->msg_obj[i] == MSG_OBJ_RX)
+			pch_can_check_if1_busy(priv, i + 1);
+		else if (priv->msg_obj[i] == MSG_OBJ_TX)
+			pch_can_check_if2_busy(priv, i + 1);
+		else
+			dev_err(&priv->ndev->dev, "Invalid OBJ\n");
+	}
+
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if (priv->msg_obj[i] == MSG_OBJ_RX) {
+			iowrite32(CAN_CMASK_RX_TX_GET,
+				&(priv->regs)->if1_cmask);
+			pch_can_check_if1_busy(priv, i+1);
+
+			pch_can_bit_clear(&(priv->regs)->if1_id2, 0x1fff);
+			pch_can_bit_clear(&(priv->regs)->if1_id2, CAN_ID2_XTD);
+			iowrite32(0, (&(priv->regs)->if1_id1));
+			pch_can_bit_set(&(priv->regs)->if1_id2, 0);
+			pch_can_bit_set(&(priv->regs)->if1_mcont,
+					CAN_IF_MCONT_UMASK);
+			pch_can_bit_set(&(priv->regs)->if2_mcont,
+					CAN_IF_MCONT_UMASK);
+
+			iowrite32(0, &(priv->regs)->if1_mask1);
+			pch_can_bit_clear(&(priv->regs)->if1_mask2, 0x1fff);
+			pch_can_bit_clear(&(priv->regs)->if1_mask2,
+					  CAN_MASK2_MDIR_MXTD);
+
+			iowrite32(0, &(priv->regs)->if2_mask1);
+			pch_can_bit_clear(&(priv->regs)->if2_mask2, 0x1fff);
+
+			/* Setting CMASK for writing */
+			iowrite32((CAN_CMASK_RDWR | CAN_CMASK_MASK |
+				   CAN_CMASK_ARB | CAN_CMASK_CTRL),
+				  (&(priv->regs)->if1_cmask));
+
+			pch_can_check_if1_busy(priv, i+1);
+		}
+	}
+	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+}
+
+static void pch_can_open(struct pch_can_priv *priv)
+{
+	/* Stopping the Can device. */
+	pch_can_set_run_mode(priv, PCH_CAN_STOP);
+
+	/* Clearing all the message object buffers. */
+	pch_can_clear_buffers(priv);
+
+	/* Configuring the respective message object as either rx/tx object. */
+	pch_can_config_rx_tx_buffers(priv);
+
+	/* Enabling all receive objects. */
+	pch_can_rx_enable_all(priv);
+
+	/* Enabling all transmit objects. */
+	pch_can_tx_enable_all(priv);
+
+	/* Enabling the interrupts. */
+	pch_can_set_int_enables(priv, PCH_CAN_ALL);
+
+	/* Setting the CAN to run mode. */
+	pch_can_set_run_mode(priv, PCH_CAN_RUN);
+}
+
+static void pch_can_release(struct pch_can_priv *priv)
+{
+	/* Stooping the CAN device. */
+	pch_can_set_run_mode(priv, PCH_CAN_STOP);
+
+	/* Disabling the interrupts. */
+	pch_can_set_int_enables(priv, PCH_CAN_NONE);
+
+	/* Disabling all the receive object. */
+	pch_can_rx_disable_all(priv);
+
+	/* Disabling all the transmit object. */
+	pch_can_tx_disable_all(priv);
+}
+
+/* This function clears interrupt(s) from the CAN device. */
+static void pch_can_int_clr(struct pch_can_priv *priv, u32 mask)
+{
+	if (mask == CAN_STATUS_INT) {
+		ioread32(&(priv->regs)->stat);
+		return;
+	}
+
+	/* Clear interrupt for transmit object */
+	if (priv->msg_obj[mask - 1] == MSG_OBJ_TX) {
+		/* Setting CMASK for clearing interrupts for
+					 frame transmission. */
+		iowrite32((CAN_CMASK_RDWR | CAN_CMASK_CTRL | CAN_CMASK_ARB),
+					(&(priv->regs)->if2_cmask));
+
+		/* Resetting the ID registers. */
+		pch_can_bit_set(&(priv->regs)->if2_id2,
+			       (CAN_ID2_DIR | (0x7ff << 2)));
+		iowrite32(0x0, (&(priv->regs)->if2_id1));
+
+		/* Claring NewDat, TxRqst & IntPnd */
+		pch_can_bit_clear(&(priv->regs)->if2_mcont,
+				  (CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND |
+				   CAN_IF_MCONT_TXRQXT));
+		pch_can_check_if2_busy(priv, mask);
+	}
+	/* Clear interrupt for receive object */
+	else if (priv->msg_obj[mask - 1] == MSG_OBJ_RX) {
+		/* Setting CMASK for clearing the reception interrupts. */
+		iowrite32((CAN_CMASK_RDWR | CAN_CMASK_CTRL | CAN_CMASK_ARB),
+			  (&(priv->regs)->if2_cmask));
+
+		/* Clearing the Dir bit. */
+		pch_can_bit_clear(&(priv->regs)->if2_id2, CAN_ID2_DIR);
+
+		/* Clearing NewDat & IntPnd */
+		pch_can_bit_clear(&(priv->regs)->if2_mcont,
+				  (CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND));
+
+		pch_can_check_if2_busy(priv, mask);
+	}
+}
+
+static int pch_can_get_buffer_status(struct pch_can_priv *priv)
+{
+	u32 reg_treq1;
+	u32 reg_treq2;
+
+	/* Reading the transmission request registers. */
+	reg_treq1 = (ioread32(&(priv->regs)->treq1) & 0xffff);
+	reg_treq2 = ((ioread32(&(priv->regs)->treq2) & 0xffff) << 16);
+
+	return reg_treq1 | reg_treq2;
+}
+
+static void pch_can_reset(struct pch_can_priv *priv)
+{
+	/* write to sw reset register */
+	iowrite32(1, (&(priv->regs)->srst));
+	iowrite32(0, (&(priv->regs)->srst));
+}
+
+static void pch_can_msg_obj(struct net_device *ndev, u32 status)
+{
+	struct pch_can_priv *priv = netdev_priv(ndev);
+	u32 reg;
+	struct sk_buff *skb;
+	struct can_frame *cf;
+	canid_t id;
+	u32 ide;
+	u32 rtr;
+	int i, j;
+	struct net_device_stats *stats = &(priv->ndev->stats);
+
+	/* Reading the messsage object from the Message RAM */
+	iowrite32(CAN_CMASK_RX_TX_GET, &(priv->regs)->if2_cmask);
+	pch_can_check_if2_busy(priv, status);
+
+	/* Reading the MCONT register. */
+	reg = ioread32(&(priv->regs)->if2_mcont);
+	reg &= 0xffff;
+
+	/* If MsgLost bit set. */
+	if (reg & CAN_IF_MCONT_MSGLOST) {
+		pch_can_bit_clear(&(priv->regs)->if2_mcont,
+				  CAN_IF_MCONT_MSGLOST);
+		dev_err(&priv->ndev->dev, "Msg Obj is overwritten.\n");
+	}
+	/* Read the direction bit for determination of remote frame . */
+	rtr = (ioread32((&(priv->regs)->if2_id2)) &  CAN_ID2_DIR);
+	/* Clearing interrupts. */
+	pch_can_int_clr(priv, status);
+	/* Hanlde reception interrupt */
+	if (priv->msg_obj[status - 1] == MSG_OBJ_RX) {
+		if (!(reg & CAN_IF_MCONT_NEWDAT)) {
+			dev_err(&priv->ndev->dev, "MCONT_NEWDAT isn't SET.\n");
+			return;
+		}
+		skb = alloc_can_skb(priv->ndev, &cf);
+		if (!skb)
+			return;
+
+		ide = ((ioread32(&(priv->regs)->if2_id2)) & CAN_ID2_XTD) >> 14;
+		if (ide) {
+			id = (ioread32(&(priv->regs)->if2_id1) & 0xffff);
+			id |= (((ioread32(&(priv->regs)->if2_id2)) &
+					    0x1fff) << 16);
+			cf->can_id = (id & CAN_EFF_MASK) | CAN_EFF_FLAG;
+		} else {
+			id = (((ioread32(&(priv->regs)->if2_id2)) &
+					  (CAN_SFF_MASK << 2)) >> 2);
+			cf->can_id = (id & CAN_SFF_MASK);
+		}
+
+		if (rtr) {
+			cf->can_dlc = 0;
+			cf->can_id |= CAN_RTR_FLAG;
+		} else {
+			cf->can_dlc = ((ioread32(&(priv->regs)->if2_mcont)) &
+						   0x0f);
+		}
+
+		/* Reading back the data. */
+		for (i = 0, j = 0; i < cf->can_dlc; j++) {
+			reg = ioread32(&(priv->regs)->if2_dataa1 + j*4);
+			cf->data[i++] = cpu_to_le32(reg & 0xff);
+			if (i == cf->can_dlc)
+				break;
+			cf->data[i++] = cpu_to_le32((reg & (0xff << 8)) >> 8);
+		}
+		netif_rx(skb);
+		stats->rx_packets++;
+		stats->rx_bytes += cf->can_dlc;
+	} else if (priv->msg_obj[status - 1] == MSG_OBJ_TX) {
+		/* Hanlde transmission interrupt */
+		can_get_echo_skb(priv->ndev, 0);
+		netif_wake_queue(priv->ndev);
+	}
+}
+
+static void pch_can_error(struct net_device *ndev, u32 status)
+{
+	struct sk_buff *skb;
+	struct pch_can_priv *priv = netdev_priv(ndev);
+	struct can_frame *cf;
+	u32 errc;
+	struct net_device_stats *stats = &(priv->ndev->stats);
+
+	skb = alloc_can_err_skb(ndev, &cf);
+	if (!skb) {
+		dev_err(&ndev->dev, "%s -> No memory.\n", __func__);
+		return;
+	}
+
+	if (status & PCH_BUS_OFF) {
+		if (!priv->bus_off_interrupt) {
+			pch_can_tx_disable_all(priv);
+			pch_can_rx_disable_all(priv);
+
+			priv->can.state = CAN_STATE_BUS_OFF;
+			cf->can_id |= CAN_ERR_BUSOFF;
+			can_bus_off(ndev);
+
+			priv->bus_off_interrupt = 1;
+			pch_can_set_run_mode(priv, PCH_CAN_RUN);
+		}
+	}
+
+	/* Warning interrupt. */
+	if (status & PCH_EWARN) {
+		priv->can.state = CAN_STATE_ERROR_WARNING;
+		priv->can.can_stats.error_warning++;
+		cf->can_id |= CAN_ERR_CRTL;
+		errc = ioread32((&(priv->regs)->errc));
+		if (((errc & CAN_REC) >> 8) > 96)
+			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
+		if ((errc & CAN_TEC) > 96)
+			cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
+		dev_warn(&ndev->dev, "%s -> Warning interrupt.\n", __func__);
+	}
+	/* Error passive interrupt. */
+	if (status & PCH_EPASSIV) {
+		priv->can.can_stats.error_passive++;
+		priv->can.state = CAN_STATE_ERROR_PASSIVE;
+		cf->can_id |= CAN_ERR_CRTL;
+		errc = ioread32((&(priv->regs)->errc));
+		if (((errc & CAN_REC) >> 8) > 127)
+			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
+		if ((errc & CAN_TEC) > 127)
+			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
+		dev_err(&ndev->dev,
+			"%s -> Error interrupt.\n", __func__);
+	}
+
+	if (status & PCH_STUF_ERR)
+		cf->data[2] |= CAN_ERR_PROT_STUFF;
+
+	if (status & PCH_FORM_ERR)
+		cf->data[2] |= CAN_ERR_PROT_FORM;
+
+	if (status & PCH_ACK_ERR)
+		cf->data[2] |= CAN_ERR_PROT_LOC_ACK | CAN_ERR_PROT_LOC_ACK_DEL;
+
+	if ((status & PCH_BIT1_ERR) || (status & PCH_BIT0_ERR))
+		cf->data[2] |= CAN_ERR_PROT_BIT;
+
+	if (status & PCH_CRC_ERR)
+		cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
+				CAN_ERR_PROT_LOC_CRC_DEL;
+
+	if (status & PCH_LEC_ALL)
+		iowrite32(status | PCH_LEC_ALL,
+			  &(priv->regs)->stat);
+
+	stats->rx_packets++;
+	stats->rx_bytes += cf->can_dlc;
+	netif_rx(skb);
+}
+
+static irqreturn_t pch_can_handler(int irq, void *dev_id)
+{
+	u32 int_stat;
+	u32 reg_stat;
+	struct net_device *ndev = (struct net_device *)dev_id;
+	struct pch_can_priv *priv = netdev_priv(ndev);
+	int_stat = pch_can_int_pending(priv);
+
+	if (!int_stat)
+		return IRQ_NONE;
+
+	if (int_stat == CAN_STATUS_INT) {
+		reg_stat = ioread32((&(priv->regs)->stat));
+		if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL | PCH_EWARN |
+								PCH_EPASSIV)) {
+			if ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL)
+				pch_can_error(ndev, reg_stat);
+		}
+
+		/* Recover from Bus Off */
+		if (!reg_stat && priv->bus_off_interrupt) {
+			priv->bus_off_interrupt = 0;
+			pch_can_tx_enable_all(priv);
+			pch_can_rx_enable_all(priv);
+
+			dev_info(&priv->ndev->dev, "BusOff stage recovered.\n");
+		}
+
+		if (reg_stat & PCH_RX_OK)
+			pch_can_bit_clear(&(priv->regs)->stat, PCH_RX_OK);
+
+		if (reg_stat & PCH_TX_OK)
+			pch_can_bit_clear(&(priv->regs)->stat, PCH_TX_OK);
+
+		int_stat = pch_can_int_pending(priv);
+	}
+
+	if ((int_stat > 0) && (int_stat <= MAX_MSG_OBJ))
+		pch_can_msg_obj(ndev, int_stat);
+
+	return IRQ_HANDLED;
+}
+
+static int pch_set_bittiming(struct net_device *ndev)
+{
+	struct pch_can_priv *priv = netdev_priv(ndev);
+	const struct can_bittiming *bt = &priv->can.bittiming;
+	u32 curr_mode;
+	u32 reg1; /* CANBIT */
+	u32 reg2; /* BEPE */
+	u32 brp;
+
+	pch_can_get_run_mode(priv, &curr_mode);
+	if (curr_mode == PCH_CAN_RUN)
+		pch_can_set_run_mode(priv, PCH_CAN_STOP);
+
+	/* Setting the CCE bit for accessing the Can Timing register. */
+	pch_can_bit_set(&(priv->regs)->cont, CAN_CTRL_CCE);
+
+	brp = (bt->tq) / (1000000/PCH_CAN_CLK) - 1;
+	reg1 = brp & MSK_BITT_BRP;
+	reg1 |= (bt->sjw - 1) << BIT_BITT_SJW;
+	reg1 |= (bt->phase_seg1 + bt->prop_seg - 1) << BIT_BITT_TSEG1;
+	reg1 |= (bt->phase_seg2 - 1) << BIT_BITT_TSEG2;
+	reg2 = (brp & MSK_BRPE_BRPE) >> BIT_BRPE_BRPE;
+	iowrite32(reg1, (&(priv->regs)->bitt));
+	iowrite32(reg2, (&(priv->regs)->brpe));
+	pch_can_bit_clear(&(priv->regs)->cont, CAN_CTRL_CCE);
+
+	if (curr_mode == PCH_CAN_RUN)
+		pch_can_set_run_mode(priv, PCH_CAN_RUN);
+
+	return 0;
+}
+
+static void pch_can_start(struct net_device *ndev)
+{
+	struct pch_can_priv *priv = netdev_priv(ndev);
+
+	if (priv->can.state != CAN_STATE_STOPPED)
+		pch_can_reset(priv);
+
+	pch_set_bittiming(ndev);
+	pch_can_set_optmode(priv);
+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+	return;
+}
+
+static int pch_can_do_set_mode(struct net_device *ndev, enum can_mode mode)
+{
+	int ret = 0;
+
+	switch (mode) {
+	case CAN_MODE_START:
+		pch_can_start(ndev);
+		netif_wake_queue(ndev);
+		break;
+	default:
+		ret = -EOPNOTSUPP;
+		break;
+	}
+
+	return ret;
+}
+
+static int pch_can_get_state(const struct net_device *ndev,
+			     enum can_state *state)
+{
+	struct pch_can_priv *priv = netdev_priv(ndev);
+
+	*state = priv->can.state;
+	return 0;
+}
+
+static int pch_open(struct net_device *ndev)
+{
+	struct pch_can_priv *priv = netdev_priv(ndev);
+	int retval;
+
+	pch_can_open(priv);
+
+	retval = pci_enable_msi(priv->dev);
+	if (retval) {
+		dev_err(&ndev->dev, "Unable to allocate MSI ret=%d\n", retval);
+		goto pci_en_msi_err;
+	}
+
+	/* Regsitering the interrupt. */
+	retval = request_irq(priv->dev->irq, pch_can_handler, IRQF_SHARED,
+			     ndev->name, ndev);
+	if (retval) {
+		dev_err(&ndev->dev, "request_irq failed.\n");
+		goto req_irq_err;
+	}
+
+	/* Assuming that no bus off interrupt. */
+	priv->bus_off_interrupt = 0;
+
+	/* Open common can device */
+	retval = open_candev(ndev);
+	if (retval) {
+		dev_err(ndev->dev.parent, "open_candev() failed %d\n", retval);
+		goto err_open_candev;
+	}
+
+	pch_can_start(ndev);
+	netif_start_queue(ndev);
+
+	return 0;
+
+err_open_candev:
+	free_irq(priv->dev->irq, ndev);
+req_irq_err:
+	pci_disable_msi(priv->dev);
+pci_en_msi_err:
+	pch_can_release(priv);
+
+	return retval;
+}
+
+static int pch_close(struct net_device *ndev)
+{
+	struct pch_can_priv *priv = netdev_priv(ndev);
+
+	netif_stop_queue(ndev);
+	pch_can_release(priv);
+	free_irq(priv->dev->irq, ndev);
+	pci_disable_msi(priv->dev);
+	close_candev(ndev);
+	priv->can.state = CAN_STATE_STOPPED;
+	return 0;
+}
+
+static int pch_get_free_msg_obj(struct net_device *ndev)
+{
+	u32 buffer_status = 0;
+	u32 tx_disable_counter = 0;
+	u32 tx_buffer_avail = 0;
+	u32 status;
+	s32 i;
+	struct pch_can_priv *priv = netdev_priv(ndev);
+
+	/* Getting the message object status. */
+	buffer_status = (u32) pch_can_get_buffer_status(priv);
+
+	/* Getting the free transmit message object. */
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if ((priv->msg_obj[i] == MSG_OBJ_TX)) {
+			/* Checking whether the object is enabled. */
+			pch_can_get_tx_enable(priv, i + 1, &status);
+			if (status == ENABLE) {
+				if (!((buffer_status >> i) & 1)) {
+					tx_buffer_avail = (i + 1);
+					break;
+				}
+			} else {
+				tx_disable_counter++;
+			}
+		}
+	}
+
+	/* If no transmit object available. */
+	if (!tx_buffer_avail) {
+		/* If no object is enabled. */
+		if ((tx_disable_counter == PCH_TX_OBJ_NUM)) {
+			dev_err(&ndev->dev, "All tx buffers are disabled.\n");
+			return -EPERM;
+		} else {
+			dev_err(&ndev->dev, "%s:No tx buf free.\n", __func__);
+			return -PCH_CAN_NO_TX_BUFF;
+		}
+	}
+	return tx_buffer_avail;
+}
+
+static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+	canid_t id;
+	u32 id1 = 0;
+	u32 id2 = 0;
+	u32 run_mode;
+	u32 i, j;
+	unsigned long flags;
+	struct pch_can_priv *priv = netdev_priv(ndev);
+	struct can_frame *cf = (struct can_frame *)skb->data;
+	struct net_device_stats *stats = &ndev->stats;
+	u32 tx_buffer_avail = 0;
+
+	if (can_dropped_invalid_skb(ndev, skb))
+		return NETDEV_TX_OK;
+
+	/* Getting the current CAN mode. */
+	pch_can_get_run_mode(priv, &run_mode);
+	if (run_mode != PCH_CAN_RUN) {
+		dev_err(&ndev->dev, "CAN stopped on transmit attempt.\n");
+		return -EPERM;
+	}
+
+	tx_buffer_avail = pch_get_free_msg_obj(ndev);
+	if (tx_buffer_avail < 0)
+		return tx_buffer_avail;
+
+	/* Attaining the lock. */
+	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
+
+	/* Reading the Msg Obj from the Msg RAM to the Interface register. */
+	iowrite32(CAN_CMASK_RX_TX_GET, &(priv->regs)->if1_cmask);
+	pch_can_check_if1_busy(priv, tx_buffer_avail);
+
+	/* Setting the CMASK register. */
+	pch_can_bit_set(&(priv->regs)->if1_cmask, CAN_CMASK_ALL);
+
+	/* If ID extended is set. */
+	if (cf->can_id & CAN_EFF_FLAG) {
+		id =  cf->can_id & CAN_EFF_MASK;
+		id1 = id & 0xffff;
+		id2 = ((id & (0x1fff << 16)) >> 16) | CAN_ID2_XTD;
+	} else {
+		id =  cf->can_id & CAN_SFF_MASK;
+		id1 = 0;
+		id2 = ((id & CAN_SFF_MASK) << 2);
+	}
+	pch_can_bit_clear(&(priv->regs)->if1_id1, 0xffff);
+	pch_can_bit_clear(&(priv->regs)->if1_id2, 0x1fff | CAN_ID2_XTD);
+	pch_can_bit_set(&(priv->regs)->if1_id1, id1);
+	pch_can_bit_set(&(priv->regs)->if1_id2, id2);
+
+	/* If remote frame has to be transmitted.. */
+	if (cf->can_id & CAN_RTR_FLAG)
+		pch_can_bit_clear(&(priv->regs)->if1_id2, CAN_ID2_DIR);
+
+	for (i = 0, j = 0; i < cf->can_dlc; j++) {
+		iowrite32(le32_to_cpu(cf->data[i++]),
+			 (&(priv->regs)->if1_dataa1) + j*4);
+		if (i == cf->can_dlc)
+			break;
+		iowrite32(le32_to_cpu(cf->data[i++] << 8),
+			 (&(priv->regs)->if1_dataa1) + j*4);
+	}
+	can_put_echo_skb(skb, ndev, 0);
+
+	/* Updating the size of the data. */
+	pch_can_bit_clear(&(priv->regs)->if1_mcont, 0x0f);
+	pch_can_bit_set(&(priv->regs)->if1_mcont, cf->can_dlc);
+
+	/* Clearing IntPend, NewDat & TxRqst */
+	pch_can_bit_clear(&(priv->regs)->if1_mcont,
+			   (CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND |
+			    CAN_IF_MCONT_TXRQXT));
+
+	/* Setting NewDat, TxRqst bits */
+	pch_can_bit_set(&(priv->regs)->if1_mcont,
+			 (CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_TXRQXT));
+
+	pch_can_check_if1_busy(priv, tx_buffer_avail);
+	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+
+	stats->tx_bytes += cf->can_dlc;
+	stats->tx_packets++;
+
+	return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops pch_can_netdev_ops = {
+	.ndo_open		= pch_open,
+	.ndo_stop		= pch_close,
+	.ndo_start_xmit		= pch_xmit,
+};
+
+static void __devexit pch_can_remove(struct pci_dev *pdev)
+{
+	struct net_device *ndev = pci_get_drvdata(pdev);
+	struct pch_can_priv *priv = netdev_priv(ndev);
+
+	unregister_candev(priv->ndev);
+	free_candev(priv->ndev);
+	pci_iounmap(pdev, priv->base);
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+	pch_can_reset(priv);
+}
+
+#ifdef CONFIG_PM
+static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	int i;			/* Counter variable. */
+	int retval;		/* Return value. */
+	u32 buf_stat;	/* Variable for reading the transmit buffer status. */
+	u32 counter = 0xFFFFFF;
+
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct pch_can_priv *priv = netdev_priv(dev);
+
+	/* Stop the CAN controller */
+	pch_can_set_run_mode(priv, PCH_CAN_STOP);
+
+	/* Indicate that we are aboutto/in suspend */
+	priv->can.state = CAN_STATE_SLEEPING;
+
+	/* Waiting for all transmission to complete. */
+	while (counter) {
+		buf_stat = pch_can_get_buffer_status(priv);
+		if (!buf_stat)
+			break;
+		counter--;
+	}
+	if (!counter)
+		dev_err(&pdev->dev, "%s -> Transmission time out.\n", __func__);
+
+	/* Save interrupt configuration and then disable them */
+	pch_can_get_int_enables(priv, &(priv->int_enables));
+	pch_can_set_int_enables(priv, PCH_CAN_DISABLE);
+
+	/* Save Tx buffer enable state */
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if (priv->msg_obj[i] == MSG_OBJ_TX)
+			pch_can_get_tx_enable(priv, (i + 1),
+					      &(priv->tx_enable[i]));
+	}
+
+	/* Disable all Transmit buffers */
+	pch_can_tx_disable_all(priv);
+
+	/* Save Rx buffer enable state */
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if (priv->msg_obj[i] == MSG_OBJ_RX) {
+			pch_can_get_rx_enable(priv, (i + 1),
+						&(priv->rx_enable[i]));
+			pch_can_get_rx_buffer_link(priv, (i + 1),
+						&(priv->rx_link[i]));
+		}
+	}
+
+	/* Disable all Receive buffers */
+	pch_can_rx_disable_all(priv);
+	retval = pci_save_state(pdev);
+	if (retval) {
+		dev_err(&pdev->dev, "pci_save_state failed.\n");
+	} else {
+		pci_enable_wake(pdev, PCI_D3hot, 0);
+		pci_disable_device(pdev);
+		pci_set_power_state(pdev, pci_choose_state(pdev, state));
+	}
+
+	return retval;
+}
+
+static int pch_can_resume(struct pci_dev *pdev)
+{
+	int i;			/* Counter variable. */
+	int retval;		/* Return variable. */
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct pch_can_priv *priv = netdev_priv(dev);
+
+	pci_set_power_state(pdev, PCI_D0);
+	pci_restore_state(pdev);
+	retval = pci_enable_device(pdev);
+	if (retval) {
+		dev_err(&pdev->dev, "pci_enable_device failed.\n");
+		return retval;
+	}
+
+	pci_enable_wake(pdev, PCI_D3hot, 0);
+
+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+	/* Disabling all interrupts. */
+	pch_can_set_int_enables(priv, PCH_CAN_DISABLE);
+
+	/* Setting the CAN device in Stop Mode. */
+	pch_can_set_run_mode(priv, PCH_CAN_STOP);
+
+	/* Configuring the transmit and receive buffers. */
+	pch_can_config_rx_tx_buffers(priv);
+
+	/* Restore the CAN state */
+	pch_set_bittiming(dev);
+
+	/* Listen/Active */
+	pch_can_set_optmode(priv);
+
+	/* Enabling the transmit buffer. */
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if (priv->msg_obj[i] == MSG_OBJ_TX) {
+			pch_can_set_tx_enable(priv, i + 1,
+					      priv->tx_enable[i]);
+		}
+	}
+
+	/* Configuring the receive buffer and enabling them. */
+	for (i = 0; i < PCH_OBJ_NUM; i++) {
+		if (priv->msg_obj[i] == MSG_OBJ_RX) {
+			/* Restore buffer link */
+			pch_can_set_rx_buffer_link(priv, i + 1,
+						   priv->rx_link[i]);
+
+			/* Restore buffer enables */
+			pch_can_set_rx_enable(priv, i + 1, priv->rx_enable[i]);
+		}
+	}
+
+	/* Enable CAN Interrupts */
+	pch_can_set_int_custom(priv);
+
+	/* Restore Run Mode */
+	pch_can_set_run_mode(priv, PCH_CAN_RUN);
+
+	return retval;
+}
+#else
+#define pch_can_suspend NULL
+#define pch_can_resume NULL
+#endif
+static int __devinit pch_can_probe(struct pci_dev *pdev,
+				   const struct pci_device_id *id)
+{
+	struct net_device *ndev;
+	struct pch_can_priv *priv;
+	int rc;
+	int index;
+	void __iomem *addr;
+
+	rc = pci_enable_device(pdev);
+	if (rc) {
+		dev_err(&pdev->dev, "Failed pci_enable_device %d\n", rc);
+		goto probe_exit_endev;
+	}
+
+	rc = pci_request_regions(pdev, KBUILD_MODNAME);
+	if (rc) {
+		dev_err(&pdev->dev, "Failed pci_request_regions %d\n", rc);
+		goto probe_exit_pcireq;
+	}
+
+	addr = pci_iomap(pdev, 1, 0);
+	if (!addr) {
+		rc = -EIO;
+		dev_err(&pdev->dev, "Failed pci_iomap\n");
+		goto probe_exit_ipmap;
+	}
+
+	ndev = alloc_candev(sizeof(struct pch_can_priv), 1);
+	if (!ndev) {
+		rc = -ENOMEM;
+		dev_err(&pdev->dev, "Failed alloc_candev\n");
+		goto probe_exit_alloc_candev;
+	}
+
+	priv = netdev_priv(ndev);
+	priv->ndev = ndev;
+	priv->base = addr;
+	priv->regs = addr;
+	priv->dev = pdev;
+	priv->can.bittiming_const = &pch_can_bittiming_const;
+	priv->can.do_set_mode = pch_can_do_set_mode;
+	priv->can.do_get_state = pch_can_get_state;
+	priv->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY |
+				       CAN_CTRLMODE_LOOPBACK;
+	ndev->irq = pdev->irq;
+	ndev->flags |= IFF_ECHO;
+
+	pci_set_drvdata(pdev, ndev);
+	SET_NETDEV_DEV(ndev, &pdev->dev);
+	ndev->netdev_ops = &pch_can_netdev_ops;
+
+	priv->can.clock.freq = PCH_CAN_CLK * 1000; /* Hz to KHz) */
+	for (index = 0; index < PCH_RX_OBJ_NUM;)
+		priv->msg_obj[index++] = MSG_OBJ_RX;
+
+	for (index = index;  index < PCH_OBJ_NUM;)
+		priv->msg_obj[index++] = MSG_OBJ_TX;
+
+	rc = register_candev(ndev);
+	if (rc) {
+		dev_err(&pdev->dev, "Failed register_candev %d\n", rc);
+		goto probe_exit_reg_candev;
+	}
+
+	return 0;
+
+probe_exit_reg_candev:
+	free_candev(ndev);
+probe_exit_alloc_candev:
+	pci_iounmap(pdev, addr);
+probe_exit_ipmap:
+	pci_release_regions(pdev);
+probe_exit_pcireq:
+	pci_disable_device(pdev);
+probe_exit_endev:
+	return rc;
+}
+
+static struct pci_driver pch_can_pcidev = {
+	.name = KBUILD_MODNAME,
+	.id_table = pch_can_pcidev_id,
+	.probe = pch_can_probe,
+	.remove = __devexit_p(pch_can_remove),
+	.suspend = pch_can_suspend,
+	.resume = pch_can_resume,
+};
+
+static int __init pch_can_pci_init(void)
+{
+	return pci_register_driver(&pch_can_pcidev);
+}
+module_init(pch_can_pci_init);
+
+static void __exit pch_can_pci_exit(void)
+{
+	pci_unregister_driver(&pch_can_pcidev);
+}
+module_exit(pch_can_pci_exit);
+
+MODULE_DESCRIPTION("Controller Area Network Driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("0.94");
+MODULE_DEVICE_TABLE(pci, pch_can_pcidev_id);
-- 
1.6.0.6


^ permalink raw reply related

* [PATCH net-next-2.6] net: loopback driver cleanup
From: Eric Dumazet @ 2010-09-24  9:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

loopback driver uses dev->ml_priv to store its percpu stats pointer.
It uses ugly casts "(void __percpu __force *)" to shut up sparse
complains.

Define an union to better document we use ml_priv in loopback driver and
define a lstats field with appropriate types.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/loopback.c    |   20 +++++---------------
 include/linux/netdevice.h |    6 ++++--
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 9a09967..4b0e30b 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -74,7 +74,6 @@ struct pcpu_lstats {
 static netdev_tx_t loopback_xmit(struct sk_buff *skb,
 				 struct net_device *dev)
 {
-	struct pcpu_lstats __percpu *pcpu_lstats;
 	struct pcpu_lstats *lb_stats;
 	int len;
 
@@ -83,8 +82,7 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
 	skb->protocol = eth_type_trans(skb, dev);
 
 	/* it's OK to use per_cpu_ptr() because BHs are off */
-	pcpu_lstats = (void __percpu __force *)dev->ml_priv;
-	lb_stats = this_cpu_ptr(pcpu_lstats);
+	lb_stats = this_cpu_ptr(dev->lstats);
 
 	len = skb->len;
 	if (likely(netif_rx(skb) == NET_RX_SUCCESS)) {
@@ -101,19 +99,17 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
 static struct rtnl_link_stats64 *loopback_get_stats64(struct net_device *dev,
 						      struct rtnl_link_stats64 *stats)
 {
-	const struct pcpu_lstats __percpu *pcpu_lstats;
 	u64 bytes = 0;
 	u64 packets = 0;
 	u64 drops = 0;
 	int i;
 
-	pcpu_lstats = (void __percpu __force *)dev->ml_priv;
 	for_each_possible_cpu(i) {
 		const struct pcpu_lstats *lb_stats;
 		u64 tbytes, tpackets;
 		unsigned int start;
 
-		lb_stats = per_cpu_ptr(pcpu_lstats, i);
+		lb_stats = per_cpu_ptr(dev->lstats, i);
 		do {
 			start = u64_stats_fetch_begin(&lb_stats->syncp);
 			tbytes = lb_stats->bytes;
@@ -147,22 +143,16 @@ static const struct ethtool_ops loopback_ethtool_ops = {
 
 static int loopback_dev_init(struct net_device *dev)
 {
-	struct pcpu_lstats __percpu *lstats;
-
-	lstats = alloc_percpu(struct pcpu_lstats);
-	if (!lstats)
+	dev->lstats = alloc_percpu(struct pcpu_lstats);
+	if (!dev->lstats)
 		return -ENOMEM;
 
-	dev->ml_priv = (void __force *)lstats;
 	return 0;
 }
 
 static void loopback_dev_free(struct net_device *dev)
 {
-	struct pcpu_lstats __percpu *lstats =
-		(void __percpu __force *)dev->ml_priv;
-
-	free_percpu(lstats);
+	free_percpu(dev->lstats);
 	free_netdev(dev);
 }
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f7f1302..65ae79c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1050,8 +1050,10 @@ struct net_device {
 #endif
 
 	/* mid-layer private */
-	void			*ml_priv;
-
+	union {
+		void				*ml_priv;
+		struct pcpu_lstats __percpu	*lstats; /* loopback stats */
+	};
 	/* GARP */
 	struct garp_port	*garp_port;
 



^ permalink raw reply related

* Re: [PATCH net-next-2.6] rps: allocate rx queues in register_netdevice only
From: Eric Dumazet @ 2010-09-24  9:21 UTC (permalink / raw)
  To: John Fastabend
  Cc: Ben Hutchings, David Miller, netdev@vger.kernel.org,
	linux-net-drivers@solarflare.com, Tom Herbert
In-Reply-To: <4C9C5E0C.1000200@intel.com>

Le vendredi 24 septembre 2010 à 01:15 -0700, John Fastabend a écrit :
> On 9/23/2010 8:26 PM, Eric Dumazet wrote:
> > 
> >> Also, I dont understand why we need to restrict
> >> netif_set_real_num_rx_queues() to lower the count.
> >> This wastes memory.
> >>
> >> Why dont we allocate dev->_rx once we know the real count, not in
> >> alloc_netdev_mq() but in register_netdevice() ?
> >>
> 
> Eric,
> 
> At least in the TX case we may not "know" until later how many
> tx_queues we want to use. For example it could change based on
> enabling/disabling features or available interrupts. So we use
> num_tx_queues as the max we ever expect to use and then
> netif_set_real_num_tx_queues() sets the number we want to use.
> 
> I presume for rx queues there are similar cases where features and
> available interrupts may determine how many rx queues are needed.
> 
> Moving the allocation later could help drivers make better max number
> of queue decisions. But, I think we still need the
> netif_set_num_rx_queues() and netif_set_num_tx_queues(). Although this
> does end up wasting memory as you pointed out.
> 

Note I am not against having netif_set_num_rx_queues() and
netif_set_num_tx_queues(). My patch was a cleanup, not an alternative.


If I take a look at sysfs stuff, on a machine with a bnx2 adapter,
single queue, I get :

/sys/class/net/eth0/queues/rx-0/rps_cpus
/sys/class/net/eth0/queues/rx-0/rps_flow_cnt
/sys/class/net/eth0/queues/rx-1/rps_cpus
/sys/class/net/eth0/queues/rx-1/rps_flow_cnt
/sys/class/net/eth0/queues/rx-2/rps_cpus
/sys/class/net/eth0/queues/rx-2/rps_flow_cnt
/sys/class/net/eth0/queues/rx-3/rps_cpus
/sys/class/net/eth0/queues/rx-3/rps_flow_cnt
/sys/class/net/eth0/queues/rx-4/rps_cpus
/sys/class/net/eth0/queues/rx-4/rps_flow_cnt
/sys/class/net/eth0/queues/rx-5/rps_cpus
/sys/class/net/eth0/queues/rx-5/rps_flow_cnt
/sys/class/net/eth0/queues/rx-6/rps_cpus
/sys/class/net/eth0/queues/rx-6/rps_flow_cnt
/sys/class/net/eth0/queues/rx-7/rps_cpus
/sys/class/net/eth0/queues/rx-7/rps_flow_cnt

Thats a lot of extra memory and administrator confusion.

We all agree :)



^ permalink raw reply

* Re: [PATCH 6/8] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
From: Richard Cochran @ 2010-09-24  8:49 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, David Miller,
	John Stultz, Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
	Thomas Gleixner
In-Reply-To: <alpine.DEB.2.00.1009231348150.2962-sBS69tsa9Uj/9pzu0YdTqQ@public.gmane.org>

On Thu, Sep 23, 2010 at 02:17:36PM -0500, Christoph Lameter wrote:
> On Thu, 23 Sep 2010, Richard Cochran wrote:
> > +  These properties set the operational parameters for the PTP
> > +  clock. You must choose these carefully for the clock to work right.
> > +  Here is how to figure good values:
> > +
> > +  TimerOsc     = system clock               MHz
> > +  tclk_period  = desired clock period       nanoseconds
> > +  NominalFreq  = 1000 / tclk_period         MHz
> > +  FreqDivRatio = TimerOsc / NominalFreq     (must be greater that 1.0)
> > +  tmr_add      = ceil(2^32 / FreqDivRatio)
> > +  OutputClock  = NominalFreq / tmr_prsc     MHz
> > +  PulseWidth   = 1 / OutputClock            microseconds
> > +  FiperFreq1   = desired frequency in Hz
> > +  FiperDiv1    = 1000000 * OutputClock / FiperFreq1
> > +  tmr_fiper1   = tmr_prsc * tclk_period * FiperDiv1 - tclk_period
> > +  max_adj      = 1000000000 * (FreqDivRatio - 1.0) - 1
> 
> Great stuff for clock synchronization...
> 
> > +  The calculation for tmr_fiper2 is the same as for tmr_fiper1. The
> > +  driver expects that tmr_fiper1 will be correctly set to produce a 1
> > +  Pulse Per Second (PPS) signal, since this will be offered to the PPS
> > +  subsystem to synchronize the Linux clock.
> 
> Argh. And conceptually completely screwed up. Why go through the PPS
> subsystem if you can directly tune the system clock based on a number of
> the cool periodic clock features that you have above? See how the other
> clocks do that easily? Look into drivers/clocksource. Add it there.
> 
> Please do not introduce useless additional layers for clock sync. Load
> these ptp clocks like the other regular clock modules and make them sync
> system time like any other clock.
> 
> Really guys: I want a PTP solution! Now! And not some idiotic additional
> kernel layers that just pass bits around because its so much fun and
> screws up clock accurary in due to the latency noise introduced while
> having so much fun with the bits.

(Sorry if this message comes twice. Mutt/Gmail flaked out again.)

I think you misunderstood this particular patch. The device tree
parameters are really just internal driver stuff. When you use the
eTSEC, you must make some design choices at the same time as you plan
your board. The proper values for some of the eTSEC registers are
based on these design choices. Since the Freescale documentation is a
bit thin on this, I added a few notes to help my fellow board
designers.

Because these values are closely related to the board itself, I think
that it is nicer to configure them via the device tree than using
either CONFIG_ variables or platform data.

Richard

^ permalink raw reply

* Re: [PATCH v6 0/8] ptp: IEEE 1588 hardware clock support
From: Richard Cochran @ 2010-09-24  8:33 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: John Stultz, Rodolfo Giometti, Peter Zijlstra,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Thomas Gleixner,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, David Miller,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Krzysztof Halasa
In-Reply-To: <alpine.DEB.2.00.1009231238170.2962-sBS69tsa9Uj/9pzu0YdTqQ@public.gmane.org>

On Thu, Sep 23, 2010 at 12:53:20PM -0500, Christoph Lameter wrote:
> On Thu, 23 Sep 2010, Richard Cochran wrote:
> > 3.3 Synchronizing the Linux System Time
> > ========================================
> >
> >    One could offer a PHC as a combined clock source and clock event
> >    device. The advantage of this approach would be that it obviates
> >    the need for synchronization when the PHC is selected as the system
> >    timer. However, some PHCs, namely the PHY based clocks, cannot be
> >    used in this way.
> 
> Why not? Do PHY based clock not at least provide a counter that increments
> in synchronized intervals throughout the network?

The counter in the PHY is accessed via the MDIO bus. One 16 bit read
takes anywhere from 25 to 40 microseconds. Reading the 64 bit time
value requires four reads, so we're talking about 100 to 160
microseconds, just for a single time reading.

In addition to that, reading MDIO bus can sleep.  So, we can't (in
general) to offer PHCs as clock sources.

> >    Instead, the patch set provides a way to offer a Pulse Per Second
> >    (PPS) event from the PHC to the Linux PPS subsystem. A user space
> >    application can read the PPS events and tune the system clock, just
> >    like when using other external time sources like radio clocks or
> >    GPS.
> 
> User space is subject to various latencies created by the OS etc. I would
> that in order to have fine grained (read microsecond) accurary we would
> have to run the portions that are relevant to obtaining the desired
> accuracy in the kernel.

The time-critical operations are all performed in hardware (packet
timestamp), or in kernel space (input PPS timestamp). User space only
runs the servo (using hardware or kernel timestamps as input) and
performs the clock correction. With a sample rate of 1 PPS, the small
user space induced delay (a few dozen microseconds) between sample
time and clock correction is not an issue.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH net-next-2.6] rps: allocate rx queues in register_netdevice only
From: John Fastabend @ 2010-09-24  8:15 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Ben Hutchings, David Miller, netdev@vger.kernel.org,
	linux-net-drivers@solarflare.com, Tom Herbert
In-Reply-To: <1285298795.2380.54.camel@edumazet-laptop>

On 9/23/2010 8:26 PM, Eric Dumazet wrote:
> 
>> Also, I dont understand why we need to restrict
>> netif_set_real_num_rx_queues() to lower the count.
>> This wastes memory.
>>
>> Why dont we allocate dev->_rx once we know the real count, not in
>> alloc_netdev_mq() but in register_netdevice() ?
>>

Eric,

At least in the TX case we may not "know" until later how many tx_queues we want to use. For example it could change based on enabling/disabling features or available interrupts. So we use num_tx_queues as the max we ever expect to use and then netif_set_real_num_tx_queues() sets the number we want to use.

I presume for rx queues there are similar cases where features and available interrupts may determine how many rx queues are needed.

Moving the allocation later could help drivers make better max number of queue decisions. But, I think we still need the netif_set_num_rx_queues() and netif_set_num_tx_queues(). Although this does end up wasting memory as you pointed out.

Thanks,
John. 

>>
> 
> Here is a patch to make this possible
> 
> I guess a similar thing could be done for tx queues.
> 
> boot tested, but please double check.
> 
> Thanks
> 
> [PATCH net-next-2.6] rps: allocate rx queues in register_netdevice()
> 
> Instead of having two places were we allocate dev->_rx, introduce
> netif_alloc_rx_queues() helper and call it only from
> register_netdevice(), not from alloc_netdev_mq()
> 
> Goal is to let drivers change dev->num_rx_queues after allocating netdev
> and before registering it.
> 
> This also removes a lot of ifdefs in net/core/dev.c
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  net/core/dev.c |   76 +++++++++++++++++++----------------------------
>  1 file changed, 32 insertions(+), 44 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 2c7934f..9110b8d 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4964,6 +4964,34 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
>  }
>  EXPORT_SYMBOL(netif_stacked_transfer_operstate);
>  
> +static int netif_alloc_rx_queues(struct net_device *dev)
> +{
> +#ifdef CONFIG_RPS
> +	unsigned int i, count = dev->num_rx_queues;
> +
> +	if (count) {
> +		struct netdev_rx_queue *rx;
> +
> +		rx = kcalloc(count, sizeof(struct netdev_rx_queue), GFP_KERNEL);
> +		if (!rx) {
> +			pr_err("netdev: Unable to allocate %u rx queues.\n",
> +			       count);
> +			return -ENOMEM;
> +		}
> +		dev->_rx = rx;
> +		atomic_set(&rx->count, count);
> +
> +		/*
> +		 * Set a pointer to first element in the array which holds the
> +		 * reference count.
> +		 */
> +		for (i = 0; i < count; i++)
> +			rx[i].first = rx;
> +	}
> +#endif
> +	return 0;
> +}
> +
>  /**
>   *	register_netdevice	- register a network device
>   *	@dev: device to register
> @@ -5001,24 +5029,10 @@ int register_netdevice(struct net_device *dev)
>  
>  	dev->iflink = -1;
>  
> -#ifdef CONFIG_RPS
> -	if (!dev->num_rx_queues) {
> -		/*
> -		 * Allocate a single RX queue if driver never called
> -		 * alloc_netdev_mq
> -		 */
> -
> -		dev->_rx = kzalloc(sizeof(struct netdev_rx_queue), GFP_KERNEL);
> -		if (!dev->_rx) {
> -			ret = -ENOMEM;
> -			goto out;
> -		}
> +	ret = netif_alloc_rx_queues(dev);
> +	if (ret)
> +		goto out;
>  
> -		dev->_rx->first = dev->_rx;
> -		atomic_set(&dev->_rx->count, 1);
> -		dev->num_rx_queues = 1;
> -	}
> -#endif
>  	/* Init, if this function is available */
>  	if (dev->netdev_ops->ndo_init) {
>  		ret = dev->netdev_ops->ndo_init(dev);
> @@ -5414,10 +5428,6 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
>  	struct net_device *dev;
>  	size_t alloc_size;
>  	struct net_device *p;
> -#ifdef CONFIG_RPS
> -	struct netdev_rx_queue *rx;
> -	int i;
> -#endif
>  
>  	BUG_ON(strlen(name) >= sizeof(dev->name));
>  
> @@ -5443,29 +5453,12 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
>  		goto free_p;
>  	}
>  
> -#ifdef CONFIG_RPS
> -	rx = kcalloc(queue_count, sizeof(struct netdev_rx_queue), GFP_KERNEL);
> -	if (!rx) {
> -		printk(KERN_ERR "alloc_netdev: Unable to allocate "
> -		       "rx queues.\n");
> -		goto free_tx;
> -	}
> -
> -	atomic_set(&rx->count, queue_count);
> -
> -	/*
> -	 * Set a pointer to first element in the array which holds the
> -	 * reference count.
> -	 */
> -	for (i = 0; i < queue_count; i++)
> -		rx[i].first = rx;
> -#endif
>  
>  	dev = PTR_ALIGN(p, NETDEV_ALIGN);
>  	dev->padded = (char *)dev - (char *)p;
>  
>  	if (dev_addr_init(dev))
> -		goto free_rx;
> +		goto free_tx;
>  
>  	dev_mc_init(dev);
>  	dev_uc_init(dev);
> @@ -5477,7 +5470,6 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
>  	dev->real_num_tx_queues = queue_count;
>  
>  #ifdef CONFIG_RPS
> -	dev->_rx = rx;
>  	dev->num_rx_queues = queue_count;
>  #endif
>  
> @@ -5495,11 +5487,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
>  	strcpy(dev->name, name);
>  	return dev;
>  
> -free_rx:
> -#ifdef CONFIG_RPS
> -	kfree(rx);
>  free_tx:
> -#endif
>  	kfree(tx);
>  free_p:
>  	kfree(p);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* Re: [PATCH 1/8] posix clocks: introduce a syscall for clock tuning.
From: Richard Cochran @ 2010-09-24  7:55 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, John Stultz,
	Rodolfo Giometti, Peter Zijlstra,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Thomas Gleixner,
	netdev-u79uwXL29TY76Z2rM5mHXA, Christoph Lameter,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, David Miller,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Krzysztof Halasa
In-Reply-To: <1285279423.5158.20.camel@pasglop>

On Fri, Sep 24, 2010 at 08:03:43AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2010-09-23 at 19:31 +0200, Richard Cochran wrote:
> > A new syscall is introduced that allows tuning of a POSIX clock. The
> > syscall is implemented for four architectures: arm, blackfin, powerpc,
> > and x86.
> > 
> > The new syscall, clock_adjtime, takes two parameters, the clock ID,
> > and a pointer to a struct timex. The semantics of the timex struct
> > have been expanded by one additional mode flag, which allows an
> > absolute offset correction. When specificied, the clock offset is
> > immediately corrected by adding the given time value to the current
> > time value.
> 
> Any reason why you CC'ed device-tree discuss ?
> 
> This list is getting way too much unrelated stuff, which I find
> annoying, it would be nice if we were all a bit more careful here with
> our CC lists.

Sorry, I only added device-tree because some one asked me to do so.

    http://marc.info/?l=linux-netdev&m=127273157912358

I'll leave it off next time.

Thanks,
Richard

^ permalink raw reply

* [PATCH net-next-2.6] net: fix rcu use in ip_route_output_slow
From: Eric Dumazet @ 2010-09-24  7:46 UTC (permalink / raw)
  To: paulmck, David Miller; +Cc: akpm, netdev
In-Reply-To: <20100922231221.GS2435@linux.vnet.ibm.com>

Le mercredi 22 septembre 2010 à 16:12 -0700, Paul E. McKenney a écrit :
> On Thu, Sep 23, 2010 at 01:04:25AM +0200, Eric Dumazet wrote:
> > Le mercredi 22 septembre 2010 à 14:44 -0700, Paul E. McKenney a écrit :
> > > > Date: Wed, 22 Sep 2010 13:52:28 -0700
> > > > From: Andrew Morton <akpm@linux-foundation.org>
> > > > To: "Paul E. McKenney" <paulmck@us.ibm.com>
> > > > Subject: rcu warning
> > > > X-Mailer: Sylpheed 2.4.8 (GTK+ 2.12.9; x86_64-pc-linux-gnu)
> > > > 
> > > > [   56.803750] 
> > > > [   56.803752] ===================================================
> > > > [   56.804082] [ INFO: suspicious rcu_dereference_check() usage. ]
> > > > [   56.804249] ---------------------------------------------------
> > > > [   56.804421] include/linux/inetdevice.h:219 invoked rcu_dereference_check() without protection!
> > > > [   56.804708] 
> > > > [   56.804709] other info that might help us debug this:
> > > > [   56.804710] 
> > > > [   56.805183] 
> > > > [   56.805184] rcu_scheduler_active = 1, debug_locks = 1
> > > > [   56.805501] 3 locks held by kworker/0:1/0:
> > > > [   56.805664]  #0:  (&in_dev->mr_ifc_timer){+.-...}, at: [<ffffffff81042466>] run_timer_softirq+0xfd/0x226
> > > > [   56.806126]  #1:  (&in_dev->mc_list_lock){++.-..}, at: [<ffffffff8133e81d>] igmp_ifc_timer_expire+0x2a/0x221
> > > > [   56.806588]  #2:  (&(&im->lock)->rlock){+.-...}, at: [<ffffffff8133e948>] igmp_ifc_timer_expire+0x155/0x221
> > > > [   56.807043] 
> > > > [   56.807044] stack backtrace:
> > > > [   56.807364] Pid: 0, comm: kworker/0:1 Not tainted 2.6.36-rc5-mm1 #1
> > > > [   56.807561] Call Trace:
> > > > [   56.807723]  <IRQ>  [<ffffffff8105b88b>] lockdep_rcu_dereference+0x99/0xa2
> > > > [   56.807948]  [<ffffffff8130dc66>] __ip_route_output_key+0x34f/0xb19
> > > > [   56.808120]  [<ffffffff8130d94a>] ? __ip_route_output_key+0x33/0xb19
> > > > [   56.814367]  [<ffffffff8130e453>] ip_route_output_flow+0x23/0x1ee
> > > > [   56.814536]  [<ffffffff8130e62c>] ip_route_output_key+0xe/0x10
> > > > [   56.814704]  [<ffffffff8133e19d>] igmpv3_newpack+0x7f/0x1c2
> > > > [   56.814873]  [<ffffffff8133e30d>] add_grhead+0x2d/0x94
> > > > [   56.815039]  [<ffffffff8133e6c2>] add_grec+0x34e/0x38c
> > > > [   56.815206]  [<ffffffff8133e9a8>] igmp_ifc_timer_expire+0x1b5/0x221
> > > > [   56.815375]  [<ffffffff810424e8>] run_timer_softirq+0x17f/0x226
> > > > [   56.815547]  [<ffffffff81042466>] ? run_timer_softirq+0xfd/0x226
> > > > [   56.815715]  [<ffffffff8133e7f3>] ? igmp_ifc_timer_expire+0x0/0x221
> > > > [   56.815885]  [<ffffffff8103ca8f>] __do_softirq+0xa5/0x13a
> > > > [   56.816051]  [<ffffffff8100390c>] call_softirq+0x1c/0x28
> > > > [   56.816219]  [<ffffffff81004eba>] do_softirq+0x38/0x82
> > > > [   56.816385]  [<ffffffff8103c9e8>] irq_exit+0x47/0x49
> > > > [   56.816553]  [<ffffffff81019ce3>] smp_apic_timer_interrupt+0x88/0x96
> > > > [   56.816722]  [<ffffffff810033d3>] apic_timer_interrupt+0x13/0x20
> > > > [   56.816888]  <EOI>  [<ffffffff8138607a>] ? __atomic_notifier_call_chain+0x0/0x84
> > > > [   56.817215]  [<ffffffff81009a9b>] ? mwait_idle+0x65/0x71
> > > > [   56.817382]  [<ffffffff81009a91>] ? mwait_idle+0x5b/0x71
> > > > [   56.817549]  [<ffffffff810014ca>] cpu_idle+0x48/0x66
> > > > [   56.817716]  [<ffffffff8137b4da>] start_secondary+0x1b9/0x1bd
> > > > [   56.817883]  [<ffffffff8137b321>] ? start_secondary+0x0/0x1bd
> > > 
> > > Hello, Eric,
> > > 
> > > In linux/master, there is an rcu_read_lock_bh() in the call path, but
> > > an rcu_dereference() instead of an rcu_dereference_bh().  Thoughts?
> > > 
> > > (I have asked Andrew what kernel this is against -- I don't see the
> > > rcu_read_lock() that I would expect to see in the lockdep output.)
> > > 
> > > 							Thanx, Paul
> > 
> > This seems strange
> > 
> > include/linux/inetdevice.h:219
> > 
> > static inline struct in_device *__in_dev_get_rtnl(const struct net_device *dev)
> > {
> > 	return rcu_dereference_check(dev->ip_ptr, lockdep_rtnl_is_held());
> > }
> > 
> > But I dont think RTNL can possibly be held at this point ???
> > 
> > Oh wait, this is line 2582 in net/ipv4/route.c
> > 
> > It seems buggy and proud of it :)
> > 
> > 	/* RACE: Check return value of inet_select_addr instead. */ 
> > 	if (__in_dev_get_rtnl(dev_out) == NULL) {
> > 
> > This should be changed to 
> > 
> > 	if (rcu_dereference_raw(dev_out->ip_ptr) == NULL) {
> > 
> > No ?
> 
> You beat me to it.  ;-)
> 
> 							Thanx, Paul

OK, here is official submission to David then.

I based it against net-next-2.6.

Thanks

[PATCH net-next-2.6] net: fix rcu use in ip_route_output_slow

__in_dev_get_rtnl(dev_out) is called while RTNL is not held, thus
triggers a lockdep fault.

At this point, we only perform a raw test of dev_out->ip_ptr being NULL,
we dont need to make sure ip_ptr cant changed right after.

We can use rcu_dereference_raw() for this.

Reported-by: Andrew Morton <akpm@linux-foundation.org>
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv4/route.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index e24d48d..8d08377 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2579,7 +2579,7 @@ static int ip_route_output_slow(struct net *net, struct rtable **rp,
 			goto out;
 
 		/* RACE: Check return value of inet_select_addr instead. */
-		if (__in_dev_get_rtnl(dev_out) == NULL) {
+		if (rcu_dereference_raw(dev_out->ip_ptr) == NULL) {
 			dev_put(dev_out);
 			goto out;	/* Wrong error code */
 		}



^ permalink raw reply related

* Re: [PATCH 1/8] posix clocks: introduce a syscall for clock tuning.
From: Richard Cochran @ 2010-09-24  7:29 UTC (permalink / raw)
  To: john stultz
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, Christoph Lameter,
	David Miller, Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
	Thomas Gleixner
In-Reply-To: <1285271331.2587.56.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>

On Thu, Sep 23, 2010 at 12:48:51PM -0700, john stultz wrote:
> On Thu, 2010-09-23 at 19:31 +0200, Richard Cochran wrote:
> > A new syscall is introduced that allows tuning of a POSIX clock. The
> > syscall is implemented for four architectures: arm, blackfin, powerpc,
> > and x86.
> > 
> > The new syscall, clock_adjtime, takes two parameters, the clock ID,
> > and a pointer to a struct timex. The semantics of the timex struct
> > have been expanded by one additional mode flag, which allows an
> > absolute offset correction. When specificied, the clock offset is
> > immediately corrected by adding the given time value to the current
> > time value.
> 
> 
> So I'd still split this patch up a little bit more.
> 
> 1) Patch that implements the ADJ_SETOFFSET  (*and its implementation*)
> in do_adjtimex.
> 
> 2) Patch that adds the new syscall and clock_id multiplexing.
> 
> 3) Patches that wire it up to the rest of the architectures (there's
> still a bunch missing here).

I was not sure what the policy is about adding syscalls. Is it the
syscall author's responsibility to add it into every arch?

The last time (see a2e2725541fad7) the commit only added half of some
archs, and ignored others. In my patch, the syscall *really* works on
the archs that are present in the patch.

(Actually, I did not test blackfin, since I don't have one, but I
included it since I know they have a PTP hardware clock.)

> > +static inline int common_clock_adj(const clockid_t which_clock, struct timex *t)
> > +{
> > +	if (CLOCK_REALTIME == which_clock)
> > +		return do_adjtimex(t);
> > +	else
> > +		return -EOPNOTSUPP;
> > +}
> 
> 
> Would it make sense to point to the do_adjtimex() in the k_clock
> definition for CLOCK_REALTIME rather then conditionalizing it here?

But what about CLOCK_MONOTONIC_RAW, for example?

Does it make sense to allow it to be adjusted?

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH net-next-2.6] rps: allocate rx queues in register_netdevice only
From: Eric Dumazet @ 2010-09-24  7:07 UTC (permalink / raw)
  To: Ben Hutchings, Tom Herbert
  Cc: David Miller, netdev, linux-net-drivers, Tom Herbert
In-Reply-To: <1285298795.2380.54.camel@edumazet-laptop>

Le vendredi 24 septembre 2010 à 05:26 +0200, Eric Dumazet a écrit :
> > Also, I dont understand why we need to restrict
> > netif_set_real_num_rx_queues() to lower the count.
> > This wastes memory.
> > 
> > Why dont we allocate dev->_rx once we know the real count, not in
> > alloc_netdev_mq() but in register_netdevice() ?
> > 
> > 
> 
> Here is a patch to make this possible
> 
> I guess a similar thing could be done for tx queues.
> 
> boot tested, but please double check.
> 
> Thanks
> 
> [PATCH net-next-2.6] rps: allocate rx queues in register_netdevice()
> 
> Instead of having two places were we allocate dev->_rx, introduce
> netif_alloc_rx_queues() helper and call it only from
> register_netdevice(), not from alloc_netdev_mq()
> 
> Goal is to let drivers change dev->num_rx_queues after allocating netdev
> and before registering it.
> 
> This also removes a lot of ifdefs in net/core/dev.c
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  net/core/dev.c |   76 +++++++++++++++++++----------------------------
>  1 file changed, 32 insertions(+), 44 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 2c7934f..9110b8d 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4964,6 +4964,34 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
>  }
>  EXPORT_SYMBOL(netif_stacked_transfer_operstate);
>  
> +static int netif_alloc_rx_queues(struct net_device *dev)
> +{
> +#ifdef CONFIG_RPS
> +	unsigned int i, count = dev->num_rx_queues;
> +

Tom

I tried to find a caller with dev->num_rx_queues = 0, but failed

If it's valid, we might add a

	if (!dev->num_rx_queues)
		dev->num_rx_queues = 1;

I wonder if you remember why you added the section :
	/*
	 * Allocate a single RX queue if driver never called
	 * alloc_netdev_mq
	 */
> 
> 
> +	if (count) {
> +		struct netdev_rx_queue *rx;
> +
> +		rx = kcalloc(count, sizeof(struct netdev_rx_queue), GFP_KERNEL);
> +		if (!rx) {
> +			pr_err("netdev: Unable to allocate %u rx queues.\n",
> +			       count);
> +			return -ENOMEM;
> +		}
> +		dev->_rx = rx;
> +		atomic_set(&rx->count, count);
> +
> +		/*
> +		 * Set a pointer to first element in the array which holds the
> +		 * reference count.
> +		 */
> +		for (i = 0; i < count; i++)
> +			rx[i].first = rx;
> +	}
> +#endif
> +	return 0;
> +}
> +
>  /**
>   *	register_netdevice	- register a network device
>   *	@dev: device to register
> @@ -5001,24 +5029,10 @@ int register_netdevice(struct net_device *dev)
>  
>  	dev->iflink = -1;
>  
> -#ifdef CONFIG_RPS
> -	if (!dev->num_rx_queues) {
> -		/*
> -		 * Allocate a single RX queue if driver never called
> -		 * alloc_netdev_mq
> -		 */
> -
> -		dev->_rx = kzalloc(sizeof(struct netdev_rx_queue), GFP_KERNEL);
> -		if (!dev->_rx) {
> -			ret = -ENOMEM;
> -			goto out;
> -		}
> +	ret = netif_alloc_rx_queues(dev);
> +	if (ret)
> +		goto out;
>  
> -		dev->_rx->first = dev->_rx;
> -		atomic_set(&dev->_rx->count, 1);
> -		dev->num_rx_queues = 1;
> -	}
> -#endif
>  	/* Init, if this function is available */
>  	if (dev->netdev_ops->ndo_init) {
>  		ret = dev->netdev_ops->ndo_init(dev);
> @@ -5414,10 +5428,6 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
>  	struct net_device *dev;
>  	size_t alloc_size;
>  	struct net_device *p;
> -#ifdef CONFIG_RPS
> -	struct netdev_rx_queue *rx;
> -	int i;
> -#endif
>  
>  	BUG_ON(strlen(name) >= sizeof(dev->name));
>  
> @@ -5443,29 +5453,12 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
>  		goto free_p;
>  	}
>  
> -#ifdef CONFIG_RPS
> -	rx = kcalloc(queue_count, sizeof(struct netdev_rx_queue), GFP_KERNEL);
> -	if (!rx) {
> -		printk(KERN_ERR "alloc_netdev: Unable to allocate "
> -		       "rx queues.\n");
> -		goto free_tx;
> -	}
> -
> -	atomic_set(&rx->count, queue_count);
> -
> -	/*
> -	 * Set a pointer to first element in the array which holds the
> -	 * reference count.
> -	 */
> -	for (i = 0; i < queue_count; i++)
> -		rx[i].first = rx;
> -#endif
>  
>  	dev = PTR_ALIGN(p, NETDEV_ALIGN);
>  	dev->padded = (char *)dev - (char *)p;
>  
>  	if (dev_addr_init(dev))
> -		goto free_rx;
> +		goto free_tx;
>  
>  	dev_mc_init(dev);
>  	dev_uc_init(dev);
> @@ -5477,7 +5470,6 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
>  	dev->real_num_tx_queues = queue_count;
>  
>  #ifdef CONFIG_RPS
> -	dev->_rx = rx;
>  	dev->num_rx_queues = queue_count;
>  #endif
>  
> @@ -5495,11 +5487,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
>  	strcpy(dev->name, name);
>  	return dev;
>  
> -free_rx:
> -#ifdef CONFIG_RPS
> -	kfree(rx);
>  free_tx:
> -#endif
>  	kfree(tx);
>  free_p:
>  	kfree(p);
> 



^ permalink raw reply

* Re: [PATCH v3] net: af_packet: don't call tpacket_destruct_skb() until the skb is sent out
From: Eric Dumazet @ 2010-09-24  7:01 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Changli Gao, David S. Miller, Oliver Hartkopp, Michael S. Tsirkin,
	netdev
In-Reply-To: <20100924063623.GA6359@ff.dom.local>

Le vendredi 24 septembre 2010 à 06:36 +0000, Jarek Poplawski a écrit :
> On 2010-09-23 12:15, Changli Gao wrote:
> > Since skb->destructor() is used to account socket memory, and maybe called
> > before the skb is sent out, a corrupt skb maybe sent out finally.
> > 
> > A new destructor is added into structure skb_shared_info(), and it won't
> > be called until the last reference to the data of an skb is put. af_packet
> > uses this destructor instead.
> 
> IMHO, we shouldn't allow for fixing the bad design of one protocol at
> the expense of others by adding more and more conditionals. The proper
> way of handling paged skbs (splice compatible) exists. And the current
> patch doesn't even fix the problem completely against things like
> pskb_expand_head or pskb_copy.
> 
> af_packet could check some flag which guarantees the queued dev can do
> skb_orphan after the real xmit and copy buffers otherwise.

Agreed.

af_packet (tx with mmap) is broken. I wonder who really uses it ?

To properly cope with paged skbs, it should not try to fit several
packets per page.

The mmap api should change so that one mmaped page belongs to at most
one skb, or else we need invasive changes in net/core

This probably makes this stuff less interesting, unless the need is to
send big packets. In this case, why splice was not used instead of
custom mmap ?



^ permalink raw reply

* Re: [PATCH v3] net: af_packet: don't call tpacket_destruct_skb() until the skb is sent out
From: Jarek Poplawski @ 2010-09-24  6:36 UTC (permalink / raw)
  To: Changli Gao
  Cc: David S. Miller, Eric Dumazet, Oliver Hartkopp,
	Michael S. Tsirkin, netdev
In-Reply-To: <1285236939-3239-1-git-send-email-xiaosuo@gmail.com>

On 2010-09-23 12:15, Changli Gao wrote:
> Since skb->destructor() is used to account socket memory, and maybe called
> before the skb is sent out, a corrupt skb maybe sent out finally.
> 
> A new destructor is added into structure skb_shared_info(), and it won't
> be called until the last reference to the data of an skb is put. af_packet
> uses this destructor instead.

IMHO, we shouldn't allow for fixing the bad design of one protocol at
the expense of others by adding more and more conditionals. The proper
way of handling paged skbs (splice compatible) exists. And the current
patch doesn't even fix the problem completely against things like
pskb_expand_head or pskb_copy.

af_packet could check some flag which guarantees the queued dev can do
skb_orphan after the real xmit and copy buffers otherwise.

Jarek P.

^ permalink raw reply

* Re: igmp: Allow mininum interval specification for igmp timers.
From: David Miller @ 2010-09-24  4:38 UTC (permalink / raw)
  To: cl-vYTEC60ixJUAvxtiuMwx3w
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	rda-x0S3BwdUo6DQT0dZR+AlfA, dlstevens-r/Jw6+rmf7HQT0dZR+AlfA
In-Reply-To: <alpine.DEB.2.00.1009221354410.32661-sBS69tsa9Uj/9pzu0YdTqQ@public.gmane.org>

From: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
Date: Wed, 22 Sep 2010 13:59:30 -0500 (CDT)

> IGMP timers sometimes fire too rapidly due to randomization of the
> intervalsfrom 0 to max_delay in igmp_start_timer().
 ...
> Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>


This change seems reasonable to me, what do you think David?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH net-next-2.6] rps: allocate rx queues in register_netdevice only
From: Eric Dumazet @ 2010-09-24  3:26 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, netdev, linux-net-drivers, Tom Herbert
In-Reply-To: <1285296505.2380.43.camel@edumazet-laptop>


> Also, I dont understand why we need to restrict
> netif_set_real_num_rx_queues() to lower the count.
> This wastes memory.
> 
> Why dont we allocate dev->_rx once we know the real count, not in
> alloc_netdev_mq() but in register_netdevice() ?
> 
> 

Here is a patch to make this possible

I guess a similar thing could be done for tx queues.

boot tested, but please double check.

Thanks

[PATCH net-next-2.6] rps: allocate rx queues in register_netdevice()

Instead of having two places were we allocate dev->_rx, introduce
netif_alloc_rx_queues() helper and call it only from
register_netdevice(), not from alloc_netdev_mq()

Goal is to let drivers change dev->num_rx_queues after allocating netdev
and before registering it.

This also removes a lot of ifdefs in net/core/dev.c

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/core/dev.c |   76 +++++++++++++++++++----------------------------
 1 file changed, 32 insertions(+), 44 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 2c7934f..9110b8d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4964,6 +4964,34 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
 }
 EXPORT_SYMBOL(netif_stacked_transfer_operstate);
 
+static int netif_alloc_rx_queues(struct net_device *dev)
+{
+#ifdef CONFIG_RPS
+	unsigned int i, count = dev->num_rx_queues;
+
+	if (count) {
+		struct netdev_rx_queue *rx;
+
+		rx = kcalloc(count, sizeof(struct netdev_rx_queue), GFP_KERNEL);
+		if (!rx) {
+			pr_err("netdev: Unable to allocate %u rx queues.\n",
+			       count);
+			return -ENOMEM;
+		}
+		dev->_rx = rx;
+		atomic_set(&rx->count, count);
+
+		/*
+		 * Set a pointer to first element in the array which holds the
+		 * reference count.
+		 */
+		for (i = 0; i < count; i++)
+			rx[i].first = rx;
+	}
+#endif
+	return 0;
+}
+
 /**
  *	register_netdevice	- register a network device
  *	@dev: device to register
@@ -5001,24 +5029,10 @@ int register_netdevice(struct net_device *dev)
 
 	dev->iflink = -1;
 
-#ifdef CONFIG_RPS
-	if (!dev->num_rx_queues) {
-		/*
-		 * Allocate a single RX queue if driver never called
-		 * alloc_netdev_mq
-		 */
-
-		dev->_rx = kzalloc(sizeof(struct netdev_rx_queue), GFP_KERNEL);
-		if (!dev->_rx) {
-			ret = -ENOMEM;
-			goto out;
-		}
+	ret = netif_alloc_rx_queues(dev);
+	if (ret)
+		goto out;
 
-		dev->_rx->first = dev->_rx;
-		atomic_set(&dev->_rx->count, 1);
-		dev->num_rx_queues = 1;
-	}
-#endif
 	/* Init, if this function is available */
 	if (dev->netdev_ops->ndo_init) {
 		ret = dev->netdev_ops->ndo_init(dev);
@@ -5414,10 +5428,6 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 	struct net_device *dev;
 	size_t alloc_size;
 	struct net_device *p;
-#ifdef CONFIG_RPS
-	struct netdev_rx_queue *rx;
-	int i;
-#endif
 
 	BUG_ON(strlen(name) >= sizeof(dev->name));
 
@@ -5443,29 +5453,12 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 		goto free_p;
 	}
 
-#ifdef CONFIG_RPS
-	rx = kcalloc(queue_count, sizeof(struct netdev_rx_queue), GFP_KERNEL);
-	if (!rx) {
-		printk(KERN_ERR "alloc_netdev: Unable to allocate "
-		       "rx queues.\n");
-		goto free_tx;
-	}
-
-	atomic_set(&rx->count, queue_count);
-
-	/*
-	 * Set a pointer to first element in the array which holds the
-	 * reference count.
-	 */
-	for (i = 0; i < queue_count; i++)
-		rx[i].first = rx;
-#endif
 
 	dev = PTR_ALIGN(p, NETDEV_ALIGN);
 	dev->padded = (char *)dev - (char *)p;
 
 	if (dev_addr_init(dev))
-		goto free_rx;
+		goto free_tx;
 
 	dev_mc_init(dev);
 	dev_uc_init(dev);
@@ -5477,7 +5470,6 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 	dev->real_num_tx_queues = queue_count;
 
 #ifdef CONFIG_RPS
-	dev->_rx = rx;
 	dev->num_rx_queues = queue_count;
 #endif
 
@@ -5495,11 +5487,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 	strcpy(dev->name, name);
 	return dev;
 
-free_rx:
-#ifdef CONFIG_RPS
-	kfree(rx);
 free_tx:
-#endif
 	kfree(tx);
 free_p:
 	kfree(p);



^ permalink raw reply related

* Re: [PATCH net-next-2.6 1/2] net: Allow changing number of RX queues after device allocation
From: Eric Dumazet @ 2010-09-24  2:48 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, netdev, linux-net-drivers, Tom Herbert
In-Reply-To: <1285273194.7794.24.camel@achroite.uk.solarflarecom.com>

Le jeudi 23 septembre 2010 à 21:19 +0100, Ben Hutchings a écrit :
> For RPS, we create a kobject for each RX queue based on the number of
> queues passed to alloc_netdev_mq().  However, drivers generally do not
> determine the numbers of hardware queues to use until much later, so
> this usually represents the maximum number the driver may use and not
> the actual number in use.
> 
> For TX queues, drivers can update the actual number using
> netif_set_real_num_tx_queues().  Add a corresponding function for RX
> queues, netif_set_real_num_rx_queues().
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---

Seems very good to me, except a minor style issue here :

All functions in this file use a single line.

> -static int rx_queue_register_kobjects(struct net_device *net)
> +int
> +net_rx_queue_update_kobjects(struct net_device *net, int old_num, int new_num)

->

int net_rx_queue_update_kobjects(struct net_device *net, int old, int new)


Also, I dont understand why we need to restrict
netif_set_real_num_rx_queues() to lower the count.
This wastes memory.

Why dont we allocate dev->_rx once we know the real count, not in
alloc_netdev_mq() but in register_netdevice() ?




^ permalink raw reply

* Re: [PATCH] net: reset skb queue mapping when rx'ing over tunnel
From: Eric Dumazet @ 2010-09-24  2:06 UTC (permalink / raw)
  To: Tom Herbert; +Cc: netdev, davem, chavey
In-Reply-To: <alpine.DEB.1.00.1009231415290.25121@pokey.mtv.corp.google.com>

Le jeudi 23 septembre 2010 à 14:19 -0700, Tom Herbert a écrit :
> Reset queue mapping when an skb is reentering the stack via a tunnel.
> On second pass, the queue mapping from the original device is no
> longer valid.
> 
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 81d1413..0238650 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -242,6 +242,7 @@ static inline void skb_tunnel_rx(struct sk_buff *skb, struct net_device *dev)
>  	dev->stats.rx_packets++;
>  	dev->stats.rx_bytes += skb->len;
>  	skb->rxhash = 0;
> +	skb_set_queue_mapping(skb, 0);
>  	skb_dst_drop(skb);
>  	nf_reset(skb);
>  }
> --

Hmm...

This would need to be reverted later when tunnels are updated to be
multiqueue aware ? I made an attempt with GRE some days ago.

I dont understand why this patch is needed, since get_rps_cpu() has a
check anyway

        if (skb_rx_queue_recorded(skb)) {
                u16 index = skb_get_rx_queue(skb);
                if (unlikely(index >= dev->num_rx_queues)) {
                        WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
                                "on queue %u, but number of RX queues is %u\n",
                                dev->name, index, dev->num_rx_queues);
                        goto done;
                }
                rxqueue = dev->_rx + index;
        } else
                rxqueue = dev->_rx;




^ permalink raw reply

* Re: [PATCH v3] xmit_compl_seq: information to reclaim vmsplice buffers
From: Eric Dumazet @ 2010-09-24  1:48 UTC (permalink / raw)
  To: Tom Herbert; +Cc: netdev, davem, sridharr
In-Reply-To: <alpine.DEB.1.00.1009231426450.11579@pokey.mtv.corp.google.com>

Le jeudi 23 septembre 2010 à 14:35 -0700, Tom Herbert a écrit :

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 3e8a4db..3d8d33f 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1768,6 +1768,12 @@ skip_copy:
>  
>  	TCP_CHECK_TIMER(sk);
>  	release_sock(sk);
> +
> +	/* Copy the first unacked seq into the receive msg control part. */
> +	if (sock_flag(sk, SOCK_XMIT_COMPL_SEQ))
> +		put_cmsg(msg, SOL_SOCKET, SCM_XMIT_COMPL_SEQ,
> +		    sizeof(tp->snd_una), &tp->snd_una);
> +
>  	return copied;
>  
>  out:

Tom,

I took the time to suggest : copy tp->snd_una before release_sock().

Why do you try to avoid this copy and introduce a bug ?

Hint : put_cmsg() (and copy_to_user()) makes no guarantee the copy is
atomic. 

It can be implemented by a 4 bytes copy, faut during this copy or being
interrupted, and application might get a garbled value, if tcp stack
modifies tp->snd_una during the copy.

If you want to optimize this (because release_sock() can process the
socket backlog, and update snd_una), please use :


	/* Copy the first unacked seq into the receive msg control part.
	 * As put_cmsg() might sleep, we must copy snd_una in a private var.
	 * Integer reads are atomic on all arches, so this copy can be
	 * performed while socket is unlocked.
	 */
	if (sock_flag(sk, SOCK_XMIT_COMPL_SEQ)) {
		u32 snd_una = tp->snd_una;

		put_cmsg(msg, SOL_SOCKET, SCM_XMIT_COMPL_SEQ,
			 sizeof(snd_una), &snd_una);
	}

Thanks !



^ permalink raw reply

* Re: [PATCH 1/8] posix clocks: introduce a syscall for clock tuning.
From: Benjamin Herrenschmidt @ 2010-09-24  1:20 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Peter Zijlstra, John Stultz, Richard Cochran,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, LKML, David Miller,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Christoph Lameter, Rodolfo Giometti,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Krzysztof Halasa
In-Reply-To: <alpine.LFD.2.00.1009240008390.2416-bi+AKbBUZKagILUCTcTcHdKyNwTtLsGr@public.gmane.org>

On Fri, 2010-09-24 at 00:12 +0200, Thomas Gleixner wrote:
> > This list is getting way too much unrelated stuff, which I find
> > annoying, it would be nice if we were all a bit more careful here
> with
> > our CC lists.
> 
> Says the guy who missed to trim the useless context of the original
> mail, which made me scroll down all the way just to find out that
> there is nothing to see. 

Heh, you can usually ignore what's after my signature :-) At least I
didn't put my reply all the way down the bottom !

Cheers,
Ben.

^ permalink raw reply

* bonding vs GSO vs checksumming.
From: Maciej Żenczykowski @ 2010-09-24  0:56 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Linux NetDev

I'm seeing the following:

bonding: caps=(0x1113a3, 0x1113ab) len=18192 data_len=18120 ip_summed=0

on a 2.6.34.5 based kernel originating from v6 tcp transmit (mtu 1280)
on a bonded device of tg3 (presumably with TSO6) and forcedeth
(presumably without TSO6).

I can't quite figure out what this truly means...

Help ;-)

^ permalink raw reply

* [PATCH 3/4] cxgb3i: fixed connection over vlan
From: kxie @ 2010-09-23 23:43 UTC (permalink / raw)
  To: netdev, linux-scsi, open-iscsi
  Cc: rranjan, kxie, James.Bottomley, michaelc, davem

[PATCH 3/4] cxgb3i: fixed connection over vlan
          
From: Karen Xie <kxie@chelsio.com>
          
This patch fixed the problem of connecting over VLAN.

Signed-off-by: Karen Xie <kxie@chelsio.com>
---

 drivers/scsi/cxgbi/cxgb3i/cxgb3i.c |   47 +++++++++++++++++++++++++++++++-----
 drivers/scsi/cxgbi/libcxgbi.c      |   37 ++++++----------------------
 drivers/scsi/cxgbi/libcxgbi.h      |    1 +
 3 files changed, 49 insertions(+), 36 deletions(-)


diff --git a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
index a2c207f..a129a17 100644
--- a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
+++ b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
@@ -589,9 +589,10 @@ static int do_act_open_rpl(struct t3cdev *tdev, struct sk_buff *skb, void *ctx)
 	struct cxgbi_sock *csk = ctx;
 	struct cpl_act_open_rpl *rpl = cplhdr(skb);
 
-	log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
-		"csk 0x%p,%u,0x%lx,%u, status %u.\n",
-		csk, csk->state, csk->flags, csk->atid, rpl->status);
+	pr_info("csk 0x%p,%u,0x%lx,%u, status %u, %pI4:%u-%pI4:%u.\n",
+		csk, csk->state, csk->flags, csk->atid, rpl->status,
+		&csk->saddr.sin_addr.s_addr, ntohs(csk->saddr.sin_port),
+		&csk->daddr.sin_addr.s_addr, ntohs(csk->daddr.sin_port));
 
 	if (rpl->status != CPL_ERR_TCAM_FULL &&
 	    rpl->status != CPL_ERR_CONN_EXIST &&
@@ -662,8 +663,7 @@ static int abort_status_to_errno(struct cxgbi_sock *csk, int abort_reason,
 	switch (abort_reason) {
 	case CPL_ERR_BAD_SYN: /* fall through */
 	case CPL_ERR_CONN_RESET:
-		return csk->state > CTP_ESTABLISHED ?
-			-EPIPE : -ECONNRESET;
+		return csk->state > CTP_ESTABLISHED ? -EPIPE : -ECONNRESET;
 	case CPL_ERR_XMIT_TIMEDOUT:
 	case CPL_ERR_PERSIST_TIMEDOUT:
 	case CPL_ERR_FINWAIT2_TIMEDOUT:
@@ -945,17 +945,44 @@ static void release_offload_resources(struct cxgbi_sock *csk)
 	csk->cdev = NULL;
 }
 
+static void update_address(struct cxgbi_hba *chba)
+{
+	if (chba->ipv4addr) {
+		if (chba->vdev &&
+		    chba->ipv4addr != cxgb3i_get_private_ipv4addr(chba->vdev)) {
+			cxgb3i_set_private_ipv4addr(chba->vdev, chba->ipv4addr);
+			cxgb3i_set_private_ipv4addr(chba->ndev, 0);
+			pr_info("%s set %pI4.\n",
+				chba->vdev->name, &chba->ipv4addr);
+		} else if (chba->ipv4addr !=
+				cxgb3i_get_private_ipv4addr(chba->ndev)) {
+			cxgb3i_set_private_ipv4addr(chba->ndev, chba->ipv4addr);
+			pr_info("%s set %pI4.\n",
+				chba->ndev->name, &chba->ipv4addr);
+		}
+	} else if (cxgb3i_get_private_ipv4addr(chba->ndev)) {
+		if (chba->vdev)
+			cxgb3i_set_private_ipv4addr(chba->vdev, 0);
+		cxgb3i_set_private_ipv4addr(chba->ndev, 0);
+	}
+}
+
 static int init_act_open(struct cxgbi_sock *csk)
 {
 	struct dst_entry *dst = csk->dst;
 	struct cxgbi_device *cdev = csk->cdev;
 	struct t3cdev *t3dev = (struct t3cdev *)cdev->lldev;
 	struct net_device *ndev = cdev->ports[csk->port_id];
+	struct cxgbi_hba *chba = cdev->hbas[csk->port_id];
 	struct sk_buff *skb = NULL;
 
 	log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
 		"csk 0x%p,%u,0x%lx.\n", csk, csk->state, csk->flags);
 
+	update_address(chba);
+	if (chba->ipv4addr)
+		csk->saddr.sin_addr.s_addr = chba->ipv4addr;
+
 	csk->rss_qid = 0;
 	csk->l2t = t3_l2t_get(t3dev, dst->neighbour, ndev);
 	if (!csk->l2t) {
@@ -984,6 +1011,12 @@ static int init_act_open(struct cxgbi_sock *csk)
 	cxgbi_sock_reset_wr_list(csk);
 	csk->err = 0;
 
+	log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
+		"csk 0x%p,%u,0x%lx, %pI4:%u-%pI4:%u.\n",
+		csk, csk->state, csk->flags,
+		&csk->saddr.sin_addr.s_addr, ntohs(csk->saddr.sin_port),
+		&csk->daddr.sin_addr.s_addr, ntohs(csk->daddr.sin_port));
+
 	cxgbi_sock_set_state(csk, CTP_ACTIVE_OPEN);
 	send_act_open_req(csk, skb, csk->l2t);
 	return 0;
@@ -1143,9 +1176,9 @@ static int ddp_alloc_gl_skb(struct cxgbi_ddp_info *ddp, int idx,
 	for (i = 0; i < cnt; i++) {
 		struct sk_buff *skb = alloc_wr(sizeof(struct ulp_mem_io) +
 						PPOD_SIZE, 0, gfp);
-		if (skb) {
+		if (skb)
 			ddp->gl_skb[idx + i] = skb;
-		} else {
+		else {
 			ddp_free_gl_skb(ddp, idx, i);
 			return -ENOMEM;
 		}
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index 7136fb2..f3fab74 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -195,16 +195,22 @@ EXPORT_SYMBOL_GPL(cxgbi_device_find_by_lldev);
 static struct cxgbi_device *cxgbi_device_find_by_netdev(struct net_device *ndev,
 							int *port)
 {
+	struct net_device *vdev = NULL;
 	struct cxgbi_device *cdev, *tmp;
 	int i;
 
-	if (ndev->priv_flags & IFF_802_1Q_VLAN)
+	if (ndev->priv_flags & IFF_802_1Q_VLAN) {
+		vdev = ndev;
 		ndev = vlan_dev_real_dev(ndev);
+		log_debug(1 << CXGBI_DBG_DEV,
+			"vlan dev %s -> %s.\n", vdev->name, ndev->name);
+	}
 
 	mutex_lock(&cdev_mutex);
 	list_for_each_entry_safe(cdev, tmp, &cdev_list, list_head) {
 		for (i = 0; i < cdev->nports; i++) {
 			if (ndev == cdev->ports[i]) {
+				cdev->hbas[i]->vdev = vdev;
 				mutex_unlock(&cdev_mutex);
 				if (port)
 					*port = i;
@@ -218,24 +224,6 @@ static struct cxgbi_device *cxgbi_device_find_by_netdev(struct net_device *ndev,
 	return NULL;
 }
 
-struct cxgbi_hba *cxgbi_hba_find_by_netdev(struct net_device *dev,
-					struct cxgbi_device *cdev)
-{
-	int i;
-
-	if (dev->priv_flags & IFF_802_1Q_VLAN)
-		dev = vlan_dev_real_dev(dev);
-
-	for (i = 0; i < cdev->nports; i++) {
-		if (cdev->hbas[i]->ndev == dev)
-			return cdev->hbas[i];
-	}
-	log_debug(1 << CXGBI_DBG_DEV,
-		"ndev 0x%p, %s, cdev 0x%p, NO match found.\n",
-		dev, dev->name, cdev);
-	return NULL;
-}
-
 void cxgbi_hbas_remove(struct cxgbi_device *cdev)
 {
 	int i;
@@ -532,12 +520,6 @@ static struct cxgbi_sock *cxgbi_check_route(struct sockaddr *dst_addr)
 			dst->neighbour->dev->name, ndev->name, mtu);
 	}
 
-	if (ndev->priv_flags & IFF_802_1Q_VLAN) {
-		ndev = vlan_dev_real_dev(ndev);
-		pr_info("rt dev %s, vlan -> %s.\n",
-			dst->neighbour->dev->name, ndev->name);
-	}
-
 	cdev = cxgbi_device_find_by_netdev(ndev, &port);
 	if (!cdev) {
 		pr_info("dst %pI4, %s, NOT cxgbi device.\n",
@@ -561,10 +543,7 @@ static struct cxgbi_sock *cxgbi_check_route(struct sockaddr *dst_addr)
 	csk->dst = dst;
 	csk->daddr.sin_addr.s_addr = daddr->sin_addr.s_addr;
 	csk->daddr.sin_port = daddr->sin_port;
-	if (cdev->hbas[port]->ipv4addr)
-		csk->saddr.sin_addr.s_addr = cdev->hbas[port]->ipv4addr;
-	else
-		csk->saddr.sin_addr.s_addr = rt->rt_src;
+	csk->saddr.sin_addr.s_addr = rt->rt_src;
 
 	return csk;
 
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 48e6d62..43025b7 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -500,6 +500,7 @@ void cxgbi_sock_free_cpl_skbs(struct cxgbi_sock *);
 
 struct cxgbi_hba {
 	struct net_device *ndev;
+	struct net_device *vdev;	/* vlan dev */
 	struct Scsi_Host *shost;
 	struct cxgbi_device *cdev;
 	__be32 ipv4addr;

^ permalink raw reply related

* [PATCH 2/4] libcxgbi: pdu read fixes
From: kxie-ut6Up61K2wZBDgjK7y7TUQ @ 2010-09-23 23:43 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA, linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw
  Cc: rranjan-ut6Up61K2wZBDgjK7y7TUQ, kxie-ut6Up61K2wZBDgjK7y7TUQ,
	James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk,
	michaelc-hcNo3dDEHLuVc3sceRu5cw, davem-fT/PcQaiUtIeIZ0/mPfg9Q

[PATCH 2/4] libcxgbi: pdu read fixes
          
From: Karen Xie <kxie-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
          
This patch
- fixed the locking and releasing skb in the case of error in the pdu read path, and
- added define iscsi_task_cxgbi_data to access the private data inside the iscsi_task.

Signed-off-by: Karen Xie <kxie-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
---

 drivers/scsi/cxgbi/libcxgbi.c |   79 ++++++++++++++++++++++++++---------------
 drivers/scsi/cxgbi/libcxgbi.h |    2 +
 2 files changed, 53 insertions(+), 28 deletions(-)


diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index db9d08a..7136fb2 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -593,11 +593,11 @@ static void cxgbi_inform_iscsi_conn_closing(struct cxgbi_sock *csk)
 		csk, csk->state, csk->flags, csk->user_data);
 
 	if (csk->state != CTP_ESTABLISHED) {
-		read_lock(&csk->callback_lock);
+		read_lock_bh(&csk->callback_lock);
 		if (csk->user_data)
 			iscsi_conn_failure(csk->user_data,
 					ISCSI_ERR_CONN_FAILED);
-		read_unlock(&csk->callback_lock);
+		read_unlock_bh(&csk->callback_lock);
 	}
 }
 
@@ -1712,12 +1712,10 @@ void cxgbi_conn_pdu_ready(struct cxgbi_sock *csk)
 			"csk 0x%p, conn 0x%p, id %d, suspend_rx %lu!\n",
 			csk, conn, conn ? conn->id : 0xFF,
 			conn ? conn->suspend_rx : 0xFF);
-		read_unlock(&csk->callback_lock);
 		return;
 	}
 
 	while (!err) {
-		read_lock(&csk->callback_lock);
 		skb = skb_peek(&csk->receive_queue);
 		if (!skb ||
 		    !(cxgbi_skcb_test_flag(skb, SKCBF_RX_STATUS))) {
@@ -1725,11 +1723,9 @@ void cxgbi_conn_pdu_ready(struct cxgbi_sock *csk)
 				log_debug(1 << CXGBI_DBG_PDU_RX,
 					"skb 0x%p, NOT ready 0x%lx.\n",
 					skb, cxgbi_skcb_flags(skb));
-			read_unlock(&csk->callback_lock);
 			break;
 		}
 		__skb_unlink(skb, &csk->receive_queue);
-		read_unlock(&csk->callback_lock);
 
 		read += cxgbi_skcb_rx_pdulen(skb);
 		log_debug(1 << CXGBI_DBG_PDU_RX,
@@ -1739,37 +1735,66 @@ void cxgbi_conn_pdu_ready(struct cxgbi_sock *csk)
 
 		if (cxgbi_skcb_test_flag(skb, SKCBF_RX_COALESCED)) {
 			err = skb_read_pdu_bhs(conn, skb);
-			if (err < 0)
-				break;
+			if (err < 0) {
+				pr_err("coalesced bhs, csk 0x%p, skb 0x%p,%u, "
+					"f 0x%lx, plen %u.\n",
+					csk, skb, skb->len,
+					cxgbi_skcb_flags(skb),
+					cxgbi_skcb_rx_pdulen(skb));
+				goto skb_done;
+			}
 			err = skb_read_pdu_data(conn, skb, skb,
 						err + cdev->skb_rx_extra);
+			if (err < 0)
+				pr_err("coalesced data, csk 0x%p, skb 0x%p,%u, "
+					"f 0x%lx, plen %u.\n",
+					csk, skb, skb->len,
+					cxgbi_skcb_flags(skb),
+					cxgbi_skcb_rx_pdulen(skb));
 		} else {
 			err = skb_read_pdu_bhs(conn, skb);
-			if (err < 0)
-				break;
+			if (err < 0) {
+				pr_err("bhs, csk 0x%p, skb 0x%p,%u, "
+					"f 0x%lx, plen %u.\n",
+					csk, skb, skb->len,
+					cxgbi_skcb_flags(skb),
+					cxgbi_skcb_rx_pdulen(skb));
+				goto skb_done;
+			}
+
 			if (cxgbi_skcb_test_flag(skb, SKCBF_RX_DATA)) {
 				struct sk_buff *dskb;
 
-				read_lock(&csk->callback_lock);
 				dskb = skb_peek(&csk->receive_queue);
 				if (!dskb) {
-					read_unlock(&csk->callback_lock);
-					pr_err("csk 0x%p, NO data.\n", csk);
-					err = -EAGAIN;
-					break;
+					pr_err("csk 0x%p, skb 0x%p,%u, f 0x%lx,"
+						" plen %u, NO data.\n",
+						csk, skb, skb->len,
+						cxgbi_skcb_flags(skb),
+						cxgbi_skcb_rx_pdulen(skb));
+					err = -EIO;
+					goto skb_done;
 				}
 				__skb_unlink(dskb, &csk->receive_queue);
-				read_unlock(&csk->callback_lock);
 
 				err = skb_read_pdu_data(conn, skb, dskb, 0);
+				if (err < 0)
+					pr_err("data, csk 0x%p, skb 0x%p,%u, "
+						"f 0x%lx, plen %u, dskb 0x%p,"
+						"%u.\n",
+						csk, skb, skb->len,
+						cxgbi_skcb_flags(skb),
+						cxgbi_skcb_rx_pdulen(skb),
+						dskb, dskb->len);
 				__kfree_skb(dskb);
 			} else
 				err = skb_read_pdu_data(conn, skb, skb, 0);
 		}
+skb_done:
+		__kfree_skb(skb);
+
 		if (err < 0)
 			break;
-
-		__kfree_skb(skb);
 	}
 
 	log_debug(1 << CXGBI_DBG_PDU_RX, "csk 0x%p, read %u.\n", csk, read);
@@ -1780,7 +1805,8 @@ void cxgbi_conn_pdu_ready(struct cxgbi_sock *csk)
 	}
 
 	if (err < 0) {
-		pr_info("csk 0x%p, 0x%p, rx failed %d.\n", csk, conn, err);
+		pr_info("csk 0x%p, 0x%p, rx failed %d, read %u.\n",
+			csk, conn, err, read);
 		iscsi_conn_failure(conn, ISCSI_ERR_CONN_FAILED);
 	}
 }
@@ -1861,7 +1887,7 @@ int cxgbi_conn_alloc_pdu(struct iscsi_task *task, u8 opcode)
 	struct cxgbi_device *cdev = cconn->chba->cdev;
 	struct iscsi_conn *conn = task->conn;
 	struct iscsi_tcp_task *tcp_task = task->dd_data;
-	struct cxgbi_task_data *tdata = task->dd_data + sizeof(*tcp_task);
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
 	struct scsi_cmnd *sc = task->sc;
 	int headroom = SKB_TX_ISCSI_PDU_HEADER_MAX;
 
@@ -1916,8 +1942,7 @@ int cxgbi_conn_init_pdu(struct iscsi_task *task, unsigned int offset,
 			      unsigned int count)
 {
 	struct iscsi_conn *conn = task->conn;
-	struct iscsi_tcp_task *tcp_task = task->dd_data;
-	struct cxgbi_task_data *tdata = tcp_task->dd_data;
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
 	struct sk_buff *skb = tdata->skb;
 	unsigned int datalen = count;
 	int i, padlen = iscsi_padding(count);
@@ -2019,8 +2044,7 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task)
 {
 	struct iscsi_tcp_conn *tcp_conn = task->conn->dd_data;
 	struct cxgbi_conn *cconn = tcp_conn->dd_data;
-	struct iscsi_tcp_task *tcp_task = task->dd_data;
-	struct cxgbi_task_data *tdata = tcp_task->dd_data;
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
 	struct sk_buff *skb = tdata->skb;
 	unsigned int datalen;
 	int err;
@@ -2072,8 +2096,7 @@ EXPORT_SYMBOL_GPL(cxgbi_conn_xmit_pdu);
 
 void cxgbi_cleanup_task(struct iscsi_task *task)
 {
-	struct cxgbi_task_data *tdata = task->dd_data +
-				sizeof(struct iscsi_tcp_task);
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
 
 	log_debug(1 << CXGBI_DBG_ISCSI,
 		"task 0x%p, skb 0x%p, itt 0x%x.\n",
@@ -2290,12 +2313,12 @@ int cxgbi_bind_conn(struct iscsi_cls_session *cls_session,
 	/*  calculate the tag idx bits needed for this conn based on cmds_max */
 	cconn->task_idx_bits = (__ilog2_u32(conn->session->cmds_max - 1)) + 1;
 
-	write_lock(&csk->callback_lock);
+	write_lock_bh(&csk->callback_lock);
 	csk->user_data = conn;
 	cconn->chba = cep->chba;
 	cconn->cep = cep;
 	cep->cconn = cconn;
-	write_unlock(&csk->callback_lock);
+	write_unlock_bh(&csk->callback_lock);
 
 	cxgbi_conn_max_xmit_dlength(conn);
 	cxgbi_conn_max_recv_dlength(conn);
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 2f2485b..48e6d62 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -592,6 +592,8 @@ struct cxgbi_task_data {
 	unsigned int count;
 	unsigned int sgoffset;
 };
+#define iscsi_task_cxgbi_data(task) \
+	((task)->dd_data + sizeof(struct iscsi_tcp_task))
 
 static inline int cxgbi_is_ddp_tag(struct cxgbi_tag_format *tformat, u32 tag)
 {

-- 
You received this message because you are subscribed to the Google Groups "open-iscsi" group.
To post to this group, send email to open-iscsi-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to open-iscsi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox