linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] rxrpc: Cleanups
@ 2016-08-23 15:26 David Howells
  2016-08-23 15:26 ` [PATCH net-next 1/5] rxrpc: Remove RXRPC_CALL_PROC_BUSY David Howells
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: David Howells @ 2016-08-23 15:26 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel



Here are some cleanups for the AF_RXRPC rewrite:

 (1) Remove some unused bits.

 (2) Call releasing on socket closure is now done in the order in which
     calls progress through the phases so that we don't miss a call
     actively moving list.

 (3) The rxrpc_call struct's channel number field is redundant and replaced
     with accesses to the masked off cid field instead.

 (4) Use a tracepoint for socket buffer accounting rather than printks.

     Unfortunately, since this would require currently non-existend
     arch-specific help to divine the current instruction location, the
     accounting functions are moved out of line so that
     __builtin_return_address() can be used.

The patches can be found here also (though they're not terminal on the branch):

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-rewrite

Tagged thusly:

	git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
	rxrpc-rewrite-20160823-1

David
---
David Howells (5):
      rxrpc: Remove RXRPC_CALL_PROC_BUSY
      rxrpc: Tidy up the rxrpc_call struct a bit
      rxrpc: When clearing a socket, clear the call sets in the right order
      rxrpc: Drop channel number field from rxrpc_call struct
      rxrpc: Use a tracepoint for skb accounting debugging


 include/trace/events/rxrpc.h |   56 +++++++++++++++++++++++++++++++++++
 net/rxrpc/af_rxrpc.c         |    1 +
 net/rxrpc/ar-internal.h      |   68 +++++++++++++-----------------------------
 net/rxrpc/call_accept.c      |    1 +
 net/rxrpc/call_event.c       |    9 ++----
 net/rxrpc/call_object.c      |   26 ++++++----------
 net/rxrpc/conn_client.c      |    2 -
 net/rxrpc/conn_event.c       |    2 +
 net/rxrpc/conn_object.c      |    5 ++-
 net/rxrpc/local_event.c      |    1 +
 net/rxrpc/output.c           |    5 ++-
 net/rxrpc/proc.c             |    6 ++--
 net/rxrpc/recvmsg.c          |    1 +
 net/rxrpc/rxkad.c            |    4 +-
 net/rxrpc/skbuff.c           |   62 ++++++++++++++++++++++++++++++++++++++
 15 files changed, 169 insertions(+), 80 deletions(-)
 create mode 100644 include/trace/events/rxrpc.h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next 1/5] rxrpc: Remove RXRPC_CALL_PROC_BUSY
  2016-08-23 15:26 [PATCH net-next 0/5] rxrpc: Cleanups David Howells
@ 2016-08-23 15:26 ` David Howells
  2016-08-23 15:26 ` [PATCH net-next 2/5] rxrpc: Tidy up the rxrpc_call struct a bit David Howells
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: David Howells @ 2016-08-23 15:26 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

Remove RXRPC_CALL_PROC_BUSY as work queue items are now 100% non-reentrant.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/ar-internal.h |    1 -
 net/rxrpc/call_event.c  |    6 ------
 2 files changed, 7 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index ff83fb1ddd47..3a2f4c214811 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -341,7 +341,6 @@ enum rxrpc_call_flag {
 	RXRPC_CALL_RCVD_LAST,		/* all packets received */
 	RXRPC_CALL_RUN_RTIMER,		/* Tx resend timer started */
 	RXRPC_CALL_TX_SOFT_ACK,		/* sent some soft ACKs */
-	RXRPC_CALL_PROC_BUSY,		/* the processor is busy */
 	RXRPC_CALL_INIT_ACCEPT,		/* acceptance was initiated */
 	RXRPC_CALL_HAS_USERID,		/* has a user ID attached */
 	RXRPC_CALL_EXPECT_OOS,		/* expect out of sequence packets */
diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index e60cf65c2232..eaa8035dcb71 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -832,11 +832,6 @@ void rxrpc_process_call(struct work_struct *work)
 	       call->debug_id, rxrpc_call_states[call->state], call->events,
 	       (jiffies - call->creation_jif) / (HZ / 10));
 
-	if (test_and_set_bit(RXRPC_CALL_PROC_BUSY, &call->flags)) {
-		_debug("XXXXXXXXXXXXX RUNNING ON MULTIPLE CPUS XXXXXXXXXXXXX");
-		return;
-	}
-
 	if (!call->conn)
 		goto skip_msg_init;
 
@@ -1281,7 +1276,6 @@ maybe_reschedule:
 	}
 
 error:
-	clear_bit(RXRPC_CALL_PROC_BUSY, &call->flags);
 	kfree(acks);
 
 	/* because we don't want two CPUs both processing the work item for one

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 2/5] rxrpc: Tidy up the rxrpc_call struct a bit
  2016-08-23 15:26 [PATCH net-next 0/5] rxrpc: Cleanups David Howells
  2016-08-23 15:26 ` [PATCH net-next 1/5] rxrpc: Remove RXRPC_CALL_PROC_BUSY David Howells
