All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/9] rxrpc: Small fixes
@ 2016-09-04 21:02 David Howells
  2016-09-04 21:02 ` [PATCH net-next 1/9] rxrpc: fix undefined behavior in rxrpc_mark_call_released David Howells
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: David Howells @ 2016-09-04 21:02 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel


Here's a set of small fix patches:

 (1) Fix some uninitialised variables.

 (2) Set the client call state before making it live by attaching it to the
     conn struct.

 (3) Randomise the epoch and starting client conn ID values, and don't
     change the epoch when the client conn ID rolls round.

 (4) Replace deprecated create_singlethread_workqueue() calls.

The patches can be found here also (non-terminally 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-20160904-1

David
---
Arnd Bergmann (1):
      rxrpc: fix undefined behavior in rxrpc_mark_call_released

Bhaktipriya Shridhar (4):
      fs/afs/vlocation: Remove deprecated create_singlethread_workqueue
      fs/afs/rxrpc: Remove deprecated create_singlethread_workqueue
      fs/afs/callback: Remove deprecated create_singlethread_workqueue
      fs/afs/flock: Remove deprecated create_singlethread_workqueue

David Howells (4):
      rxrpc: Fix uninitialised variable warning
      rxrpc: The client call state must be changed before attachment to conn
      rxrpc: Randomise epoch and starting client conn ID values
      rxrpc: Don't change the epoch


 fs/afs/callback.c       |    4 ++--
 fs/afs/flock.c          |    4 ++--
 fs/afs/rxrpc.c          |    2 +-
 fs/afs/vlocation.c      |    4 ++--
 include/rxrpc/packet.h  |    1 +
 net/rxrpc/af_rxrpc.c    |    9 ++++++++-
 net/rxrpc/call_event.c  |    5 ++---
 net/rxrpc/call_object.c |    4 +---
 net/rxrpc/conn_client.c |   36 ++++++++++++------------------------
 9 files changed, 31 insertions(+), 38 deletions(-)

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

* [PATCH net-next 1/9] rxrpc: fix undefined behavior in rxrpc_mark_call_released
  2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
@ 2016-09-04 21:02 ` David Howells
  2016-09-04 21:02 ` [PATCH net-next 2/9] rxrpc: Fix uninitialised variable warning David Howells
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: David Howells @ 2016-09-04 21:02 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

From: Arnd Bergmann <arnd@arndb.de>

gcc -Wmaybe-initialized correctly points out a newly introduced bug
through which we can end up calling rxrpc_queue_call() for a dead
connection:

net/rxrpc/call_object.c: In function 'rxrpc_mark_call_released':
net/rxrpc/call_object.c:600:5: error: 'sched' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This sets the 'sched' variable to zero to restore the previous
behavior.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: f5c17aaeb2ae ("rxrpc: Calls should only have one terminal state")
Signed-off-by: David Howells <dhowells@redhat.com>
---

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

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 516d8ea82f02..57e00fc9cff2 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -586,7 +586,7 @@ static void rxrpc_dead_call_expired(unsigned long _call)
  */
 static void rxrpc_mark_call_released(struct rxrpc_call *call)
 {
-	bool sched;
+	bool sched = false;
 
 	rxrpc_see_call(call);
 	write_lock(&call->state_lock);

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

* [PATCH net-next 2/9] rxrpc: Fix uninitialised variable warning
  2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
  2016-09-04 21:02 ` [PATCH net-next 1/9] rxrpc: fix undefined behavior in rxrpc_mark_call_released David Howells
@ 2016-09-04 21:02 ` David Howells
  2016-09-04 21:02 ` [PATCH net-next 3/9] rxrpc: The client call state must be changed before attachment to conn David Howells
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: David Howells @ 2016-09-04 21:02 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

Fix the following uninitialised variable warning:

