Netdev List
 help / color / mirror / Atom feed
* [PATCH 11/24] TTY: ircomm, use tty_port_close_start helper
From: Jiri Slaby @ 2012-06-04 11:35 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby, Samuel Ortiz, netdev
In-Reply-To: <1338809738-18967-1-git-send-email-jslaby@suse.cz>

Again, the code is identical, so leverage the helper code.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
---
 net/irda/ircomm/ircomm_tty.c |   48 +-----------------------------------------
 1 file changed, 1 insertion(+), 47 deletions(-)

diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index cfe352d..4e35b45 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -509,64 +509,18 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 {
 	struct ircomm_tty_cb *self = (struct ircomm_tty_cb *) tty->driver_data;
 	struct tty_port *port = &self->port;
-	unsigned long flags;
 
 	IRDA_DEBUG(0, "%s()\n", __func__ );
 
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
 
-	spin_lock_irqsave(&port->lock, flags);
-
-	if (tty_hung_up_p(filp)) {
-		spin_unlock_irqrestore(&port->lock, flags);
-
-		IRDA_DEBUG(0, "%s(), returning 1\n", __func__ );
-		return;
-	}
-
-	if ((tty->count == 1) && (port->count != 1)) {
-		/*
-		 * Uh, oh.  tty->count is 1, which means that the tty
-		 * structure will be freed.  state->count should always
-		 * be one in these conditions.  If it's greater than
-		 * one, we've got real problems, since it means the
-		 * serial port won't be shutdown.
-		 */
-		IRDA_DEBUG(0, "%s(), bad serial port count; "
-			   "tty->count is 1, state->count is %d\n", __func__ ,
-			   port->count);
-		port->count = 1;
-	}
-
-	if (--port->count < 0) {
-		IRDA_ERROR("%s(), bad serial port count for ttys%d: %d\n",
-			   __func__, self->line, port->count);
-		port->count = 0;
-	}
-	if (port->count) {
-		spin_unlock_irqrestore(&port->lock, flags);
-
-		IRDA_DEBUG(0, "%s(), open count > 0\n", __func__ );
+	if (tty_port_close_start(port, tty, filp) == 0)
 		return;
-	}
-
-	set_bit(ASYNCB_CLOSING, &port->flags);
-
-	spin_unlock_irqrestore(&port->lock, flags);
-
-	/*
-	 * Now we wait for the transmit buffer to clear; and we notify
-	 * the line discipline to only process XON/XOFF characters.
-	 */
-	tty->closing = 1;
-	if (port->closing_wait != ASYNC_CLOSING_WAIT_NONE)
-		tty_wait_until_sent_from_close(tty, port->closing_wait);
 
 	ircomm_tty_shutdown(self);
 
 	tty_driver_flush_buffer(tty);
-	tty_ldisc_flush(tty);
 
 	tty_port_close_end(port, tty);
 	tty_port_tty_set(port, NULL);
-- 
1.7.10.3

^ permalink raw reply related

* [PATCH 10/24] TTY: ircomm, use tty_port_close_end helper
From: Jiri Slaby @ 2012-06-04 11:35 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby, Samuel Ortiz, netdev
In-Reply-To: <1338809738-18967-1-git-send-email-jslaby@suse.cz>

Again, the code is identical, so leverage the helper code.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
---
 net/irda/ircomm/ircomm_tty.c |   16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 3fdce18..cfe352d 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -568,21 +568,7 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	tty_driver_flush_buffer(tty);
 	tty_ldisc_flush(tty);
 
-	spin_lock_irqsave(&port->lock, flags);
-	tty->closing = 0;
-
-	if (port->blocked_open) {
-		if (port->close_delay) {
-			spin_unlock_irqrestore(&port->lock, flags);
-			schedule_timeout_interruptible(port->close_delay);
-			spin_lock_irqsave(&port->lock, flags);
-		}
-		wake_up_interruptible(&port->open_wait);
-	}
-
-	port->flags &= ~(ASYNC_NORMAL_ACTIVE|ASYNC_CLOSING);
-	spin_unlock_irqrestore(&port->lock, flags);
-	wake_up_interruptible(&port->close_wait);
+	tty_port_close_end(port, tty);
 	tty_port_tty_set(port, NULL);
 }
 
-- 
1.7.10.3

^ permalink raw reply related

* [PATCH 09/24] TTY: ircomm, define carrier routines
From: Jiri Slaby @ 2012-06-04 11:35 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby, Samuel Ortiz, netdev
In-Reply-To: <1338809738-18967-1-git-send-email-jslaby@suse.cz>

These will be used by the tty_port wait_til_ready later. (Now they are
used by our code.)

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
---
 net/irda/ircomm/ircomm_tty.c |   43 ++++++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 199d9cb..3fdce18 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -104,6 +104,35 @@ static const struct tty_operations ops = {
 #endif /* CONFIG_PROC_FS */
 };
 
+static void ircomm_port_raise_dtr_rts(struct tty_port *port, int raise)
+{
+	struct ircomm_tty_cb *self = container_of(port, struct ircomm_tty_cb,
+			port);
+	/*
+	 * Here, we use to lock those two guys, but as ircomm_param_request()
+	 * does it itself, I don't see the point (and I see the deadlock).
+	 * Jean II
+	 */
+	if (raise)
+		self->settings.dte |= IRCOMM_RTS | IRCOMM_DTR;
+	else
+		self->settings.dte &= ~(IRCOMM_RTS | IRCOMM_DTR);
+
+	ircomm_param_request(self, IRCOMM_DTE, TRUE);
+}
+
+static int ircomm_port_carrier_raised(struct tty_port *port)
+{
+	struct ircomm_tty_cb *self = container_of(port, struct ircomm_tty_cb,
+			port);
+	return self->settings.dce & IRCOMM_CD;
+}
+
+static const struct tty_port_operations ircomm_port_ops = {
+	.dtr_rts = ircomm_port_raise_dtr_rts,
+	.carrier_raised = ircomm_port_carrier_raised,
+};
+
 /*
  * Function ircomm_tty_init()
  *
@@ -290,15 +319,8 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 	port->blocked_open++;
 
 	while (1) {
-		if (tty->termios->c_cflag & CBAUD) {
-			/* Here, we use to lock those two guys, but
-			 * as ircomm_param_request() does it itself,
-			 * I don't see the point (and I see the deadlock).
-			 * Jean II */
-			self->settings.dte |= IRCOMM_RTS + IRCOMM_DTR;
-
-			ircomm_param_request(self, IRCOMM_DTE, TRUE);
-		}
+		if (tty->termios->c_cflag & CBAUD)
+			tty_port_raise_dtr_rts(port);
 
 		current->state = TASK_INTERRUPTIBLE;
 
@@ -315,7 +337,7 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 		 * ready
 		 */
 		if (!test_bit(ASYNCB_CLOSING, &port->flags) &&
-		    (do_clocal || (self->settings.dce & IRCOMM_CD)) &&
+		    (do_clocal || tty_port_carrier_raised(port)) &&
 		    self->state == IRCOMM_TTY_READY)
 		{
 			break;
@@ -379,6 +401,7 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 		}
 
 		tty_port_init(&self->port);
+		self->port.ops = &ircomm_port_ops;
 		self->magic = IRCOMM_TTY_MAGIC;
 		self->flow = FLOW_STOP;
 
-- 
1.7.10.3

^ permalink raw reply related

* [PATCH 07/24] TTY: ircomm, use tty from tty_port
From: Jiri Slaby @ 2012-06-04 11:35 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby, Samuel Ortiz, netdev
In-Reply-To: <1338809738-18967-1-git-send-email-jslaby@suse.cz>

This also includes a switch to tty refcounting. It makes sure, the
code no longer can access a freed TTY struct.

Sometimes the only thing needed is to pass tty down to the callies.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
---
 include/net/irda/ircomm_tty.h       |    1 -
 net/irda/ircomm/ircomm_param.c      |    5 ---
 net/irda/ircomm/ircomm_tty.c        |   62 ++++++++++++++++++++++-------------
 net/irda/ircomm/ircomm_tty_attach.c |   33 ++++++++++++++-----
 net/irda/ircomm/ircomm_tty_ioctl.c  |   11 ++++---
 5 files changed, 70 insertions(+), 42 deletions(-)

diff --git a/include/net/irda/ircomm_tty.h b/include/net/irda/ircomm_tty.h
index a9027d8..80ffde3 100644
--- a/include/net/irda/ircomm_tty.h
+++ b/include/net/irda/ircomm_tty.h
@@ -62,7 +62,6 @@ struct ircomm_tty_cb {
 
 	int state;                /* Connect state */
 
-	struct tty_struct *tty;
 	struct ircomm_cb *ircomm; /* IrCOMM layer instance */
 
 	struct sk_buff *tx_skb;   /* Transmit buffer */
diff --git a/net/irda/ircomm/ircomm_param.c b/net/irda/ircomm/ircomm_param.c
index 8b915f3..3089391 100644
--- a/net/irda/ircomm/ircomm_param.c
+++ b/net/irda/ircomm/ircomm_param.c
@@ -99,7 +99,6 @@ pi_param_info_t ircomm_param_info = { pi_major_call_table, 3, 0x0f, 4 };
  */
 int ircomm_param_request(struct ircomm_tty_cb *self, __u8 pi, int flush)
 {
-	struct tty_struct *tty;
 	unsigned long flags;
 	struct sk_buff *skb;
 	int count;
@@ -109,10 +108,6 @@ int ircomm_param_request(struct ircomm_tty_cb *self, __u8 pi, int flush)
 	IRDA_ASSERT(self != NULL, return -1;);
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return -1;);
 
-	tty = self->tty;
-	if (!tty)
-		return 0;
-
 	/* Make sure we don't send parameters for raw mode */
 	if (self->service_type == IRCOMM_3_WIRE_RAW)
 		return 0;
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 7b2152c..03acc07 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -242,18 +242,15 @@ err:
  *
  */
 static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
-				      struct file *filp)
+		struct tty_struct *tty, struct file *filp)
 {
 	DECLARE_WAITQUEUE(wait, current);
 	int		retval;
 	int		do_clocal = 0, extra_count = 0;
 	unsigned long	flags;
-	struct tty_struct *tty;
 
 	IRDA_DEBUG(2, "%s()\n", __func__ );
 
-	tty = self->tty;
-
 	/*
 	 * If non-blocking mode is set, or the port is not enabled,
 	 * then make the check up front and then exit.
@@ -412,8 +409,8 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 	self->port.count++;
 
 	tty->driver_data = self;
-	self->tty = tty;
 	spin_unlock_irqrestore(&self->port.lock, flags);
+	tty_port_tty_set(&self->port, tty);
 
 	IRDA_DEBUG(1, "%s(), %s%d, count = %d\n", __func__ , tty->driver->name,
 		   self->line, self->port.count);
@@ -467,7 +464,7 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 	if (ret)
 		return ret;
 
-	ret = ircomm_tty_block_til_ready(self, filp);
+	ret = ircomm_tty_block_til_ready(self, tty, filp);
 	if (ret) {
 		IRDA_DEBUG(2,
 		      "%s(), returning after block_til_ready with %d\n", __func__ ,
@@ -548,7 +545,6 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 
 	spin_lock_irqsave(&self->port.lock, flags);
 	tty->closing = 0;
-	self->tty = NULL;
 
 	if (self->port.blocked_open) {
 		if (self->port.close_delay) {
@@ -562,6 +558,7 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	self->port.flags &= ~(ASYNC_NORMAL_ACTIVE|ASYNC_CLOSING);
 	spin_unlock_irqrestore(&self->port.lock, flags);
 	wake_up_interruptible(&self->port.close_wait);
+	tty_port_tty_set(&self->port, NULL);
 }
 
 /*
@@ -604,7 +601,7 @@ static void ircomm_tty_do_softint(struct work_struct *work)
 	if (!self || self->magic != IRCOMM_TTY_MAGIC)
 		return;
 
-	tty = self->tty;
+	tty = tty_port_tty_get(&self->port);
 	if (!tty)
 		return;
 
@@ -625,7 +622,7 @@ static void ircomm_tty_do_softint(struct work_struct *work)
 	}
 
 	if (tty->hw_stopped)
-		return;
+		goto put;
 
 	/* Unlink transmit buffer */
 	spin_lock_irqsave(&self->spinlock, flags);
@@ -644,6 +641,8 @@ static void ircomm_tty_do_softint(struct work_struct *work)
 
 	/* Check if user (still) wants to be waken up */
 	tty_wakeup(tty);
+put:
+	tty_kref_put(tty);
 }
 
 /*
@@ -1004,7 +1003,11 @@ static void ircomm_tty_hangup(struct tty_struct *tty)
 
 	spin_lock_irqsave(&self->port.lock, flags);
 	self->port.flags &= ~ASYNC_NORMAL_ACTIVE;
-	self->tty = NULL;
+	if (self->port.tty) {
+		set_bit(TTY_IO_ERROR, &self->port.tty->flags);
+		tty_kref_put(self->port.tty);
+	}
+	self->port.tty = NULL;
 	self->port.count = 0;
 	spin_unlock_irqrestore(&self->port.lock, flags);
 
@@ -1068,7 +1071,7 @@ void ircomm_tty_check_modem_status(struct ircomm_tty_cb *self)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
 
-	tty = self->tty;
+	tty = tty_port_tty_get(&self->port);
 
 	status = self->settings.dce;
 
@@ -1089,10 +1092,10 @@ void ircomm_tty_check_modem_status(struct ircomm_tty_cb *self)
 				tty_hangup(tty);
 
 			/* Hangup will remote the tty, so better break out */
-			return;
+			goto put;
 		}
 	}