@ 2016-08-23 15:26 ` David Howells
  2016-08-23 15:26 ` [PATCH net-next 3/5] rxrpc: When clearing a socket, clear the call sets in the right order David Howells
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: David Howells @ 2016-08-23 15:26 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

Do a little tidying of the rxrpc_call struct:

 (1) in_clientflag is no longer compared against the value that's in the
     packet, so keeping it in this form isn't necessary.  Use a flag in
     flags instead and provide a pair of wrapper functions.

 (2) We don't read the epoch value, so that can go.

 (3) Move what remains of the data that were used for hashing up in the
     struct to be with the channel number.

 (4) Get rid of the local pointer.  We can get at this via the socket
     struct and we only use this in the procfs viewer.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/ar-internal.h |   23 +++++++++++++++--------
 net/rxrpc/call_object.c |    7 +------
 net/rxrpc/conn_client.c |    1 -
 net/rxrpc/output.c      |    4 ++--
 net/rxrpc/proc.c        |    6 +++---
 5 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 3a2f4c214811..0e6bc8227d54 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -344,6 +344,7 @@ enum rxrpc_call_flag {
 	RXRPC_CALL_INIT_ACCEPT,		/* acceptance was initiated */
 	RXRPC_CALL_HAS_USERID,		/* has a user ID attached */
 	RXRPC_CALL_EXPECT_OOS,		/* expect out of sequence packets */
+	RXRPC_CALL_IS_SERVICE,		/* Call is service call */
 };
 
 /*
@@ -431,8 +432,11 @@ struct rxrpc_call {
 	int			error_report;	/* Network error (ICMP/local transport) */
 	int			error;		/* Local error incurred */
 	enum rxrpc_call_state	state : 8;	/* current state of call */
-	int			debug_id;	/* debug ID for printks */
 	u8			channel;	/* connection channel occupied by this call */
+	u16			service_id;	/* service ID */
+	u32			call_id;	/* call ID on connection  */
+	u32			cid;		/* connection ID plus channel index */
+	int			debug_id;	/* debug ID for printks */
 
 	/* transmission-phase ACK management */
 	u8			acks_head;	/* offset into window of first entry */
