Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1373518 > unrolled thread

[PATCH 0/9] RxRPC: 2nd rewrite part 1

Started byDavid Howells <dhowells@redhat.com>
First post2016-04-07 18:30 +0200
Last post2016-04-12 03:30 +0200
Articles 20 on this page of 24 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/9] RxRPC: 2nd rewrite part 1 David Howells <dhowells@redhat.com> - 2016-04-07 18:30 +0200
    [PATCH 6/9] rxrpc: Don't pass gfp around in incoming call handling  functions David Howells <dhowells@redhat.com> - 2016-04-07 18:30 +0200
    [PATCH 2/9] rxrpc: Disable a debugging statement that has been left  enabled. David Howells <dhowells@redhat.com> - 2016-04-07 18:30 +0200
      Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been  left enabled. Joe Perches <joe@perches.com> - 2016-04-07 18:40 +0200
        Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled. David Howells <dhowells@redhat.com> - 2016-04-07 19:10 +0200
          Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been  left enabled. Joe Perches <joe@perches.com> - 2016-04-07 19:20 +0200
            Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled. David Howells <dhowells@redhat.com> - 2016-04-07 21:50 +0200
              Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been  left enabled. Joe Perches <joe@perches.com> - 2016-04-07 22:30 +0200
                Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled. David Howells <dhowells@redhat.com> - 2016-04-07 22:50 +0200
        Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been  left enabled. David Miller <davem@davemloft.net> - 2016-04-07 19:40 +0200
          Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled. David Howells <dhowells@redhat.com> - 2016-04-07 21:50 +0200
            Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been  left enabled. David Miller <davem@davemloft.net> - 2016-04-07 22:40 +0200
              Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled. David Howells <dhowells@redhat.com> - 2016-04-07 22:50 +0200
                Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been  left enabled. David Miller <davem@davemloft.net> - 2016-04-07 23:00 +0200
                  Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled. David Howells <dhowells@redhat.com> - 2016-04-07 23:30 +0200
    [PATCH 4/9] rxrpc: Static arrays of strings should be const char  *const[] David Howells <dhowells@redhat.com> - 2016-04-07 18:30 +0200
    [PATCH 5/9] rxrpc: Differentiate local and remote abort codes in  structs David Howells <dhowells@redhat.com> - 2016-04-07 18:30 +0200
    [PATCH 1/9] afs: Wait for outstanding async calls before closing  rxrpc socket David Howells <dhowells@redhat.com> - 2016-04-07 18:30 +0200
    [PATCH 9/9] rxrpc: Create a null security type and get rid of  conditional calls David Howells <dhowells@redhat.com> - 2016-04-07 18:30 +0200
    Re: [PATCH 0/9] RxRPC: 2nd rewrite part 1 David Miller <davem@davemloft.net> - 2016-04-11 21:40 +0200
      Re: [PATCH 0/9] RxRPC: 2nd rewrite part 1 David Howells <dhowells@redhat.com> - 2016-04-11 22:40 +0200
      Re: [PATCH 0/9] RxRPC: 2nd rewrite part 1 David Howells <dhowells@redhat.com> - 2016-04-12 00:10 +0200
        Re: [PATCH 0/9] RxRPC: 2nd rewrite part 1 David Howells <dhowells@redhat.com> - 2016-04-12 00:40 +0200
        Re: [PATCH 0/9] RxRPC: 2nd rewrite part 1 David Miller <davem@davemloft.net> - 2016-04-12 03:30 +0200

Page 1 of 2  [1] 2  Next page →


#1373518 — [PATCH 0/9] RxRPC: 2nd rewrite part 1

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 18:30 +0200
Subject[PATCH 0/9] RxRPC: 2nd rewrite part 1
Message-ID<rlkYG-6F9-9@gated-at.bofh.it>
Okay, I'm in the process of rewriting the RxRPC rewrite.  The primary aim of
this second rewrite is to strictly control the number of active connections we
know about and to get rid of connections we don't need much more quickly.

On top of this, there are fixes to the protocol handling which will all occur
in later parts.

Here's the first set of patches from the second go, aimed at net-next.  These
are all fixes and cleanups preparatory to the main event.

Notable parts of this set include:

 (1) A fix for the AFS filesystem to wait for outstanding calls to complete
     before closing the RxRPC socket.

 (2) Differentiation of local and remote abort codes.  At a future point
     userspace will get to see this via control message data on recvmsg().

 (3) Absorb the rxkad module into the af_rxrpc module to prevent a dependency
     loop.

 (4) Create a null security module and unconditionalise calls into the
     security module that's in force (there will always be a security module
     applied to a connection, even if it's just the null one).


The patches can be found here also:

	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-20160407

This is based on net-next/master

David
---
David Howells (9):
      afs: Wait for outstanding async calls before closing rxrpc socket
      rxrpc: Disable a debugging statement that has been left enabled.
      rxrpc: Move some miscellaneous bits out into their own file
      rxrpc: Static arrays of strings should be const char *const[]
      rxrpc: Differentiate local and remote abort codes in structs
      rxrpc: Don't pass gfp around in incoming call handling functions
      rxrpc: Don't assume transport address family and size when using it
      rxrpc: Absorb the rxkad security module
      rxrpc: Create a null security type and get rid of conditional calls


 fs/afs/rxrpc.c            |   30 +++++++-
 include/net/af_rxrpc.h    |    4 +
 include/rxrpc/packet.h    |    2 -
 net/rxrpc/Kconfig         |    2 -
 net/rxrpc/Makefile        |    7 +-
 net/rxrpc/af_rxrpc.c      |    9 ++
 net/rxrpc/ar-accept.c     |    4 +
 net/rxrpc/ar-ack.c        |   81 ++--------------------
 net/rxrpc/ar-call.c       |   11 +--
 net/rxrpc/ar-connection.c |   14 ++--
 net/rxrpc/ar-connevent.c  |   27 +++----
 net/rxrpc/ar-input.c      |   14 +---
 net/rxrpc/ar-internal.h   |   70 ++++++++++++-------
 net/rxrpc/ar-output.c     |    6 +-
 net/rxrpc/ar-proc.c       |    2 -
 net/rxrpc/ar-recvmsg.c    |   18 ++++-
 net/rxrpc/ar-security.c   |  166 +++++++++------------------------------------
 net/rxrpc/insecure.c      |   83 +++++++++++++++++++++++
 net/rxrpc/misc.c          |   89 ++++++++++++++++++++++++
 net/rxrpc/rxkad.c         |   61 +++++++----------
 20 files changed, 374 insertions(+), 326 deletions(-)
 create mode 100644 net/rxrpc/insecure.c
 create mode 100644 net/rxrpc/misc.c

[toc] | [next] | [standalone]


#1373522 — [PATCH 6/9] rxrpc: Don't pass gfp around in incoming call handling functions

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 18:30 +0200
Subject[PATCH 6/9] rxrpc: Don't pass gfp around in incoming call handling functions
Message-ID<rlkYG-6F9-21@gated-at.bofh.it>
In reply to#1373518
Don't pass gfp around in incoming call handling functions, but rather hard
code it at the points where we actually need it since the value comes from
within the rxrpc driver and is always the same.

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

 net/rxrpc/ar-accept.c     |    4 ++--
 net/rxrpc/ar-call.c       |    7 +++----
 net/rxrpc/ar-connection.c |    5 ++---
 net/rxrpc/ar-internal.h   |    5 ++---
 4 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/net/rxrpc/ar-accept.c b/net/rxrpc/ar-accept.c