-	if (self->port.flags & ASYNC_CTS_FLOW) {
+	if (tty && self->port.flags & ASYNC_CTS_FLOW) {
 		if (tty->hw_stopped) {
 			if (status & IRCOMM_CTS) {
 				IRDA_DEBUG(2,
@@ -1103,7 +1106,7 @@ void ircomm_tty_check_modem_status(struct ircomm_tty_cb *self)
 				wake_up_interruptible(&self->port.open_wait);
 
 				schedule_work(&self->tqueue);
-				return;
+				goto put;
 			}
 		} else {
 			if (!(status & IRCOMM_CTS)) {
@@ -1113,6 +1116,8 @@ void ircomm_tty_check_modem_status(struct ircomm_tty_cb *self)
 			}
 		}
 	}
+put:
+	tty_kref_put(tty);
 }
 
 /*
@@ -1125,6 +1130,7 @@ static int ircomm_tty_data_indication(void *instance, void *sap,
 				      struct sk_buff *skb)
 {
 	struct ircomm_tty_cb *self = (struct ircomm_tty_cb *) instance;
+	struct tty_struct *tty;
 
 	IRDA_DEBUG(2, "%s()\n", __func__ );
 
@@ -1132,7 +1138,8 @@ static int ircomm_tty_data_indication(void *instance, void *sap,
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return -1;);
 	IRDA_ASSERT(skb != NULL, return -1;);
 
-	if (!self->tty) {
+	tty = tty_port_tty_get(&self->port);
+	if (!tty) {
 		IRDA_DEBUG(0, "%s(), no tty!\n", __func__ );
 		return 0;
 	}
@@ -1143,7 +1150,7 @@ static int ircomm_tty_data_indication(void *instance, void *sap,
 	 * Devices like WinCE can do this, and since they don't send any
 	 * params, we can just as well declare the hardware for running.
 	 */
-	if (self->tty->hw_stopped && (self->flow == FLOW_START)) {
+	if (tty->hw_stopped && (self->flow == FLOW_START)) {
 		IRDA_DEBUG(0, "%s(), polling for line settings!\n", __func__ );
 		ircomm_param_request(self, IRCOMM_POLL, TRUE);
 
@@ -1156,8 +1163,9 @@ static int ircomm_tty_data_indication(void *instance, void *sap,
 	 * Use flip buffer functions since the code may be called from interrupt
 	 * context
 	 */
-	tty_insert_flip_string(self->tty, skb->data, skb->len);
-	tty_flip_buffer_push(self->tty);
+	tty_insert_flip_string(tty, skb->data, skb->len);
+	tty_flip_buffer_push(tty);
+	tty_kref_put(tty);
 
 	/* No need to kfree_skb - see ircomm_ttp_data_indication() */
 
@@ -1208,12 +1216,13 @@ static void ircomm_tty_flow_indication(void *instance, void *sap,
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
 
-	tty = self->tty;
+	tty = tty_port_tty_get(&self->port);
 
 	switch (cmd) {
 	case FLOW_START:
 		IRDA_DEBUG(2, "%s(), hw start!\n", __func__ );
-		tty->hw_stopped = 0;
+		if (tty)
+			tty->hw_stopped = 0;
 
 		/* ircomm_tty_do_softint will take care of the rest */
 		schedule_work(&self->tqueue);
@@ -1221,15 +1230,19 @@ static void ircomm_tty_flow_indication(void *instance, void *sap,
 	default:  /* If we get here, something is very wrong, better stop */
 	case FLOW_STOP:
 		IRDA_DEBUG(2, "%s(), hw stopped!\n", __func__ );
-		tty->hw_stopped = 1;
+		if (tty)
+			tty->hw_stopped = 1;
 		break;
 	}
+
+	tty_kref_put(tty);
 	self->flow = cmd;
 }
 
 #ifdef CONFIG_PROC_FS
 static void ircomm_tty_line_info(struct ircomm_tty_cb *self, struct seq_file *m)
 {
+	struct tty_struct *tty;
 	char sep;
 
 	seq_printf(m, "State: %s\n", ircomm_tty_state[self->state]);
@@ -1356,9 +1369,12 @@ static void ircomm_tty_line_info(struct ircomm_tty_cb *self, struct seq_file *m)
 	seq_printf(m, "Max data size: %d\n", self->max_data_size);
 	seq_printf(m, "Max header size: %d\n", self->max_header_size);
 
-	if (self->tty)
+	tty = tty_port_tty_get(&self->port);
+	if (tty) {
 		seq_printf(m, "Hardware: %s\n",
-			       self->tty->hw_stopped ? "Stopped" : "Running");
+			       tty->hw_stopped ? "Stopped" : "Running");
+		tty_kref_put(tty);
+	}
 }
 
 static int ircomm_tty_proc_show(struct seq_file *m, void *v)
diff --git a/net/irda/ircomm/ircomm_tty_attach.c b/net/irda/ircomm/ircomm_tty_attach.c
index bed311a..3ab70e7 100644
--- a/net/irda/ircomm/ircomm_tty_attach.c
+++ b/net/irda/ircomm/ircomm_tty_attach.c
@@ -130,6 +130,8 @@ static int (*state[])(struct ircomm_tty_cb *self, IRCOMM_TTY_EVENT event,
  */
 int ircomm_tty_attach_cable(struct ircomm_tty_cb *self)
 {
+	struct tty_struct *tty;
+
 	IRDA_DEBUG(0, "%s()\n", __func__ );
 
 	IRDA_ASSERT(self != NULL, return -1;);
@@ -142,7 +144,11 @@ int ircomm_tty_attach_cable(struct ircomm_tty_cb *self)
 	}
 
 	/* Make sure nobody tries to write before the link is up */
-	self->tty->hw_stopped = 1;
+	tty = tty_port_tty_get(&self->port);
+	if (tty) {
+		tty->hw_stopped = 1;
+		tty_kref_put(tty);
+	}
 
 	ircomm_tty_ias_register(self);
 
@@ -398,23 +404,26 @@ void ircomm_tty_disconnect_indication(void *instance, void *sap,
 				      struct sk_buff *skb)
 {
 	struct ircomm_tty_cb *self = (struct ircomm_tty_cb *) instance;
+	struct tty_struct *tty;
 
 	IRDA_DEBUG(2, "%s()\n", __func__ );
 
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
 
-	if (!self->tty)
+	tty = tty_port_tty_get(&self->port);
+	if (!tty)
 		return;
 
 	/* This will stop control data transfers */
 	self->flow = FLOW_STOP;
 
 	/* Stop data transfers */
-	self->tty->hw_stopped = 1;
+	tty->hw_stopped = 1;
 
 	ircomm_tty_do_event(self, IRCOMM_TTY_DISCONNECT_INDICATION, NULL,
 			    NULL);
+	tty_kref_put(tty);
 }
 
 /*
@@ -550,12 +559,15 @@ void ircomm_tty_connect_indication(void *instance, void *sap,
  */
 void ircomm_tty_link_established(struct ircomm_tty_cb *self)
 {
+	struct tty_struct *tty;
+
 	IRDA_DEBUG(2, "%s()\n", __func__ );
 
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
 
-	if (!self->tty)
+	tty = tty_port_tty_get(&self->port);
+	if (!tty)
 		return;
 
 	del_timer(&self->watchdog_timer);
@@ -569,17 +581,19 @@ void ircomm_tty_link_established(struct ircomm_tty_cb *self)
 	if ((self->port.flags & ASYNC_CTS_FLOW) &&
 			((self->settings.dce & IRCOMM_CTS) == 0)) {
 		IRDA_DEBUG(0, "%s(), waiting for CTS ...\n", __func__ );
-		return;
+		goto put;
 	} else {
 		IRDA_DEBUG(1, "%s(), starting hardware!\n", __func__ );
 
-		self->tty->hw_stopped = 0;
+		tty->hw_stopped = 0;
 
 		/* Wake up processes blocked on open */
 		wake_up_interruptible(&self->port.open_wait);
 	}
 
 	schedule_work(&self->tqueue);
+put:
+	tty_kref_put(tty);
 }
 
 /*
@@ -983,9 +997,12 @@ static int ircomm_tty_state_ready(struct ircomm_tty_cb *self,
 			self->settings.dce = IRCOMM_DELTA_CD;
 			ircomm_tty_check_modem_status(self);
 		} else {
+			struct tty_struct *tty = tty_port_tty_get(&self->port);
 			IRDA_DEBUG(0, "%s(), hanging up!\n", __func__ );
-			if (self->tty)
-				tty_hangup(self->tty);
+			if (tty) {
+				tty_hangup(tty);
+				tty_kref_put(tty);
+			}
 		}
 		break;
 	default:
diff --git a/net/irda/ircomm/ircomm_tty_ioctl.c b/net/irda/ircomm/ircomm_tty_ioctl.c
index 31b917e..0eab650 100644
--- a/net/irda/ircomm/ircomm_tty_ioctl.c
+++ b/net/irda/ircomm/ircomm_tty_ioctl.c
@@ -52,17 +52,18 @@
  *    Change speed of the driver. If the remote device is a DCE, then this
  *    should make it change the speed of its serial port
  */
-static void ircomm_tty_change_speed(struct ircomm_tty_cb *self)
+static void ircomm_tty_change_speed(struct ircomm_tty_cb *self,
+		struct tty_struct *tty)
 {
 	unsigned int cflag, cval;
 	int baud;
 
 	IRDA_DEBUG(2, "%s()\n", __func__ );
 
-	if (!self->tty || !self->tty->termios || !self->ircomm)
+	if (!self->ircomm)
 		return;
 
-	cflag = self->tty->termios->c_cflag;
+	cflag = tty->termios->c_cflag;
 
 	/*  byte size and parity */
 	switch (cflag & CSIZE) {
@@ -81,7 +82,7 @@ static void ircomm_tty_change_speed(struct ircomm_tty_cb *self)
 		cval |= IRCOMM_PARITY_EVEN;
 
 	/* Determine divisor based on baud rate */
-	baud = tty_get_baud_rate(self->tty);
+	baud = tty_get_baud_rate(tty);
 	if (!baud)
 		baud = 9600;	/* B0 transition handled in rs_set_termios */
 
@@ -159,7 +160,7 @@ void ircomm_tty_set_termios(struct tty_struct *tty,
 		return;
 	}
 
-	ircomm_tty_change_speed(self);
+	ircomm_tty_change_speed(self, tty);
 
 	/* Handle transition to B0 status */
 	if ((old_termios->c_cflag & CBAUD) &&
-- 
1.7.10.3

^ permalink raw reply related

* [PATCH 06/24] TTY: ircomm, revamp locking
From: Jiri Slaby @ 2012-06-04 11:35 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby, Samuel Ortiz, netdev
In-Reply-To: <1338809738-18967-1-git-send-email-jslaby@suse.cz>

Use self->spinlock only for ctrl_skb and tx_skb. TTY stuff is now
protected by tty_port->lock. This is needed for further cleanup (and
conversion to tty_port helpers).

This also closes the race in the end of close.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
---
 include/net/irda/ircomm_tty.h |    1 -
 net/irda/ircomm/ircomm_tty.c  |   38 ++++++++++++++++++--------------------
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/include/net/irda/ircomm_tty.h b/include/net/irda/ircomm_tty.h
index e4db3b5..a9027d8 100644
--- a/include/net/irda/ircomm_tty.h
+++ b/include/net/irda/ircomm_tty.h
@@ -96,7 +96,6 @@ struct ircomm_tty_cb {
 	struct work_struct  tqueue;
 
 	/* Protect concurent access to :
-	 *	o self->open_count
 	 *	o self->ctrl_skb
 	 *	o self->tx_skb
 	 * Maybe other things may gain to be protected as well...
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 8e61026..7b2152c 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -283,13 +283,12 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 	IRDA_DEBUG(2, "%s(%d):block_til_ready before block on %s open_count=%d\n",
 	      __FILE__, __LINE__, tty->driver->name, self->port.count);
 
-	/* As far as I can see, we protect port.count - Jean II */
-	spin_lock_irqsave(&self->spinlock, flags);
+	spin_lock_irqsave(&self->port.lock, flags);
 	if (!tty_hung_up_p(filp)) {
 		extra_count = 1;
 		self->port.count--;
 	}
-	spin_unlock_irqrestore(&self->spinlock, flags);
+	spin_unlock_irqrestore(&self->port.lock, flags);
 	self->port.blocked_open++;
 
 	while (1) {
@@ -340,9 +339,9 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 
 	if (extra_count) {
 		/* ++ is not atomic, so this should be protected - Jean II */
-		spin_lock_irqsave(&self->spinlock, flags);
+		spin_lock_irqsave(&self->port.lock, flags);
 		self->port.count++;
-		spin_unlock_irqrestore(&self->spinlock, flags);
+		spin_unlock_irqrestore(&self->port.lock, flags);
 	}
 	self->port.blocked_open--;
 
@@ -409,12 +408,12 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 		hashbin_insert(ircomm_tty, (irda_queue_t *) self, line, NULL);
 	}
 	/* ++ is not atomic, so this should be protected - Jean II */
-	spin_lock_irqsave(&self->spinlock, flags);
+	spin_lock_irqsave(&self->port.lock, flags);
 	self->port.count++;
 
 	tty->driver_data = self;
 	self->tty = tty;
-	spin_unlock_irqrestore(&self->spinlock, flags);
+	spin_unlock_irqrestore(&self->port.lock, flags);
 
 	IRDA_DEBUG(1, "%s(), %s%d, count = %d\n", __func__ , tty->driver->name,
 		   self->line, self->port.count);
@@ -495,10 +494,10 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
 
-	spin_lock_irqsave(&self->spinlock, flags);
+	spin_lock_irqsave(&self->port.lock, flags);
 
 	if (tty_hung_up_p(filp)) {
-		spin_unlock_irqrestore(&self->spinlock, flags);
+		spin_unlock_irqrestore(&self->port.lock, flags);
 
 		IRDA_DEBUG(0, "%s(), returning 1\n", __func__ );
 		return;
@@ -524,20 +523,15 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 		self->port.count = 0;
 	}
 	if (self->port.count) {
-		spin_unlock_irqrestore(&self->spinlock, flags);
+		spin_unlock_irqrestore(&self->port.lock, flags);
 
 		IRDA_DEBUG(0, "%s(), open count > 0\n", __func__ );
 		return;
 	}
 
-	/* Hum... Should be test_and_set_bit ??? - Jean II */
 	set_bit(ASYNCB_CLOSING, &self->port.flags);
 
-	/* We need to unlock here (we were unlocking at the end of this
-	 * function), because tty_wait_until_sent() may schedule.
-	 * I don't know if the rest should be protected somehow,
-	 * so someone should check. - Jean II */
-	spin_unlock_irqrestore(&self->spinlock, flags);
+	spin_unlock_irqrestore(&self->port.lock, flags);
 
 	/*
 	 * Now we wait for the transmit buffer to clear; and we notify
@@ -552,16 +546,21 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	tty_driver_flush_buffer(tty);
 	tty_ldisc_flush(tty);
 
+	spin_lock_irqsave(&self->port.lock, flags);
 	tty->closing = 0;
 	self->tty = NULL;
 
 	if (self->port.blocked_open) {
-		if (self->port.close_delay)
+		if (self->port.close_delay) {
+			spin_unlock_irqrestore(&self->port.lock, flags);
 			schedule_timeout_interruptible(self->port.close_delay);
+			spin_lock_irqsave(&self->port.lock, flags);
+		}
 		wake_up_interruptible(&self->port.open_wait);
 	}
 
 	self->port.flags &= ~(ASYNC_NORMAL_ACTIVE|ASYNC_CLOSING);
+	spin_unlock_irqrestore(&self->port.lock, flags);
 	wake_up_interruptible(&self->port.close_wait);
 }
 
@@ -1003,12 +1002,11 @@ static void ircomm_tty_hangup(struct tty_struct *tty)
 	/* ircomm_tty_flush_buffer(tty); */
 	ircomm_tty_shutdown(self);
 
-	/* I guess we need to lock here - Jean II */
-	spin_lock_irqsave(&self->spinlock, flags);
+	spin_lock_irqsave(&self->port.lock, flags);
 	self->port.flags &= ~ASYNC_NORMAL_ACTIVE;
 	self->tty = NULL;
 	self->port.count = 0;
-	spin_unlock_irqrestore(&self->spinlock, flags);
+	spin_unlock_irqrestore(&self->port.lock, flags);
 
 	wake_up_interruptible(&self->port.open_wait);
 }
-- 
1.7.10.3

^ permalink raw reply related

* [PATCH 05/24] TTY: ircomm, use flags from tty_port
From: Jiri Slaby @ 2012-06-04 11:35 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby, Samuel Ortiz, netdev
In-Reply-To: <1338809738-18967-1-git-send-email-jslaby@suse.cz>

Switch to tty_port->flags. And while at it, remove redefined flags for
them.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
---
 include/net/irda/ircomm_tty.h       |    6 -----
 net/irda/ircomm/ircomm_tty.c        |   46 +++++++++++++++++------------------
 net/irda/ircomm/ircomm_tty_attach.c |    5 ++--
 net/irda/ircomm/ircomm_tty_ioctl.c  |   10 ++++----
 4 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/include/net/irda/ircomm_tty.h b/include/net/irda/ircomm_tty.h
index 5e94bad..e4db3b5 100644
--- a/include/net/irda/ircomm_tty.h
+++ b/include/net/irda/ircomm_tty.h
@@ -52,11 +52,6 @@
 /* Same for payload size. See qos.c for the smallest max data size */
 #define IRCOMM_TTY_DATA_UNINITIALISED	(64 - IRCOMM_TTY_HDR_UNINITIALISED)
 
-/* Those are really defined in include/linux/serial.h - Jean II */
-#define ASYNC_B_INITIALIZED	31	/* Serial port was initialized */
-#define ASYNC_B_NORMAL_ACTIVE	29	/* Normal device is active */
-#define ASYNC_B_CLOSING		27	/* Serial port is closing */
-
 /*
  * IrCOMM TTY driver state
  */
@@ -81,7 +76,6 @@ struct ircomm_tty_cb {
 	LOCAL_FLOW flow;          /* IrTTP flow status */
 
 	int line;
-	unsigned long flags;
 
 	__u8 dlsap_sel;
 	__u8 slsap_sel;
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 787578f..8e61026 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -194,7 +194,7 @@ static int ircomm_tty_startup(struct ircomm_tty_cb *self)
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return -1;);
 
 	/* Check if already open */
-	if (test_and_set_bit(ASYNC_B_INITIALIZED, &self->flags)) {
+	if (test_and_set_bit(ASYNCB_INITIALIZED, &self->port.flags)) {
 		IRDA_DEBUG(2, "%s(), already open so break out!\n", __func__ );
 		return 0;
 	}
@@ -231,7 +231,7 @@ static int ircomm_tty_startup(struct ircomm_tty_cb *self)
 
 	return 0;
 err:
-	clear_bit(ASYNC_B_INITIALIZED, &self->flags);
+	clear_bit(ASYNCB_INITIALIZED, &self->port.flags);
 	return ret;
 }
 
@@ -260,7 +260,7 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 	 */
 	if (filp->f_flags & O_NONBLOCK || tty->flags & (1 << TTY_IO_ERROR)){
 		/* nonblock mode is set or port is not enabled */
-		self->flags |= ASYNC_NORMAL_ACTIVE;
+		self->port.flags |= ASYNC_NORMAL_ACTIVE;
 		IRDA_DEBUG(1, "%s(), O_NONBLOCK requested!\n", __func__ );
 		return 0;
 	}
@@ -306,8 +306,8 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 		current->state = TASK_INTERRUPTIBLE;
 
 		if (tty_hung_up_p(filp) ||
-		    !test_bit(ASYNC_B_INITIALIZED, &self->flags)) {
-			retval = (self->flags & ASYNC_HUP_NOTIFY) ?
+		    !test_bit(ASYNCB_INITIALIZED, &self->port.flags)) {
+			retval = (self->port.flags & ASYNC_HUP_NOTIFY) ?
 					-EAGAIN : -ERESTARTSYS;
 			break;
 		}
@@ -317,7 +317,7 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 		 * specified, we cannot return before the IrCOMM link is
 		 * ready
 		 */
-		if (!test_bit(ASYNC_B_CLOSING, &self->flags) &&
+		if (!test_bit(ASYNCB_CLOSING, &self->port.flags) &&
 		    (do_clocal || (self->settings.dce & IRCOMM_CD)) &&
 		    self->state == IRCOMM_TTY_READY)
 		{
@@ -350,7 +350,7 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 	      __FILE__, __LINE__, tty->driver->name, self->port.count);
 
 	if (!retval)
-		self->flags |= ASYNC_NORMAL_ACTIVE;
+		self->port.flags |= ASYNC_NORMAL_ACTIVE;
 
 	return retval;
 }
@@ -420,13 +420,13 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 		   self->line, self->port.count);
 
 	/* Not really used by us, but lets do it anyway */
-	self->tty->low_latency = (self->flags & ASYNC_LOW_LATENCY) ? 1 : 0;
+	tty->low_latency = (self->port.flags & ASYNC_LOW_LATENCY) ? 1 : 0;
 
 	/*
 	 * If the port is the middle of closing, bail out now
 	 */
 	if (tty_hung_up_p(filp) ||
-	    test_bit(ASYNC_B_CLOSING, &self->flags)) {
+	    test_bit(ASYNCB_CLOSING, &self->port.flags)) {
 
 		/* Hm, why are we blocking on ASYNC_CLOSING if we
 		 * do return -EAGAIN/-ERESTARTSYS below anyway?
@@ -437,14 +437,14 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 		 */
 
 		if (wait_event_interruptible(self->port.close_wait,
-				!test_bit(ASYNC_B_CLOSING, &self->flags))) {
+				!test_bit(ASYNCB_CLOSING, &self->port.flags))) {
 			IRDA_WARNING("%s - got signal while blocking on ASYNC_CLOSING!\n",
 				     __func__);
 			return -ERESTARTSYS;
 		}
 
 #ifdef SERIAL_DO_RESTART
-		return (self->flags & ASYNC_HUP_NOTIFY) ?
+		return (self->port.flags & ASYNC_HUP_NOTIFY) ?
 			-EAGAIN : -ERESTARTSYS;
 #else
 		return -EAGAIN;
@@ -531,7 +531,7 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	}
 
 	/* Hum... Should be test_and_set_bit ??? - Jean II */
-	set_bit(ASYNC_B_CLOSING, &self->flags);
+	set_bit(ASYNCB_CLOSING, &self->port.flags);
 
 	/* We need to unlock here (we were unlocking at the end of this
 	 * function), because tty_wait_until_sent() may schedule.
@@ -561,7 +561,7 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 		wake_up_interruptible(&self->port.open_wait);
 	}
 
-	self->flags &= ~(ASYNC_NORMAL_ACTIVE|ASYNC_CLOSING);
+	self->port.flags &= ~(ASYNC_NORMAL_ACTIVE|ASYNC_CLOSING);
 	wake_up_interruptible(&self->port.close_wait);
 }
 
@@ -954,7 +954,7 @@ static void ircomm_tty_shutdown(struct ircomm_tty_cb *self)
 
 	IRDA_DEBUG(0, "%s()\n", __func__ );
 
-	if (!test_and_clear_bit(ASYNC_B_INITIALIZED, &self->flags))
+	if (!test_and_clear_bit(ASYNCB_INITIALIZED, &self->port.flags))
 		return;
 
 	ircomm_tty_detach_cable(self);
@@ -1005,7 +1005,7 @@ static void ircomm_tty_hangup(struct tty_struct *tty)
 
 	/* I guess we need to lock here - Jean II */
 	spin_lock_irqsave(&self->spinlock, flags);
-	self->flags &= ~ASYNC_NORMAL_ACTIVE;
+	self->port.flags &= ~ASYNC_NORMAL_ACTIVE;
 	self->tty = NULL;
 	self->port.count = 0;
 	spin_unlock_irqrestore(&self->spinlock, flags);
@@ -1077,7 +1077,7 @@ void ircomm_tty_check_modem_status(struct ircomm_tty_cb *self)
 	if (status & IRCOMM_DCE_DELTA_ANY) {
 		/*wake_up_interruptible(&self->delta_msr_wait);*/
 	}
-	if ((self->flags & ASYNC_CHECK_CD) && (status & IRCOMM_DELTA_CD)) {
+	if ((self->port.flags & ASYNC_CHECK_CD) && (status & IRCOMM_DELTA_CD)) {
 		IRDA_DEBUG(2,
 			   "%s(), ircomm%d CD now %s...\n", __func__ , self->line,
 			   (status & IRCOMM_CD) ? "on" : "off");
@@ -1094,7 +1094,7 @@ void ircomm_tty_check_modem_status(struct ircomm_tty_cb *self)
 			return;
 		}
 	}
-	if (self->flags & ASYNC_CTS_FLOW) {
+	if (self->port.flags & ASYNC_CTS_FLOW) {
 		if (tty->hw_stopped) {
 			if (status & IRCOMM_CTS) {
 				IRDA_DEBUG(2,
@@ -1327,27 +1327,27 @@ static void ircomm_tty_line_info(struct ircomm_tty_cb *self, struct seq_file *m)
 
 	seq_puts(m, "Flags:");
 	sep = ' ';
-	if (self->flags & ASYNC_CTS_FLOW) {
+	if (self->port.flags & ASYNC_CTS_FLOW) {
 		seq_printf(m, "%cASYNC_CTS_FLOW", sep);
 		sep = '|';
 	}
-	if (self->flags & ASYNC_CHECK_CD) {
+	if (self->port.flags & ASYNC_CHECK_CD) {
 		seq_printf(m, "%cASYNC_CHECK_CD", sep);
 		sep = '|';
 	}
-	if (self->flags & ASYNC_INITIALIZED) {
+	if (self->port.flags & ASYNC_INITIALIZED) {
 		seq_printf(m, "%cASYNC_INITIALIZED", sep);
 		sep = '|';
 	}
-	if (self->flags & ASYNC_LOW_LATENCY) {
+	if (self->port.flags & ASYNC_LOW_LATENCY) {
 		seq_printf(m, "%cASYNC_LOW_LATENCY", sep);
 		sep = '|';
 	}
-	if (self->flags & ASYNC_CLOSING) {
+	if (self->port.flags & ASYNC_CLOSING) {
 		seq_printf(m, "%cASYNC_CLOSING", sep);
 		sep = '|';
 	}
-	if (self->flags & ASYNC_NORMAL_ACTIVE) {
+	if (self->port.flags & ASYNC_NORMAL_ACTIVE) {
 		seq_printf(m, "%cASYNC_NORMAL_ACTIVE", sep);
 		sep = '|';
 	}
diff --git a/net/irda/ircomm/ircomm_tty_attach.c b/net/irda/ircomm/ircomm_tty_attach.c
index bb1e935..bed311a 100644
--- a/net/irda/ircomm/ircomm_tty_attach.c
+++ b/net/irda/ircomm/ircomm_tty_attach.c
@@ -566,7 +566,8 @@ void ircomm_tty_link_established(struct ircomm_tty_cb *self)
 	 * will have to wait for the peer device (DCE) to raise the CTS
 	 * line.
 	 */
-	if ((self->flags & ASYNC_CTS_FLOW) && ((self->settings.dce & IRCOMM_CTS) == 0)) {
+	if ((self->port.flags & ASYNC_CTS_FLOW) &&
+			((self->settings.dce & IRCOMM_CTS) == 0)) {
 		IRDA_DEBUG(0, "%s(), waiting for CTS ...\n", __func__ );
 		return;
 	} else {
@@ -977,7 +978,7 @@ static int ircomm_tty_state_ready(struct ircomm_tty_cb *self,
 		ircomm_tty_next_state(self, IRCOMM_TTY_SEARCH);
 		ircomm_tty_start_watchdog_timer(self, 3*HZ);
 
-		if (self->flags & ASYNC_CHECK_CD) {
+		if (self->port.flags & ASYNC_CHECK_CD) {
 			/* Drop carrier */
 			self->settings.dce = IRCOMM_DELTA_CD;
 			ircomm_tty_check_modem_status(self);
diff --git a/net/irda/ircomm/ircomm_tty_ioctl.c b/net/irda/ircomm/ircomm_tty_ioctl.c
index a6d25e3..31b917e 100644
--- a/net/irda/ircomm/ircomm_tty_ioctl.c
+++ b/net/irda/ircomm/ircomm_tty_ioctl.c
@@ -90,19 +90,19 @@ static void ircomm_tty_change_speed(struct ircomm_tty_cb *self)
 
 	/* CTS flow control flag and modem status interrupts */
 	if (cflag & CRTSCTS) {
-		self->flags |= ASYNC_CTS_FLOW;
+		self->port.flags |= ASYNC_CTS_FLOW;
 		self->settings.flow_control |= IRCOMM_RTS_CTS_IN;
 		/* This got me. Bummer. Jean II */
 		if (self->service_type == IRCOMM_3_WIRE_RAW)
 			IRDA_WARNING("%s(), enabling RTS/CTS on link that doesn't support it (3-wire-raw)\n", __func__);
 	} else {
-		self->flags &= ~ASYNC_CTS_FLOW;
+		self->port.flags &= ~ASYNC_CTS_FLOW;
 		self->settings.flow_control &= ~IRCOMM_RTS_CTS_IN;
 	}
 	if (cflag & CLOCAL)
-		self->flags &= ~ASYNC_CHECK_CD;
+		self->port.flags &= ~ASYNC_CHECK_CD;
 	else
-		self->flags |= ASYNC_CHECK_CD;
+		self->port.flags |= ASYNC_CHECK_CD;
 #if 0
 	/*
 	 * Set up parity check flag
@@ -270,7 +270,7 @@ static int ircomm_tty_get_serial_info(struct ircomm_tty_cb *self,
 
 	memset(&info, 0, sizeof(info));
 	info.line = self->line;
-	info.flags = self->flags;
+	info.flags = self->port.flags;
 	info.baud_base = self->settings.data_rate;
 	info.close_delay = self->port.close_delay;
 	info.closing_wait = self->port.closing_wait;
-- 
1.7.10.3

^ permalink raw reply related

* [PATCH 04/24] TTY: ircomm, use open counts from tty_port
From: Jiri Slaby @ 2012-06-04 11:35 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby, Samuel Ortiz, netdev
In-Reply-To: <1338809738-18967-1-git-send-email-jslaby@suse.cz>

Switch to tty_port->count and blocked_open.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
---
 include/net/irda/ircomm_tty.h |    3 ---
 net/irda/ircomm/ircomm_tty.c  |   42 ++++++++++++++++++++---------------------
 2 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/include/net/irda/ircomm_tty.h b/include/net/irda/ircomm_tty.h
index b4184d0..5e94bad 100644
--- a/include/net/irda/ircomm_tty.h
+++ b/include/net/irda/ircomm_tty.h
@@ -101,9 +101,6 @@ struct ircomm_tty_cb {
 	struct timer_list watchdog_timer;
 	struct work_struct  tqueue;
 
-	int  open_count;
-	int  blocked_open;	/* # of blocked opens */
-
 	/* Protect concurent access to :
 	 *	o self->open_count
 	 *	o self->ctrl_skb
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 61e0adc..787578f 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -272,7 +272,7 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 
 	/* Wait for carrier detect and the line to become
 	 * free (i.e., not in use by the callout).  While we are in
-	 * this loop, self->open_count is dropped by one, so that
+	 * this loop, self->port.count is dropped by one, so that
 	 * mgsl_close() knows when to free things.  We restore it upon
 	 * exit, either normal or abnormal.
 	 */
@@ -281,16 +281,16 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 	add_wait_queue(&self->port.open_wait, &wait);
 
 	IRDA_DEBUG(2, "%s(%d):block_til_ready before block on %s open_count=%d\n",
-	      __FILE__,__LINE__, tty->driver->name, self->open_count );
+	      __FILE__, __LINE__, tty->driver->name, self->port.count);
 
-	/* As far as I can see, we protect open_count - Jean II */
+	/* As far as I can see, we protect port.count - Jean II */
 	spin_lock_irqsave(&self->spinlock, flags);
 	if (!tty_hung_up_p(filp)) {
 		extra_count = 1;
-		self->open_count--;
+		self->port.count--;
 	}
 	spin_unlock_irqrestore(&self->spinlock, flags);
-	self->blocked_open++;
+	self->port.blocked_open++;
 
 	while (1) {
 		if (tty->termios->c_cflag & CBAUD) {
@@ -330,7 +330,7 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 		}
 
 		IRDA_DEBUG(1, "%s(%d):block_til_ready blocking on %s open_count=%d\n",
-		      __FILE__,__LINE__, tty->driver->name, self->open_count );
+		      __FILE__, __LINE__, tty->driver->name, self->port.count);
 
 		schedule();
 	}
@@ -341,13 +341,13 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 	if (extra_count) {
 		/* ++ is not atomic, so this should be protected - Jean II */
 		spin_lock_irqsave(&self->spinlock, flags);
-		self->open_count++;
+		self->port.count++;
 		spin_unlock_irqrestore(&self->spinlock, flags);
 	}
-	self->blocked_open--;
+	self->port.blocked_open--;
 
 	IRDA_DEBUG(1, "%s(%d):block_til_ready after blocking on %s open_count=%d\n",
-	      __FILE__,__LINE__, tty->driver->name, self->open_count);
+	      __FILE__, __LINE__, tty->driver->name, self->port.count);
 
 	if (!retval)
 		self->flags |= ASYNC_NORMAL_ACTIVE;
@@ -410,14 +410,14 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 	}
 	/* ++ is not atomic, so this should be protected - Jean II */
 	spin_lock_irqsave(&self->spinlock, flags);
-	self->open_count++;
+	self->port.count++;
 
 	tty->driver_data = self;
 	self->tty = tty;
 	spin_unlock_irqrestore(&self->spinlock, flags);
 
 	IRDA_DEBUG(1, "%s(), %s%d, count = %d\n", __func__ , tty->driver->name,
-		   self->line, self->open_count);
+		   self->line, self->port.count);
 
 	/* Not really used by us, but lets do it anyway */
 	self->tty->low_latency = (self->flags & ASYNC_LOW_LATENCY) ? 1 : 0;
@@ -504,7 +504,7 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 		return;
 	}
 
-	if ((tty->count == 1) && (self->open_count != 1)) {
+	if ((tty->count == 1) && (self->port.count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  state->count should always
@@ -514,16 +514,16 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 		 */
 		IRDA_DEBUG(0, "%s(), bad serial port count; "
 			   "tty->count is 1, state->count is %d\n", __func__ ,
-			   self->open_count);
-		self->open_count = 1;
+			   self->port.count);
+		self->port.count = 1;
 	}
 
-	if (--self->open_count < 0) {
+	if (--self->port.count < 0) {
 		IRDA_ERROR("%s(), bad serial port count for ttys%d: %d\n",
-			   __func__, self->line, self->open_count);
-		self->open_count = 0;
+			   __func__, self->line, self->port.count);
+		self->port.count = 0;
 	}
-	if (self->open_count) {
+	if (self->port.count) {
 		spin_unlock_irqrestore(&self->spinlock, flags);
 
 		IRDA_DEBUG(0, "%s(), open count > 0\n", __func__ );
@@ -555,7 +555,7 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	tty->closing = 0;
 	self->tty = NULL;
 
-	if (self->blocked_open) {
+	if (self->port.blocked_open) {
 		if (self->port.close_delay)
 			schedule_timeout_interruptible(self->port.close_delay);
 		wake_up_interruptible(&self->port.open_wait);
@@ -1007,7 +1007,7 @@ static void ircomm_tty_hangup(struct tty_struct *tty)
 	spin_lock_irqsave(&self->spinlock, flags);
 	self->flags &= ~ASYNC_NORMAL_ACTIVE;
 	self->tty = NULL;
-	self->open_count = 0;
+	self->port.count = 0;
 	spin_unlock_irqrestore(&self->spinlock, flags);
 
 	wake_up_interruptible(&self->port.open_wait);
@@ -1354,7 +1354,7 @@ static void ircomm_tty_line_info(struct ircomm_tty_cb *self, struct seq_file *m)
 	seq_putc(m, '\n');
 
 	seq_printf(m, "Role: %s\n", self->client ? "client" : "server");
-	seq_printf(m, "Open count: %d\n", self->open_count);
+	seq_printf(m, "Open count: %d\n", self->port.count);
 	seq_printf(m, "Max data size: %d\n", self->max_data_size);
 	seq_printf(m, "Max header size: %d\n", self->max_header_size);
 
-- 
1.7.10.3

^ permalink raw reply related

* [PATCH 03/24] TTY: ircomm, use close times from tty_port
From: Jiri Slaby @ 2012-06-04 11:35 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby, Samuel Ortiz, netdev
In-Reply-To: <1338809738-18967-1-git-send-email-jslaby@suse.cz>

Switch to tty_port->close_delay and closing_wait.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
---
 include/net/irda/ircomm_tty.h      |    3 ---
 net/irda/ircomm/ircomm_tty.c       |   10 ++++------
 net/irda/ircomm/ircomm_tty_ioctl.c |    4 ++--
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/include/net/irda/ircomm_tty.h b/include/net/irda/ircomm_tty.h
index 365fa6e..b4184d0 100644
--- a/include/net/irda/ircomm_tty.h
+++ b/include/net/irda/ircomm_tty.h
@@ -101,9 +101,6 @@ struct ircomm_tty_cb {
 	struct timer_list watchdog_timer;
 	struct work_struct  tqueue;
 
-        unsigned short    close_delay;
-        unsigned short    closing_wait; /* time to wait before closing */
-
 	int  open_count;
 	int  blocked_open;	/* # of blocked opens */
 
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 8eeaa8b..61e0adc 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -389,8 +389,6 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 		INIT_WORK(&self->tqueue, ircomm_tty_do_softint);
 		self->max_header_size = IRCOMM_TTY_HDR_UNINITIALISED;
 		self->max_data_size = IRCOMM_TTY_DATA_UNINITIALISED;
-		self->close_delay = 5*HZ/10;
-		self->closing_wait = 30*HZ;
 
 		/* Init some important stuff */
 		init_timer(&self->watchdog_timer);
@@ -546,8 +544,8 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	 * the line discipline to only process XON/XOFF characters.
 	 */
 	tty->closing = 1;
-	if (self->closing_wait != ASYNC_CLOSING_WAIT_NONE)
-		tty_wait_until_sent_from_close(tty, self->closing_wait);
+	if (self->port.closing_wait != ASYNC_CLOSING_WAIT_NONE)
+		tty_wait_until_sent_from_close(tty, self->port.closing_wait);
 
 	ircomm_tty_shutdown(self);
 
@@ -558,8 +556,8 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	self->tty = NULL;
 
 	if (self->blocked_open) {
-		if (self->close_delay)
-			schedule_timeout_interruptible(self->close_delay);
+		if (self->port.close_delay)
+			schedule_timeout_interruptible(self->port.close_delay);
 		wake_up_interruptible(&self->port.open_wait);
 	}
 
diff --git a/net/irda/ircomm/ircomm_tty_ioctl.c b/net/irda/ircomm/ircomm_tty_ioctl.c
index d0667d6..a6d25e3 100644
--- a/net/irda/ircomm/ircomm_tty_ioctl.c
+++ b/net/irda/ircomm/ircomm_tty_ioctl.c
@@ -272,8 +272,8 @@ static int ircomm_tty_get_serial_info(struct ircomm_tty_cb *self,
 	info.line = self->line;
 	info.flags = self->flags;
 	info.baud_base = self->settings.data_rate;
-	info.close_delay = self->close_delay;
-	info.closing_wait = self->closing_wait;
+	info.close_delay = self->port.close_delay;
+	info.closing_wait = self->port.closing_wait;
 
 	/* For compatibility  */
 	info.type = PORT_16550A;
-- 
1.7.10.3

^ permalink raw reply related

* [PATCH 02/24] TTY: ircomm, add tty_port
From: Jiri Slaby @ 2012-06-04 11:35 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby, Samuel Ortiz, netdev
In-Reply-To: <1338809738-18967-1-git-send-email-jslaby@suse.cz>

And use close/open_wait from there.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: netdev@vger.kernel.org
---
 include/net/irda/ircomm_tty.h       |    3 +--
 net/irda/ircomm/ircomm_tty.c        |   21 +++++++++++----------
 net/irda/ircomm/ircomm_tty_attach.c |    2 +-
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/net/irda/ircomm_tty.h b/include/net/irda/ircomm_tty.h
index 59ba38bc..365fa6e 100644
--- a/include/net/irda/ircomm_tty.h
+++ b/include/net/irda/ircomm_tty.h
@@ -62,6 +62,7 @@
  */
 struct ircomm_tty_cb {
 	irda_queue_t queue;            /* Must be first */
+	struct tty_port port;
 	magic_t magic;
 
 	int state;                /* Connect state */
@@ -97,8 +98,6 @@ struct ircomm_tty_cb {
 	void *skey;
 	void *ckey;
 
-	wait_queue_head_t open_wait;
-	wait_queue_head_t close_wait;
 	struct timer_list watchdog_timer;
 	struct work_struct  tqueue;
 
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 6b9d5a0..8eeaa8b 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -278,7 +278,7 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 	 */
 
 	retval = 0;
-	add_wait_queue(&self->open_wait, &wait);
+	add_wait_queue(&self->port.open_wait, &wait);
 
 	IRDA_DEBUG(2, "%s(%d):block_til_ready before block on %s open_count=%d\n",
 	      __FILE__,__LINE__, tty->driver->name, self->open_count );
@@ -336,7 +336,7 @@ static int ircomm_tty_block_til_ready(struct ircomm_tty_cb *self,
 	}
 
 	__set_current_state(TASK_RUNNING);
-	remove_wait_queue(&self->open_wait, &wait);
+	remove_wait_queue(&self->port.open_wait, &wait);
 
 	if (extra_count) {
 		/* ++ is not atomic, so this should be protected - Jean II */
@@ -381,6 +381,7 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 			return -ENOMEM;
 		}
 
+		tty_port_init(&self->port);
 		self->magic = IRCOMM_TTY_MAGIC;
 		self->flow = FLOW_STOP;
 
@@ -393,8 +394,6 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 
 		/* Init some important stuff */
 		init_timer(&self->watchdog_timer);
-		init_waitqueue_head(&self->open_wait);
-		init_waitqueue_head(&self->close_wait);
 		spin_lock_init(&self->spinlock);
 
 		/*
@@ -408,6 +407,7 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 		tty->termios->c_oflag = 0;
 
 		/* Insert into hash */
+		/* FIXME there is a window from find to here */
 		hashbin_insert(ircomm_tty, (irda_queue_t *) self, line, NULL);
 	}
 	/* ++ is not atomic, so this should be protected - Jean II */
@@ -438,7 +438,8 @@ static int ircomm_tty_open(struct tty_struct *tty, struct file *filp)
 		 * probably better sleep uninterruptible?
 		 */
 
-		if (wait_event_interruptible(self->close_wait, !test_bit(ASYNC_B_CLOSING, &self->flags))) {
+		if (wait_event_interruptible(self->port.close_wait,
+				!test_bit(ASYNC_B_CLOSING, &self->flags))) {
 			IRDA_WARNING("%s - got signal while blocking on ASYNC_CLOSING!\n",
 				     __func__);
 			return -ERESTARTSYS;
@@ -559,11 +560,11 @@ static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
 	if (self->blocked_open) {
 		if (self->close_delay)
 			schedule_timeout_interruptible(self->close_delay);
-		wake_up_interruptible(&self->open_wait);
+		wake_up_interruptible(&self->port.open_wait);
 	}
 
 	self->flags &= ~(ASYNC_NORMAL_ACTIVE|ASYNC_CLOSING);
-	wake_up_interruptible(&self->close_wait);
+	wake_up_interruptible(&self->port.close_wait);
 }
 
 /*
@@ -1011,7 +1012,7 @@ static void ircomm_tty_hangup(struct tty_struct *tty)
 	self->open_count = 0;
 	spin_unlock_irqrestore(&self->spinlock, flags);
 
-	wake_up_interruptible(&self->open_wait);
+	wake_up_interruptible(&self->port.open_wait);
 }
 
 /*
@@ -1084,7 +1085,7 @@ void ircomm_tty_check_modem_status(struct ircomm_tty_cb *self)
 			   (status & IRCOMM_CD) ? "on" : "off");
 
 		if (status & IRCOMM_CD) {
-			wake_up_interruptible(&self->open_wait);
+			wake_up_interruptible(&self->port.open_wait);
 		} else {
 			IRDA_DEBUG(2,
 				   "%s(), Doing serial hangup..\n", __func__ );
@@ -1103,7 +1104,7 @@ void ircomm_tty_check_modem_status(struct ircomm_tty_cb *self)
 				tty->hw_stopped = 0;
 
 				/* Wake up processes blocked on open */
-				wake_up_interruptible(&self->open_wait);
+				wake_up_interruptible(&self->port.open_wait);
 
 				schedule_work(&self->tqueue);
 				return;
diff --git a/net/irda/ircomm/ircomm_tty_attach.c b/net/irda/ircomm/ircomm_tty_attach.c
index b65d66e..bb1e935 100644
--- a/net/irda/ircomm/ircomm_tty_attach.c
+++ b/net/irda/ircomm/ircomm_tty_attach.c
@@ -575,7 +575,7 @@ void ircomm_tty_link_established(struct ircomm_tty_cb *self)
 		self->tty->hw_stopped = 0;
 
 		/* Wake up processes blocked on open */
-		wake_up_interruptible(&self->open_wait);
+		wake_up_interruptible(&self->port.open_wait);
 	}
 
 	schedule_work(&self->tqueue);
-- 
1.7.10.3

^ permalink raw reply related

* [PATCH net-next] net: use consume_skb() in place of kfree_skb()
From: Eric Dumazet @ 2012-06-04 11:17 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

Remove some dropwatch/drop_monitor false positives.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/atm/lec.c                   |    6 ++++--
 net/atm/pppoatm.c               |    2 +-
 net/ax25/ax25_out.c             |    2 +-
 net/ax25/ax25_route.c           |    2 +-
 net/decnet/dn_neigh.c           |    6 +++---
 net/ipv4/ip_output.c            |    4 ++--
 net/netfilter/ipvs/ip_vs_xmit.c |    4 ++--
 7 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index a7d1721..3da125c 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -231,9 +231,11 @@ static netdev_tx_t lec_start_xmit(struct sk_buff *skb,
 	if (skb_headroom(skb) < 2) {
 		pr_debug("reallocating skb\n");
 		skb2 = skb_realloc_headroom(skb, LEC_HEADER_LEN);
-		kfree_skb(skb);
-		if (skb2 == NULL)
+		if (unlikely(!skb2)) {
+			kfree_skb(skb);
 			return NETDEV_TX_OK;
+		}
+		consume_skb(skb);
 		skb = skb2;
 	}
 	skb_push(skb, 2);
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index ce1e59f..226dca9 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -283,7 +283,7 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
 				kfree_skb(n);
 				goto nospace;
 			}
-			kfree_skb(skb);
+			consume_skb(skb);
 			skb = n;
 			if (skb == NULL)
 				return DROP_PACKET;
diff --git a/net/ax25/ax25_out.c b/net/ax25/ax25_out.c
index be8a25e..be2acab 100644
--- a/net/ax25/ax25_out.c
+++ b/net/ax25/ax25_out.c
@@ -350,7 +350,7 @@ void ax25_transmit_buffer(ax25_cb *ax25, struct sk_buff *skb, int type)
 		if (skb->sk != NULL)
 			skb_set_owner_w(skbn, skb->sk);
 
-		kfree_skb(skb);
+		consume_skb(skb);
 		skb = skbn;
 	}
 
diff --git a/net/ax25/ax25_route.c b/net/ax25/ax25_route.c
index a655880..d390977 100644
--- a/net/ax25/ax25_route.c
+++ b/net/ax25/ax25_route.c
@@ -474,7 +474,7 @@ struct sk_buff *ax25_rt_build_path(struct sk_buff *skb, ax25_address *src,
 		if (skb->sk != NULL)
 			skb_set_owner_w(skbn, skb->sk);
 
-		kfree_skb(skb);
+		consume_skb(skb);
 
 		skb = skbn;
 	}
diff --git a/net/decnet/dn_neigh.c b/net/decnet/dn_neigh.c
index ac90f658..8e9a35b 100644
--- a/net/decnet/dn_neigh.c
+++ b/net/decnet/dn_neigh.c
@@ -240,7 +240,7 @@ static int dn_long_output(struct neighbour *neigh, struct sk_buff *skb)
 			kfree_skb(skb);
 			return -ENOBUFS;
 		}
-		kfree_skb(skb);
+		consume_skb(skb);
 		skb = skb2;
 		net_info_ratelimited("dn_long_output: Increasing headroom\n");
 	}
@@ -283,7 +283,7 @@ static int dn_short_output(struct neighbour *neigh, struct sk_buff *skb)
 			kfree_skb(skb);
 			return -ENOBUFS;
 		}
-		kfree_skb(skb);
+		consume_skb(skb);
 		skb = skb2;
 		net_info_ratelimited("dn_short_output: Increasing headroom\n");
 	}
@@ -322,7 +322,7 @@ static int dn_phase3_output(struct neighbour *neigh, struct sk_buff *skb)
 			kfree_skb(skb);
 			return -ENOBUFS;
 		}
-		kfree_skb(skb);
+		consume_skb(skb);
 		skb = skb2;
 		net_info_ratelimited("dn_phase3_output: Increasing headroom\n");
 	}
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 451f97c..157ebd2 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -200,7 +200,7 @@ static inline int ip_finish_output2(struct sk_buff *skb)
 		}
 		if (skb->sk)
 			skb_set_owner_w(skb2, skb->sk);
-		kfree_skb(skb);
+		consume_skb(skb);
 		skb = skb2;
 	}
 
@@ -709,7 +709,7 @@ slow_path:
 
 		IP_INC_STATS(dev_net(dev), IPSTATS_MIB_FRAGCREATES);
 	}
-	kfree_skb(skb);
+	consume_skb(skb);
 	IP_INC_STATS(dev_net(dev), IPSTATS_MIB_FRAGOKS);
 	return err;
 
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 7fd66de..71d6ecb 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -823,7 +823,7 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 			IP_VS_ERR_RL("%s(): no memory\n", __func__);
 			return NF_STOLEN;
 		}
-		kfree_skb(skb);
+		consume_skb(skb);
 		skb = new_skb;
 		old_iph = ip_hdr(skb);
 	}
@@ -942,7 +942,7 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 			IP_VS_ERR_RL("%s(): no memory\n", __func__);
 			return NF_STOLEN;
 		}
-		kfree_skb(skb);
+		consume_skb(skb);
 		skb = new_skb;
 		old_iph = ipv6_hdr(skb);
 	}

^ permalink raw reply related

* Re: [PATCH v5 6/6 resend] net: sh_eth: use NAPI
From: Shimoda, Yoshihiro @ 2012-06-04 11:17 UTC (permalink / raw)
  To: Jan Ceuleers; +Cc: netdev, SH-Linux
In-Reply-To: <4FC9E56C.4000707@computer.org>

2012/06/02 19:05, Jan Ceuleers wrote:
> On 05/29/2012 10:15 AM, Shimoda, Yoshihiro wrote:
>> @@ -1087,13 +1088,17 @@ static int sh_eth_rx(struct net_device *ndev)
>>  				skb_reserve(skb, NET_IP_ALIGN);
>>  			skb_put(skb, pkt_len);
>>  			skb->protocol = eth_type_trans(skb, ndev);
>> -			netif_rx(skb);
>> -			ndev->stats.rx_packets++;
>> -			ndev->stats.rx_bytes += pkt_len;
>> +			if (netif_receive_skb(skb) == NET_RX_DROP) {
>> +				ndev->stats.rx_dropped++;
>> +			} else {
>> +				ndev->stats.rx_packets++;
>> +				ndev->stats.rx_bytes += pkt_len;
>> +			}
>>  		}
>>  		rxdesc->status |= cpu_to_edmac(mdp, RD_RACT);
>>  		entry = (++mdp->cur_rx) % mdp->num_rx_ring;
>>  		rxdesc = &mdp->rx_ring[entry];
>> +		(*work)++;
>>  	}
>>
>>  	/* Refill the Rx ring buffers. */
> 
> Please forgive a newbie's question/comment; feel free to ignore if I'm
> wasting your time. Particularly because it's about an aspect of the
> driver that you're not changing in this patch. (And yes, I know that
> you've been asked to sit on this patch series until net-next opens up
> again).
> 
> I see that most users of netif_receive_skb() ignore its return value.
> Some drivers (including this-one) do check it and use it to determine
> whether counters should be updated. But looking at netif_receive_skb()
> itself I see that there's counter infrastructure there already.
> 
> So why this in-driver set of counters, I wonder?

Thank you for the comment.
I also think that the driver should not update the rx_dropped counter
when netif_receive_skb() returns NET_RX_DROP. I will fix this.

Best regards,
Yoshihiro Shimoda

^ permalink raw reply

* Re: [PATCH v2] drop_monitor: dont sleep in atomic context
From: Neil Horman @ 2012-06-04 10:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1338805099.2760.1798.camel@edumazet-glaptop>

On Mon, Jun 04, 2012 at 12:18:19PM +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> drop_monitor calls several sleeping functions while in atomic context.
> 
>  BUG: sleeping function called from invalid context at mm/slub.c:943
>  in_atomic(): 1, irqs_disabled(): 0, pid: 2103, name: kworker/0:2
>  Pid: 2103, comm: kworker/0:2 Not tainted 3.5.0-rc1+ #55
>  Call Trace:
>   [<ffffffff810697ca>] __might_sleep+0xca/0xf0
>   [<ffffffff811345a3>] kmem_cache_alloc_node+0x1b3/0x1c0
>   [<ffffffff8105578c>] ? queue_delayed_work_on+0x11c/0x130
>   [<ffffffff815343fb>] __alloc_skb+0x4b/0x230
>   [<ffffffffa00b0360>] ? reset_per_cpu_data+0x160/0x160 [drop_monitor]
>   [<ffffffffa00b022f>] reset_per_cpu_data+0x2f/0x160 [drop_monitor]
>   [<ffffffffa00b03ab>] send_dm_alert+0x4b/0xb0 [drop_monitor]
>   [<ffffffff810568e0>] process_one_work+0x130/0x4c0
>   [<ffffffff81058249>] worker_thread+0x159/0x360
>   [<ffffffff810580f0>] ? manage_workers.isra.27+0x240/0x240
>   [<ffffffff8105d403>] kthread+0x93/0xa0
>   [<ffffffff816be6d4>] kernel_thread_helper+0x4/0x10
>   [<ffffffff8105d370>] ? kthread_freezable_should_stop+0x80/0x80
>   [<ffffffff816be6d0>] ? gs_change+0xb/0xb
> 
> Rework the logic to call the sleeping functions in right context.
> 
> Use standard timer/workqueue api to let system chose any cpu to perform
> the allocation and netlink send.
> 
> Also avoid a loop if reset_per_cpu_data() cannot allocate memory :
> use mod_timer() to wait 1/10 second before next try.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
Reviewed-by: Neil Horman <nhorman@tuxdriver.com>

Thanks Eric!
Neil

^ permalink raw reply

* Re: [PATCH v2 2/2] drop_monitor: Make updating data->skb smp safe
From: Neil Horman @ 2012-06-04 10:39 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, David Miller
In-Reply-To: <1338795910.2760.1739.camel@edumazet-glaptop>

On Mon, Jun 04, 2012 at 09:45:10AM +0200, Eric Dumazet wrote:
> On Fri, 2012-04-27 at 16:11 -0400, Neil Horman wrote:
> > Eric Dumazet pointed out to me that the drop_monitor protocol has some holes in
> > its smp protections.  Specifically, its possible to replace data->skb while its
> > being written.  This patch corrects that by making data->skb and rcu protected
> > variable.  That will prevent it from being overwritten while a tracepoint is
> > modifying it.
> > 
> 
> >  static void send_dm_alert(struct work_struct *unused)
> >  {
> >  	struct sk_buff *skb;
> > -	struct per_cpu_dm_data *data = &__get_cpu_var(dm_cpu_data);
> > +	struct per_cpu_dm_data *data = &get_cpu_var(dm_cpu_data);
> >  
> >  	/*
> >  	 * Grab the skb we're about to send
> >  	 */
> > -	skb = data->skb;
> > +	skb = rcu_dereference_protected(data->skb, 1);
> >  
> >  	/*
> >  	 * Replace it with a new one
> > @@ -111,8 +134,10 @@ static void send_dm_alert(struct work_struct *unused)
> >  	/*
> >  	 * Ship it!
> >  	 */
> > -	genlmsg_multicast(skb, 0, NET_DM_GRP_ALERT, GFP_KERNEL);
> > +	if (skb)
> > +		genlmsg_multicast(skb, 0, NET_DM_GRP_ALERT, GFP_KERNEL);
> >  
> > +	put_cpu_var(dm_cpu_data);
> >  }
> >  
> 
> Oh well, drop_monitor can still trigger alerts :
> 
Grr, Not sure why I didn't see this before.  I'll take care of it shortly.
Neil

^ permalink raw reply

* [PATCH v2] drop_monitor: dont sleep in atomic context
From: Eric Dumazet @ 2012-06-04 10:18 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Neil Horman

From: Eric Dumazet <edumazet@google.com>

drop_monitor calls several sleeping functions while in atomic context.

 BUG: sleeping function called from invalid context at mm/slub.c:943
 in_atomic(): 1, irqs_disabled(): 0, pid: 2103, name: kworker/0:2
 Pid: 2103, comm: kworker/0:2 Not tainted 3.5.0-rc1+ #55
 Call Trace:
  [<ffffffff810697ca>] __might_sleep+0xca/0xf0
  [<ffffffff811345a3>] kmem_cache_alloc_node+0x1b3/0x1c0
  [<ffffffff8105578c>] ? queue_delayed_work_on+0x11c/0x130
  [<ffffffff815343fb>] __alloc_skb+0x4b/0x230
  [<ffffffffa00b0360>] ? reset_per_cpu_data+0x160/0x160 [drop_monitor]
  [<ffffffffa00b022f>] reset_per_cpu_data+0x2f/0x160 [drop_monitor]
  [<ffffffffa00b03ab>] send_dm_alert+0x4b/0xb0 [drop_monitor]
  [<ffffffff810568e0>] process_one_work+0x130/0x4c0
  [<ffffffff81058249>] worker_thread+0x159/0x360
  [<ffffffff810580f0>] ? manage_workers.isra.27+0x240/0x240
  [<ffffffff8105d403>] kthread+0x93/0xa0
  [<ffffffff816be6d4>] kernel_thread_helper+0x4/0x10
  [<ffffffff8105d370>] ? kthread_freezable_should_stop+0x80/0x80
  [<ffffffff816be6d0>] ? gs_change+0xb/0xb

Rework the logic to call the sleeping functions in right context.

Use standard timer/workqueue api to let system chose any cpu to perform
the allocation and netlink send.

Also avoid a loop if reset_per_cpu_data() cannot allocate memory :
use mod_timer() to wait 1/10 second before next try.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
---
 net/core/drop_monitor.c |  102 ++++++++++++--------------------------
 1 file changed, 33 insertions(+), 69 deletions(-)

diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index ea5fb9f..d23b668 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -36,9 +36,6 @@
 #define TRACE_ON 1
 #define TRACE_OFF 0
 
-static void send_dm_alert(struct work_struct *unused);
-
-
 /*
  * Globals, our netlink socket pointer
  * and the work handle that will send up
@@ -48,11 +45,10 @@ static int trace_state = TRACE_OFF;
 static DEFINE_MUTEX(trace_state_mutex);
 
 struct per_cpu_dm_data {
-	struct work_struct dm_alert_work;
-	struct sk_buff __rcu *skb;
-	atomic_t dm_hit_count;
-	struct timer_list send_timer;
-	int cpu;
+	spinlock_t		lock;
+	struct sk_buff		*skb;
+	struct work_struct	dm_alert_work;
+	struct timer_list	send_timer;
 };
 
 struct dm_hw_stat_delta {
@@ -78,13 +74,13 @@ static int dm_delay = 1;
 static unsigned long dm_hw_check_delta = 2*HZ;
 static LIST_HEAD(hw_stats_list);
 
-static void reset_per_cpu_data(struct per_cpu_dm_data *data)
+static struct sk_buff *reset_per_cpu_data(struct per_cpu_dm_data *data)
 {
 	size_t al;
 	struct net_dm_alert_msg *msg;
 	struct nlattr *nla;
 	struct sk_buff *skb;
-	struct sk_buff *oskb = rcu_dereference_protected(data->skb, 1);
+	unsigned long flags;
 
 	al = sizeof(struct net_dm_alert_msg);
 	al += dm_hit_limit * sizeof(struct net_dm_drop_point);
@@ -99,65 +95,40 @@ static void reset_per_cpu_data(struct per_cpu_dm_data *data)
 				  sizeof(struct net_dm_alert_msg));
 		msg = nla_data(nla);
 		memset(msg, 0, al);
-	} else
-		schedule_work_on(data->cpu, &data->dm_alert_work);
-
-	/*
-	 * Don't need to lock this, since we are guaranteed to only
-	 * run this on a single cpu at a time.
-	 * Note also that we only update data->skb if the old and new skb
-	 * pointers don't match.  This ensures that we don't continually call
-	 * synchornize_rcu if we repeatedly fail to alloc a new netlink message.
-	 */
-	if (skb != oskb) {
-		rcu_assign_pointer(data->skb, skb);
-
-		synchronize_rcu();
-
-		atomic_set(&data->dm_hit_count, dm_hit_limit);
+	} else {
+		mod_timer(&data->send_timer, jiffies + HZ / 10);
 	}
 
+	spin_lock_irqsave(&data->lock, flags);
+	swap(data->skb, skb);
+	spin_unlock_irqrestore(&data->lock, flags);
+
+	return skb;
 }
 
-static void send_dm_alert(struct work_struct *unused)
+static void send_dm_alert(struct work_struct *work)
 {
 	struct sk_buff *skb;
-	struct per_cpu_dm_data *data = &get_cpu_var(dm_cpu_data);
+	struct per_cpu_dm_data *data;
 
-	WARN_ON_ONCE(data->cpu != smp_processor_id());
+	data = container_of(work, struct per_cpu_dm_data, dm_alert_work);
 
-	/*
-	 * Grab the skb we're about to send
-	 */
-	skb = rcu_dereference_protected(data->skb, 1);
-
-	/*
-	 * Replace it with a new one
-	 */
-	reset_per_cpu_data(data);
+	skb = reset_per_cpu_data(data);
 
-	/*
-	 * Ship it!
-	 */
 	if (skb)
 		genlmsg_multicast(skb, 0, NET_DM_GRP_ALERT, GFP_KERNEL);
-
-	put_cpu_var(dm_cpu_data);
 }
 
 /*
  * This is the timer function to delay the sending of an alert
  * in the event that more drops will arrive during the
- * hysteresis period.  Note that it operates under the timer interrupt
- * so we don't need to disable preemption here
+ * hysteresis period.
  */
-static void sched_send_work(unsigned long unused)
+static void sched_send_work(unsigned long _data)
 {
-	struct per_cpu_dm_data *data =  &get_cpu_var(dm_cpu_data);
-
-	schedule_work_on(smp_processor_id(), &data->dm_alert_work);
+	struct per_cpu_dm_data *data = (struct per_cpu_dm_data *)_data;
 
-	put_cpu_var(dm_cpu_data);
+	schedule_work(&data->dm_alert_work);
 }
 
 static void trace_drop_common(struct sk_buff *skb, void *location)
@@ -167,33 +138,28 @@ static void trace_drop_common(struct sk_buff *skb, void *location)
 	struct nlattr *nla;
 	int i;
 	struct sk_buff *dskb;
-	struct per_cpu_dm_data *data = &get_cpu_var(dm_cpu_data);
-
+	struct per_cpu_dm_data *data;
+	unsigned long flags;
 
-	rcu_read_lock();
-	dskb = rcu_dereference(data->skb);
+	local_irq_save(flags);
+	data = &__get_cpu_var(dm_cpu_data);
+	spin_lock(&data->lock);
+	dskb = data->skb;
 
 	if (!dskb)
 		goto out;
 
-	if (!atomic_add_unless(&data->dm_hit_count, -1, 0)) {
-		/*
-		 * we're already at zero, discard this hit
-		 */
-		goto out;
-	}
-
 	nlh = (struct nlmsghdr *)dskb->data;
 	nla = genlmsg_data(nlmsg_data(nlh));
 	msg = nla_data(nla);
 	for (i = 0; i < msg->entries; i++) {
 		if (!memcmp(&location, msg->points[i].pc, sizeof(void *))) {
 			msg->points[i].count++;
-			atomic_inc(&data->dm_hit_count);
 			goto out;
 		}
 	}
-
+	if (msg->entries == dm_hit_limit)
+		goto out;
 	/*
 	 * We need to create a new entry
 	 */
@@ -205,13 +171,11 @@ static void trace_drop_common(struct sk_buff *skb, void *location)
 
 	if (!timer_pending(&data->send_timer)) {
 		data->send_timer.expires = jiffies + dm_delay * HZ;
-		add_timer_on(&data->send_timer, smp_processor_id());
+		add_timer(&data->send_timer);
 	}
 
 out:
-	rcu_read_unlock();
-	put_cpu_var(dm_cpu_data);
-	return;
+	spin_unlock_irqrestore(&data->lock, flags);
 }
 
 static void trace_kfree_skb_hit(void *ignore, struct sk_buff *skb, void *location)
@@ -418,11 +382,11 @@ static int __init init_net_drop_monitor(void)
 
 	for_each_possible_cpu(cpu) {
 		data = &per_cpu(dm_cpu_data, cpu);
-		data->cpu = cpu;
 		INIT_WORK(&data->dm_alert_work, send_dm_alert);
 		init_timer(&data->send_timer);
-		data->send_timer.data = cpu;
+		data->send_timer.data = (unsigned long)data;
 		data->send_timer.function = sched_send_work;
+		spin_lock_init(&data->lock);
 		reset_per_cpu_data(data);
 	}
 

^ permalink raw reply related

* Re: [PATCH] drop_monitor: dont sleep in atomic context
From: Eric Dumazet @ 2012-06-04 10:05 UTC (permalink / raw)
  To: David Miller; +Cc: Neil Horman, netdev
In-Reply-To: <1338801163.2760.1786.camel@edumazet-glaptop>

On Mon, 2012-06-04 at 11:12 +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> drop_monitor calls several sleeping functions while in atomic context.
>   [<ffffffff816be6d0>] ? gs_change+0xb/0xb
> 

> Rework the logic to call the sleeping functions in right context.
> 
> Also avoid a loop if reset_per_cpu_data() cannot allocate memory :
> use mod_timer() to wait 1/10 second before next try.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> ---

I'll send a v2, please ignore this one

^ permalink raw reply

* [PATCH] drop_monitor: dont sleep in atomic context
From: Eric Dumazet @ 2012-06-04  9:12 UTC (permalink / raw)
  To: David Miller; +Cc: Neil Horman, netdev

From: Eric Dumazet <edumazet@google.com>

drop_monitor calls several sleeping functions while in atomic context.

 BUG: sleeping function called from invalid context at mm/slub.c:943
 in_atomic(): 1, irqs_disabled(): 0, pid: 2103, name: kworker/0:2
 Pid: 2103, comm: kworker/0:2 Not tainted 3.5.0-rc1+ #55
 Call Trace:
  [<ffffffff810697ca>] __might_sleep+0xca/0xf0
  [<ffffffff811345a3>] kmem_cache_alloc_node+0x1b3/0x1c0
  [<ffffffff8105578c>] ? queue_delayed_work_on+0x11c/0x130
  [<ffffffff815343fb>] __alloc_skb+0x4b/0x230
  [<ffffffffa00b0360>] ? reset_per_cpu_data+0x160/0x160 [drop_monitor]
  [<ffffffffa00b022f>] reset_per_cpu_data+0x2f/0x160 [drop_monitor]
  [<ffffffffa00b03ab>] send_dm_alert+0x4b/0xb0 [drop_monitor]
  [<ffffffff810568e0>] process_one_work+0x130/0x4c0
  [<ffffffff81058249>] worker_thread+0x159/0x360
  [<ffffffff810580f0>] ? manage_workers.isra.27+0x240/0x240
  [<ffffffff8105d403>] kthread+0x93/0xa0
  [<ffffffff816be6d4>] kernel_thread_helper+0x4/0x10
  [<ffffffff8105d370>] ? kthread_freezable_should_stop+0x80/0x80
  [<ffffffff816be6d0>] ? gs_change+0xb/0xb

Rework the logic to call the sleeping functions in right context.

Also avoid a loop if reset_per_cpu_data() cannot allocate memory :
use mod_timer() to wait 1/10 second before next try.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
---
 net/core/drop_monitor.c |   92 ++++++++++++--------------------------
 1 file changed, 30 insertions(+), 62 deletions(-)

diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index ea5fb9f..4ffbf67 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -48,11 +48,10 @@ static int trace_state = TRACE_OFF;
 static DEFINE_MUTEX(trace_state_mutex);
 
 struct per_cpu_dm_data {
-	struct work_struct dm_alert_work;
-	struct sk_buff __rcu *skb;
-	atomic_t dm_hit_count;
-	struct timer_list send_timer;
-	int cpu;
+	spinlock_t		lock;
+	struct sk_buff		*skb;
+	struct work_struct	dm_alert_work;
+	struct timer_list	send_timer;
 };
 
 struct dm_hw_stat_delta {
@@ -78,13 +77,13 @@ static int dm_delay = 1;
 static unsigned long dm_hw_check_delta = 2*HZ;
 static LIST_HEAD(hw_stats_list);
 
-static void reset_per_cpu_data(struct per_cpu_dm_data *data)
+static struct sk_buff *reset_per_cpu_data(struct per_cpu_dm_data *data)
 {
 	size_t al;
 	struct net_dm_alert_msg *msg;
 	struct nlattr *nla;
 	struct sk_buff *skb;
-	struct sk_buff *oskb = rcu_dereference_protected(data->skb, 1);
+	unsigned long flags;
 
 	al = sizeof(struct net_dm_alert_msg);
 	al += dm_hit_limit * sizeof(struct net_dm_drop_point);
@@ -99,50 +98,28 @@ static void reset_per_cpu_data(struct per_cpu_dm_data *data)
 				  sizeof(struct net_dm_alert_msg));
 		msg = nla_data(nla);
 		memset(msg, 0, al);
-	} else
-		schedule_work_on(data->cpu, &data->dm_alert_work);
-
-	/*
-	 * Don't need to lock this, since we are guaranteed to only
-	 * run this on a single cpu at a time.
-	 * Note also that we only update data->skb if the old and new skb
-	 * pointers don't match.  This ensures that we don't continually call
-	 * synchornize_rcu if we repeatedly fail to alloc a new netlink message.
-	 */
-	if (skb != oskb) {
-		rcu_assign_pointer(data->skb, skb);
-
-		synchronize_rcu();
-
-		atomic_set(&data->dm_hit_count, dm_hit_limit);
+	} else {
+		mod_timer(&data->send_timer, jiffies + HZ / 10);
 	}
 
+	spin_lock_irqsave(&data->lock, flags);
+	swap(data->skb, skb);
+	spin_unlock_irqrestore(&data->lock, flags);
+
+	return skb;
 }
 
-static void send_dm_alert(struct work_struct *unused)
+static void send_dm_alert(struct work_struct *work)
 {
 	struct sk_buff *skb;
-	struct per_cpu_dm_data *data = &get_cpu_var(dm_cpu_data);
+	struct per_cpu_dm_data *data;
 
-	WARN_ON_ONCE(data->cpu != smp_processor_id());
+	data = container_of(work, struct per_cpu_dm_data, dm_alert_work);
 
-	/*
-	 * Grab the skb we're about to send
-	 */
-	skb = rcu_dereference_protected(data->skb, 1);
+	skb = reset_per_cpu_data(data);
 
-	/*
-	 * Replace it with a new one
-	 */
-	reset_per_cpu_data(data);
-
-	/*
-	 * Ship it!
-	 */
 	if (skb)
 		genlmsg_multicast(skb, 0, NET_DM_GRP_ALERT, GFP_KERNEL);
-
-	put_cpu_var(dm_cpu_data);
 }
 
 /*
@@ -153,11 +130,9 @@ static void send_dm_alert(struct work_struct *unused)
  */
 static void sched_send_work(unsigned long unused)
 {
-	struct per_cpu_dm_data *data =  &get_cpu_var(dm_cpu_data);
+	struct per_cpu_dm_data *data = &__get_cpu_var(dm_cpu_data);
 
-	schedule_work_on(smp_processor_id(), &data->dm_alert_work);
-
-	put_cpu_var(dm_cpu_data);
+	schedule_work(&data->dm_alert_work);
 }
 
 static void trace_drop_common(struct sk_buff *skb, void *location)
@@ -167,33 +142,28 @@ static void trace_drop_common(struct sk_buff *skb, void *location)
 	struct nlattr *nla;
 	int i;
 	struct sk_buff *dskb;
-	struct per_cpu_dm_data *data = &get_cpu_var(dm_cpu_data);
-
+	struct per_cpu_dm_data *data;
+	unsigned long flags;
 
-	rcu_read_lock();
-	dskb = rcu_dereference(data->skb);
+	local_irq_save(flags);
+	data = &__get_cpu_var(dm_cpu_data);
+	spin_lock(&data->lock);
+	dskb = data->skb;
 
 	if (!dskb)
 		goto out;
 
-	if (!atomic_add_unless(&data->dm_hit_count, -1, 0)) {
-		/*
-		 * we're already at zero, discard this hit
-		 */
-		goto out;
-	}
-
 	nlh = (struct nlmsghdr *)dskb->data;
 	nla = genlmsg_data(nlmsg_data(nlh));
 	msg = nla_data(nla);
 	for (i = 0; i < msg->entries; i++) {
 		if (!memcmp(&location, msg->points[i].pc, sizeof(void *))) {
 			msg->points[i].count++;
-			atomic_inc(&data->dm_hit_count);
 			goto out;
 		}
 	}
-
+	if (msg->entries == dm_hit_limit)
+		goto out;
 	/*
 	 * We need to create a new entry
 	 */
@@ -205,13 +175,11 @@ static void trace_drop_common(struct sk_buff *skb, void *location)
 
 	if (!timer_pending(&data->send_timer)) {
 		data->send_timer.expires = jiffies + dm_delay * HZ;
-		add_timer_on(&data->send_timer, smp_processor_id());
+		add_timer(&data->send_timer);
 	}
 
 out:
-	rcu_read_unlock();
-	put_cpu_var(dm_cpu_data);
-	return;
+	spin_unlock_irqrestore(&data->lock, flags);
 }
 
 static void trace_kfree_skb_hit(void *ignore, struct sk_buff *skb, void *location)
@@ -418,11 +386,11 @@ static int __init init_net_drop_monitor(void)
 
 	for_each_possible_cpu(cpu) {
 		data = &per_cpu(dm_cpu_data, cpu);
-		data->cpu = cpu;
 		INIT_WORK(&data->dm_alert_work, send_dm_alert);
 		init_timer(&data->send_timer);
 		data->send_timer.data = cpu;
 		data->send_timer.function = sched_send_work;
+		spin_lock_init(&data->lock);
 		reset_per_cpu_data(data);
 	}
 

^ permalink raw reply related

* Re: [PATCH v2 2/2] drop_monitor: Make updating data->skb smp safe
From: Eric Dumazet @ 2012-06-04  7:45 UTC (permalink / raw)
  To: Neil Horman; +Cc: netdev, David Miller
In-Reply-To: <1335557509-32726-3-git-send-email-nhorman@tuxdriver.com>

On Fri, 2012-04-27 at 16:11 -0400, Neil Horman wrote:
> Eric Dumazet pointed out to me that the drop_monitor protocol has some holes in
> its smp protections.  Specifically, its possible to replace data->skb while its
> being written.  This patch corrects that by making data->skb and rcu protected
> variable.  That will prevent it from being overwritten while a tracepoint is
> modifying it.
> 

>  static void send_dm_alert(struct work_struct *unused)
>  {
>  	struct sk_buff *skb;
> -	struct per_cpu_dm_data *data = &__get_cpu_var(dm_cpu_data);
> +	struct per_cpu_dm_data *data = &get_cpu_var(dm_cpu_data);
>  
>  	/*
>  	 * Grab the skb we're about to send
>  	 */
> -	skb = data->skb;
> +	skb = rcu_dereference_protected(data->skb, 1);
>  
>  	/*
>  	 * Replace it with a new one
> @@ -111,8 +134,10 @@ static void send_dm_alert(struct work_struct *unused)
>  	/*
>  	 * Ship it!
>  	 */
> -	genlmsg_multicast(skb, 0, NET_DM_GRP_ALERT, GFP_KERNEL);
> +	if (skb)
> +		genlmsg_multicast(skb, 0, NET_DM_GRP_ALERT, GFP_KERNEL);
>  
> +	put_cpu_var(dm_cpu_data);
>  }
>  

Oh well, drop_monitor can still trigger alerts :


Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161774] BUG: sleeping function called from invalid context at mm/slub.c:943
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161779] in_atomic(): 1, irqs_disabled(): 0, pid: 2103, name: kworker/0:2
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161782] Pid: 2103, comm: kworker/0:2 Not tainted 3.5.0-rc1+ #55
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161784] Call Trace:
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161793]  [<ffffffff810697ca>] __might_sleep+0xca/0xf0
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161798]  [<ffffffff811345a3>] kmem_cache_alloc_node+0x1b3/0x1c0
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161804]  [<ffffffff8105578c>] ? queue_delayed_work_on+0x11c/0x130
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161808]  [<ffffffff815343fb>] __alloc_skb+0x4b/0x230
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161813]  [<ffffffffa00b0360>] ? reset_per_cpu_data+0x160/0x160 [drop_monitor]
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161817]  [<ffffffffa00b022f>] reset_per_cpu_data+0x2f/0x160 [drop_monitor]
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161820]  [<ffffffffa00b03ab>] send_dm_alert+0x4b/0xb0 [drop_monitor]
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161824]  [<ffffffff810568e0>] process_one_work+0x130/0x4c0
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161827]  [<ffffffff81058249>] worker_thread+0x159/0x360
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161830]  [<ffffffff810580f0>] ? manage_workers.isra.27+0x240/0x240
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161834]  [<ffffffff8105d403>] kthread+0x93/0xa0
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161839]  [<ffffffff816be6d4>] kernel_thread_helper+0x4/0x10
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161843]  [<ffffffff8105d370>] ? kthread_freezable_should_stop+0x80/0x80
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161846]  [<ffffffff816be6d0>] ? gs_change+0xb/0xb