@@ -460,13 +464,6 @@ struct rxrpc_call {
 	/* received packet records, 1 bit per record */
 #define RXRPC_ACKR_WINDOW_ASZ DIV_ROUND_UP(RXRPC_MAXACKS, BITS_PER_LONG)
 	unsigned long		ackr_window[RXRPC_ACKR_WINDOW_ASZ + 1];
-
-	u8			in_clientflag;	/* Copy of conn->in_clientflag */
-	struct rxrpc_local	*local;		/* Local endpoint. */
-	u32			call_id;	/* call ID on connection  */
-	u32			cid;		/* connection ID plus channel index */
-	u32			epoch;		/* epoch of this connection */
-	u16			service_id;	/* service ID */
 };
 
 /*
@@ -527,6 +524,16 @@ void rxrpc_release_calls_on_socket(struct rxrpc_sock *);
 void __rxrpc_put_call(struct rxrpc_call *);
 void __exit rxrpc_destroy_all_calls(void);
 
+static inline bool rxrpc_is_service_call(const struct rxrpc_call *call)
+{
+	return test_bit(RXRPC_CALL_IS_SERVICE, &call->flags);
+}
+
+static inline bool rxrpc_is_client_call(const struct rxrpc_call *call)
+{
+	return !rxrpc_is_service_call(call);
+}
+
 /*
  * conn_client.c
  */
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index ae057e0740f3..5007e7ac889f 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -167,10 +167,7 @@ static struct rxrpc_call *rxrpc_alloc_client_call(struct rxrpc_sock *rx,
 	sock_hold(&rx->sk);
 	call->socket = rx;
 	call->rx_data_post = 1;
-
-	call->local = rx->local;
 	call->service_id = srx->srx_service;
-	call->in_clientflag = 0;
 
 	_leave(" = %p", call);
 	return call;
@@ -323,6 +320,7 @@ struct rxrpc_call *rxrpc_incoming_call(struct rxrpc_sock *rx,
 	candidate->channel	= chan;
 	candidate->rx_data_post	= 0;
 	candidate->state	= RXRPC_CALL_SERVER_ACCEPTING;
+	candidate->flags	|= (1 << RXRPC_CALL_IS_SERVICE);
 	if (conn->security_ix > 0)
 		candidate->state = RXRPC_CALL_SERVER_SECURING;
 
@@ -397,10 +395,7 @@ struct rxrpc_call *rxrpc_incoming_call(struct rxrpc_sock *rx,
 	list_add_tail(&call->link, &rxrpc_calls);
 	write_unlock_bh(&rxrpc_call_lock);
 
-	call->local = conn->params.local;
-	call->epoch = conn->proto.epoch;
 	call->service_id = conn->params.service_id;
-	call->in_clientflag = RXRPC_CLIENT_INITIATED;
 
 	_net("CALL incoming %d on CONN %d", call->debug_id, call->conn->debug_id);
 
diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index 9e91f27b0d0f..d8dd8e6bb172 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -281,7 +281,6 @@ found_channel:
 	_debug("found chan");
 	call->conn	= conn;
 	call->channel	= chan;
-	call->epoch	= conn->proto.epoch;
 	call->cid	= conn->proto.cid | chan;
 	call->call_id	= ++conn->channels[chan].call_counter;
 	conn->channels[chan].call_id = call->call_id;
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index f4bda06b7d2d..9e626f1e2668 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -218,11 +218,11 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
 		ret = 0;
 	} else if (cmd != RXRPC_CMD_SEND_DATA) {
 		ret = -EINVAL;
-	} else if (!call->in_clientflag &&
+	} else if (rxrpc_is_client_call(call) &&
 		   call->state != RXRPC_CALL_CLIENT_SEND_REQUEST) {
 		/* request phase complete for this client call */
 		ret = -EPROTO;
-	} else if (call->in_clientflag &&
+	} else if (rxrpc_is_service_call(call) &&
 		   call->state != RXRPC_CALL_SERVER_ACK_REQUEST &&
 		   call->state != RXRPC_CALL_SERVER_SEND_REPLY) {
 		/* Reply phase not begun or not complete for service call. */
diff --git a/net/rxrpc/proc.c b/net/rxrpc/proc.c
index ced5f07444e5..f92de18b5893 100644
--- a/net/rxrpc/proc.c
+++ b/net/rxrpc/proc.c
@@ -61,8 +61,8 @@ static int rxrpc_call_seq_show(struct seq_file *seq, void *v)
 	call = list_entry(v, struct rxrpc_call, link);
 
 	sprintf(lbuff, "%pI4:%u",
-		&call->local->srx.transport.sin.sin_addr,
-		ntohs(call->local->srx.transport.sin.sin_port));
+		&call->socket->local->srx.transport.sin.sin_addr,
+		ntohs(call->socket->local->srx.transport.sin.sin_port));
 
 	conn = call->conn;
 	if (conn)
@@ -80,7 +80,7 @@ static int rxrpc_call_seq_show(struct seq_file *seq, void *v)
 		   call->service_id,
 		   call->cid,
 		   call->call_id,
-		   call->in_clientflag ? "Svc" : "Clt",
+		   rxrpc_is_service_call(call) ? "Svc" : "Clt",
 		   atomic_read(&call->usage),
 		   rxrpc_call_states[call->state],
 		   call->remote_abort ?: call->local_abort,

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 3/5] rxrpc: When clearing a socket, clear the call sets in the right order
  2016-08-23 15:26 [PATCH net-next 0/5] rxrpc: Cleanups David Howells
  2016-08-23 15:26 ` [PATCH net-next 1/5] rxrpc: Remove RXRPC_CALL_PROC_BUSY David Howells
  2016-08-23 15:26 ` [PATCH net-next 2/5] rxrpc: Tidy up the rxrpc_call struct a bit David Howells
@ 2016-08-23 15:26 ` David Howells
  2016-08-23 15:26 ` [PATCH net-next 4/5] rxrpc: Drop channel number field from rxrpc_call struct David Howells
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: David Howells @ 2016-08-23 15:26 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

When clearing a socket, we should clear the securing-in-progress list
first, then the accept queue and last the main call tree because that's the
order in which a call progresses.  Not that a call should move from the
accept queue to the main tree whilst we're shutting down a socket, but it a
call could possibly move from sequreq to acceptq whilst we're clearing up.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/call_object.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 5007e7ac889f..008188103fd6 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -564,12 +564,6 @@ void rxrpc_release_calls_on_socket(struct rxrpc_sock *rx)
 
 	read_lock_bh(&rx->call_lock);
 
-	/* mark all the calls as no longer wanting incoming packets */
-	for (p = rb_first(&rx->calls); p; p = rb_next(p)) {
-		call = rb_entry(p, struct rxrpc_call, sock_node);
-		rxrpc_mark_call_released(call);
-	}
-
 	/* kill the not-yet-accepted incoming calls */
 	list_for_each_entry(call, &rx->secureq, accept_link) {
 		rxrpc_mark_call_released(call);
@@ -579,6 +573,12 @@ void rxrpc_release_calls_on_socket(struct rxrpc_sock *rx)
 		rxrpc_mark_call_released(call);
 	}
 
+	/* mark all the calls as no longer wanting incoming packets */
+	for (p = rb_first(&rx->calls); p; p = rb_next(p)) {
+		call = rb_entry(p, struct rxrpc_call, sock_node);
+		rxrpc_mark_call_released(call);
+	}
+
 	read_unlock_bh(&rx->call_lock);
 	_leave("");
 }

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 4/5] rxrpc: Drop channel number field from rxrpc_call struct
  2016-08-23 15:26 [PATCH net-next 0/5] rxrpc: Cleanups David Howells
                   ` (2 preceding siblings ...)
  2016-08-23 15:26 ` [PATCH net-next 3/5] rxrpc: When clearing a socket, clear the call sets in the right order David Howells