index 277731a5e67a..e7a7f05f13e2 100644
--- a/net/rxrpc/ar-accept.c
+++ b/net/rxrpc/ar-accept.c
@@ -108,7 +108,7 @@ static int rxrpc_accept_incoming_call(struct rxrpc_local *local,
 		goto error;
 	}
 
-	conn = rxrpc_incoming_connection(trans, &sp->hdr, GFP_NOIO);
+	conn = rxrpc_incoming_connection(trans, &sp->hdr);
 	rxrpc_put_transport(trans);
 	if (IS_ERR(conn)) {
 		_debug("no conn");
@@ -116,7 +116,7 @@ static int rxrpc_accept_incoming_call(struct rxrpc_local *local,
 		goto error;
 	}
 
-	call = rxrpc_incoming_call(rx, conn, &sp->hdr, GFP_NOIO);
+	call = rxrpc_incoming_call(rx, conn, &sp->hdr);
 	rxrpc_put_connection(conn);
 	if (IS_ERR(call)) {
 		_debug("no call");
diff --git a/net/rxrpc/ar-call.c b/net/rxrpc/ar-call.c
index 67a211f0ebba..571a41fd5a32 100644
--- a/net/rxrpc/ar-call.c
+++ b/net/rxrpc/ar-call.c
@@ -411,18 +411,17 @@ found_extant_second:
  */
 struct rxrpc_call *rxrpc_incoming_call(struct rxrpc_sock *rx,
 				       struct rxrpc_connection *conn,
-				       struct rxrpc_host_header *hdr,
-				       gfp_t gfp)
+				       struct rxrpc_host_header *hdr)
 {
 	struct rxrpc_call *call, *candidate;
 	struct rb_node **p, *parent;
 	u32 call_id;
 
-	_enter(",%d,,%x", conn->debug_id, gfp);
+	_enter(",%d", conn->debug_id);
 
 	ASSERT(rx != NULL);
 
-	candidate = rxrpc_alloc_call(gfp);
+	candidate = rxrpc_alloc_call(GFP_NOIO);
 	if (!candidate)
 		return ERR_PTR(-EBUSY);
 
diff --git a/net/rxrpc/ar-connection.c b/net/rxrpc/ar-connection.c
index 9942da1edbf6..9b6966777633 100644
--- a/net/rxrpc/ar-connection.c
+++ b/net/rxrpc/ar-connection.c
@@ -619,8 +619,7 @@ interrupted:
  */
 struct rxrpc_connection *
 rxrpc_incoming_connection(struct rxrpc_transport *trans,
-			  struct rxrpc_host_header *hdr,
-			  gfp_t gfp)
+			  struct rxrpc_host_header *hdr)
 {
 	struct rxrpc_connection *conn, *candidate = NULL;
 	struct rb_node *p, **pp;
@@ -659,7 +658,7 @@ rxrpc_incoming_connection(struct rxrpc_transport *trans,
 
 	/* not yet present - create a candidate for a new record and then
 	 * redo the search */
-	candidate = rxrpc_alloc_connection(gfp);
+	candidate = rxrpc_alloc_connection(GFP_NOIO);
 	if (!candidate) {
 		_leave(" = -ENOMEM");
 		return ERR_PTR(-ENOMEM);
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 258b74a2a23f..d38071b09f72 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -503,7 +503,7 @@ struct rxrpc_call *rxrpc_get_client_call(struct rxrpc_sock *,
 					 unsigned long, int, gfp_t);
 struct rxrpc_call *rxrpc_incoming_call(struct rxrpc_sock *,
 				       struct rxrpc_connection *,
-				       struct rxrpc_host_header *, gfp_t);
+				       struct rxrpc_host_header *);
 struct rxrpc_call *rxrpc_find_server_call(struct rxrpc_sock *, unsigned long);
 void rxrpc_release_call(struct rxrpc_call *);
 void rxrpc_release_calls_on_socket(struct rxrpc_sock *);
@@ -528,8 +528,7 @@ void __exit rxrpc_destroy_all_connections(void);
 struct rxrpc_connection *rxrpc_find_connection(struct rxrpc_transport *,
 					       struct rxrpc_host_header *);
 extern struct rxrpc_connection *
-rxrpc_incoming_connection(struct rxrpc_transport *, struct rxrpc_host_header *,
-			  gfp_t);
+rxrpc_incoming_connection(struct rxrpc_transport *, struct rxrpc_host_header *);
 
 /*
  * ar-connevent.c

[toc] | [prev] | [next] | [standalone]


#1373525 — [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 18:30 +0200
Subject[PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rlkYH-6F9-43@gated-at.bofh.it>
In reply to#1373518
Disable a debugging statement that has been left enabled

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

 net/rxrpc/ar-ack.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index 16d967075eaf..01a017a05f14 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -426,7 +426,7 @@ static void rxrpc_rotate_tx_window(struct rxrpc_call *call, u32 hard)
 	int tail = call->acks_tail, old_tail;
 	int win = CIRC_CNT(call->acks_head, tail, call->acks_winsz);
 
-	kenter("{%u,%u},%u", call->acks_hard, win, hard);
+	_enter("{%u,%u},%u", call->acks_hard, win, hard);
 
 	ASSERTCMP(hard - call->acks_hard, <=, win);
 

[toc] | [prev] | [next] | [standalone]


#1373540 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromJoe Perches <joe@perches.com>
Date2016-04-07 18:40 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rll8m-6JT-27@gated-at.bofh.it>
In reply to#1373525
On Thu, 2016-04-07 at 17:23 +0100, David Howells wrote:
> Disable a debugging statement that has been left enabled
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  net/rxrpc/ar-ack.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
> index 16d967075eaf..01a017a05f14 100644
> --- a/net/rxrpc/ar-ack.c
> +++ b/net/rxrpc/ar-ack.c
> @@ -426,7 +426,7 @@ static void rxrpc_rotate_tx_window(struct rxrpc_call *call, u32 hard)
>  	int tail = call->acks_tail, old_tail;
>  	int win = CIRC_CNT(call->acks_head, tail, call->acks_winsz);
>  
> -	kenter("{%u,%u},%u", call->acks_hard, win, hard);
> +	_enter("{%u,%u},%u", call->acks_hard, win, hard);
>  
>  	ASSERTCMP(hard - call->acks_hard, <=, win);

It might be better to remove kenter and _enter
altogether and use function tracing instead.

[toc] | [prev] | [next] | [standalone]


#1373578 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 19:10 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rllBn-7dK-11@gated-at.bofh.it>
In reply to#1373540
Joe Perches <joe@perches.com> wrote:

> It might be better to remove kenter and _enter
> altogether and use function tracing instead.

Possibly - but not at this time.  Besides, isn't the function tracing log lost
if the machine crashes?

David

[toc] | [prev] | [next] | [standalone]


#1373583 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromJoe Perches <joe@perches.com>
Date2016-04-07 19:20 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rllL4-7hx-5@gated-at.bofh.it>
In reply to#1373578
On Thu, 2016-04-07 at 18:05 +0100, David Howells wrote:
> Joe Perches <joe@perches.com> wrote:
> > It might be better to remove kenter and _enter
> > altogether and use function tracing instead.
> Possibly - but not at this time.

Swell.

> Besides, isn't the function tracing log lost
> if the machine crashes?

I believe yes, but would it matter?

[toc] | [prev] | [next] | [standalone]


#1373711 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 21:50 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rlo6e-o4-21@gated-at.bofh.it>
In reply to#1373583
Joe Perches <joe@perches.com> wrote:

> > Joe Perches <joe@perches.com> wrote:
> > > It might be better to remove kenter and _enter
> > > altogether and use function tracing instead.
> > Possibly - but not at this time.
> 
> Swell.

I didn't say I wouldn't do it - it's just that I'm trying to fix other stuff
at the moment and don't particularly want to add that to the list just now.
kenter, _enter and co. are serving me very well.

> > Besides, isn't the function tracing log lost
> > if the machine crashes?
> 
> I believe yes, but would it matter?

Let's see...  If the machine panics whilst I'm developing stuff (quite likely
if something goes wrong in BH context), how do I get at the function tracing
log to find out why it panicked if the log is then lost?  With the serial
console, at least I automatically capture the output of kenter and co..

David

[toc] | [prev] | [next] | [standalone]


#1373723 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromJoe Perches <joe@perches.com>
Date2016-04-07 22:30 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rloIW-Ti-17@gated-at.bofh.it>
In reply to#1373711
On Thu, 2016-04-07 at 20:43 +0100, David Howells wrote:
> Joe Perches <joe@perches.com> wrote:
> > > Joe Perches <joe@perches.com> wrote:
> > > > It might be better to remove kenter and _enter
> > > > altogether and use function tracing instead.
> > > Possibly - but not at this time.
> > Swell.
> I didn't say I wouldn't do it - it's just that I'm trying to fix other stuff
> at the moment and don't particularly want to add that to the list just now.
> kenter, _enter and co. are serving me very well.

No worries, I didn't mean to imply you wouldn't.
It's your code, do what and when you want.

> > > Besides, isn't the function tracing log lost
> > > if the machine crashes?
> > I believe yes, but would it matter?
> Let's see...  If the machine panics whilst I'm developing stuff (quite likely
> if something goes wrong in BH context), how do I get at the function tracing
> log to find out why it panicked if the log is then lost?  With the serial
> console, at least I automatically capture the output of kenter and co..

But you also don't get a bunch of other useful stuff.

btw: there is ftrace_dump_on_oops

https://lwn.net/Articles/366796/

[toc] | [prev] | [next] | [standalone]


#1373744 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 22:50 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rlp2i-13g-15@gated-at.bofh.it>
In reply to#1373723
Joe Perches <joe@perches.com> wrote:

> > Let's see...  If the machine panics whilst I'm developing stuff (quite
> > likely if something goes wrong in BH context), how do I get at the
> > function tracing log to find out why it panicked if the log is then
> > lost?  With the serial console, at least I automatically capture the
> > output of kenter and co..
> 
> But you also don't get a bunch of other useful stuff.
> 
> btw: there is ftrace_dump_on_oops
> 
> https://lwn.net/Articles/366796/

Cute.

How do I get at the argument values and suchlike that kenter() and co print?
I presume I would need to replace any kenter() or suchlike that printed more
than just the address with a tracing hook?

David

[toc] | [prev] | [next] | [standalone]


#1373595 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromDavid Miller <davem@davemloft.net>
Date2016-04-07 19:40 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rlm4r-7rS-25@gated-at.bofh.it>
In reply to#1373540
From: Joe Perches <joe@perches.com>
Date: Thu, 07 Apr 2016 09:31:17 -0700

> On Thu, 2016-04-07 at 17:23 +0100, David Howells wrote:
>> Disable a debugging statement that has been left enabled
>> 
>> Signed-off-by: David Howells <dhowells@redhat.com>
>> ---
>> 
>>  net/rxrpc/ar-ack.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
>> index 16d967075eaf..01a017a05f14 100644
>> --- a/net/rxrpc/ar-ack.c
>> +++ b/net/rxrpc/ar-ack.c
>> @@ -426,7 +426,7 @@ static void rxrpc_rotate_tx_window(struct rxrpc_call *call, u32 hard)
>>  	int tail = call->acks_tail, old_tail;
>>  	int win = CIRC_CNT(call->acks_head, tail, call->acks_winsz);
>>  
>> -	kenter("{%u,%u},%u", call->acks_hard, win, hard);
>> +	_enter("{%u,%u},%u", call->acks_hard, win, hard);
>>  
>>  	ASSERTCMP(hard - call->acks_hard, <=, win);
> 
> It might be better to remove kenter and _enter
> altogether and use function tracing instead.

Yeah this custom stuff is really inappropriate given the excellent
infrastructure we have these days...

[toc] | [prev] | [next] | [standalone]


#1373707 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 21:50 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rlo6e-o4-5@gated-at.bofh.it>
In reply to#1373595
David Miller <davem@davemloft.net> wrote:

> Yeah this custom stuff is really inappropriate given the excellent
> infrastructure we have these days...

Excellent only if I can get at it to find out why the something went wrong.
If it's lost because the machine panics, then it is worthless.

David

[toc] | [prev] | [next] | [standalone]


#1373729 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromDavid Miller <davem@davemloft.net>
Date2016-04-07 22:40 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rloSC-YH-21@gated-at.bofh.it>
In reply to#1373707
From: David Howells <dhowells@redhat.com>
Date: Thu, 07 Apr 2016 20:45:08 +0100

> David Miller <davem@davemloft.net> wrote:
> 
>> Yeah this custom stuff is really inappropriate given the excellent
>> infrastructure we have these days...
> 
> Excellent only if I can get at it to find out why the something went wrong.
> If it's lost because the machine panics, then it is worthless.

If you're ok with these kenter things spewing into the logs with the
current facility, you can run the function tracer and have it record
into the logs all the time too.

I don't see any argument which is appropriate for keeping this stuff
around.

[toc] | [prev] | [next] | [standalone]


#1373746 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 22:50 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rlp2j-13g-27@gated-at.bofh.it>
In reply to#1373729
David Miller <davem@davemloft.net> wrote:

> > Excellent only if I can get at it to find out why the something went wrong.
> > If it's lost because the machine panics, then it is worthless.
> 
> If you're ok with these kenter things spewing into the logs with the
> current facility, you can run the function tracer and have it record
> into the logs all the time too.

They don't do that unless you enable them.  And I can enable them individually
at compile time or by set at runtime.

> I don't see any argument which is appropriate for keeping this stuff
> around.

Sigh.

As I said to Joe: I don't want to deal with converting to using ftrace at the
moment whilst I'm towards the end of reworking the rewrite of the code.  I
have a stack of patches that would need reworking yet again.

David

[toc] | [prev] | [next] | [standalone]


#1373757 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromDavid Miller <davem@davemloft.net>
Date2016-04-07 23:00 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rlpbY-16R-15@gated-at.bofh.it>
In reply to#1373746
From: David Howells <dhowells@redhat.com>
Date: Thu, 07 Apr 2016 21:47:19 +0100

> David Miller <davem@davemloft.net> wrote:
> 
>> > Excellent only if I can get at it to find out why the something went wrong.
>> > If it's lost because the machine panics, then it is worthless.
>> 
>> If you're ok with these kenter things spewing into the logs with the
>> current facility, you can run the function tracer and have it record
>> into the logs all the time too.
> 
> They don't do that unless you enable them.  And I can enable them individually
> at compile time or by set at runtime.

As you can with the function tracer and tracepoints.

We're not telling you to kill this now, we're just saying that you should
thinking about tossing this custom stuff when you have a chance.

[toc] | [prev] | [next] | [standalone]


#1373790 — Re: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 23:30 +0200
SubjectRe: [PATCH 2/9] rxrpc: Disable a debugging statement that has been left enabled.
Message-ID<rlpF0-1zn-17@gated-at.bofh.it>
In reply to#1373757
David Miller <davem@davemloft.net> wrote:

> As you can with the function tracer and tracepoints.

I've had experience with tracepoints before (i2c and smbus).  It wasn't
particularly fun.  There's got to be some easier way to write them.

Hmmm...

Of the _enter() and _leave() macros in my tree at the moment, I have 261 that
record more information than just the name of the function.  I presume each
would need to be converted to a TRACE_EVENT*() macro as, as far as I can see
with a cursory examination, the function tracer doesn't record arguments or the
things arguments point to (which I do a lot of).  There are also another 84 of
these macros that only record the name of the function which are simpler
propositions.

Add to that 286 _debug(), _net() and _proto() macros, all of which record more
information than just the name of the calling function and don't have anything
particularly to do with function trace.

Now, I have been considering that the _net() and _proto() macros would probably
work well as TRACE_EVENT()-type constructs, allowing the tracing of protocol
flow.

I don't suppose you have a script for automatically converting something like
a printk()-type thing into a TRACE_EVENT()?

David

[toc] | [prev] | [next] | [standalone]


#1373528 — [PATCH 4/9] rxrpc: Static arrays of strings should be const char *const[]

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 18:30 +0200
Subject[PATCH 4/9] rxrpc: Static arrays of strings should be const char *const[]
Message-ID<rlkYH-6F9-55@gated-at.bofh.it>
In reply to#1373518
Static arrays of strings should be const char *const[].

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

 include/rxrpc/packet.h  |    2 --
 net/rxrpc/ar-internal.h |    2 +-
 net/rxrpc/misc.c        |    2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/rxrpc/packet.h b/include/rxrpc/packet.h
index 9ebab3a8cf0a..b2017440b765 100644
--- a/include/rxrpc/packet.h
+++ b/include/rxrpc/packet.h
@@ -68,8 +68,6 @@ struct rxrpc_wire_header {
 
 } __packed;
 
-extern const char *rxrpc_pkts[];
-
 #define RXRPC_SUPPORTED_PACKET_TYPES (			\
 		(1 << RXRPC_PACKET_TYPE_DATA) |		\
 		(1 << RXRPC_PACKET_TYPE_ACK) |		\
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 24126d954f38..eeb829e837e1 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -637,7 +637,7 @@ extern unsigned int rxrpc_rx_window_size;
 extern unsigned int rxrpc_rx_mtu;
 extern unsigned int rxrpc_rx_jumbo_max;
 
-extern const char *rxrpc_pkts[];
+extern const char *const rxrpc_pkts[];
 extern const s8 rxrpc_ack_priority[];
 
 extern const char *rxrpc_acks(u8 reason);
diff --git a/net/rxrpc/misc.c b/net/rxrpc/misc.c
index 8ebeec3384e1..1afe9876e79f 100644
--- a/net/rxrpc/misc.c
+++ b/net/rxrpc/misc.c
@@ -58,7 +58,7 @@ unsigned int rxrpc_rx_mtu = 5692;
  */
 unsigned int rxrpc_rx_jumbo_max = 4;
 
-const char *rxrpc_pkts[] = {
+const char *const rxrpc_pkts[] = {
 	"?00",
 	"DATA", "ACK", "BUSY", "ABORT", "ACKALL", "CHALL", "RESP", "DEBUG",
 	"?09", "?10", "?11", "?12", "VERSION", "?14", "?15"

[toc] | [prev] | [next] | [standalone]


#1373530 — [PATCH 5/9] rxrpc: Differentiate local and remote abort codes in structs

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 18:30 +0200
Subject[PATCH 5/9] rxrpc: Differentiate local and remote abort codes in structs
Message-ID<rlkYH-6F9-61@gated-at.bofh.it>
In reply to#1373518
In the rxrpc_connection and rxrpc_call structs, there's one field to hold
the abort code, no matter whether that value was generated locally to be
sent or was received from the peer via an abort packet.

Split the abort code fields in two for cleanliness sake and add an error
field to hold the Linux error number to the rxrpc_call struct too
(sometimes this is generated in a context where we can't return it to
userspace directly).

Furthermore, add a skb mark to indicate a packet that caused a local abort
to be generated so that recvmsg() can pick up the correct abort code.  A
future addition will need to be to indicate to userspace the difference
between aborts via a control message.

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

 fs/afs/rxrpc.c           |   14 +++++++++++---
 include/net/af_rxrpc.h   |    3 ++-
 net/rxrpc/ar-ack.c       |    4 ++--
 net/rxrpc/ar-call.c      |    4 ++--
 net/rxrpc/ar-connevent.c |   12 +++++++-----
 net/rxrpc/ar-input.c     |    6 +++---
 net/rxrpc/ar-internal.h  |   10 +++++++---
 net/rxrpc/ar-output.c    |    2 +-
 net/rxrpc/ar-proc.c      |    2 +-
 net/rxrpc/ar-recvmsg.c   |   18 ++++++++++++++----
 10 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index 7c2567a711ea..e24ffa9ef166 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -430,9 +430,11 @@ error_kill_call:
 }
 
 /*
- * handles intercepted messages that were arriving in the socket's Rx queue
- * - called with the socket receive queue lock held to ensure message ordering
- * - called with softirqs disabled
+ * Handles intercepted messages that were arriving in the socket's Rx queue.
+ *
+ * Called from the AF_RXRPC call processor in waitqueue process context.  For
+ * each call, it is guaranteed this will be called in order of packet to be
+ * delivered.
  */
 static void afs_rx_interceptor(struct sock *sk, unsigned long user_call_ID,
 			       struct sk_buff *skb)
@@ -523,6 +525,12 @@ static void afs_deliver_to_call(struct afs_call *call)
 			call->state = AFS_CALL_ABORTED;
 			_debug("Rcv ABORT %u -> %d", abort_code, call->error);
 			break;
+		case RXRPC_SKB_MARK_LOCAL_ABORT:
+			abort_code = rxrpc_kernel_get_abort_code(skb);
+			call->error = call->type->abort_to_error(abort_code);
+			call->state = AFS_CALL_ABORTED;
+			_debug("Loc ABORT %u -> %d", abort_code, call->error);
+			break;
 		case RXRPC_SKB_MARK_NET_ERROR:
 			call->error = -rxrpc_kernel_get_error_number(skb);
 			call->state = AFS_CALL_ERROR;
diff --git a/include/net/af_rxrpc.h b/include/net/af_rxrpc.h
index 4fd3e4a2cadd..ac1bc3c49fbd 100644
--- a/include/net/af_rxrpc.h
+++ b/include/net/af_rxrpc.h
@@ -20,11 +20,12 @@ struct rxrpc_call;
 /*
  * the mark applied to socket buffers that may be intercepted
  */
-enum {
+enum rxrpc_skb_mark {
 	RXRPC_SKB_MARK_DATA,		/* data message */
 	RXRPC_SKB_MARK_FINAL_ACK,	/* final ACK received message */
 	RXRPC_SKB_MARK_BUSY,		/* server busy message */
 	RXRPC_SKB_MARK_REMOTE_ABORT,	/* remote abort message */
+	RXRPC_SKB_MARK_LOCAL_ABORT,	/* local abort message */
 	RXRPC_SKB_MARK_NET_ERROR,	/* network error message */
 	RXRPC_SKB_MARK_LOCAL_ERROR,	/* local error message */
 	RXRPC_SKB_MARK_NEW_CALL,	/* local error message */
diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index 54bf43ba9aa8..d0eb98e1391c 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -905,7 +905,7 @@ void rxrpc_process_call(struct work_struct *work)
 				       ECONNABORTED, true) < 0)
 			goto no_mem;
 		whdr.type = RXRPC_PACKET_TYPE_ABORT;
-		data = htonl(call->abort_code);
+		data = htonl(call->local_abort);
 		iov[1].iov_base = &data;
 		iov[1].iov_len = sizeof(data);
 		genbit = RXRPC_CALL_EV_ABORT;
@@ -968,7 +968,7 @@ void rxrpc_process_call(struct work_struct *work)
 		write_lock_bh(&call->state_lock);
 		if (call->state <= RXRPC_CALL_COMPLETE) {
 			call->state = RXRPC_CALL_LOCALLY_ABORTED;
-			call->abort_code = RX_CALL_TIMEOUT;
+			call->local_abort = RX_CALL_TIMEOUT;
 			set_bit(RXRPC_CALL_EV_ABORT, &call->events);
 		}
 		write_unlock_bh(&call->state_lock);
diff --git a/net/rxrpc/ar-call.c b/net/rxrpc/ar-call.c
index 7c8d300ade9b..67a211f0ebba 100644
--- a/net/rxrpc/ar-call.c
+++ b/net/rxrpc/ar-call.c
@@ -682,7 +682,7 @@ void rxrpc_release_call(struct rxrpc_call *call)
 	    call->state != RXRPC_CALL_CLIENT_FINAL_ACK) {
 		_debug("+++ ABORTING STATE %d +++\n", call->state);
 		call->state = RXRPC_CALL_LOCALLY_ABORTED;
-		call->abort_code = RX_CALL_DEAD;
+		call->local_abort = RX_CALL_DEAD;
 		set_bit(RXRPC_CALL_EV_ABORT, &call->events);
 		rxrpc_queue_call(call);
 	}
@@ -758,7 +758,7 @@ static void rxrpc_mark_call_released(struct rxrpc_call *call)
 		if (call->state < RXRPC_CALL_COMPLETE) {
 			_debug("abort call %p", call);
 			call->state = RXRPC_CALL_LOCALLY_ABORTED;
-			call->abort_code = RX_CALL_DEAD;
+			call->local_abort = RX_CALL_DEAD;
 			if (!test_and_set_bit(RXRPC_CALL_EV_ABORT, &call->events))
 				sched = true;
 		}
diff --git a/net/rxrpc/ar-connevent.c b/net/rxrpc/ar-connevent.c
index 1bdaaed8cdc4..4dc6ab81fd2f 100644
--- a/net/rxrpc/ar-connevent.c
+++ b/net/rxrpc/ar-connevent.c
@@ -40,11 +40,13 @@ static void rxrpc_abort_calls(struct rxrpc_connection *conn, int state,
 		write_lock(&call->state_lock);
 		if (call->state <= RXRPC_CALL_COMPLETE) {
 			call->state = state;
-			call->abort_code = abort_code;
-			if (state == RXRPC_CALL_LOCALLY_ABORTED)
+			if (state == RXRPC_CALL_LOCALLY_ABORTED) {
+				call->local_abort = conn->local_abort;
 				set_bit(RXRPC_CALL_EV_CONN_ABORT, &call->events);
-			else
+			} else {
+				call->remote_abort = conn->remote_abort;
 				set_bit(RXRPC_CALL_EV_RCVD_ABORT, &call->events);
+			}
 			rxrpc_queue_call(call);
 		}
 		write_unlock(&call->state_lock);
@@ -101,7 +103,7 @@ static int rxrpc_abort_connection(struct rxrpc_connection *conn,
 	whdr._rsvd	= 0;
 	whdr.serviceId	= htons(conn->service_id);
 
-	word = htonl(abort_code);
+	word		= htonl(conn->local_abort);
 
 	iov[0].iov_base	= &whdr;
 	iov[0].iov_len	= sizeof(whdr);
@@ -112,7 +114,7 @@ static int rxrpc_abort_connection(struct rxrpc_connection *conn,
 
 	serial = atomic_inc_return(&conn->serial);
 	whdr.serial = htonl(serial);
-	_proto("Tx CONN ABORT %%%u { %d }", serial, abort_code);
+	_proto("Tx CONN ABORT %%%u { %d }", serial, conn->local_abort);
 
 	ret = kernel_sendmsg(conn->trans->local->socket, &msg, iov, 2, len);
 	if (ret < 0) {
diff --git a/net/rxrpc/ar-input.c b/net/rxrpc/ar-input.c
index f9d97fcb21ff..401ea55fb773 100644
--- a/net/rxrpc/ar-input.c
+++ b/net/rxrpc/ar-input.c
@@ -349,7 +349,7 @@ void rxrpc_fast_process_packet(struct rxrpc_call *call, struct sk_buff *skb)
 		write_lock_bh(&call->state_lock);
 		if (call->state < RXRPC_CALL_COMPLETE) {
 			call->state = RXRPC_CALL_REMOTELY_ABORTED;
-			call->abort_code = abort_code;
+			call->remote_abort = abort_code;
 			set_bit(RXRPC_CALL_EV_RCVD_ABORT, &call->events);
 			rxrpc_queue_call(call);
 		}
@@ -422,7 +422,7 @@ protocol_error:
 protocol_error_locked:
 	if (call->state <= RXRPC_CALL_COMPLETE) {
 		call->state = RXRPC_CALL_LOCALLY_ABORTED;
-		call->abort_code = RX_PROTOCOL_ERROR;
+		call->local_abort = RX_PROTOCOL_ERROR;
 		set_bit(RXRPC_CALL_EV_ABORT, &call->events);
 		rxrpc_queue_call(call);
 	}
@@ -494,7 +494,7 @@ protocol_error:
 	write_lock_bh(&call->state_lock);
 	if (call->state <= RXRPC_CALL_COMPLETE) {
 		call->state = RXRPC_CALL_LOCALLY_ABORTED;
-		call->abort_code = RX_PROTOCOL_ERROR;
+		call->local_abort = RX_PROTOCOL_ERROR;
 		set_bit(RXRPC_CALL_EV_ABORT, &call->events);
 		rxrpc_queue_call(call);
 	}
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index eeb829e837e1..258b74a2a23f 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -289,7 +289,9 @@ struct rxrpc_connection {
 		RXRPC_CONN_LOCALLY_ABORTED,	/* - conn aborted locally */
 		RXRPC_CONN_NETWORK_ERROR,	/* - conn terminated by network error */
 	} state;
-	int			error;		/* error code for local abort */
+	u32			local_abort;	/* local abort code */
+	u32			remote_abort;	/* remote abort code */
+	int			error;		/* local error incurred */
 	int			debug_id;	/* debug ID for printks */
 	unsigned int		call_counter;	/* call ID counter */
 	atomic_t		serial;		/* packet serial number counter */
@@ -399,7 +401,9 @@ struct rxrpc_call {
 	rwlock_t		state_lock;	/* lock for state transition */
 	atomic_t		usage;
 	atomic_t		sequence;	/* Tx data packet sequence counter */
-	u32			abort_code;	/* local/remote abort code */
+	u32			local_abort;	/* local abort code */
+	u32			remote_abort;	/* remote abort code */
+	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 */
@@ -453,7 +457,7 @@ static inline void rxrpc_abort_call(struct rxrpc_call *call, u32 abort_code)
 {
 	write_lock_bh(&call->state_lock);
 	if (call->state < RXRPC_CALL_COMPLETE) {
-		call->abort_code = abort_code;
+		call->local_abort = abort_code;
 		call->state = RXRPC_CALL_LOCALLY_ABORTED;
 		set_bit(RXRPC_CALL_EV_ABORT, &call->events);
 	}
diff --git a/net/rxrpc/ar-output.c b/net/rxrpc/ar-output.c
index d36fb6e1a29c..94e7d9537437 100644
--- a/net/rxrpc/ar-output.c
+++ b/net/rxrpc/ar-output.c
@@ -110,7 +110,7 @@ static void rxrpc_send_abort(struct rxrpc_call *call, u32 abort_code)
 
 	if (call->state <= RXRPC_CALL_COMPLETE) {
 		call->state = RXRPC_CALL_LOCALLY_ABORTED;
-		call->abort_code = abort_code;
+		call->local_abort = abort_code;
 		set_bit(RXRPC_CALL_EV_ABORT, &call->events);
 		del_timer_sync(&call->resend_timer);
 		del_timer_sync(&call->ack_timer);
diff --git a/net/rxrpc/ar-proc.c b/net/rxrpc/ar-proc.c
index 525b2ba5a8f4..225163bc658d 100644
--- a/net/rxrpc/ar-proc.c
+++ b/net/rxrpc/ar-proc.c
@@ -80,7 +80,7 @@ static int rxrpc_call_seq_show(struct seq_file *seq, void *v)
 		   call->conn->in_clientflag ? "Svc" : "Clt",
 		   atomic_read(&call->usage),
 		   rxrpc_call_states[call->state],
-		   call->abort_code,
+		   call->remote_abort ?: call->local_abort,
 		   call->user_call_ID);
 
 	return 0;
diff --git a/net/rxrpc/ar-recvmsg.c b/net/rxrpc/ar-recvmsg.c
index 64facba24a45..160f0927aa3e 100644
--- a/net/rxrpc/ar-recvmsg.c
+++ b/net/rxrpc/ar-recvmsg.c
@@ -288,7 +288,11 @@ receive_non_data_message:
 		ret = put_cmsg(msg, SOL_RXRPC, RXRPC_BUSY, 0, &abort_code);
 		break;
 	case RXRPC_SKB_MARK_REMOTE_ABORT:
-		abort_code = call->abort_code;
+		abort_code = call->remote_abort;
+		ret = put_cmsg(msg, SOL_RXRPC, RXRPC_ABORT, 4, &abort_code);
+		break;
+	case RXRPC_SKB_MARK_LOCAL_ABORT:
+		abort_code = call->local_abort;
 		ret = put_cmsg(msg, SOL_RXRPC, RXRPC_ABORT, 4, &abort_code);
 		break;
 	case RXRPC_SKB_MARK_NET_ERROR:
@@ -303,6 +307,7 @@ receive_non_data_message:
 			       &abort_code);
 		break;
 	default:
+		pr_err("RxRPC: Unknown packet mark %u\n", skb->mark);
 		BUG();
 		break;
 	}
@@ -401,9 +406,14 @@ u32 rxrpc_kernel_get_abort_code(struct sk_buff *skb)
 {
 	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
 
-	ASSERTCMP(skb->mark, ==, RXRPC_SKB_MARK_REMOTE_ABORT);
-
-	return sp->call->abort_code;
+	switch (skb->mark) {
+	case RXRPC_SKB_MARK_REMOTE_ABORT:
+		return sp->call->remote_abort;
+	case RXRPC_SKB_MARK_LOCAL_ABORT:
+		return sp->call->local_abort;
+	default:
+		BUG();
+	}
 }
 
 EXPORT_SYMBOL(rxrpc_kernel_get_abort_code);

[toc] | [prev] | [next] | [standalone]


#1373532 — [PATCH 1/9] afs: Wait for outstanding async calls before closing rxrpc socket

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 18:30 +0200
Subject[PATCH 1/9] afs: Wait for outstanding async calls before closing rxrpc socket
Message-ID<rlkYI-6F9-65@gated-at.bofh.it>
In reply to#1373518
The afs filesystem needs to wait for any outstanding asynchronous calls
(such as FS.GiveUpCallBacks cleaning up the callbacks lodged with a server)
to complete before closing the AF_RXRPC socket when unloading the module.

This may occur if the module is removed too quickly after unmounting all
filesystems.  This will produce an error report that looks like:

	AFS: Assertion failed
	1 == 0 is false
	0x1 == 0x0 is false
	------------[ cut here ]------------
	kernel BUG at ../fs/afs/rxrpc.c:135!
	...
	RIP: 0010:[<ffffffffa004111c>] afs_close_socket+0xec/0x107 [kafs]
	...
	Call Trace:
	 [<ffffffffa004a160>] afs_exit+0x1f/0x57 [kafs]
	 [<ffffffff810c30a0>] SyS_delete_module+0xec/0x17d
	 [<ffffffff81610417>] entry_SYSCALL_64_fastpath+0x12/0x6b

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

 fs/afs/rxrpc.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index b50642870a43..7c2567a711ea 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -65,6 +65,12 @@ static void afs_async_workfn(struct work_struct *work)
 	call->async_workfn(call);
 }
 
+static int afs_wait_atomic_t(atomic_t *p)
+{
+	schedule();
+	return 0;
+}
+
 /*
  * open an RxRPC socket and bind it to be a server for callback notifications
  * - the socket is left in blocking mode and non-blocking ops use MSG_DONTWAIT
@@ -126,13 +132,16 @@ void afs_close_socket(void)
 {
 	_enter("");
 
+	wait_on_atomic_t(&afs_outstanding_calls, afs_wait_atomic_t,
+			 TASK_UNINTERRUPTIBLE);
+	_debug("no outstanding calls");
+	
 	sock_release(afs_socket);
 
 	_debug("dework");
 	destroy_workqueue(afs_async_calls);
 
 	ASSERTCMP(atomic_read(&afs_outstanding_skbs), ==, 0);
-	ASSERTCMP(atomic_read(&afs_outstanding_calls), ==, 0);
 	_leave("");
 }
 
@@ -178,8 +187,6 @@ static void afs_free_call(struct afs_call *call)
 {
 	_debug("DONE %p{%s} [%d]",
 	       call, call->type->name, atomic_read(&afs_outstanding_calls));
-	if (atomic_dec_return(&afs_outstanding_calls) == -1)
-		BUG();
 
 	ASSERTCMP(call->rxcall, ==, NULL);
 	ASSERT(!work_pending(&call->async_work));
@@ -188,6 +195,9 @@ static void afs_free_call(struct afs_call *call)
 
 	kfree(call->request);
 	kfree(call);
+
+	if (atomic_dec_and_test(&afs_outstanding_calls))
+		wake_up_atomic_t(&afs_outstanding_calls);
 }
 
 /*

[toc] | [prev] | [next] | [standalone]


#1373533 — [PATCH 9/9] rxrpc: Create a null security type and get rid of conditional calls

FromDavid Howells <dhowells@redhat.com>
Date2016-04-07 18:30 +0200
Subject[PATCH 9/9] rxrpc: Create a null security type and get rid of conditional calls
Message-ID<rlkYI-6F9-67@gated-at.bofh.it>
In reply to#1373518
Create a null security type for security index 0 and get rid of all
conditional calls to the security operations.  We expect normally to be
using security, so this should be of little negative impact.

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

 net/rxrpc/Makefile        |    1 +
 net/rxrpc/ar-ack.c        |    3 +-
 net/rxrpc/ar-connection.c |    9 +++--
 net/rxrpc/ar-connevent.c  |   11 +-----
 net/rxrpc/ar-input.c      |    2 +
 net/rxrpc/ar-internal.h   |   10 +++--
 net/rxrpc/ar-output.c     |    4 +-
 net/rxrpc/ar-security.c   |   43 +----------------------
 net/rxrpc/insecure.c      |   83 +++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 105 insertions(+), 61 deletions(-)
 create mode 100644 net/rxrpc/insecure.c

diff --git a/net/rxrpc/Makefile b/net/rxrpc/Makefile
index fa09cb55bfce..e05a06ef2254 100644
--- a/net/rxrpc/Makefile
+++ b/net/rxrpc/Makefile
@@ -19,6 +19,7 @@ af-rxrpc-y := \
 	ar-security.o \
 	ar-skbuff.o \
 	ar-transport.o \
+	insecure.o \
 	misc.o
 
 af-rxrpc-$(CONFIG_PROC_FS) += ar-proc.o
diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index 3cd9264806a4..374478e006e7 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -588,7 +588,8 @@ process_further:
 		_proto("OOSQ DATA %%%u { #%u }", sp->hdr.serial, sp->hdr.seq);
 
 		/* secured packets must be verified and possibly decrypted */
-		if (rxrpc_verify_packet(call, skb, _abort_code) < 0)
+		if (call->conn->security->verify_packet(call, skb,
+							_abort_code) < 0)
 			goto protocol_error;
 
 		rxrpc_insert_oos_packet(call, skb);
diff --git a/net/rxrpc/ar-connection.c b/net/rxrpc/ar-connection.c
index 9b6966777633..97f4fae74bca 100644
--- a/net/rxrpc/ar-connection.c
+++ b/net/rxrpc/ar-connection.c
@@ -207,6 +207,7 @@ static struct rxrpc_connection *rxrpc_alloc_connection(gfp_t gfp)
 		INIT_LIST_HEAD(&conn->bundle_link);
 		conn->calls = RB_ROOT;
 		skb_queue_head_init(&conn->rx_queue);
+		conn->security = &rxrpc_no_security;
 		rwlock_init(&conn->lock);
 		spin_lock_init(&conn->state_lock);
 		atomic_set(&conn->usage, 1);
@@ -564,8 +565,7 @@ int rxrpc_connect_call(struct rxrpc_sock *rx,
 		     candidate->debug_id, candidate->trans->debug_id);
 
 		rxrpc_assign_connection_id(candidate);
-		if (candidate->security)
-			candidate->security->prime_packet_security(candidate);
+		candidate->security->prime_packet_security(candidate);
 
 		/* leave the candidate lurking in zombie mode attached to the
 		 * bundle until we're ready for it */
@@ -830,7 +830,10 @@ static void rxrpc_destroy_connection(struct rxrpc_connection *conn)
 	ASSERT(RB_EMPTY_ROOT(&conn->calls));
 	rxrpc_purge_queue(&conn->rx_queue);
 
-	rxrpc_clear_conn_security(conn);
+	conn->security->clear(conn);
+	key_put(conn->key);
+	key_put(conn->server_key);
+
 	rxrpc_put_transport(conn->trans);
 	kfree(conn);
 	_leave("");
diff --git a/net/rxrpc/ar-connevent.c b/net/rxrpc/ar-connevent.c
index 291522392ac7..5f9563968a5b 100644
--- a/net/rxrpc/ar-connevent.c
+++ b/net/rxrpc/ar-connevent.c
@@ -174,15 +174,10 @@ static int rxrpc_process_event(struct rxrpc_connection *conn,
 		return -ECONNABORTED;
 
 	case RXRPC_PACKET_TYPE_CHALLENGE:
-		if (conn->security)
-			return conn->security->respond_to_challenge(
-				conn, skb, _abort_code);
-		return -EPROTO;
+		return conn->security->respond_to_challenge(conn, skb,
+							    _abort_code);
 
 	case RXRPC_PACKET_TYPE_RESPONSE:
-		if (!conn->security)
-			return -EPROTO;
-
 		ret = conn->security->verify_response(conn, skb, _abort_code);
 		if (ret < 0)
 			return ret;
@@ -238,8 +233,6 @@ static void rxrpc_secure_connection(struct rxrpc_connection *conn)
 		}
 	}
 
-	ASSERT(conn->security != NULL);
-
 	if (conn->security->issue_challenge(conn) < 0) {
 		abort_code = RX_CALL_DEAD;
 		ret = -ENOMEM;
diff --git a/net/rxrpc/ar-input.c b/net/rxrpc/ar-input.c
index 401ea55fb773..c91420eda26c 100644
--- a/net/rxrpc/ar-input.c
+++ b/net/rxrpc/ar-input.c
@@ -193,7 +193,7 @@ static int rxrpc_fast_process_data(struct rxrpc_call *call,
 
 	/* if the packet need security things doing to it, then it goes down
 	 * the slow path */
-	if (call->conn->security)
+	if (call->conn->security_ix)
 		goto enqueue_packet;
 
 	sp->call = call;
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 72fd675a891e..f0b807a163fa 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -9,6 +9,7 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#include <net/sock.h>
 #include <rxrpc/packet.h>
 
 #if 0
@@ -612,10 +613,6 @@ int __init rxrpc_init_security(void);
 void rxrpc_exit_security(void);
 int rxrpc_init_client_conn_security(struct rxrpc_connection *);
 int rxrpc_init_server_conn_security(struct rxrpc_connection *);
-int rxrpc_secure_packet(const struct rxrpc_call *, struct sk_buff *, size_t,
-			void *);
-int rxrpc_verify_packet(const struct rxrpc_call *, struct sk_buff *, u32 *);
-void rxrpc_clear_conn_security(struct rxrpc_connection *);
 
 /*
  * ar-skbuff.c
@@ -635,6 +632,11 @@ struct rxrpc_transport *rxrpc_find_transport(struct rxrpc_local *,
 					     struct rxrpc_peer *);
 
 /*
+ * insecure.c
+ */
+extern const struct rxrpc_security rxrpc_no_security;
+
+/*
  * misc.c
  */
 extern unsigned int rxrpc_requested_ack_delay;
diff --git a/net/rxrpc/ar-output.c b/net/rxrpc/ar-output.c
index 94e7d9537437..51cb10062a8d 100644
--- a/net/rxrpc/ar-output.c
+++ b/net/rxrpc/ar-output.c
@@ -663,7 +663,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 			size_t pad;
 
 			/* pad out if we're using security */
-			if (conn->security) {
+			if (conn->security_ix) {
 				pad = conn->security_size + skb->mark;
 				pad = conn->size_align - pad;
 				pad &= conn->size_align - 1;
@@ -695,7 +695,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 			if (more && seq & 1)
 				sp->hdr.flags |= RXRPC_REQUEST_ACK;
 
-			ret = rxrpc_secure_packet(
+			ret = conn->security->secure_packet(
 				call, skb, skb->mark,
 				skb->head + sizeof(struct rxrpc_wire_header));
 			if (ret < 0)
diff --git a/net/rxrpc/ar-security.c b/net/rxrpc/ar-security.c
index 6946aec7ab1f..d223253b22fa 100644
--- a/net/rxrpc/ar-security.c
+++ b/net/rxrpc/ar-security.c
@@ -23,6 +23,7 @@ static LIST_HEAD(rxrpc_security_methods);
 static DECLARE_RWSEM(rxrpc_security_sem);
 
 static const struct rxrpc_security *rxrpc_security_types[] = {
+	[RXRPC_SECURITY_NONE]	= &rxrpc_no_security,
 #ifdef CONFIG_RXKAD
 	[RXRPC_SECURITY_RXKAD]	= &rxkad,
 #endif
@@ -98,7 +99,7 @@ int rxrpc_init_client_conn_security(struct rxrpc_connection *conn)
 
 	ret = conn->security->init_connection_security(conn);
 	if (ret < 0) {
-		conn->security = NULL;
+		conn->security = &rxrpc_no_security;
 		return ret;
 	}
 
@@ -165,43 +166,3 @@ found_service:
 	_leave(" = 0");
 	return 0;
 }
-
-/*
- * secure a packet prior to transmission
- */
-int rxrpc_secure_packet(const struct rxrpc_call *call,
-			struct sk_buff *skb,
-			size_t data_size,
-			void *sechdr)
-{
-	if (call->conn->security)
-		return call->conn->security->secure_packet(
-			call, skb, data_size, sechdr);
-	return 0;
-}
-
-/*
- * secure a packet prior to transmission
- */
-int rxrpc_verify_packet(const struct rxrpc_call *call, struct sk_buff *skb,
-			u32 *_abort_code)
-{
-	if (call->conn->security)
-		return call->conn->security->verify_packet(
-			call, skb, _abort_code);
-	return 0;
-}
-
-/*
- * clear connection security
- */
-void rxrpc_clear_conn_security(struct rxrpc_connection *conn)
-{
-	_enter("{%d}", conn->debug_id);
-
-	if (conn->security)
-		conn->security->clear(conn);
-
-	key_put(conn->key);
-	key_put(conn->server_key);
-}
diff --git a/net/rxrpc/insecure.c b/net/rxrpc/insecure.c
new file mode 100644
index 000000000000..e571403613c1
--- /dev/null
+++ b/net/rxrpc/insecure.c
@@ -0,0 +1,83 @@
+/* Null security operations.
+ *
+ * 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.
+ */
+
+#include <net/af_rxrpc.h>
+#include "ar-internal.h"
+
+static int none_init_connection_security(struct rxrpc_connection *conn)
+{
+	return 0;
+}
+
+static void none_prime_packet_security(struct rxrpc_connection *conn)
+{
+}
+
+static int none_secure_packet(const struct rxrpc_call *call,
+			       struct sk_buff *skb,
+			       size_t data_size,
+			       void *sechdr)
+{
+	return 0;
+}
+
+static int none_verify_packet(const struct rxrpc_call *call,
+			       struct sk_buff *skb,
+			       u32 *_abort_code)
+{
+	return 0;
+}
+
+static int none_respond_to_challenge(struct rxrpc_connection *conn,
+				      struct sk_buff *skb,
+				      u32 *_abort_code)
+{
+	*_abort_code = RX_PROTOCOL_ERROR;
+	return -EPROTO;
+}
+
+static int none_verify_response(struct rxrpc_connection *conn,
+				 struct sk_buff *skb,
+				 u32 *_abort_code)
+{
+	*_abort_code = RX_PROTOCOL_ERROR;
+	return -EPROTO;
+}
+
+static void none_clear(struct rxrpc_connection *conn)
+{
+}
+
+static int none_init(void)
+{
+	return 0;
+}
+
+static void none_exit(void)
+{
+}
+
+/*
+ * RxRPC Kerberos-based security
+ */
+const struct rxrpc_security rxrpc_no_security = {
+	.name				= "none",
+	.security_index			= RXRPC_SECURITY_NONE,
+	.init				= none_init,
+	.exit				= none_exit,
+	.init_connection_security	= none_init_connection_security,
+	.prime_packet_security		= none_prime_packet_security,
+	.secure_packet			= none_secure_packet,
+	.verify_packet			= none_verify_packet,
+	.respond_to_challenge		= none_respond_to_challenge,
+	.verify_response		= none_verify_response,
+	.clear				= none_clear,
+};

[toc] | [prev] | [next] | [standalone]


#1376253

FromDavid Miller <davem@davemloft.net>
Date2016-04-11 21:40 +0200
Message-ID<rmPQK-2Io-17@gated-at.bofh.it>
In reply to#1373518
From: David Howells <dhowells@redhat.com>
Date: Thu, 07 Apr 2016 17:22:56 +0100

> Okay, I'm in the process of rewriting the RxRPC rewrite.  The primary aim of
> this second rewrite is to strictly control the number of active connections we
> know about and to get rid of connections we don't need much more quickly.
> 
> On top of this, there are fixes to the protocol handling which will all occur
> in later parts.

Series applied, but I had to fix up some trailing whitespace (reported by GIT)
by hand.

Thanks.

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web