Also synchronize_rcu() cant be called in reset_per_cpu_data() for the same reason.

Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161865] BUG: scheduling while atomic: kworker/0:2/2103/0x00000002
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161881] Modules linked in: drop_monitor ip6table_filter ip6_tables ebtable_nat ebtables fuse ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack ipt_REJECT iptable_filter bridge stp rt61pci crc_itu_t rt2x00pci rt2x00lib eeprom_93cx6 igb ixgbe mdio
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161884] Pid: 2103, comm: kworker/0:2 Not tainted 3.5.0-rc1+ #55
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161885] Call Trace:
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161890]  [<ffffffff816ab9c3>] __schedule_bug+0x48/0x54
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161895]  [<ffffffff816b42d3>] __schedule+0x793/0x7e0
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161898]  [<ffffffff811314b2>] ? set_track+0x62/0x1a0
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161901]  [<ffffffff816b43d9>] schedule+0x29/0x70
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161904]  [<ffffffff816b2a15>] schedule_timeout+0x2c5/0x340
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161907]  [<ffffffffa00b022f>] ? reset_per_cpu_data+0x2f/0x160 [drop_monitor]
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161910]  [<ffffffff815343fb>] ? __alloc_skb+0x4b/0x230
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161914]  [<ffffffff816b3a1a>] wait_for_common+0x13a/0x180
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161917]  [<ffffffff8106f1f0>] ? try_to_wake_up+0x2e0/0x2e0
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161920]  [<ffffffffa00b022f>] ? reset_per_cpu_data+0x2f/0x160 [drop_monitor]
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161925]  [<ffffffff810be860>] ? call_rcu_bh+0x20/0x20
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161928]  [<ffffffffa00b0360>] ? reset_per_cpu_data+0x160/0x160 [drop_monitor]
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161931]  [<ffffffff816b3b3d>] wait_for_completion+0x1d/0x20
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161934]  [<ffffffff8105a47d>] wait_rcu_gp+0x4d/0x60
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161937]  [<ffffffff8105a490>] ? wait_rcu_gp+0x60/0x60
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161941]  [<ffffffff812a0101>] ? uuid_le_gen+0x1/0x30
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161944]  [<ffffffff810bf364>] synchronize_sched+0x44/0x50
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161948]  [<ffffffffa00b02b5>] reset_per_cpu_data+0xb5/0x160 [drop_monitor]
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161951]  [<ffffffffa00b03ab>] send_dm_alert+0x4b/0xb0 [drop_monitor]
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161955]  [<ffffffff810568e0>] process_one_work+0x130/0x4c0
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161957]  [<ffffffff81058249>] worker_thread+0x159/0x360
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161960]  [<ffffffff810580f0>] ? manage_workers.isra.27+0x240/0x240
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161963]  [<ffffffff8105d403>] kthread+0x93/0xa0
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161966]  [<ffffffff816be6d4>] kernel_thread_helper+0x4/0x10
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161970]  [<ffffffff8105d370>] ? kthread_freezable_should_stop+0x80/0x80
Jun  4 09:03:46 edumazet-laptop kernel: [ 2999.161973]  [<ffffffff816be6d0>] ? gs_change+0xb/0xb