@ 2016-08-23 15:26 ` David Howells
  2016-08-23 15:26 ` [PATCH net-next 5/5] rxrpc: Use a tracepoint for skb accounting debugging David Howells
  2016-08-24  0:21 ` [PATCH net-next 0/5] rxrpc: Cleanups David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: David Howells @ 2016-08-23 15:26 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

Drop the channel number (channel) field from the rxrpc_call struct to
reduce the size of the call struct.  The field is redundant: if the call is
attached to a connection, the channel can be obtained from there by AND'ing
with RXRPC_CHANNELMASK.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/ar-internal.h |    1 -
 net/rxrpc/call_object.c |    7 +++----
 net/rxrpc/conn_client.c |    1 -
 net/rxrpc/conn_object.c |    5 +++--
 net/rxrpc/rxkad.c       |    4 ++--
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 0e6bc8227d54..648060a5df35 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -432,7 +432,6 @@ struct rxrpc_call {
 	int			error_report;	/* Network error (ICMP/local transport) */
 	int			error;		/* Local error incurred */
 	enum rxrpc_call_state	state : 8;	/* current state of call */
-	u8			channel;	/* connection channel occupied by this call */
 	u16			service_id;	/* service ID */
 	u32			call_id;	/* call ID on connection  */
 	u32			cid;		/* connection ID plus channel index */
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 008188103fd6..4af01805bfc7 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -317,7 +317,6 @@ struct rxrpc_call *rxrpc_incoming_call(struct rxrpc_sock *rx,
 	candidate->conn		= conn;
 	candidate->cid		= sp->hdr.cid;
 	candidate->call_id	= sp->hdr.callNumber;
-	candidate->channel	= chan;
 	candidate->rx_data_post	= 0;
 	candidate->state	= RXRPC_CALL_SERVER_ACCEPTING;
 	candidate->flags	|= (1 << RXRPC_CALL_IS_SERVICE);
@@ -330,7 +329,7 @@ struct rxrpc_call *rxrpc_incoming_call(struct rxrpc_sock *rx,
 	call = rcu_dereference_protected(conn->channels[chan].call,
 					 lockdep_is_held(&conn->channel_lock));
 
-	_debug("channel[%u] is %p", candidate->channel, call);
+	_debug("channel[%u] is %p", candidate->cid & RXRPC_CHANNELMASK, call);
 	if (call && call->call_id == sp->hdr.callNumber) {
 		/* already set; must've been a duplicate packet */
 		_debug("extant call [%d]", call->state);
@@ -677,8 +676,8 @@ static void rxrpc_destroy_call(struct work_struct *work)
 	struct rxrpc_call *call =
 		container_of(work, struct rxrpc_call, destroyer);
 
-	_enter("%p{%d,%d,%p}",
-	       call, atomic_read(&call->usage), call->channel, call->conn);
+	_enter("%p{%d,%x,%p}",
+	       call, atomic_read(&call->usage), call->cid, call->conn);
 
 	ASSERTCMP(call->state, ==, RXRPC_CALL_DEAD);
 
diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index d8dd8e6bb172..fc32cc67c2de 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -280,7 +280,6 @@ attached:
 found_channel:
 	_debug("found chan");
 	call->conn	= conn;
-	call->channel	= chan;
 	call->cid	= conn->proto.cid | chan;
 	call->call_id	= ++conn->channels[chan].call_counter;
 	conn->channels[chan].call_id = call->call_id;
diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
index 896d84493a05..6a5a17efc538 100644
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -156,9 +156,10 @@ not_found:
 void __rxrpc_disconnect_call(struct rxrpc_call *call)
 {
 	struct rxrpc_connection *conn = call->conn;
-	struct rxrpc_channel *chan = &conn->channels[call->channel];
+	struct rxrpc_channel *chan =
+		&conn->channels[call->cid & RXRPC_CHANNELMASK];
 
-	_enter("%d,%d", conn->debug_id, call->channel);
+	_enter("%d,%x", conn->debug_id, call->cid);
 
 	if (rcu_access_pointer(chan->call) == call) {
 		/* Save the result of the call so that we can repeat it if necessary
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index 63afa9e9cc08..89f475febfd7 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -275,7 +275,7 @@ static int rxkad_secure_packet(struct rxrpc_call *call,
 	memcpy(&iv, call->conn->csum_iv.x, sizeof(iv));
 
 	/* calculate the security checksum */
-	x = call->channel << (32 - RXRPC_CIDSHIFT);
+	x = (call->cid & RXRPC_CHANNELMASK) << (32 - RXRPC_CIDSHIFT);
 	x |= sp->hdr.seq & 0x3fffffff;
 	call->crypto_buf[0] = htonl(sp->hdr.callNumber);
 	call->crypto_buf[1] = htonl(x);
@@ -507,7 +507,7 @@ static int rxkad_verify_packet(struct rxrpc_call *call,
 	memcpy(&iv, call->conn->csum_iv.x, sizeof(iv));
 
 	/* validate the security checksum */
-	x = call->channel << (32 - RXRPC_CIDSHIFT);
+	x = (call->cid & RXRPC_CHANNELMASK) << (32 - RXRPC_CIDSHIFT);
 	x |= sp->hdr.seq & 0x3fffffff;
 	call->crypto_buf[0] = htonl(call->call_id);
 	call->crypto_buf[1] = htonl(x);

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 5/5] rxrpc: Use a tracepoint for skb accounting debugging
  2016-08-23 15:26 [PATCH net-next 0/5] rxrpc: Cleanups David Howells
                   ` (3 preceding siblings ...)
  2016-08-23 15:26 ` [PATCH net-next 4/5] rxrpc: Drop channel number field from rxrpc_call struct David Howells
@ 2016-08-23 15:26 ` David Howells
  2016-08-24  0:21 ` [PATCH net-next 0/5] rxrpc: Cleanups David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: David Howells @ 2016-08-23 15:26 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

Use a tracepoint to log various skb accounting points to help in debugging
refcounting errors.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/trace/events/rxrpc.h |   56 ++++++++++++++++++++++++++++++++++++++
 net/rxrpc/af_rxrpc.c         |    1 +
 net/rxrpc/ar-internal.h      |   45 +++++-------------------------
 net/rxrpc/call_accept.c      |    1 +
 net/rxrpc/call_event.c       |    3 ++
 net/rxrpc/conn_event.c       |    2 +
 net/rxrpc/local_event.c      |    1 +
 net/rxrpc/output.c           |    1 +
 net/rxrpc/recvmsg.c          |    1 +
 net/rxrpc/skbuff.c           |   62 ++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 135 insertions(+), 38 deletions(-)
 create mode 100644 include/trace/events/rxrpc.h

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
new file mode 100644
index 000000000000..15283ee3e41a
--- /dev/null
+++ b/include/trace/events/rxrpc.h
@@ -0,0 +1,56 @@
+/* AF_RXRPC tracepoints
+ *
+ * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM rxrpc
+
+#if !defined(_TRACE_RXRPC_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_RXRPC_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(rxrpc_skb,
+	    TP_PROTO(struct sk_buff *skb, int op, int usage, int mod_count,
+		     const void *where),
+
+	    TP_ARGS(skb, op, usage, mod_count, where),
+
+	    TP_STRUCT__entry(
+		    __field(struct sk_buff *,		skb		)
+		    __field(int,			op		)
+		    __field(int,			usage		)
+		    __field(int,			mod_count	)
+		    __field(const void *,		where		)
+			     ),
+
+	    TP_fast_assign(
+		    __entry->skb = skb;
+		    __entry->op = op;
+		    __entry->usage = usage;
+		    __entry->mod_count = mod_count;
+		    __entry->where = where;
+			   ),
+
+	    TP_printk("s=%p %s u=%d m=%d p=%pSR",
+		      __entry->skb,
+		      (__entry->op == 0 ? "NEW" :
+		       __entry->op == 1 ? "SEE" :
+		       __entry->op == 2 ? "GET" :
+		       __entry->op == 3 ? "FRE" :
+		       "PUR"),
+		      __entry->usage,
+		      __entry->mod_count,
+		      __entry->where)
+	    );
+
+#endif /* _TRACE_RXRPC_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 88effadd4b16..c7cf356b42b8 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -22,6 +22,7 @@
 #include <net/net_namespace.h>
 #include <net/sock.h>
 #include <net/af_rxrpc.h>
+#define CREATE_TRACE_POINTS
 #include "ar-internal.h"
 
 MODULE_DESCRIPTION("RxRPC network protocol");
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 648060a5df35..8cb517fbbd23 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -479,6 +479,8 @@ static inline void rxrpc_abort_call(struct rxrpc_call *call, u32 abort_code)
 	write_unlock_bh(&call->state_lock);
 }
 
+#include <trace/events/rxrpc.h>
+
 /*
  * af_rxrpc.c
  */
@@ -752,6 +754,11 @@ int rxrpc_init_server_conn_security(struct rxrpc_connection *);
  * skbuff.c
  */
 void rxrpc_packet_destructor(struct sk_buff *);
+void rxrpc_new_skb(struct sk_buff *);
+void rxrpc_see_skb(struct sk_buff *);
+void rxrpc_get_skb(struct sk_buff *);
+void rxrpc_free_skb(struct sk_buff *);
+void rxrpc_purge_queue(struct sk_buff_head *);
 
 /*
  * sysctl.c
@@ -899,44 +906,6 @@ do {						\
 
 #endif /* __KDEBUGALL */
 
-/*
- * socket buffer accounting / leak finding
- */
-static inline void __rxrpc_new_skb(struct sk_buff *skb, const char *fn)
-{
-	//_net("new skb %p %s [%d]", skb, fn, atomic_read(&rxrpc_n_skbs));
-	//atomic_inc(&rxrpc_n_skbs);
-}
-
-#define rxrpc_new_skb(skb) __rxrpc_new_skb((skb), __func__)
-
-static inline void __rxrpc_kill_skb(struct sk_buff *skb, const char *fn)
-{
-	//_net("kill skb %p %s [%d]", skb, fn, atomic_read(&rxrpc_n_skbs));
-	//atomic_dec(&rxrpc_n_skbs);
-}
-
-#define rxrpc_kill_skb(skb) __rxrpc_kill_skb((skb), __func__)
-
-static inline void __rxrpc_free_skb(struct sk_buff *skb, const char *fn)
-{
-	if (skb) {
-		CHECK_SLAB_OKAY(&skb->users);
-		//_net("free skb %p %s [%d]",
-		//     skb, fn, atomic_read(&rxrpc_n_skbs));
-		//atomic_dec(&rxrpc_n_skbs);
-		kfree_skb(skb);
-	}
-}
-
-#define rxrpc_free_skb(skb) __rxrpc_free_skb((skb), __func__)
-
-static inline void rxrpc_purge_queue(struct sk_buff_head *list)
-{
-	struct sk_buff *skb;
-	while ((skb = skb_dequeue((list))) != NULL)
-		rxrpc_free_skb(skb);
-}
 
 #define rxrpc_get_call(CALL)				\
 do {							\
diff --git a/net/rxrpc/call_accept.c b/net/rxrpc/call_accept.c
index 9bae21e66d65..669ac79d3b44 100644
--- a/net/rxrpc/call_accept.c
+++ b/net/rxrpc/call_accept.c
@@ -203,6 +203,7 @@ void rxrpc_accept_incoming_calls(struct rxrpc_local *local)
 
 	_net("incoming call skb %p", skb);
 
+	rxrpc_see_skb(skb);
 	sp = rxrpc_skb(skb);
 
 	/* Set up a response packet header in case we need it */
diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index eaa8035dcb71..3d1267cea9ea 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -407,6 +407,7 @@ static int rxrpc_drain_rx_oos_queue(struct rxrpc_call *call)
 
 	skb = skb_dequeue(&call->rx_oos_queue);
 	if (skb) {
+		rxrpc_see_skb(skb);
 		sp = rxrpc_skb(skb);
 
 		_debug("drain OOS packet %d [%d]",
@@ -427,6 +428,7 @@ static int rxrpc_drain_rx_oos_queue(struct rxrpc_call *call)
 
 			/* find out what the next packet is */
 			skb = skb_peek(&call->rx_oos_queue);
+			rxrpc_see_skb(skb);
 			if (skb)
 				call->rx_first_oos = rxrpc_skb(skb)->hdr.seq;
 			else
@@ -576,6 +578,7 @@ process_further:
 	if (!skb)
 		return -EAGAIN;
 
+	rxrpc_see_skb(skb);
 	_net("deferred skb %p", skb);
 
 	sp = rxrpc_skb(skb);
diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index cee0f35bc1cf..c631d926f4db 100644
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -277,6 +277,7 @@ void rxrpc_process_connection(struct work_struct *work)
 	/* go through the conn-level event packets, releasing the ref on this
 	 * connection that each one has when we've finished with it */
 	while ((skb = skb_dequeue(&conn->rx_queue))) {
+		rxrpc_see_skb(skb);
 		ret = rxrpc_process_event(conn, skb, &abort_code);
 		switch (ret) {
 		case -EPROTO:
@@ -365,6 +366,7 @@ void rxrpc_reject_packets(struct rxrpc_local *local)
 	whdr.type = RXRPC_PACKET_TYPE_ABORT;
 
 	while ((skb = skb_dequeue(&local->reject_queue))) {
+		rxrpc_see_skb(skb);
 		sp = rxrpc_skb(skb);
 		switch (sa.sa.sa_family) {
 		case AF_INET:
diff --git a/net/rxrpc/local_event.c b/net/rxrpc/local_event.c
index 31a3f86ef2f6..bcc6593b4cdb 100644
--- a/net/rxrpc/local_event.c
+++ b/net/rxrpc/local_event.c
@@ -93,6 +93,7 @@ void rxrpc_process_local_events(struct rxrpc_local *local)
 	if (skb) {
 		struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
 
+		rxrpc_see_skb(skb);
 		_debug("{%d},{%u}", local->debug_id, sp->hdr.type);
 
 		switch (sp->hdr.type) {
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index 9e626f1e2668..e3a08d542fb7 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -548,6 +548,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 
 	skb = call->tx_pending;
 	call->tx_pending = NULL;
+	rxrpc_see_skb(skb);
 
 	copied = 0;
 	do {
diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index 9ed66d533002..b964c2d49a88 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -111,6 +111,7 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		}
 
 	peek_next_packet:
+		rxrpc_see_skb(skb);
 		sp = rxrpc_skb(skb);
 		call = sp->call;
 		ASSERT(call != NULL);
diff --git a/net/rxrpc/skbuff.c b/net/rxrpc/skbuff.c
index 06c51d4b622d..d28058a97bc1 100644
--- a/net/rxrpc/skbuff.c
+++ b/net/rxrpc/skbuff.c
@@ -163,3 +163,65 @@ void rxrpc_kernel_free_skb(struct sk_buff *skb)
 	rxrpc_free_skb(skb);
 }
 EXPORT_SYMBOL(rxrpc_kernel_free_skb);
+
+/*
+ * Note the existence of a new-to-us socket buffer (allocated or dequeued).
+ */
+void rxrpc_new_skb(struct sk_buff *skb)
+{
+	const void *here = __builtin_return_address(0);
+	int n = atomic_inc_return(&rxrpc_n_skbs);
+	trace_rxrpc_skb(skb, 0, atomic_read(&skb->users), n, here);
+}
+
+/*
+ * Note the re-emergence of a socket buffer from a queue or buffer.
+ */
+void rxrpc_see_skb(struct sk_buff *skb)
+{
+	const void *here = __builtin_return_address(0);
+	if (skb) {
+		int n = atomic_read(&rxrpc_n_skbs);
+		trace_rxrpc_skb(skb, 1, atomic_read(&skb->users), n, here);
+	}
+}
+
+/*
+ * Note the addition of a ref on a socket buffer.
+ */
+void rxrpc_get_skb(struct sk_buff *skb)
+{
+	const void *here = __builtin_return_address(0);
+	int n = atomic_inc_return(&rxrpc_n_skbs);
+	trace_rxrpc_skb(skb, 2, atomic_read(&skb->users), n, here);
+	skb_get(skb);
+}
+
+/*
+ * Note the destruction of a socket buffer.
+ */
+void rxrpc_free_skb(struct sk_buff *skb)
+{
+	const void *here = __builtin_return_address(0);
+	if (skb) {
+		int n;
+		CHECK_SLAB_OKAY(&skb->users);
+		n = atomic_dec_return(&rxrpc_n_skbs);
+		trace_rxrpc_skb(skb, 3, atomic_read(&skb->users), n, here);
+		kfree_skb(skb);
+	}
+}
+
+/*
+ * Clear a queue of socket buffers.
+ */
+void rxrpc_purge_queue(struct sk_buff_head *list)
+{
+	const void *here = __builtin_return_address(0);
+	struct sk_buff *skb;
+	while ((skb = skb_dequeue((list))) != NULL) {
+		int n = atomic_dec_return(&rxrpc_n_skbs);
+		trace_rxrpc_skb(skb, 4, atomic_read(&skb->users), n, here);
+		kfree_skb(skb);
+	}
+}

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next 0/5] rxrpc: Cleanups
  2016-08-23 15:26 [PATCH net-next 0/5] rxrpc: Cleanups David Howells
                   ` (4 preceding siblings ...)
  2016-08-23 15:26 ` [PATCH net-next 5/5] rxrpc: Use a tracepoint for skb accounting debugging David Howells
@ 2016-08-24  0:21 ` David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2016-08-24  0:21 UTC (permalink / raw)
  To: dhowells; +Cc: netdev, linux-afs, linux-kernel

From: David Howells <dhowells@redhat.com>
Date: Tue, 23 Aug 2016 16:26:18 +0100

> Here are some cleanups for the AF_RXRPC rewrite:
...
> 	git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
> 	rxrpc-rewrite-20160823-1

Pulled.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-08-24  0:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-23 15:26 [PATCH net-next 0/5] rxrpc: Cleanups David Howells
2016-08-23 15:26 ` [PATCH net-next 1/5] rxrpc: Remove RXRPC_CALL_PROC_BUSY David Howells
2016-08-23 15:26 ` [PATCH net-next 2/5] rxrpc: Tidy up the rxrpc_call struct a bit David Howells
2016-08-23 15:26 ` [PATCH net-next 3/5] rxrpc: When clearing a socket, clear the call sets in the right order David Howells
2016-08-23 15:26 ` [PATCH net-next 4/5] rxrpc: Drop channel number field from rxrpc_call struct David Howells
2016-08-23 15:26 ` [PATCH net-next 5/5] rxrpc: Use a tracepoint for skb accounting debugging David Howells
2016-08-24  0:21 ` [PATCH net-next 0/5] rxrpc: Cleanups David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).