../net/rxrpc/call_event.c: In function 'rxrpc_process_call':
../net/rxrpc/call_event.c:879:58: warning: 'error' may be used uninitialized in this function [-Wmaybe-uninitialized]
    _debug("post net error %d", error);
                                                          ^

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

 net/rxrpc/call_event.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index de72de662044..4754c7fb6242 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -868,7 +868,6 @@ skip_msg_init:
 	/* deal with events of a final nature */
 	if (test_bit(RXRPC_CALL_EV_RCVD_ERROR, &call->events)) {
 		enum rxrpc_skb_mark mark;
-		int error;
 
 		clear_bit(RXRPC_CALL_EV_CONN_ABORT, &call->events);
 		clear_bit(RXRPC_CALL_EV_REJECT_BUSY, &call->events);
@@ -876,10 +875,10 @@ skip_msg_init:
 
 		if (call->completion == RXRPC_CALL_NETWORK_ERROR) {
 			mark = RXRPC_SKB_MARK_NET_ERROR;
-			_debug("post net error %d", error);
+			_debug("post net error %d", call->error);
 		} else {
 			mark = RXRPC_SKB_MARK_LOCAL_ERROR;
-			_debug("post net local error %d", error);
+			_debug("post net local error %d", call->error);
 		}
 
 		if (rxrpc_post_message(call, mark, call->error, true) < 0)

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

* [PATCH net-next 3/9] rxrpc: The client call state must be changed before attachment to conn
  2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
  2016-09-04 21:02 ` [PATCH net-next 1/9] rxrpc: fix undefined behavior in rxrpc_mark_call_released David Howells
  2016-09-04 21:02 ` [PATCH net-next 2/9] rxrpc: Fix uninitialised variable warning David Howells
@ 2016-09-04 21:02 ` David Howells
  2016-09-04 21:02 ` [PATCH net-next 4/9] rxrpc: Randomise epoch and starting client conn ID values David Howells
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: David Howells @ 2016-09-04 21:02 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

We must set the client call state to RXRPC_CALL_CLIENT_SEND_REQUEST before
attaching the call to the connection struct, not after, as it's liable to
receive errors and conn aborts as soon as the assignment is made - and
these will cause its state to be changed outside of the initiating thread's
control.

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

 net/rxrpc/call_object.c |    2 --
 net/rxrpc/conn_client.c |    4 ++++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 57e00fc9cff2..65691742199b 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -197,8 +197,6 @@ static int rxrpc_begin_client_call(struct rxrpc_call *call,
 	if (ret < 0)
 		return ret;
 
-	call->state = RXRPC_CALL_CLIENT_SEND_REQUEST;
-
 	spin_lock(&call->conn->params.peer->lock);
 	hlist_add_head(&call->error_link, &call->conn->params.peer->error_targets);
 	spin_unlock(&call->conn->params.peer->lock);
diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index 4b213bc0f554..e19804dd6c8d 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -537,6 +537,10 @@ static void rxrpc_activate_one_channel(struct rxrpc_connection *conn,
 					     struct rxrpc_call, chan_wait_link);
 	u32 call_id = chan->call_counter + 1;
 
+	write_lock_bh(&call->state_lock);
+	call->state = RXRPC_CALL_CLIENT_SEND_REQUEST;
+	write_unlock_bh(&call->state_lock);
+
 	rxrpc_see_call(call);
 	list_del_init(&call->chan_wait_link);
 	conn->active_chans |= 1 << channel;

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

* [PATCH net-next 4/9] rxrpc: Randomise epoch and starting client conn ID values
  2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
                   ` (2 preceding siblings ...)
  2016-09-04 21:02 ` [PATCH net-next 3/9] rxrpc: The client call state must be changed before attachment to conn David Howells
@ 2016-09-04 21:02 ` David Howells
  2016-09-05 15:01   ` David Laight
  2016-09-05 16:24   ` David Howells
  2016-09-04 21:02 ` [PATCH net-next 5/9] rxrpc: Don't change the epoch David Howells
                   ` (5 subsequent siblings)
  9 siblings, 2 replies; 14+ messages in thread
From: David Howells @ 2016-09-04 21:02 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

Create a random epoch value rather than a time-based one on startup and set
the top bit to indicate that this is the case.

Also create a random starting client connection ID value.  This will be
incremented from here as new client connections are created.

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

 include/rxrpc/packet.h |    1 +
 net/rxrpc/af_rxrpc.c   |    9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/rxrpc/packet.h b/include/rxrpc/packet.h
index b2017440b765..3c6128e1fdbe 100644
--- a/include/rxrpc/packet.h
+++ b/include/rxrpc/packet.h
@@ -24,6 +24,7 @@ typedef __be32	rxrpc_serial_net_t; /* on-the-wire Rx message serial number */
  */
 struct rxrpc_wire_header {
 	__be32		epoch;		/* client boot timestamp */
+#define RXRPC_RANDOM_EPOCH	0x80000000	/* Random if set, date-based if not */
 
 	__be32		cid;		/* connection and channel ID */
 #define RXRPC_MAXCALLS		4			/* max active calls per conn */
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 32d544995dda..b66a9e6f8d04 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -16,6 +16,7 @@
 #include <linux/net.h>
 #include <linux/slab.h>
 #include <linux/skbuff.h>
+#include <linux/random.h>
 #include <linux/poll.h>
 #include <linux/proc_fs.h>
 #include <linux/key-type.h>
@@ -700,7 +701,13 @@ static int __init af_rxrpc_init(void)
 
 	BUILD_BUG_ON(sizeof(struct rxrpc_skb_priv) > FIELD_SIZEOF(struct sk_buff, cb));
 
-	rxrpc_epoch = get_seconds();
+	get_random_bytes(&rxrpc_epoch, sizeof(rxrpc_epoch));
+	rxrpc_epoch |= RXRPC_RANDOM_EPOCH;
+	get_random_bytes(&rxrpc_client_conn_ids.cur,
+			 sizeof(rxrpc_client_conn_ids.cur));
+	rxrpc_client_conn_ids.cur &= 0x3fffffff;
+	if (rxrpc_client_conn_ids.cur == 0)
+		rxrpc_client_conn_ids.cur = 1;
 
 	ret = -ENOMEM;
 	rxrpc_call_jar = kmem_cache_create(

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

* [PATCH net-next 5/9] rxrpc: Don't change the epoch
  2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
                   ` (3 preceding siblings ...)
  2016-09-04 21:02 ` [PATCH net-next 4/9] rxrpc: Randomise epoch and starting client conn ID values David Howells
@ 2016-09-04 21:02 ` David Howells
  2016-09-04 21:03 ` [PATCH net-next 6/9] fs/afs/vlocation: Remove deprecated create_singlethread_workqueue David Howells
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: David Howells @ 2016-09-04 21:02 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

It seems the local epoch should only be changed on boot, so remove the code
that changes it for client connections.

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

 net/rxrpc/conn_client.c |   32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index e19804dd6c8d..82de1aeaef21 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -108,12 +108,12 @@ static DECLARE_DELAYED_WORK(rxrpc_client_conn_reap,
 /*
  * Get a connection ID and epoch for a client connection from the global pool.
  * The connection struct pointer is then recorded in the idr radix tree.  The
- * epoch is changed if this wraps.
+ * epoch doesn't change until the client is rebooted (or, at least, unless the
+ * module is unloaded).
  */
 static int rxrpc_get_client_connection_id(struct rxrpc_connection *conn,
 					  gfp_t gfp)
 {
-	u32 epoch;
 	int id;
 
 	_enter("");
@@ -121,34 +121,18 @@ static int rxrpc_get_client_connection_id(struct rxrpc_connection *conn,
 	idr_preload(gfp);
 	spin_lock(&rxrpc_conn_id_lock);
 
-	epoch = rxrpc_epoch;
-
-	/* We could use idr_alloc_cyclic() here, but we really need to know
-	 * when the thing wraps so that we can advance the epoch.
-	 */
-	if (rxrpc_client_conn_ids.cur == 0)
-		rxrpc_client_conn_ids.cur = 1;
-	id = idr_alloc(&rxrpc_client_conn_ids, conn,
-		       rxrpc_client_conn_ids.cur, 0x40000000, GFP_NOWAIT);
-	if (id < 0) {
-		if (id != -ENOSPC)
-			goto error;
-		id = idr_alloc(&rxrpc_client_conn_ids, conn,
-			       1, 0x40000000, GFP_NOWAIT);
-		if (id < 0)
-			goto error;
-		epoch++;
-		rxrpc_epoch = epoch;
-	}
-	rxrpc_client_conn_ids.cur = id + 1;
+	id = idr_alloc_cyclic(&rxrpc_client_conn_ids, conn,
+			      1, 0x40000000, GFP_NOWAIT);
+	if (id < 0)
+		goto error;
 
 	spin_unlock(&rxrpc_conn_id_lock);
 	idr_preload_end();
 
-	conn->proto.epoch = epoch;
+	conn->proto.epoch = rxrpc_epoch;
 	conn->proto.cid = id << RXRPC_CIDSHIFT;
 	set_bit(RXRPC_CONN_HAS_IDR, &conn->flags);
-	_leave(" [CID %x:%x]", epoch, conn->proto.cid);
+	_leave(" [CID %x]", conn->proto.cid);
 	return 0;
 
 error:

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

* [PATCH net-next 6/9] fs/afs/vlocation: Remove deprecated create_singlethread_workqueue
  2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
                   ` (4 preceding siblings ...)
  2016-09-04 21:02 ` [PATCH net-next 5/9] rxrpc: Don't change the epoch David Howells
@ 2016-09-04 21:03 ` David Howells
  2016-09-04 21:03 ` [PATCH net-next 7/9] fs/afs/rxrpc: " David Howells
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: David Howells @ 2016-09-04 21:03 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

From: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>

The workqueue "afs_vlocation_update_worker" queues a single work item
&afs_vlocation_update and hence it doesn't require execution ordering.
Hence, alloc_workqueue has been used to replace the deprecated
create_singlethread_workqueue instance.

Since the workqueue is being used on a memory reclaim path, WQ_MEM_RECLAIM
flag has been set to ensure forward progress under memory pressure.

Since there are fixed number of work items, explicit concurrency
limit is unnecessary here.

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/vlocation.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/afs/vlocation.c b/fs/afs/vlocation.c
index 52976785a32c..45a86396fd2d 100644
--- a/fs/afs/vlocation.c
+++ b/fs/afs/vlocation.c
@@ -594,8 +594,8 @@ static void afs_vlocation_reaper(struct work_struct *work)
  */
 int __init afs_vlocation_update_init(void)
 {
-	afs_vlocation_update_worker =
-		create_singlethread_workqueue("kafs_vlupdated");
+	afs_vlocation_update_worker = alloc_workqueue("kafs_vlupdated",
+						      WQ_MEM_RECLAIM, 0);
 	return afs_vlocation_update_worker ? 0 : -ENOMEM;
 }
 

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

* [PATCH net-next 7/9] fs/afs/rxrpc: Remove deprecated create_singlethread_workqueue
  2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
                   ` (5 preceding siblings ...)
  2016-09-04 21:03 ` [PATCH net-next 6/9] fs/afs/vlocation: Remove deprecated create_singlethread_workqueue David Howells
@ 2016-09-04 21:03 ` David Howells
  2016-09-04 21:03 ` [PATCH net-next 8/9] fs/afs/callback: " David Howells
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: David Howells @ 2016-09-04 21:03 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

From: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>

The workqueue "afs_async_calls" queues work item
&call->async_work per afs_call. Since there could be multiple calls and since
these calls can be run concurrently, alloc_workqueue has been used to replace
the deprecated create_singlethread_workqueue instance.

The WQ_MEM_RECLAIM flag has been set to ensure forward progress under
memory pressure because the workqueue is being used on a memory reclaim
path.

Since there are fixed number of work items, explicit concurrency
limit is unnecessary here.

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/rxrpc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index 244896baf241..37608be52abd 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -76,7 +76,7 @@ int afs_open_socket(void)
 	_enter("");
 
 	ret = -ENOMEM;
-	afs_async_calls = create_singlethread_workqueue("kafsd");
+	afs_async_calls = alloc_workqueue("kafsd", WQ_MEM_RECLAIM, 0);
 	if (!afs_async_calls)
 		goto error_0;
 

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

* [PATCH net-next 8/9] fs/afs/callback: Remove deprecated create_singlethread_workqueue
  2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
                   ` (6 preceding siblings ...)
  2016-09-04 21:03 ` [PATCH net-next 7/9] fs/afs/rxrpc: " David Howells
@ 2016-09-04 21:03 ` David Howells
  2016-09-04 21:03 ` [PATCH net-next 9/9] fs/afs/flock: " David Howells
  2016-09-06 20:53 ` [PATCH net-next 0/9] rxrpc: Small fixes David Miller
  9 siblings, 0 replies; 14+ messages in thread
From: David Howells @ 2016-09-04 21:03 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

From: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>

The workqueue "afs_callback_update_worker" queues multiple work items
viz  &vnode->cb_broken_work, &server->cb_break_work which require strict
execution ordering. Hence, an ordered dedicated workqueue has been used.

Since the workqueue is being used on a memory reclaim path, WQ_MEM_RECLAIM
has been set to ensure forward progress under memory pressure.

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/callback.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/afs/callback.c b/fs/afs/callback.c
index 7ef637d7f3a5..1e9d2f84e5b5 100644
--- a/fs/afs/callback.c
+++ b/fs/afs/callback.c
@@ -461,8 +461,8 @@ static void afs_callback_updater(struct work_struct *work)
  */
 int __init afs_callback_update_init(void)
 {
-	afs_callback_update_worker =
-		create_singlethread_workqueue("kafs_callbackd");
+	afs_callback_update_worker = alloc_ordered_workqueue("kafs_callbackd",
+							     WQ_MEM_RECLAIM);
 	return afs_callback_update_worker ? 0 : -ENOMEM;
 }
 

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

* [PATCH net-next 9/9] fs/afs/flock: Remove deprecated create_singlethread_workqueue
  2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
                   ` (7 preceding siblings ...)
  2016-09-04 21:03 ` [PATCH net-next 8/9] fs/afs/callback: " David Howells
@ 2016-09-04 21:03 ` David Howells
  2016-09-06 20:53 ` [PATCH net-next 0/9] rxrpc: Small fixes David Miller
  9 siblings, 0 replies; 14+ messages in thread
From: David Howells @ 2016-09-04 21:03 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel

From: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>

The workqueue "afs_lock_manager" queues work item &vnode->lock_work,
per vnode. Since there can be multiple vnodes and since their work items
can be executed concurrently, alloc_workqueue has been used to replace
the deprecated create_singlethread_workqueue instance.

The WQ_MEM_RECLAIM flag has been set to ensure forward progress under
memory pressure because the workqueue is being used on a memory reclaim
path.

Since there are fixed number of work items, explicit concurrency
limit is unnecessary here.

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/flock.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/afs/flock.c b/fs/afs/flock.c
index d91a9c9cfbd0..3191dff2c156 100644
--- a/fs/afs/flock.c
+++ b/fs/afs/flock.c
@@ -36,8 +36,8 @@ static int afs_init_lock_manager(void)
 	if (!afs_lock_manager) {
 		mutex_lock(&afs_lock_manager_mutex);
 		if (!afs_lock_manager) {
-			afs_lock_manager =
-				create_singlethread_workqueue("kafs_lockd");
+			afs_lock_manager = alloc_workqueue("kafs_lockd",
+							   WQ_MEM_RECLAIM, 0);
 			if (!afs_lock_manager)
 				ret = -ENOMEM;
 		}

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

* RE: [PATCH net-next 4/9] rxrpc: Randomise epoch and starting client conn ID values
  2016-09-04 21:02 ` [PATCH net-next 4/9] rxrpc: Randomise epoch and starting client conn ID values David Howells
@ 2016-09-05 15:01   ` David Laight
  2016-09-05 16:24   ` David Howells
  1 sibling, 0 replies; 14+ messages in thread
From: David Laight @ 2016-09-05 15:01 UTC (permalink / raw)
  To: 'David Howells', netdev; +Cc: linux-afs, linux-kernel

From: David Howells
> Sent: 04 September 2016 22:03
> Create a random epoch value rather than a time-based one on startup and set
> the top bit to indicate that this is the case.

Why set the top bit?
There is nothing to stop the time (in seconds) from having the top bit set.
Nothing else can care - otherwise this wouldn't work.

> Also create a random starting client connection ID value.  This will be
> incremented from here as new client connections are created.

I'm guessing this is to make duplicates less likely after a restart?
You may want to worry about duplicate allocations (after 2^32 connects).
There are id allocation algorithms that guarantee not to generate duplicates
and not to reuse values quickly while still being fixed cost.
Look at the code NetBSD uses to allocate process ids for an example.

	David

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

* Re: [PATCH net-next 4/9] rxrpc: Randomise epoch and starting client conn ID values
  2016-09-04 21:02 ` [PATCH net-next 4/9] rxrpc: Randomise epoch and starting client conn ID values David Howells
  2016-09-05 15:01   ` David Laight
@ 2016-09-05 16:24   ` David Howells
  2016-09-06  2:12     ` Jeffrey Altman
  1 sibling, 1 reply; 14+ messages in thread
From: David Howells @ 2016-09-05 16:24 UTC (permalink / raw)
  To: David Laight, Jeffrey Altman; +Cc: dhowells, netdev, linux-afs, linux-kernel

[cc'ing Jeff Altman for comment]

David Laight <David.Laight@ACULAB.COM> wrote:

> > Create a random epoch value rather than a time-based one on startup and set
> > the top bit to indicate that this is the case.
> 
> Why set the top bit?
> There is nothing to stop the time (in seconds) from having the top bit set.
> Nothing else can care - otherwise this wouldn't work.

This is what I'm told I should do by purveyors of other RxRPC solutions.

> > Also create a random starting client connection ID value.  This will be
> > incremented from here as new client connections are created.
> 
> I'm guessing this is to make duplicates less likely after a restart?

Again, it's been suggested that I do this, but I would guess so.

> You may want to worry about duplicate allocations (after 2^32 connects).

It's actually a quarter of that, but connection != call, so a connection may
be used for up to ~16 billion RPC operations before it *has* to be flushed.

> There are id allocation algorithms that guarantee not to generate duplicates
> and not to reuse values quickly while still being fixed cost.
> Look at the code NetBSD uses to allocate process ids for an example.

I'm using idr_alloc_cyclic()[*] with a fixed size "window" on the active conn
ID values.  Client connections with IDs outside of that window are discarded
as soon as possible to keep the memory consumption of the tree down (and to
force security renegotiation occasionally).  However, given that there are a
billion IDs to cycle through, it will take quite a while for reuse to become
an issue.

I like the idea of incrementing the epoch every time we cycle through the ID
space, but I'm told that a change in the epoch value is an indication that the
client rebooted - with what consequences I cannot say.

[*] which is what Linux uses to allocate process IDs.

David

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

* Re: [PATCH net-next 4/9] rxrpc: Randomise epoch and starting client conn ID values
  2016-09-05 16:24   ` David Howells
@ 2016-09-06  2:12     ` Jeffrey Altman
  0 siblings, 0 replies; 14+ messages in thread
From: Jeffrey Altman @ 2016-09-06  2:12 UTC (permalink / raw)
  To: David Howells, David Laight; +Cc: netdev, linux-afs, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2504 bytes --]

Reply inline ....

On 9/5/2016 12:24 PM, David Howells wrote:
> [cc'ing Jeff Altman for comment]
> 
> David Laight <David.Laight@ACULAB.COM> wrote:
> 
>>> Create a random epoch value rather than a time-based one on startup and set
>>> the top bit to indicate that this is the case.
>>
>> Why set the top bit?
>> There is nothing to stop the time (in seconds) from having the top bit set.
>> Nothing else can care - otherwise this wouldn't work.
> 
> This is what I'm told I should do by purveyors of other RxRPC solutions.

The protocol specification requires that the top bit be 1 for a random
epoch and 0 for a time derived epoch.
> 
>>> Also create a random starting client connection ID value.  This will be
>>> incremented from here as new client connections are created.
>>
>> I'm guessing this is to make duplicates less likely after a restart?

Its to reduce the possibility of duplicates on multiple machines that
might at some point exchange an endpoint address either due to mobility
or NAT/PAT.
> 
> Again, it's been suggested that I do this, but I would guess so.
> 
>> You may want to worry about duplicate allocations (after 2^32 connects).
> 
> It's actually a quarter of that, but connection != call, so a connection may
> be used for up to ~16 billion RPC operations before it *has* to be flushed.
> 
>> There are id allocation algorithms that guarantee not to generate duplicates
>> and not to reuse values quickly while still being fixed cost.
>> Look at the code NetBSD uses to allocate process ids for an example.
> 
> I'm using idr_alloc_cyclic()[*] with a fixed size "window" on the active conn
> ID values.  Client connections with IDs outside of that window are discarded
> as soon as possible to keep the memory consumption of the tree down (and to
> force security renegotiation occasionally).  However, given that there are a
> billion IDs to cycle through, it will take quite a while for reuse to become
> an issue.
> 
> I like the idea of incrementing the epoch every time we cycle through the ID
> space, but I'm told that a change in the epoch value is an indication that the
> client rebooted - with what consequences I cannot say.

State information might be recorded about an rx peer with the assumption
that state will be reset when the epoch changes.  The most frequent use
of this technique is for rx rpc statistics monitoring.


> 
> [*] which is what Linux uses to allocate process IDs.
> 
> David
> 


[-- Attachment #1.2: jaltman.vcf --]
[-- Type: text/x-vcard, Size: 410 bytes --]

begin:vcard
fn:Jeffrey Altman
n:Altman;Jeffrey
org:AuriStor, Inc.
adr:Suite 6B;;255 West 94Th Street;New York;New York;10025-6985;United States
email;internet:jaltman@auristor.com
title:Founder and CEO
tel;work:+1-212-769-9018
note;quoted-printable:LinkedIn: https://www.linkedin.com/in/jeffreyaltman=0D=0A=
	Skype: jeffrey.e.altman=0D=0A=
	
url:https://www.auristor.com/
version:2.1
end:vcard


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4333 bytes --]

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

* Re: [PATCH net-next 0/9] rxrpc: Small fixes
  2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
                   ` (8 preceding siblings ...)
  2016-09-04 21:03 ` [PATCH net-next 9/9] fs/afs/flock: " David Howells
@ 2016-09-06 20:53 ` David Miller
  9 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2016-09-06 20:53 UTC (permalink / raw)
  To: dhowells; +Cc: netdev, linux-afs, linux-kernel

From: David Howells <dhowells@redhat.com>
Date: Sun, 04 Sep 2016 22:02:24 +0100

> 
> Here's a set of small fix patches:
> 
>  (1) Fix some uninitialised variables.
> 
>  (2) Set the client call state before making it live by attaching it to the
>      conn struct.
> 
>  (3) Randomise the epoch and starting client conn ID values, and don't
>      change the epoch when the client conn ID rolls round.
> 
>  (4) Replace deprecated create_singlethread_workqueue() calls.
> 
> The patches can be found here also (non-terminally 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-20160904-1

Both rxrpc-rewrite-20160904-1 and rxrpc-rewrite-20160904-2 pulled, thanks.

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

end of thread, other threads:[~2016-09-06 20:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-04 21:02 [PATCH net-next 0/9] rxrpc: Small fixes David Howells
2016-09-04 21:02 ` [PATCH net-next 1/9] rxrpc: fix undefined behavior in rxrpc_mark_call_released David Howells
2016-09-04 21:02 ` [PATCH net-next 2/9] rxrpc: Fix uninitialised variable warning David Howells
2016-09-04 21:02 ` [PATCH net-next 3/9] rxrpc: The client call state must be changed before attachment to conn David Howells
2016-09-04 21:02 ` [PATCH net-next 4/9] rxrpc: Randomise epoch and starting client conn ID values David Howells
2016-09-05 15:01   ` David Laight
2016-09-05 16:24   ` David Howells
2016-09-06  2:12     ` Jeffrey Altman
2016-09-04 21:02 ` [PATCH net-next 5/9] rxrpc: Don't change the epoch David Howells
2016-09-04 21:03 ` [PATCH net-next 6/9] fs/afs/vlocation: Remove deprecated create_singlethread_workqueue David Howells
2016-09-04 21:03 ` [PATCH net-next 7/9] fs/afs/rxrpc: " David Howells
2016-09-04 21:03 ` [PATCH net-next 8/9] fs/afs/callback: " David Howells
2016-09-04 21:03 ` [PATCH net-next 9/9] fs/afs/flock: " David Howells
2016-09-06 20:53 ` [PATCH net-next 0/9] rxrpc: Small fixes David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.