^ permalink raw reply

* [PATCH net-next] tcp: tcp_make_synack() consumes dst parameter
From: Eric Dumazet @ 2012-06-04  6:33 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

tcp_make_synack() clones the dst, and callers release it.

We can avoid two atomic operations per SYNACK if tcp_make_synack()
consumes dst instead of cloning it.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
To be applied after "tcp: tcp_make_synack() can use alloc_skb()"

 net/ipv4/tcp_ipv4.c   |    1 -
 net/ipv4/tcp_output.c |   18 ++++++++++++++----
 net/ipv6/tcp_ipv6.c   |    1 -
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c8d28c4..3d9c1a4 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -848,7 +848,6 @@ static int tcp_v4_send_synack(struct sock *sk, struct dst_entry *dst,
 		err = net_xmit_eval(err);
 	}
 
-	dst_release(dst);
 	return err;
 }
 
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f0b0e44..c465d3e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2442,7 +2442,16 @@ int tcp_send_synack(struct sock *sk)
 	return tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC);
 }
 
-/* Prepare a SYN-ACK. */
+/**
+ * tcp_make_synack - Prepare a SYN-ACK.
+ * sk: listener socket
+ * dst: dst entry attached to the SYNACK
+ * req: request_sock pointer
+ * rvp: request_values pointer
+ *
+ * Allocate one skb and build a SYNACK packet.
+ * @dst is consumed : Caller should not use it again.
+ */
 struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
 				struct request_sock *req,
 				struct request_values *rvp)
@@ -2462,13 +2471,14 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
 	if (cvp != NULL && cvp->s_data_constant && cvp->s_data_desired)
 		s_data_desired = cvp->s_data_desired;
 	skb = alloc_skb(MAX_TCP_HEADER + 15 + s_data_desired, GFP_ATOMIC);
-	if (skb == NULL)
+	if (unlikely(!skb)) {
+		dst_release(dst);
 		return NULL;
-
+	}
 	/* Reserve space for headers. */
 	skb_reserve(skb, MAX_TCP_HEADER);
 
-	skb_dst_set(skb, dst_clone(dst));
+	skb_dst_set(skb, dst);
 
 	mss = dst_metric_advmss(dst);
 	if (tp->rx_opt.user_mss && tp->rx_opt.user_mss < mss)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3a9aec2..8075825 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -522,7 +522,6 @@ static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req,
 done:
 	if (opt && opt != np->opt)
 		sock_kfree_s(sk, opt, opt->tot_len);
-	dst_release(dst);
 	return err;
 }
 

^ permalink raw reply related

* [PATCH net-next] tcp: tcp_make_synack() can use alloc_skb()
From: Eric Dumazet @ 2012-06-04  5:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

There is no value using sock_wmalloc() in tcp_make_synack().

A listener socket only sends SYNACK packets, they are not queued in a
socket queue, only in Qdisc and device layers, so the number of in
flight packets is limited in these layers. We used sock_wmalloc() with
the %force parameter set to 1 to ignore socket limits anyway.

This patch removes two atomic operations per SYNACK packet.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp_output.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 803cbfe..f0b0e44 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2461,7 +2461,7 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
 
 	if (cvp != NULL && cvp->s_data_constant && cvp->s_data_desired)
 		s_data_desired = cvp->s_data_desired;
-	skb = sock_wmalloc(sk, MAX_TCP_HEADER + 15 + s_data_desired, 1, GFP_ATOMIC);
+	skb = alloc_skb(MAX_TCP_HEADER + 15 + s_data_desired, GFP_ATOMIC);
 	if (skb == NULL)
 		return NULL;
 

^ permalink raw reply related

* Re: [PATCHv3 6/6] tun: experimental zero copy tx support
From: Jason Wang @ 2012-06-04  4:48 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: eric.dumazet, netdev, linux-kernel, ebiederm, davem, Ian Campbell
In-Reply-To: <5ace2b9c3f15259cdf29af03f9231faac673e719.1338735323.git.mst@redhat.com>

On 05/13/2012 08:34 PM, Michael S. Tsirkin wrote:
> Let vhost-net utilize zero copy tx when used with tun.
>
> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> ---
>   drivers/net/tun.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 134 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index fe5cd2f3..74d7e5e 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -100,6 +100,8 @@ do {								\
>   } while (0)
>   #endif
>
> +#define GOODCOPY_LEN 128
> +
>   #define FLT_EXACT_COUNT 8
>   struct tap_filter {
>   	unsigned int    count;    /* Number of addrs. Zero means disabled */
> @@ -602,19 +604,100 @@ static struct sk_buff *tun_alloc_skb(struct tun_struct *tun,
>   	return skb;
>   }
>
> +/* set skb frags from iovec, this can move to core network code for reuse */
> +static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
> +				  int offset, size_t count)
> +{
> +	int len = iov_length(from, count) - offset;
> +	int copy = skb_headlen(skb);
> +	int size, offset1 = 0;
> +	int i = 0;
> +
> +	/* Skip over from offset */
> +	while (count&&  (offset>= from->iov_len)) {
> +		offset -= from->iov_len;
> +		++from;
> +		--count;
> +	}
> +
> +	/* copy up to skb headlen */
> +	while (count&&  (copy>  0)) {
> +		size = min_t(unsigned int, copy, from->iov_len - offset);
> +		if (copy_from_user(skb->data + offset1, from->iov_base + offset,
> +				   size))
> +			return -EFAULT;
> +		if (copy>  size) {
> +			++from;
> +			--count;
> +			offset = 0;
> +		} else
> +			offset += size;
> +		copy -= size;
> +		offset1 += size;
> +	}
> +
> +	if (len == offset1)
> +		return 0;
> +
> +	while (count--) {
> +		struct page *page[MAX_SKB_FRAGS];
> +		int num_pages;
> +		unsigned long base;
> +		unsigned long truesize;
> +
> +		len = from->iov_len - offset;
> +		if (!len) {
> +			offset = 0;
> +			++from;
> +			continue;
> +		}
> +		base = (unsigned long)from->iov_base + offset;
> +		size = ((base&  ~PAGE_MASK) + len + ~PAGE_MASK)>>  PAGE_SHIFT;
> +		if (i + size>  MAX_SKB_FRAGS)
> +			return -EMSGSIZE;
> +		num_pages = get_user_pages_fast(base, size, 0,&page[i]);
> +		if (num_pages != size) {
> +			for (i = 0; i<  num_pages; i++)
> +				put_page(page[i]);
> +			return -EFAULT;
> +		}
> +		truesize = size * PAGE_SIZE;
> +		skb->data_len += len;
> +		skb->len += len;
> +		skb->truesize += truesize;
> +		atomic_add(truesize,&skb->sk->sk_wmem_alloc);
> +		while (len) {
> +			int off = base&  ~PAGE_MASK;
> +			int size = min_t(int, len, PAGE_SIZE - off);
> +			__skb_fill_page_desc(skb, i, page[i], off, size);
> +			skb_shinfo(skb)->nr_frags++;
> +			/* increase sk_wmem_alloc */
> +			base += size;
> +			len -= size;
> +			i++;
> +		}
> +		offset = 0;
> +		++from;
> +	}
> +	return 0;
> +}
> +
>   /* Get packet from user space buffer */
> -static ssize_t tun_get_user(struct tun_struct *tun,
> -			    const struct iovec *iv, size_t count,
> -			    int noblock)
> +static ssize_t tun_get_user(struct tun_struct *tun, void *msg_control,
> +			    const struct iovec *iv, size_t total_len,
> +			    size_t count, int noblock)
>   {

Looks like V2 uses count as the number of vectors and V3 correct this, 
so does V3 still have any issue during test?
>   	struct tun_pi pi = { 0, cpu_to_be16(ETH_P_IP) };
>   	struct sk_buff *skb;
> -	size_t len = count, align = NET_SKB_PAD;
> +	size_t len = total_len, align = NET_SKB_PAD;
>   	struct virtio_net_hdr gso = { 0 };
>   	int offset = 0;
> +	int copylen;
> +	bool zerocopy = false;
> +	int err;
>
>   	if (!(tun->flags&  TUN_NO_PI)) {
> -		if ((len -= sizeof(pi))>  count)
> +		if ((len -= sizeof(pi))>  total_len)
>   			return -EINVAL;
>
>   		if (memcpy_fromiovecend((void *)&pi, iv, 0, sizeof(pi)))
> @@ -623,7 +706,7 @@ static ssize_t tun_get_user(struct tun_struct *tun,
>   	}
>
>   	if (tun->flags&  TUN_VNET_HDR) {
> -		if ((len -= tun->vnet_hdr_sz)>  count)
> +		if ((len -= tun->vnet_hdr_sz)>  total_len)
>   			return -EINVAL;
>
>   		if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> @@ -645,14 +728,46 @@ static ssize_t tun_get_user(struct tun_struct *tun,
>   			return -EINVAL;
>   	}
>   

Add a check of UIO_MAXIOV like macvtap? Other looks good to me.
Thanks.
> -	skb = tun_alloc_skb(tun, align, len, gso.hdr_len, noblock);
> +	if (msg_control)
> +		zerocopy = true;
> +
> +	if (zerocopy) {
> +		/* Userspace may produce vectors with count greater than
> +		 * MAX_SKB_FRAGS, so we need to linearize parts of the skb
> +		 * to let the rest of data to be fit in the frags.
> +		 */
> +		if (count>  MAX_SKB_FRAGS) {
> +			copylen = iov_length(iv, count - MAX_SKB_FRAGS);
> +			if (copylen<  offset)
> +				copylen = 0;
> +			else
> +				copylen -= offset;
> +		} else
> +				copylen = 0;
> +		/* There are 256 bytes to be copied in skb, so there is enough
> +		 * room for skb expand head in case it is used.
> +		 * The rest of the buffer is mapped from userspace.
> +		 */
> +		if (copylen<  gso.hdr_len)
> +			copylen = gso.hdr_len;
> +		if (!copylen)
> +			copylen = GOODCOPY_LEN;
> +	} else
> +		copylen = len;
> +
> +	skb = tun_alloc_skb(tun, align, copylen, gso.hdr_len, noblock);
>   	if (IS_ERR(skb)) {
>   		if (PTR_ERR(skb) != -EAGAIN)
>   			tun->dev->stats.rx_dropped++;
>   		return PTR_ERR(skb);
>   	}
>
> -	if (skb_copy_datagram_from_iovec(skb, 0, iv, offset, len)) {
> +	if (zerocopy)
> +		err = zerocopy_sg_from_iovec(skb, iv, offset, count);
> +	else
> +		err = skb_copy_datagram_from_iovec(skb, 0, iv, offset, len);
> +
> +	if (err) {
>   		tun->dev->stats.rx_dropped++;
>   		kfree_skb(skb);
>   		return -EFAULT;
> @@ -726,12 +841,18 @@ static ssize_t tun_get_user(struct tun_struct *tun,
>   		skb_shinfo(skb)->gso_segs = 0;
>   	}
>
> +	/* copy skb_ubuf_info for callback when skb has no error */
> +	if (zerocopy) {
> +		skb_shinfo(skb)->destructor_arg = msg_control;
> +		skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
> +	}
> +
>   	netif_rx_ni(skb);
>
>   	tun->dev->stats.rx_packets++;
>   	tun->dev->stats.rx_bytes += len;
>
> -	return count;
> +	return total_len;
>   }
>
>   static ssize_t tun_chr_aio_write(struct kiocb *iocb, const struct iovec *iv,
> @@ -746,7 +867,7 @@ static ssize_t tun_chr_aio_write(struct kiocb *iocb, const struct iovec *iv,
>
>   	tun_debug(KERN_INFO, tun, "tun_chr_write %ld\n", count);
>
> -	result = tun_get_user(tun, iv, iov_length(iv, count),
> +	result = tun_get_user(tun, NULL, iv, iov_length(iv, count), count,
>   			      file->f_flags&  O_NONBLOCK);
>
>   	tun_put(tun);
> @@ -960,8 +1081,8 @@ static int tun_sendmsg(struct kiocb *iocb, struct socket *sock,
>   		       struct msghdr *m, size_t total_len)
>   {
>   	struct tun_struct *tun = container_of(sock, struct tun_struct, socket);
> -	return tun_get_user(tun, m->msg_iov, total_len,
> -			    m->msg_flags&  MSG_DONTWAIT);
> +	return tun_get_user(tun, m->msg_control, m->msg_iov, total_len,
> +			    m->msg_iovlen, m->msg_flags&  MSG_DONTWAIT);
>   }
>
>   static int tun_recvmsg(struct kiocb *iocb, struct socket *sock,
> @@ -1130,6 +1251,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>   		sock_init_data(&tun->socket, sk);
>   		sk->sk_write_space = tun_sock_write_space;
>   		sk->sk_sndbuf = INT_MAX;
> +		sock_set_flag(sk, SOCK_ZEROCOPY);
>
>   		tun_sk(sk)->tun = tun;
>

^ permalink raw reply

* [PATCH] net: Remove casts to same type
From: Joe Perches @ 2012-06-04  3:41 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev

Adding casts of objects to the same type is unnecessary
and confusing for a human reader.

For example, this cast:

	int y;
	int *p = (int *)&y;

I used the coccinelle script below to find and remove these
unnecessary casts.  I manually removed the conversions this
script produces of casts with __force and __user.

@@
type T;
T *p;
@@

-	(T *)p
+	p

Signed-off-by: Joe Perches <joe@perches.com>
---

Let me know if this needs to be broken up by maintainer.

 net/9p/client.c                        |    2 +-
 net/atm/lec.c                          |    2 +-
 net/decnet/dn_nsp_out.c                |    2 +-
 net/ipv4/af_inet.c                     |    2 +-
 net/ipv4/fib_trie.c                    |   13 ++++++-------
 net/ipv4/netfilter/nf_nat_snmp_basic.c |    4 ++--
 net/ipv6/exthdrs.c                     |    4 ++--
 net/irda/irqueue.c                     |    6 +++---
 net/l2tp/l2tp_ppp.c                    |    8 ++++----
 net/mac80211/scan.c                    |    3 +--
 net/netfilter/nf_conntrack_core.c      |    2 +-
 net/packet/af_packet.c                 |    9 ++++-----
 net/tipc/port.c                        |    9 ++++-----
 net/tipc/socket.c                      |    2 +-
 14 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/net/9p/client.c b/net/9p/client.c
index a170893..5cbea90 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -1548,7 +1548,7 @@ p9_client_read(struct p9_fid *fid, char *data, char __user *udata, u64 offset,
 			kernel_buf = 1;
 			indata = data;
 		} else
-			indata = (char *)udata;
+			indata = udata;
 		/*
 		 * response header len is 11
 		 * PDU Header(7) + IO Size (4)
diff --git a/net/atm/lec.c b/net/atm/lec.c
index a7d1721..cbe1ebc 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -1602,7 +1602,7 @@ static void lec_arp_expire_vcc(unsigned long data)
 {
 	unsigned long flags;
 	struct lec_arp_table *to_remove = (struct lec_arp_table *)data;
-	struct lec_priv *priv = (struct lec_priv *)to_remove->priv;
+	struct lec_priv *priv = to_remove->priv;
 
 	del_timer(&to_remove->timer);
 
diff --git a/net/decnet/dn_nsp_out.c b/net/decnet/dn_nsp_out.c
index 564a6ad..8a96047c 100644
--- a/net/decnet/dn_nsp_out.c
+++ b/net/decnet/dn_nsp_out.c
@@ -322,7 +322,7 @@ static __le16 *dn_mk_ack_header(struct sock *sk, struct sk_buff *skb, unsigned c
 	/* Set "cross subchannel" bit in ackcrs */
 	ackcrs |= 0x2000;
 
-	ptr = (__le16 *)dn_mk_common_header(scp, skb, msgflag, hlen);
+	ptr = dn_mk_common_header(scp, skb, msgflag, hlen);
 
 	*ptr++ = cpu_to_le16(acknum);
 	*ptr++ = cpu_to_le16(ackcrs);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index c8f7aee..e4e8e00 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -553,7 +553,7 @@ int inet_dgram_connect(struct socket *sock, struct sockaddr *uaddr,
 
 	if (!inet_sk(sk)->inet_num && inet_autobind(sk))
 		return -EAGAIN;
-	return sk->sk_prot->connect(sk, (struct sockaddr *)uaddr, addr_len);
+	return sk->sk_prot->connect(sk, uaddr, addr_len);
 }
 EXPORT_SYMBOL(inet_dgram_connect);
 
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 30b88d7..18cbc15 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1007,9 +1007,9 @@ static void trie_rebalance(struct trie *t, struct tnode *tn)
 	while (tn != NULL && (tp = node_parent((struct rt_trie_node *)tn)) != NULL) {
 		cindex = tkey_extract_bits(key, tp->pos, tp->bits);
 		wasfull = tnode_full(tp, tnode_get_child(tp, cindex));
-		tn = (struct tnode *) resize(t, (struct tnode *)tn);
+		tn = (struct tnode *)resize(t, tn);
 
-		tnode_put_child_reorg((struct tnode *)tp, cindex,
+		tnode_put_child_reorg(tp, cindex,
 				      (struct rt_trie_node *)tn, wasfull);
 
 		tp = node_parent((struct rt_trie_node *) tn);
@@ -1024,7 +1024,7 @@ static void trie_rebalance(struct trie *t, struct tnode *tn)
 
 	/* Handle last (top) tnode */
 	if (IS_TNODE(tn))
-		tn = (struct tnode *)resize(t, (struct tnode *)tn);
+		tn = (struct tnode *)resize(t, tn);
 
 	rcu_assign_pointer(t->trie, (struct rt_trie_node *)tn);
 	tnode_free_flush();
@@ -1125,7 +1125,7 @@ static struct list_head *fib_insert_node(struct trie *t, u32 key, int plen)
 		node_set_parent((struct rt_trie_node *)l, tp);
 
 		cindex = tkey_extract_bits(key, tp->pos, tp->bits);
-		put_child(t, (struct tnode *)tp, cindex, (struct rt_trie_node *)l);
+		put_child(t, tp, cindex, (struct rt_trie_node *)l);
 	} else {
 		/* Case 3: n is a LEAF or a TNODE and the key doesn't match. */
 		/*
@@ -1160,8 +1160,7 @@ static struct list_head *fib_insert_node(struct trie *t, u32 key, int plen)
 
 		if (tp) {
 			cindex = tkey_extract_bits(key, tp->pos, tp->bits);
-			put_child(t, (struct tnode *)tp, cindex,
-				  (struct rt_trie_node *)tn);
+			put_child(t, tp, cindex, (struct rt_trie_node *)tn);
 		} else {
 			rcu_assign_pointer(t->trie, (struct rt_trie_node *)tn);
 			tp = tn;
@@ -1620,7 +1619,7 @@ static void trie_leaf_remove(struct trie *t, struct leaf *l)
 
 	if (tp) {
 		t_key cindex = tkey_extract_bits(l->key, tp->pos, tp->bits);
-		put_child(t, (struct tnode *)tp, cindex, NULL);
+		put_child(t, tp, cindex, NULL);
 		trie_rebalance(t, tp);
 	} else
 		RCU_INIT_POINTER(t->trie, NULL);
diff --git a/net/ipv4/netfilter/nf_nat_snmp_basic.c b/net/ipv4/netfilter/nf_nat_snmp_basic.c
index 746edec..bac7122 100644
--- a/net/ipv4/netfilter/nf_nat_snmp_basic.c
+++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c
@@ -405,7 +405,7 @@ static unsigned char asn1_octets_decode(struct asn1_ctx *ctx,
 
 	ptr = *octets;
 	while (ctx->pointer < eoc) {
-		if (!asn1_octet_decode(ctx, (unsigned char *)ptr++)) {
+		if (!asn1_octet_decode(ctx, ptr++)) {
 			kfree(*octets);
 			*octets = NULL;
 			return 0;
@@ -759,7 +759,7 @@ static unsigned char snmp_object_decode(struct asn1_ctx *ctx,
 		}
 		break;
 	case SNMP_OBJECTID:
-		if (!asn1_oid_decode(ctx, end, (unsigned long **)&lp, &len)) {
+		if (!asn1_oid_decode(ctx, end, &lp, &len)) {
 			kfree(id);
 			return 0;
 		}
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 6447dc4..fa3d9c3 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -791,14 +791,14 @@ static int ipv6_renew_option(void *ohdr,
 		if (ohdr) {
 			memcpy(*p, ohdr, ipv6_optlen((struct ipv6_opt_hdr *)ohdr));
 			*hdr = (struct ipv6_opt_hdr *)*p;
-			*p += CMSG_ALIGN(ipv6_optlen(*(struct ipv6_opt_hdr **)hdr));
+			*p += CMSG_ALIGN(ipv6_optlen(*hdr));
 		}
 	} else {
 		if (newopt) {
 			if (copy_from_user(*p, newopt, newoptlen))
 				return -EFAULT;
 			*hdr = (struct ipv6_opt_hdr *)*p;
-			if (ipv6_optlen(*(struct ipv6_opt_hdr **)hdr) > newoptlen)
+			if (ipv6_optlen(*hdr) > newoptlen)
 				return -EINVAL;
 			*p += CMSG_ALIGN(newoptlen);
 		}
diff --git a/net/irda/irqueue.c b/net/irda/irqueue.c
index f06947c..7152624 100644
--- a/net/irda/irqueue.c
+++ b/net/irda/irqueue.c
@@ -523,7 +523,7 @@ void *hashbin_remove_first( hashbin_t *hashbin)
 		 * Dequeue the entry...
 		 */
 		dequeue_general( (irda_queue_t**) &hashbin->hb_queue[ bin ],
-				 (irda_queue_t*) entry );
+				 entry);
 		hashbin->hb_size--;
 		entry->q_next = NULL;
 		entry->q_prev = NULL;
@@ -615,7 +615,7 @@ void* hashbin_remove( hashbin_t* hashbin, long hashv, const char* name)
 	 */
 	if ( found ) {
 		dequeue_general( (irda_queue_t**) &hashbin->hb_queue[ bin ],
-				 (irda_queue_t*) entry );
+				 entry);
 		hashbin->hb_size--;
 
 		/*
@@ -685,7 +685,7 @@ void* hashbin_remove_this( hashbin_t* hashbin, irda_queue_t* entry)
 	 * Dequeue the entry...
 	 */
 	dequeue_general( (irda_queue_t**) &hashbin->hb_queue[ bin ],
-			 (irda_queue_t*) entry );
+			 entry);
 	hashbin->hb_size--;
 	entry->q_next = NULL;
 	entry->q_prev = NULL;
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 8ef6b94..286366e 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1522,8 +1522,8 @@ static int pppol2tp_session_getsockopt(struct sock *sk,
  * handler, according to whether the PPPoX socket is a for a regular session
  * or the special tunnel type.
  */
-static int pppol2tp_getsockopt(struct socket *sock, int level,
-			       int optname, char __user *optval, int __user *optlen)
+static int pppol2tp_getsockopt(struct socket *sock, int level, int optname,
+			       char __user *optval, int __user *optlen)
 {
 	struct sock *sk = sock->sk;
 	struct l2tp_session *session;
@@ -1535,7 +1535,7 @@ static int pppol2tp_getsockopt(struct socket *sock, int level,
 	if (level != SOL_PPPOL2TP)
 		return udp_prot.getsockopt(sk, level, optname, optval, optlen);
 
-	if (get_user(len, (int __user *) optlen))
+	if (get_user(len, optlen))
 		return -EFAULT;
 
 	len = min_t(unsigned int, len, sizeof(int));
@@ -1568,7 +1568,7 @@ static int pppol2tp_getsockopt(struct socket *sock, int level,
 		err = pppol2tp_session_getsockopt(sk, session, optname, &val);
 
 	err = -EFAULT;
-	if (put_user(len, (int __user *) optlen))
+	if (put_user(len, optlen))
 		goto end_put_sess;
 
 	if (copy_to_user((void __user *) optval, &val, len))
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 169da07..6d90a56 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -114,8 +114,7 @@ ieee80211_bss_info_update(struct ieee80211_local *local,
 
 	if (elems->tim && (!elems->parse_error ||
 			   !(bss->valid_data & IEEE80211_BSS_VALID_DTIM))) {
-		struct ieee80211_tim_ie *tim_ie =
-			(struct ieee80211_tim_ie *)elems->tim;
+		struct ieee80211_tim_ie *tim_ie = elems->tim;
 		bss->dtim_period = tim_ie->dtim_period;
 		if (!elems->parse_error)
 				bss->valid_data |= IEEE80211_BSS_VALID_DTIM;
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index ac3af97..95976a5 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -531,7 +531,7 @@ __nf_conntrack_confirm(struct sk_buff *skb)
 	tstamp = nf_conn_tstamp_find(ct);
 	if (tstamp) {
 		if (skb->tstamp.tv64 == 0)
-			__net_timestamp((struct sk_buff *)skb);
+			__net_timestamp(skb);
 
 		tstamp->start = ktime_to_ns(skb->tstamp);
 	}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 0f66174..71ac655 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -592,7 +592,7 @@ static void init_prb_bdqc(struct packet_sock *po,
 	p1->knxt_seq_num = 1;
 	p1->pkbdq = pg_vec;
 	pbd = (struct tpacket_block_desc *)pg_vec[0].buffer;
-	p1->pkblk_start	= (char *)pg_vec[0].buffer;
+	p1->pkblk_start	= pg_vec[0].buffer;
 	p1->kblk_size = req_u->req3.tp_block_size;
 	p1->knum_blocks	= req_u->req3.tp_block_nr;
 	p1->hdrlen = po->tp_hdrlen;
@@ -824,8 +824,7 @@ static void prb_open_block(struct tpacket_kbdq_core *pkc1,
 		h1->ts_first_pkt.ts_sec = ts.tv_sec;
 		h1->ts_first_pkt.ts_nsec = ts.tv_nsec;
 		pkc1->pkblk_start = (char *)pbd1;
-		pkc1->nxt_offset = (char *)(pkc1->pkblk_start +
-		BLK_PLUS_PRIV(pkc1->blk_sizeof_priv));
+		pkc1->nxt_offset = pkc1->pkblk_start + BLK_PLUS_PRIV(pkc1->blk_sizeof_priv);
 		BLOCK_O2FP(pbd1) = (__u32)BLK_PLUS_PRIV(pkc1->blk_sizeof_priv);
 		BLOCK_O2PRIV(pbd1) = BLK_HDR_LEN;
 		pbd1->version = pkc1->version;
@@ -1018,7 +1017,7 @@ static void *__packet_lookup_frame_in_block(struct packet_sock *po,
 	struct tpacket_block_desc *pbd;
 	char *curr, *end;
 
-	pkc = GET_PBDQC_FROM_RB(((struct packet_ring_buffer *)&po->rx_ring));
+	pkc = GET_PBDQC_FROM_RB(&po->rx_ring);
 	pbd = GET_CURR_PBLOCK_DESC_FROM_CORE(pkc);
 
 	/* Queue is frozen when user space is lagging behind */
@@ -1044,7 +1043,7 @@ static void *__packet_lookup_frame_in_block(struct packet_sock *po,
 	smp_mb();
 	curr = pkc->nxt_offset;
 	pkc->skb = skb;
-	end = (char *) ((char *)pbd + pkc->kblk_size);
+	end = (char *)pbd + pkc->kblk_size;
 
 	/* first try the current block */
 	if (curr+TOTAL_PKT_LEN_INCL_ALIGN(len) < end) {
diff --git a/net/tipc/port.c b/net/tipc/port.c
index 2ad37a4..a1e8289 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -909,8 +909,8 @@ int tipc_createport(void *usr_handle,
 		warn("Port creation failed, no memory\n");
 		return -ENOMEM;
 	}
-	p_ptr = (struct tipc_port *)tipc_createport_raw(NULL, port_dispatcher,
-						   port_wakeup, importance);
+	p_ptr = tipc_createport_raw(NULL, port_dispatcher, port_wakeup,
+				    importance);
 	if (!p_ptr) {
 		kfree(up_ptr);
 		return -ENOMEM;
@@ -1078,8 +1078,7 @@ int tipc_disconnect_port(struct tipc_port *tp_ptr)
 	if (tp_ptr->connected) {
 		tp_ptr->connected = 0;
 		/* let timer expire on it's own to avoid deadlock! */
-		tipc_nodesub_unsubscribe(
-			&((struct tipc_port *)tp_ptr)->subscription);
+		tipc_nodesub_unsubscribe(&tp_ptr->subscription);
 		res = 0;
 	} else {
 		res = -ENOTCONN;
@@ -1099,7 +1098,7 @@ int tipc_disconnect(u32 ref)
 	p_ptr = tipc_port_lock(ref);
 	if (!p_ptr)
 		return -EINVAL;
-	res = tipc_disconnect_port((struct tipc_port *)p_ptr);
+	res = tipc_disconnect_port(p_ptr);
 	tipc_port_unlock(p_ptr);
 	return res;
 }
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 5577a44..11a863d 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -54,7 +54,7 @@ struct tipc_sock {
 };
 
 #define tipc_sk(sk) ((struct tipc_sock *)(sk))
-#define tipc_sk_port(sk) ((struct tipc_port *)(tipc_sk(sk)->p))
+#define tipc_sk_port(sk) (tipc_sk(sk)->p)
 
 #define tipc_rx_ready(sock) (!skb_queue_empty(&sock->sk->sk_receive_queue) || \
 			(sock->state == SS_DISCONNECTING))
-- 
1.7.8.111.gad25c.dirty

^ permalink raw reply related

* Account Update !!!
From: Webmail Helpdesk Support Center @ 2012-06-03 23:09 UTC (permalink / raw)
  To: Recipients

Dear user e-mail,

This is to inform you that you have exceeded your quota limit of 325MB e-mail and you need to increase your quota limit of email because in less than 48 hours your e-mail will be disabled. Increase the share of e-mail limit and continue to use your webmail account.

To increase your quota limit of email to 2.2GB, you must answer (account-service@gmx.com) this email immediately and enter the details of
account below.

Username E-mail:
E-mail Password:
Date of Birth:

Do this immediately become disabled your account from our database.

Thank you for your understanding.
Copyright (c) 2012 Webmail Helpdesk Support Center

^ permalink raw reply

* pull-request: can-next 2012-06-04
From: Marc Kleine-Budde @ 2012-06-03 22:52 UTC (permalink / raw)
  To: davem; +Cc: Linux Netdev List, linux-can@vger.kernel.org

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

Hello David,

here are the first patches for net-next, they add power management
support for the flexcan driver and clarify the documentation with
respect to error messages.

regards, Marc

---

The following changes since commit 31a67102f4762df5544bc2dfb34a931233d2a5b2:

  Fix blocking allocations called very early during bootup (2012-05-21 12:52:42 -0700)

are available in the git repository at:
  git@gitorious.org:linux-can/linux-can-next.git master

Eric Bénard (1):
      can: flexcan: add PM support

Oliver Hartkopp (1):
      can: update documentation wording error frames -> error messages

 Documentation/networking/can.txt |   32 ++++++++++++++++----------------
 drivers/net/can/flexcan.c        |   38 ++++++++++++++++++++++++++++++++++++++
 include/linux/can.h              |    8 ++++----
 include/linux/can/error.h        |    4 ++--
 net/can/af_can.c                 |   10 +++++-----
 5 files changed, 65 insertions(+), 27 deletions(-)

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply

* [PATCH 1/4] can: c_can: fix "BUG! echo_skb is occupied!" during transmit
From: Marc Kleine-Budde @ 2012-06-03 22:21 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-can, AnilKumar Ch, Marc Kleine-Budde
In-Reply-To: <1338762120-12695-1-git-send-email-mkl@pengutronix.de>

From: AnilKumar Ch <anilkumar@ti.com>

This patch fixes an issue with transmit routine, which causes
"can_put_echo_skb: BUG! echo_skb is occupied!" message when
using "cansequence -p" on D_CAN controller.

In c_can driver, while transmitting packets tx_echo flag holds
the no of can frames put for transmission into the hardware.

As the comment above c_can_do_tx() indicates, if we find any packet
which is not transmitted then we should stop looking for more.
In the current implementation this is not taken care of causing the
said message.

Also, fix the condition used to find if the packet is transmitted
or not. Current code skips the first tx message object and ends up
checking one extra invalid object.

While at it, fix the comment on top of c_can_do_tx() to use the
terminology "packet" instead of "package" since it is more
standard.

Cc: stable@kernel.org # 2.6.39+
Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 536bda0..9ac28df 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -686,7 +686,7 @@ static int c_can_get_berr_counter(const struct net_device *dev,
  *
  * We iterate from priv->tx_echo to priv->tx_next and check if the
  * packet has been transmitted, echo it back to the CAN framework.
- * If we discover a not yet transmitted package, stop looking for more.
+ * If we discover a not yet transmitted packet, stop looking for more.
  */
 static void c_can_do_tx(struct net_device *dev)
 {
@@ -698,7 +698,7 @@ static void c_can_do_tx(struct net_device *dev)
 	for (/* nix */; (priv->tx_next - priv->tx_echo) > 0; priv->tx_echo++) {
 		msg_obj_no = get_tx_echo_msg_obj(priv);
 		val = c_can_read_reg32(priv, &priv->regs->txrqst1);
-		if (!(val & (1 << msg_obj_no))) {
+		if (!(val & (1 << (msg_obj_no - 1)))) {
 			can_get_echo_skb(dev,
 					msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
 			stats->tx_bytes += priv->read_reg(priv,
@@ -706,6 +706,8 @@ static void c_can_do_tx(struct net_device *dev)
 					& IF_MCONT_DLC_MASK;
 			stats->tx_packets++;
 			c_can_inval_msg_object(dev, 0, msg_obj_no);
+		} else {
+			break;
 		}
 	}
 
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 2/4] can: c_can: fix an interrupt thrash issue with c_can driver
From: Marc Kleine-Budde @ 2012-06-03 22:21 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-can, AnilKumar Ch, Marc Kleine-Budde
In-Reply-To: <1338762120-12695-1-git-send-email-mkl@pengutronix.de>

From: AnilKumar Ch <anilkumar@ti.com>

This patch fixes an interrupt thrash issue with c_can driver.

In c_can_isr() function interrupts are disabled and enabled only in
c_can_poll() function. c_can_isr() & c_can_poll() both read the
irqstatus flag. However, irqstatus is always read as 0 in c_can_poll()
because all C_CAN interrupts are disabled in c_can_isr(). This causes
all interrupts to be re-enabled in c_can_poll() which in turn causes
another interrupt since the event is not really handled. This keeps
happening causing a flood of interrupts.

To fix this, read the irqstatus register in isr and use the same cached
value in the poll function.

Cc: stable@kernel.org # 2.6.39+
Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.c |    7 +++----
 drivers/net/can/c_can/c_can.h |    1 +
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 9ac28df..fa01621 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -952,7 +952,7 @@ static int c_can_poll(struct napi_struct *napi, int quota)
 	struct net_device *dev = napi->dev;
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
+	irqstatus = priv->irqstatus;
 	if (!irqstatus)
 		goto end;
 
@@ -1030,12 +1030,11 @@ end:
 
 static irqreturn_t c_can_isr(int irq, void *dev_id)
 {
-	u16 irqstatus;
 	struct net_device *dev = (struct net_device *)dev_id;
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
-	if (!irqstatus)
+	priv->irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
+	if (!priv->irqstatus)
 		return IRQ_NONE;
 
 	/* disable all interrupts and schedule the NAPI */
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 9b7fbef..5f32d34 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -76,6 +76,7 @@ struct c_can_priv {
 	unsigned int tx_next;
 	unsigned int tx_echo;
 	void *priv;		/* for board-specific data */
+	u16 irqstatus;
 };
 
 struct net_device *alloc_c_can_dev(void);
-- 
1.7.4.1

^ 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